diff --git a/UX_REVIEW_2026-07-27.md b/UX_REVIEW_2026-07-27.md index 951dc57..238d9d3 100644 --- a/UX_REVIEW_2026-07-27.md +++ b/UX_REVIEW_2026-07-27.md @@ -166,6 +166,52 @@ in the status bar. Two independent levers, both worth doing: offline; during a full check, at least one card appears before the last chunk finishes; the status bar shows a running count. +### 3b — DONE (third session). The rules existed; the latency didn't. + +Scoped against the code as the handoff advised, and the handoff was right: +`prose.ts` already carries every rule this item asks for — `articles` +(a/an), `pluralAfterNumber`, `subjectVerbAgreement`, `uncountables` — and +they already surface as real cards via the `mechanics` family. Nothing to +write there. The gap was purely *when* they render: `mechanicsFindings` ran +only inside `runCheck`, behind the same 4000 ms checkpoint debounce as the +LLM, and only reached the screen via the server's reply. So a free, +instant, offline-capable detection was being delivered at network speed on +an LLM-shaped delay. + +**Implemented:** +- `useCheckpoint.ts` — the rule pack gets its own `FAST_MS = 250` fuse, + separate from the 4 s checkpoint. It renders its findings as + *provisional* suggestions with no network at all, then persists them; the + server's reply is authoritative and clears the provisional set. If the + reply never comes (offline, server down) the cards simply stay — which is + the point of a rule pack. +- Provisional cards carry a `local: ` id. The merge + matches on wording, not position, so a card can't flicker into a + duplicate of its own persisted twin while she types around it. +- `resolveServerId` maps a card to the row the API can act on, awaiting the + in-flight submit if she accepts inside that window — so an early accept + still records the keep and plants its word in the garden instead of being + silently dropped. Null means no row exists (offline); the edit has landed + regardless. +- Findings she actions while provisional are remembered client-side + (`actionedRef`), because the detector has no memory between runs. The + server already keeps the equivalent record for persisted rows. +- `runCheck` no longer re-submits mechanics for text the fast pass already + filed; it's now a catch-up path for when that submit failed. +- `App.tsx` — accept/dismiss go through `resolveServerId`; the arrival + chime keys rule-pack cards by wording so one finding doesn't chime twice + (once provisional, once persisted). + +**Deliberately not done:** no distinct "modest style" for unconfirmed local +hits. The rail renders LLM and rule-pack cards identically on purpose (see +the note on `Suggestion.source` in `client.ts`), and a provisional card now +lives for one LAN round-trip. Styling it differently would be a visible +regression against an existing decision, not polish. + +**Still open from item 3:** the incremental-surfacing half (per-chunk LLM +results) and the running count in the status bar — both belong with item 2's +chunking and item 8's status-bar summary. + ## 4. Rail scrolls away from the text With ~7 cards the rail is taller than the viewport; scrolling to reach @@ -289,6 +335,9 @@ if you're comparing against memory of the live site, that's why. reproducible) and item 5's original premise (re-scoped, much cheaper now). **Untouched:** items 2, 3, 4, 6, 7, 8. +*(Third session: item 3b done — see the subsection under item 3. Item 3's +incremental-surfacing half remains. Untouched: 2, 4, 6, 7, 8.)* + **Suggested next:** item 3b, the instant local rules layer — but it is **largely already built, in `main`**, and the item as written doesn't know that. Before writing any rules engine, read: diff --git a/web/src/App.tsx b/web/src/App.tsx index ff2b504..95606cf 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -1,7 +1,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { api, type DocSummary, type DocUpdate, type Document, type Suggestion, type Tag, type TagColor } from './api/client' import { useAutoSave } from './hooks/useAutoSave' -import { useCheckpoint } from './hooks/useCheckpoint' +import { findingKey, useCheckpoint } from './hooks/useCheckpoint' import { useSpellChecker } from './hooks/useSpellChecker' import { useTags } from './hooks/useTags' import { DocList } from './components/DocList/DocList' @@ -90,6 +90,7 @@ export default function App() { runVoice, runCollocation, removeSuggestion, + resolveServerId, } = useCheckpoint(currentDoc?.id ?? null) // Browser-side spell checker — loads the en-US dictionary once per session. const { checker: spellChecker, addWord } = useSpellChecker() @@ -350,17 +351,20 @@ export default function App() { // Accept applies the replacement in the editor (handled in EditorCore) and // marks the suggestion accepted; dismiss just rejects it. Both drop it locally. + // A rule-pack card can be accepted before its row exists — the edit has already + // landed either way, so a missing id just means there's nothing to file. const handleAccept = useCallback( async (s: Suggestion) => { removeSuggestion(s.id) setAcceptTick((n) => n + 1) try { - await api.acceptSuggestion(s.id) + const id = await resolveServerId(s) + if (id) await api.acceptSuggestion(id) } catch (err) { console.error('accept failed', err) } }, - [removeSuggestion], + [removeSuggestion, resolveServerId], ) // After restoring a version, swap the restored doc into the editor. Bumping @@ -398,12 +402,13 @@ export default function App() { async (s: Suggestion) => { removeSuggestion(s.id) try { - await api.dismissSuggestion(s.id) + const id = await resolveServerId(s) + if (id) await api.dismissSuggestion(id) } catch (err) { console.error('dismiss failed', err) } }, - [removeSuggestion], + [removeSuggestion, resolveServerId], ) // Play a soft sound when freshly-checked suggestions arrive — one per distinct @@ -411,10 +416,14 @@ export default function App() { // a pile-up. We track which ids we've already chimed for, and only chime for // recently-created suggestions so opening a doc with old pending advice stays // silent (the existing set was created in a past session). + // Rule-pack findings are chimed by their wording, not their id: the same fix + // appears first as a provisional card and then as its persisted row, and the + // writer should hear it once. const chimedRef = useRef>(new Set()) useEffect(() => { - const fresh = suggestions.filter((s) => !chimedRef.current.has(s.id)) - fresh.forEach((s) => chimedRef.current.add(s.id)) + const key = (s: Suggestion) => (s.source === 'local' ? `local:${findingKey(s)}` : s.id) + const fresh = suggestions.filter((s) => !chimedRef.current.has(key(s))) + fresh.forEach((s) => chimedRef.current.add(key(s))) const justMade = fresh.filter( (s) => Date.now() - new Date(s.created_at).getTime() < 12_000, ) diff --git a/web/src/hooks/useCheckpoint.ts b/web/src/hooks/useCheckpoint.ts index a0a8f1f..b4b578a 100644 Binary files a/web/src/hooks/useCheckpoint.ts and b/web/src/hooks/useCheckpoint.ts differ