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 b4b578a..d38d250 100644 Binary files a/web/src/hooks/useCheckpoint.ts and b/web/src/hooks/useCheckpoint.ts differ diff --git a/web/src/i18n/i18n.test.ts b/web/src/i18n/i18n.test.ts index c1cea2c..d38ad20 100644 --- a/web/src/i18n/i18n.test.ts +++ b/web/src/i18n/i18n.test.ts @@ -180,6 +180,26 @@ describe('the zh pack', () => { 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)) + } +}