From b2d50e9136f9e8e17a6aa90ef320488e85123901 Mon Sep 17 00:00:00 2001 From: prosolis <5590409+prosolis@users.noreply.github.com> Date: Tue, 28 Jul 2026 07:27:49 -0700 Subject: [PATCH] A dismissed card stays dismissed, even offline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server already suppressed every span she had accepted or dismissed, on both the LLM reconcile and the mechanics pass, with tests either side. What had no memory was the half that never asks it: item 3b's rule pack renders 250 ms after a keystroke with no network, and its record of "she already answered this" was a set cleared on every document switch and added to only for cards dismissed while still provisional. So dismissing a persisted rule-pack card recorded nothing client-side and the next keystroke put it straight back until the server's reply removed it again; and after a reload the client knew nothing at all — permanently so with the server unreachable, which is the case the rule pack exists for. GET /docs/{id}/settled hands over the normalized originals of the document's actioned rows, scoped through documents because an original quotes her sentence. The client seeds a SettledSpans from it on open and adds to it for every card that leaves, keyed on the original alone the way the server keys it. The load adds rather than assigns, so a dismissal made while it is in flight survives it. normalizeForDedup now exists in both languages, compared across a network boundary, so the same nine cases are asserted on both sides and each test names the other. Also: the status-bar count — "🌸 5片花瓣待打磨 · 5 petals to polish" beside the word count, from the packs, hidden at zero. An empty rail already says nothing is waiting; a badge announcing it after every check is a verdict, which the review's non-goals rule out. Verified in Chrome at 1517x810 with the server killed: a new violation was detected, underlined and counted with no network, while the dismissed span stayed gone. --- UX_REVIEW_2026-07-27.md | 139 +++++++++++++++++++- internal/suggestions/handlers.go | 71 +++++++++++ internal/suggestions/isolation_test.go | 10 ++ internal/suggestions/settled_test.go | 142 +++++++++++++++++++++ web/src/App.tsx | 1 + web/src/api/client.ts | 6 + web/src/components/StatusBar/StatusBar.tsx | 31 ++++- web/src/hooks/useCheckpoint.ts | Bin 14498 -> 15484 bytes web/src/i18n/i18n.test.ts | 20 +++ web/src/i18n/packs/fr.ts | 4 + web/src/i18n/packs/pt-PT.ts | 4 + web/src/i18n/packs/zh.ts | 6 + web/src/i18n/types.ts | 5 + web/src/lib/settled.test.ts | 73 +++++++++++ web/src/lib/settled.ts | 57 +++++++++ 15 files changed, 562 insertions(+), 7 deletions(-) create mode 100644 internal/suggestions/settled_test.go create mode 100644 web/src/lib/settled.test.ts create mode 100644 web/src/lib/settled.ts diff --git a/UX_REVIEW_2026-07-27.md b/UX_REVIEW_2026-07-27.md index 42b4ad2..121e5f2 100644 --- a/UX_REVIEW_2026-07-27.md +++ b/UX_REVIEW_2026-07-27.md @@ -755,6 +755,112 @@ written down as such rather than covered by a test that would pass regardless. the word count — the gentle version of Grammarly's score. No numeric grade, per the north star. Acceptance: count updates live with the rail. +### 8 — dismissal persistence and the status-bar summary DONE (ninth session). + +The two the handoff picked. They turned out to be opposite shapes: one was +almost entirely built and needed a small piece in an unexpected place; the +other was new but tiny. + +**Dismissal persistence was already true of everything the server stores.** +`buildSuppressor` indexes accepted *and* rejected rows and is wired into both +the LLM reconcile and `replaceMechanics`, with tests either side +(`TestResolvedSuggestionsNotReproposed`, `TestMechanicsActionedSuppression`). +The item's own acceptance criterion — dismiss → edit elsewhere → recheck → +it doesn't return — held before this session started. + +**What wasn't true was the half that never asks the server.** Item 3b gave the +rule pack a 250 ms fuse that renders findings with no network at all, and the +detector reads the text alone, so something has to tell it what she has +already answered. That memory was `actionedRef`: a set of +`original + replacement` keys, added to *only* for cards dismissed while still +provisional, and cleared on every document switch. Two consequences, both +real: + +- Dismiss a **persisted** rule-pack card and nothing recorded it client-side. + The next keystroke re-detected it and put it back on screen; the server's + reply then removed it again. A flicker every few keystrokes, on a card she + had just answered. +- After a **reload** the client knew nothing at all — and with the server + unreachable, which is the case the rule pack exists for, the reply that + would have corrected it never comes. The dismissed card simply stays. + +**Implemented:** + +- `GET /docs/{id}/settled` (`handlers.go`) — the normalized originals of every + accepted or dismissed row on the document. Scoped through `documents` like + `fetchPending`, and for the same reason: an `original` is a verbatim + quotation of her sentence, so an unscoped read here leaks prose to anyone + holding a doc id. Wrapped in an object rather than returned as a bare array, + so it can grow a field later. +- `lib/settled.ts` — `SettledSpans`, and a TypeScript `normalizeForDedup` + mirroring the Go one. Keyed on the **original alone**, which is how the + server keys it: dismissing an edit settles the span, not one rewrite of it. +- `useCheckpoint.ts` — the set is seeded from that endpoint when the document + opens and added to by `removeSuggestion` for *every* card that leaves, not + just provisional ones. The load `add`s rather than assigns, so a card she + dismisses while the fetch is in flight isn't forgotten when it lands. + +**The status-bar summary** is `petalsToPolish(n)` in the three packs plus six +lines in `StatusBar.tsx`. Two decisions worth keeping: + +- **Nothing is shown at zero.** An empty rail already says there is nothing + waiting; a badge that appears after every check to announce it is a verdict + on each pass, which is the pressure this review's own non-goals rule out. +- **Native half first**, against the item's example, which wrote it + English-first. Everything else in Petal leads with the pair language — it is + the order the packs use and the one item 6 settled — and the status bar is + not the place to be inconsistent about it. + +**A duplicated function, deliberately, with the duplication tested.** The +server normalizes the spans it sends and the client normalizes the findings it +compares against them, across a network boundary, in two languages. A drift +there is silent — a dismissed card quietly coming back — so the same nine +cases are asserted on both sides (`TestNormalizeMatchesTheClient` and the head +of `settled.test.ts`), each naming the other and saying: add to both or +neither. + +**Verified in a real browser at the review's own 1517×810**, on a local build +with no model (the rule pack needs none), against a fresh database: + +- Five findings in one paragraph; the bar read `🌸 5片花瓣待打磨 · 5 petals to + polish`. Dismissed "a apple" → **4**, live, and the underline went with it. +- Typed elsewhere so the 250 ms pass ran: the dismissed card **did not come + back** — the in-session half. +- **Reloaded.** `/settled` fires alongside `/suggestions` at doc open (both at + 269 ms). Typed again: still gone, though the text still contains "a apple" + and the detector had flagged that exact string twenty minutes earlier. +- **Killed the server and kept typing.** A *new* violation ("a office") was + detected, underlined and counted with no network at all — proving the local + pass really was running — while the dismissed span stayed gone, and the bar + went to 5 next to "Couldn't save". That is the case the whole item is worth + anything for, and it is the one the old code could not have passed. + +**An observation, not fixed, and not this item's:** "He walk to a office" got +a card for the article and none for the verb. `subjectVerbAgreement` in +`prose.ts` catches "She have" but not "He walk", so it is narrower than item +3b's summary of it implies. Untouched here — a rule-pack gap belongs with +whoever next opens `prose.ts`. + +**A trap worth recording, and it is the eighth session's trap wearing a +different hat.** The first local run served a bundle hash that didn't match +`web/dist` — because a *stale petal from an earlier session was still holding +the port* and the new process died unbound. Same failure mode as last time, +different cause: the check that catches it is the same one, comparing the +served `index-*.js` against `dist/index.html` **before** believing anything on +screen. + +Coverage: `settled_test.go` (accepted and dismissed both settle, pending never +does, normalization collapses two spellings into one, the empty case is a list +and not a `null` the client would throw on, and the mirrored normalize table), +an isolation subtest proving a stranger reads no settled span from her +document, `settled.test.ts` (the mirrored table, plus the in-flight-dismissal +case the `add`-don't-assign choice exists for), and an `i18n.test.ts` case that +every pack counts in both halves, keeps its English half in English, and knows +one from many. + +**Still open in item 8:** Accept All per category, and the keyboard triage +flow. Both untouched. + --- ## Explicit non-goals (from this review) @@ -837,6 +943,18 @@ restart that silently failed left an old binary serving an old bundle through two rounds of measurement, and nothing about the running app looked wrong. Untouched: 8, item 3's incremental half.)* +*(Ninth session: item 8's dismissal persistence and status-bar summary done — +see the subsection under item 8. Two things to carry forward. First, **"the +server already does this" is not the same as "Petal does this"**: every +suppression the item asked for was in place and tested, and the defect lived +entirely in the 250 ms pass that by design never asks the server. Any item +whose answer is "the server handles it" should now be checked against the +offline path too, because since item 3b there is always one. Second, **a stale +server from an earlier session can hold the port**, so a new binary dies +unbound and the old bundle keeps serving — the eighth session's lesson with a +different cause, and the same bundle-hash check catches it. Untouched: item +8's Accept All and keyboard flow, item 3's incremental half.)* + **Migration 0015 on the live database.** It rebuilds the suggestions table, so unlike 0014 it could have dropped her rows. Backed up first — and the backup had to be the whole WAL set (`petal.db`, `-wal`, `-shm` in @@ -851,12 +969,21 @@ CHECK, and every existing row still carrying the label she has already read (the seven `clarity` rows include the mislabelled Chinese one — by design, only new findings get the new type; if you want that card relabelled, edit the sentence). -**Suggested next (eighth session onward):** only **item 8**'s four small ones -and item 3's incremental half remain. **Dismissal persistence** is still the one -with real value now that item 2 gives suggestions stable identity across checks; -the status-bar summary is the cheapest. Item 3's incremental surfacing needs -streaming, which the current `/check` response shape doesn't do — it remains the -largest of what's left. +**Suggested next (ninth session onward):** three things remain in the whole +review. **Accept All per category** is the one with real value left — five +tense fixes are still five clicks, and item 2's stable ids make a batch safe to +reason about; the one design question it has to answer is that its undo must be +a single step. **Keyboard triage** is next, and is bigger than it looks: item 7 +made the anchored popover the primary surface in both layouts, so "cycle the +underlines" now means driving that popover, not the rail. **Item 3's +incremental surfacing** is still the largest — it needs a streaming `/check`, +which the current response shape doesn't do — and it is the only one left that +changes how the app *feels* rather than what it can do. + +*(Superseded, kept for the reading list: the eighth session recommended +dismissal persistence and the status-bar summary, both now done. Its reasoning +— that stable identity from item 2 made dismissal worth doing — was right, but +for a different reason than it supposed: see the subsection under item 8.)* *(Superseded, kept for the reading list: the seventh session recommended item 6, which is now done.)* **item 6** was the obvious pick — diff --git a/internal/suggestions/handlers.go b/internal/suggestions/handlers.go index 57b1a89..f2d1e40 100644 --- a/internal/suggestions/handlers.go +++ b/internal/suggestions/handlers.go @@ -56,6 +56,7 @@ func (h *Handler) RegisterDocRoutes(r chi.Router) { r.Post("/{id}/collocation", h.collocation) r.Post("/{id}/rewrite", h.rewrite) r.Get("/{id}/suggestions", h.listForDoc) + r.Get("/{id}/settled", h.listSettled) } // Routes returns the router mounted at /api/suggestions for per-suggestion @@ -567,6 +568,76 @@ func (h *Handler) listForDoc(w http.ResponseWriter, r *http.Request) { httputil.WriteJSON(w, http.StatusOK, out) } +// listSettled returns the normalized originals of every edit the user has +// already accepted or dismissed on this document — the same spans buildSuppressor +// drops on the server, handed to the client so its instant rule-pack pass can +// drop them too. +// +// Without this the offline half of the loop has no memory. The rule pack detects +// from the text alone and re-runs 250 ms after a keystroke, so a dismissed "the +// the" comes straight back the moment she types anywhere in the document; the +// server's reply then removes it again. That flicker is the visible symptom, but +// the real one is worse: with the server unreachable — the case the rule pack +// exists for — the reply never comes and a card she dismissed simply stays. +// +// Only the originals are sent. Replacements are the model's words, not hers, and +// the client only needs to answer "has she settled this span?" +func (h *Handler) listSettled(w http.ResponseWriter, r *http.Request) { + out, err := h.fetchSettled(auth.UserID(r.Context()), chi.URLParam(r, "id")) + if err != nil { + httputil.ServerError(w, err) + return + } + httputil.WriteJSON(w, http.StatusOK, settledResponse{Originals: out}) +} + +// settledResponse wraps the list so the endpoint can grow a second field without +// breaking a client that reads a bare array. +type settledResponse struct { + Originals []string `json:"originals"` +} + +// fetchSettled loads the distinct normalized originals of the document's actioned +// rows. Scoped through documents for the same reason fetchPending is: an original +// is a quotation of her writing. +func (h *Handler) fetchSettled(userID, docID string) ([]string, error) { + rows, err := h.DB.Query( + `SELECT DISTINCT s.original + FROM suggestions s + JOIN documents d ON d.id = s.doc_id + WHERE s.doc_id = ? AND d.user_id = ? AND s.status IN (?, ?)`, + docID, userID, db.SuggestionStatusAccepted, db.SuggestionStatusRejected, + ) + if err != nil { + return nil, err + } + defer rows.Close() + + // DISTINCT is on the raw text; normalizing can collapse two rows into one, so + // dedupe again on this side to keep the payload honest. + seen := map[string]struct{}{} + out := []string{} + for rows.Next() { + var original string + if err := rows.Scan(&original); err != nil { + return nil, err + } + norm := normalizeForDedup(original) + if norm == "" { + continue + } + if _, dup := seen[norm]; dup { + continue + } + seen[norm] = struct{}{} + out = append(out, norm) + } + if err := rows.Err(); err != nil { + return nil, err + } + return out, nil +} + // fetchPending loads a document's pending suggestions, joined through documents // so the rows are only reachable by the document's owner. A suggestion quotes the // sentence it corrects, so an unscoped read here would leak document text to diff --git a/internal/suggestions/isolation_test.go b/internal/suggestions/isolation_test.go index 932a3a1..ee75c14 100644 --- a/internal/suggestions/isolation_test.go +++ b/internal/suggestions/isolation_test.go @@ -116,4 +116,14 @@ func TestSuggestionIsolation(t *testing.T) { if rec.Code != http.StatusNoContent { t.Fatalf("owner accept = %d, want 204 (body: %s)", rec.Code, rec.Body) } + + // That accept created a settled span, which is the other read of this table. + // It carries originals only — but an original is a verbatim quotation of her + // sentence, so it is the same leak as the pending list through a smaller hole. + if got := getSettled(t, owner, docID); len(got) != 1 { + t.Fatalf("owner should see their own settled span, got %v", got) + } + if got := getSettled(t, stranger, docID); len(got) != 0 { + t.Fatalf("stranger read %d settled span(s) (leaking %q)", len(got), got[0]) + } } diff --git a/internal/suggestions/settled_test.go b/internal/suggestions/settled_test.go new file mode 100644 index 0000000..635c45b --- /dev/null +++ b/internal/suggestions/settled_test.go @@ -0,0 +1,142 @@ +package suggestions + +import ( + "encoding/json" + "net/http" + "testing" + + "gitea.parodia.dev/drwily/petal/internal/db" +) + +// The settled endpoint exists for the offline half of the loop. The rule pack +// detects from the text alone, 250 ms after a keystroke, and has no memory +// between runs — so without the document's record of what she has already +// answered, a dismissed finding is re-detected and re-rendered on the next +// keystroke, and stays there for as long as the server can't be reached. + +func getSettled(t *testing.T, srv http.Handler, docID string) []string { + t.Helper() + rec := do(t, srv, http.MethodGet, "/docs/"+docID+"/settled", "") + if rec.Code != http.StatusOK { + t.Fatalf("settled: code=%d body=%s", rec.Code, rec.Body) + } + var out struct { + Originals []string `json:"originals"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v", err) + } + return out.Originals +} + +func contains(list []string, want string) bool { + for _, s := range list { + if s == want { + return true + } + } + return false +} + +// TestSettledListsActionedSpans proves the endpoint reports exactly the spans the +// suppressor would drop: accepted and dismissed, never pending. A pending row +// leaking in would be the damaging direction — the client would hide a card she +// has never been shown an answer to. +func TestSettledListsActionedSpans(t *testing.T) { + client := &stubClient{response: `{"suggestions":[ + {"original":"I has","replacement":"I have","explanation":"agreement","type":"grammar"}, + {"original":"two apple","replacement":"two apples","explanation":"plural","type":"grammar"} + ]}`} + srv, docID, _ := newTestServer(t, client) + + if got := getSettled(t, srv, docID); len(got) != 0 { + t.Fatalf("nothing actioned yet, got %v", got) + } + + rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "") + var got []db.Suggestion + _ = json.Unmarshal(rec.Body.Bytes(), &got) + if len(got) != 2 { + t.Fatalf("first pass: want 2, got %d", len(got)) + } + + // One accepted, one still pending: only the accepted span is settled. + do(t, srv, http.MethodPost, "/suggestions/"+got[0].ID+"/accept", "") + settled := getSettled(t, srv, docID) + if len(settled) != 1 || settled[0] != got[0].Original { + t.Fatalf("want just %q settled, got %v", got[0].Original, settled) + } + + // A dismissal settles a span just as an accept does — the whole point of the + // item: "you already decided about this one" doesn't mean "you agreed". + do(t, srv, http.MethodPost, "/suggestions/"+got[1].ID+"/dismiss", "") + settled = getSettled(t, srv, docID) + if len(settled) != 2 || !contains(settled, got[1].Original) { + t.Fatalf("dismissed span missing from %v", settled) + } +} + +// TestSettledNormalizesAndDedupes proves the payload is normalized server-side +// and collapsed. The client compares its freshly-detected findings against these +// strings, so the two sides have to agree on what "the same span" is — the +// editor's quote churn is the case that breaks a byte-exact match, and it is why +// normalizeForDedup exists at all. +func TestSettledNormalizesAndDedupes(t *testing.T) { + client := &stubClient{response: `{"suggestions":[]}`} + srv, docID, h := newTestServer(t, client) + + // The same span twice, differing only in quote style and line breaks — one + // accepted, one dismissed. Distinct rows; one settled span. + a := seedSuggestion(t, h, docID, "text", db.SuggestionTypeGrammar, + "She said \"hello\"\n to me", "She said 'hello' to me", "quotes") + b := seedSuggestion(t, h, docID, "text", db.SuggestionTypeGrammar, + "She said “hello” to me", "She said 'hello' to me", "quotes") + do(t, srv, http.MethodPost, "/suggestions/"+a+"/accept", "") + do(t, srv, http.MethodPost, "/suggestions/"+b+"/dismiss", "") + + settled := getSettled(t, srv, docID) + if len(settled) != 1 { + t.Fatalf("two spellings of one span should collapse to one, got %v", settled) + } + if want := "She said 'hello' to me"; settled[0] != want { + t.Fatalf("settled[0] = %q, want normalized %q", settled[0], want) + } +} + +// TestNormalizeMatchesTheClient is the Go half of a pair. Every case here also +// appears in web/src/lib/settled.test.ts, asserted against the TypeScript +// reimplementation of this function. The two are compared across a network +// boundary — the server normalizes what it sends, the client normalizes what it +// checks against it — so they have to fold the same characters the same way, and +// nothing but a shared list of cases can say so. Add to both or neither. +func TestNormalizeMatchesTheClient(t *testing.T) { + cases := []struct{ in, want string }{ + {"She said “hello”", "She said 'hello'"}, + {"She said \"hello\"", "She said 'hello'"}, + {"it‘s", "it's"}, + {"it’s", "it's"}, + {"`code´", "'code'"}, + {" a apple\n here ", "a apple here"}, + {"a\tapple", "a apple"}, + {" \n ", ""}, + {"我想说这句话", "我想说这句话"}, + } + for _, c := range cases { + if got := normalizeForDedup(c.in); got != c.want { + t.Errorf("normalizeForDedup(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +// TestSettledEmptyIsAList guards the shape rather than the content: the client +// spreads this array into its settled set, and a null would throw there. Go +// marshals a nil slice as null, so this is one `[]string{}` away from breaking. +func TestSettledEmptyIsAList(t *testing.T) { + client := &stubClient{response: `{"suggestions":[]}`} + srv, docID, _ := newTestServer(t, client) + + rec := do(t, srv, http.MethodGet, "/docs/"+docID+"/settled", "") + if body := rec.Body.String(); body != "{\"originals\":[]}\n" && body != "{\"originals\":[]}" { + t.Fatalf("empty settled body = %q, want an empty list", body) + } +} diff --git a/web/src/App.tsx b/web/src/App.tsx index bc3fa11..bf9a289 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -589,6 +589,7 @@ export default function App() { voicing={voicing} collocating={collocating} llmDown={llmDown} + suggestionCount={suggestions.length} /> diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 4b963a8..77b8a6d 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -298,6 +298,12 @@ export const api = { }), // Pending suggestions for a doc, loaded when the editor opens it. listSuggestions: (id: string) => req(`/docs/${id}/suggestions`), + // The spans she has already accepted or dismissed on this doc, normalized. The + // server suppresses these itself; the client needs them so the instant rule-pack + // pass doesn't hand back a dismissed card before the server can say otherwise — + // or, with the server unreachable, at all. See lib/settled.ts. + listSettled: (id: string) => + req<{ originals: string[] }>(`/docs/${id}/settled`), acceptSuggestion: (id: string) => req(`/suggestions/${id}/accept`, { method: 'POST' }), dismissSuggestion: (id: string) => diff --git a/web/src/components/StatusBar/StatusBar.tsx b/web/src/components/StatusBar/StatusBar.tsx index f0fc713..e3b2fd2 100644 --- a/web/src/components/StatusBar/StatusBar.tsx +++ b/web/src/components/StatusBar/StatusBar.tsx @@ -19,6 +19,9 @@ interface Props { // True when Petal can't reach its LLM helper — shows a gentle, reassuring note // (the writing still saves locally, so this is awareness, not an error). llmDown: boolean + // How many suggestions the rail is holding. Shown as a soft bilingual line + // beside the word count; hidden at zero. + suggestionCount: number } // Save-state labels. English except for the lapsed-session case, which is the @@ -48,9 +51,23 @@ interface Indicator { label: string } -export function StatusBar({ wordCount, text, saveStatus, checking, voicing, collocating, llmDown }: Props) { +export function StatusBar({ + wordCount, + text, + saveStatus, + checking, + voicing, + collocating, + llmDown, + suggestionCount, +}: Props) { const t = usePack() const label = saveLabel(saveStatus, t) + // The gentle counterpart of Grammarly's score: how much is waiting, never how + // well she wrote. Nothing waiting says itself — an empty rail — so a zero here + // would only be a verdict delivered after every check, which is the pressure + // this app's non-goals rule out. + const petals = suggestionCount > 0 ? t.status.petalsToPolish(suggestionCount) : null const indicators: Indicator[] = [ { @@ -109,6 +126,18 @@ export function StatusBar({ wordCount, text, saveStatus, checking, voicing, coll {statsOpen && } + {petals && ( + <> + · + {/* Native half first, as everywhere else in Petal. (The review wrote it + English-first; the pack's order is the one she reads all day.) */} + + 🌸 + {petals.native} + · {petals.en} + + + )} {indicators .filter((i) => i.active) .map((i) => ( diff --git a/web/src/hooks/useCheckpoint.ts b/web/src/hooks/useCheckpoint.ts index b4b578acd1688aa67bdab41c0bde0e5b3b7dc042..d38d2505c5f2ab5f1bf2910ceb60c363f7d5c5a9 100644 GIT binary patch delta 1471 zcmZWp&5ImG6lXUiktX0KK^zh^FS;Q!%WO|}q6b$7Sv-Z{#SuJ+keaUQ=_1`#oqE+f z8JA?e3PJ+yQ4qX((m8n5|3EL|P4MU~2%bFntL~mLA`Hw-eZ2R6U-k8azaKvRb*PKd zI!`AwRo>@HPD^2!9#QISK{w-gl4QuW<5>MBEl~cy&OZxrOFUT75xMiK5Ya?=8 zwMGq5y;)v}W%JF-9V-_~FlirSXhoX=&np>wjt8qtXa2l!DNM-gNg^W5d1dF?`OpiE z>iJIpY`gko=e1LsiR6BVtZgC6GS`aRuA)%nRAQYZZ<|}BFvrqo+RQNsvA0g~+c*t? z_yj1qO@Je@<#I@Do2624*aJgAE|p`NsT4PFwl%}84k2b=gnco03R5h!&(Np-Yv=Rb zj?4E|N)xJB;YZ)7#eW1`eRJ`%v(KH3lgb6MW2n+k<2B2JK~z8Oy}SDA)*PY z)4-&Be*o^9@jD8DuY?HFCRKr&QT?oUb@jvkcU!>#8~wX-qzx(Iz@-B=W^D~`_;6LO zs08(tSz{~U1()91nC`05gOnf}u9dat*!i&Gz6$_XWeNMiG9>U=Gz14R>|#i> z%7eoIrNo#y)WxCby~ZrgRz~i7r%Mv4@AMqMd5a3THpK7wcd107wcgDW5S}4U5IIvn zK6qz0fmuUVTgISJKL~S3(`r6f;KG`ZKCWLJzj8M$4@jrf=U7XcOeUn|pk?>}%JD|H pL1Rin3tu^dr;L4lGQPeQ)j#(xN9)E&>9gMD7uJ7QUyje6`v)`*{b>LI delta 707 zcmZuuKX21O6jxFaq^d(e0wD$H1B9>yI|t_j!NP*o|LX*AgMQkTZ8< z6JR`slxh2UxE<7~$gt&DLXC>f7?EXAsJs$duoRp=W;%?>_I2Oj&%i zVnchii%|@4LON(#UaTR6sWY&205s_*7oa_4?c?x@gP6I1VSh^(qsP+ z&Av8@Bu9}au?8A6)2r_;-Cu(Taj6s$1mwkAIedXjFY%kL8#~uW|AWREGUfS=3BL83 z_qz{!lV_ckb;mIyxuT6)U-Uql8I7D;`~C6lm)#!a7pG*AaZN2{OK0BT!aQ { expect(native).not.toBe(en) }) + // The status-bar count is the one line in Petal that grows a number, so it is + // the one that can be quietly ungrammatical in three languages at once — and + // the count itself has to survive translation, since it is the whole content. + it.each(PACKS)('counts petals to polish in both halves, and agrees on the number ($code)', (p) => { + for (const n of [1, 2, 5, 21]) { + const { native, en } = p.status.petalsToPolish(n) + expect(native, `${p.code} has no pair-language half for n=${n}`).toBeTruthy() + expect(en, `${p.code} has no English half for n=${n}`).toBeTruthy() + expect(native, `${p.code} drops the count from its native half`).toContain(String(n)) + expect(en).toContain(String(n)) + // English is the half every pack shares; a pack that translated it has lost + // the point, exactly as with chatFailed above. + expect(en).toMatch(/petals? to polish/) + } + // One is not many, in every language Petal ships. + expect(p.status.petalsToPolish(1).native).not.toBe(p.status.petalsToPolish(2).native) + expect(p.status.petalsToPolish(1).en).toContain('petal to polish') + expect(p.status.petalsToPolish(2).en).toContain('petals to polish') + }) + it.each(PACKS)('labels every companion, tone and style ($code)', async (p) => { const { COMPANIONS } = await import('../components/Companion/companions') for (const c of COMPANIONS) { diff --git a/web/src/i18n/packs/fr.ts b/web/src/i18n/packs/fr.ts index 702031a..465ff3e 100644 --- a/web/src/i18n/packs/fr.ts +++ b/web/src/i18n/packs/fr.ts @@ -450,6 +450,10 @@ export const fr: Pack = { savedLocally: 'Gardé sur cet appareil · Kept on this device', helperRestingNative: 'L’assistant se repose', helperRestingEn: "· Petal's helper is resting · ton texte est enregistré", + petalsToPolish: (n) => ({ + native: `${n} ${n === 1 ? 'pétale' : 'pétales'} à polir`, + en: `${n} ${n === 1 ? 'petal' : 'petals'} to polish`, + }), soundsOn: 'Sons activés · Sounds on', soundsOff: 'Sons coupés · Sounds off', petalsOn: 'Pétales activés · Petals on', diff --git a/web/src/i18n/packs/pt-PT.ts b/web/src/i18n/packs/pt-PT.ts index 2f517c8..24874a2 100644 --- a/web/src/i18n/packs/pt-PT.ts +++ b/web/src/i18n/packs/pt-PT.ts @@ -427,6 +427,10 @@ export const ptPT: Pack = { savedLocally: 'Guardado neste dispositivo · Kept on this device', helperRestingNative: 'O ajudante está a descansar', helperRestingEn: "· Petal's helper is resting · o teu texto está guardado", + petalsToPolish: (n) => ({ + native: `${n} ${n === 1 ? 'pétala' : 'pétalas'} para polir`, + en: `${n} ${n === 1 ? 'petal' : 'petals'} to polish`, + }), soundsOn: 'Som ligado · Sounds on', soundsOff: 'Som desligado · Sounds off', petalsOn: 'Pétalas ligadas · Petals on', diff --git a/web/src/i18n/packs/zh.ts b/web/src/i18n/packs/zh.ts index 480d321..4e05863 100644 --- a/web/src/i18n/packs/zh.ts +++ b/web/src/i18n/packs/zh.ts @@ -327,6 +327,12 @@ export const zh: Pack = { savedLocally: '已保存在本机 · Kept on this device', helperRestingNative: '小助手在休息', helperRestingEn: "· Petal's helper is resting · 文字已保存", + // 片 is the measure word for petals; a digit reads perfectly naturally in + // Chinese and spares every pack a numeral table. + petalsToPolish: (n) => ({ + native: `${n}片花瓣待打磨`, + en: `${n} ${n === 1 ? 'petal' : 'petals'} to polish`, + }), soundsOn: '声音开 · Sounds on', soundsOff: '声音关 · Sounds off', petalsOn: '花瓣开 · Petals on', diff --git a/web/src/i18n/types.ts b/web/src/i18n/types.ts index 8a06126..a56bd85 100644 --- a/web/src/i18n/types.ts +++ b/web/src/i18n/types.ts @@ -297,6 +297,11 @@ export interface Pack { savedLocally: string helperRestingNative: string helperRestingEn: string + // How many suggestions are waiting, said gently — the status-bar counterpart + // of the rail. Never a score: it counts what is there, and says nothing about + // how well she is writing. Only ever called with n >= 1 (nothing waiting is + // said by the rail being empty, not by a badge announcing it). + petalsToPolish: (n: number) => Line soundsOn: string soundsOff: string petalsOn: string diff --git a/web/src/lib/settled.test.ts b/web/src/lib/settled.test.ts new file mode 100644 index 0000000..4e5276d --- /dev/null +++ b/web/src/lib/settled.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from 'vitest' +import { SettledSpans, normalizeForDedup } from './settled' + +describe('normalizeForDedup', () => { + // These cases are the client half of a pair: every one of them is also asserted + // against the Go implementation in TestNormalizeMatchesTheClient. The server + // normalizes the spans it sends and the client normalizes the findings it + // checks against them, so a divergence would be silent — a dismissed card + // quietly coming back. Add a case to both or neither. + it('folds every quote variant onto one character', () => { + expect(normalizeForDedup('She said “hello”')).toBe("She said 'hello'") + expect(normalizeForDedup('She said "hello"')).toBe("She said 'hello'") + expect(normalizeForDedup('it‘s')).toBe("it's") + expect(normalizeForDedup('it’s')).toBe("it's") + expect(normalizeForDedup('`code´')).toBe("'code'") + }) + + it('collapses runs of whitespace and trims', () => { + expect(normalizeForDedup(' a apple\n here ')).toBe('a apple here') + expect(normalizeForDedup('a\tapple')).toBe('a apple') + }) + + it('is empty for whitespace only, so it can never settle everything', () => { + expect(normalizeForDedup(' \n ')).toBe('') + }) + + it('leaves her Chinese alone', () => { + expect(normalizeForDedup('我想说这句话')).toBe('我想说这句话') + }) +}) + +describe('SettledSpans', () => { + it('recognises a span it was told about, in any spelling', () => { + const settled = new SettledSpans() + settled.add('a apple') + expect(settled.has('a apple')).toBe(true) + expect(settled.has('a apple')).toBe(true) + expect(settled.has('an apple')).toBe(false) + }) + + it('takes the document record and later dismissals through the same door', () => { + const settled = new SettledSpans() + settled.add('the the', 'a apple') // as loaded on open + settled.add('two apple') // as dismissed just now + expect(settled.has('the the')).toBe(true) + expect(settled.has('two apple')).toBe(true) + }) + + // The load is async and she can dismiss a card while it is in flight. Adding + // rather than assigning is what keeps that dismissal — an assignment here would + // hand the card straight back, which is the bug this whole file is about. + it('keeps a dismissal made before the document record lands', () => { + const settled = new SettledSpans() + settled.add('two apple') // she dismisses + settled.add('the the') // the fetch lands afterwards + expect(settled.has('two apple')).toBe(true) + expect(settled.has('the the')).toBe(true) + }) + + it('forgets everything on reset, because the record is per document', () => { + const settled = new SettledSpans() + settled.add('a apple') + settled.reset() + expect(settled.has('a apple')).toBe(false) + }) + + it('ignores an empty span rather than storing one nothing can match', () => { + const settled = new SettledSpans() + settled.add(' ') + expect(settled.has('')).toBe(false) + expect(settled.has('anything')).toBe(false) + }) +}) diff --git a/web/src/lib/settled.ts b/web/src/lib/settled.ts new file mode 100644 index 0000000..bc89ebb --- /dev/null +++ b/web/src/lib/settled.ts @@ -0,0 +1,57 @@ +// Whether a span is one she has already settled — accepted or dismissed. +// +// The rule pack (Companion/prose.ts) detects from the document text alone and has +// no memory between runs, so something has to tell it "she has already answered +// this one". The server keeps that record and applies it to everything it stores +// (see buildSuppressor in internal/suggestions/handlers.go); this is the same +// question asked locally, for the 250 ms pass that renders before — and, offline, +// instead of — the server's reply. +// +// Keyed on the original alone, deliberately, because that is how the server keys +// it: dismissing an edit settles the span, not one particular rewrite of it. + +// normalizeForDedup mirrors the Go function of the same name: quote variants +// folded onto one character, runs of whitespace collapsed. The editor rewrites +// quotes as she types and a reflowed paragraph changes its line breaks, so +// comparing raw text would miss spans that are plainly the same. +// +// The two implementations must agree, because the server normalizes the strings +// it sends and the client normalizes the findings it compares against them. They +// differ only on characters neither her writing nor the model produces (JS folds +// a BOM as whitespace, Go folds U+0085); a mismatch there costs one redundant +// card, not a wrong one. +export function normalizeForDedup(s: string): string { + return s.replace(/[‘’‚‛“”„″"`´]/g, "'").trim().split(/\s+/).join(' ') +} + +// SettledSpans answers "has she already dealt with this?" for a rule-pack +// finding. Built from the server's record when the document opens, and added to +// as she accepts or dismisses, so the answer is right for cards the server has +// never heard of — the provisional ones. +export class SettledSpans { + private spans = new Set() + + // Forget everything (on switching documents — the record is per document). + reset(): void { + this.spans = new Set() + } + + // Remember a span she has just actioned. Called for every card that leaves the + // rail, not only the provisional ones: a persisted card is suppressed by the + // server, but its reply arrives a round-trip after the local pass has already + // put the card back on screen. + // + // The document's stored record is loaded through this same door rather than by + // replacing the set, so a card she dismisses while that fetch is in flight isn't + // forgotten when it lands. + add(...originals: string[]): void { + for (const original of originals) { + const norm = normalizeForDedup(original) + if (norm) this.spans.add(norm) + } + } + + has(original: string): boolean { + return this.spans.has(normalizeForDedup(original)) + } +}