Merge the suggestion-loop stack: instant rules, stable cards, reachable rail

Three sessions of UX_REVIEW work land together, because they are one
change to how suggestions arrive and sit:

- item 3b — the deterministic rule pack renders on its own 250 ms fuse
  instead of waiting behind the LLM's 4 s checkpoint and a round-trip.
- item 2 — passes reconcile instead of replacing, so an untouched card
  keeps its id, its arrival chime and its original explanation, and an
  unchanged document doesn't call the model at all.
- item 4 — the rail's overhang becomes real scrollable page, with the
  prose pinned bottom-anchored so the sentences the lower cards flag
  stay on screen.

Green: go test ./... , tsc --noEmit, 195 vitest tests.

Claude-Session: https://claude.ai/code/session_016y6gyuHkQXPiEuW8RGQyua
This commit is contained in:
prosolis
2026-07-27 23:37:56 -07:00
12 changed files with 1231 additions and 89 deletions
+206
View File
@@ -143,6 +143,81 @@ instability, doubles the pause after each accept, and burns qwen3.5 tokens.
or position of any other card; re-check traffic after a one-sentence edit
contains only that sentence's chunk; explanations are stable across rounds.
### 2 — DONE (fourth session). Server-side; the client needed nothing.
Both halves shipped, and they turned out to be one idea. The root cause of
the vanish/reappear was structural: **every pass deleted its whole family
and re-inserted it**, so each round minted new row ids. The rail keys its
cards on `suggestion.id`, so a full remount was guaranteed — new id, new
`created_at` (hence the re-fired arrival chime), and a freshly-worded
explanation from a model that re-reasons every time it's asked.
**Implemented:**
- `chunk.go` — splits the document into sentences and hashes each. Newlines
always break; ASCII terminators need trailing whitespace (so `3.50` and
`Ms.` stay whole); `。!?` break outright, since Chinese runs sentences
together with no space and she writes both languages in one document. The
hash normalizes quotes and whitespace runs through the existing
`normalizeForDedup`, so the editor's constant quote rewriting and a
reflowed paragraph cost nothing. Identity is the hash, not the position —
insert a paragraph at the top and every sentence below keeps its cards.
- `reconcile.go` — passes now *reconcile* rather than replace. A row on a
sentence this pass didn't ask about is kept untouched; a row whose
sentence is gone is dropped; a row on a sentence that was re-read survives
only if the model proposed the same edit again, keeping its id,
`created_at` and its **original explanation**. Re-proposals are matched on
`(original, replacement)` normalized — not on type, so a re-labelled edit
keeps the label she's already reading.
- `checked_chunks` (migration `0014`) records which sentences a family has
read. The grammar checkpoint asks only about the difference. **When
nothing changed it doesn't call the model at all** — and doesn't consume
its rate-limit slot, so an idle check can't throttle the next real edit.
- The tone is folded into a sentence's hash, so switching doc type still
re-reads every line: the same sentence gets different advice as an
academic essay than as a journal entry, and cached advice was written for
the old register.
- `replaceMechanics` reconciles too. This mattered more than expected: the
rule pack fires 250 ms after a keystroke (item 3b), so it was re-minting
every local card's id several times a sentence.
**Deliberately not done:**
- Only the grammar checkpoint is chunked. Voice is a property of the
document as a whole — a sentence isn't inconsistent with itself — and the
collocation coach is a button she presses asking for a fresh read. Both
still read everything, but both now reconcile, so they keep their ids.
- No client change. With stable ids the existing code already does what the
item asked for: the rail keeps its card DOM, an expanded card survives a
re-check, and the chime (which keys on id) stops re-firing for advice she
is already reading. The one-card optimistic removal on accept was already
there.
**Sentences the model can't be trusted to have read.** Two guards the plan
didn't anticipate, both found while writing the tests: a finding is
attributed to a sentence the model was *actually shown* before falling back
to the whole document (a short span like "the the" can occur twice, and
crediting the cached copy would drop it); and a cached row whose quoted span
no longer matches byte-for-byte is dropped *and* its sentence re-opened,
rather than caching advice the frontend can't anchor.
**Verified on the running binary**, not just in tests — per the handoff's
own advice. Against a stand-in model server: three checks over a two-
sentence document, editing only the second. The model received exactly
`She goes to market yesterday.` and never saw the first sentence; the
untouched card kept its id and its first explanation across all three
rounds; the fixed sentence's card was dropped; the idle re-check made zero
model calls. Mechanics identity confirmed the same way (a finding kept its
row id while its span moved).
Coverage: `chunk_test.go` (splitting, CJK, decimals, cosmetic churn) and
`stability_test.go` (untouched cards keep id + explanation, unchanged
document skips the model, deleted sentence drops its card, tone change
re-opens everything, mechanics rows keep identity). Two existing tests
changed contract deliberately — `TestFickleEditsSuppressed` and
`TestCollocationPassCoexists` both re-checked a document nobody had edited,
which is now a no-op; they edit the text between passes, as she always does.
## 3. Perceived latency: mask the LLM round-trip
Measured ~815 s from typing-stop to cards, with only a small "Checking…"
@@ -166,6 +241,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:<original> <replacement>` 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
@@ -181,6 +302,74 @@ takes over when the stack exceeds the viewport.
**Acceptance:** with 10+ suggestions, the flagged text stays visible while
scrolling the card list; hover-linking still highlights the right span.
### 4 — DONE (fifth session). The cards weren't distant; they were unreachable.
Measured on the live build before touching anything, and the item understates
its own bug. Her open document: **four cards, 173 px each, all anchored inside
126 px of text** — the stack resolved to tops 4 / 189 / 374 / 559, so ~714 px
of cards beside four lines of prose. And because `.petal-rail` is
`position: absolute`, none of that counts as layout height: the page reported
`scrollHeight === clientHeight`, **no scroll container at all**. On the review's
810 px viewport the lower cards weren't merely severed from their sentence,
they were off-screen with no way to scroll to them. That is the real defect,
and it is why the item read as a scrolling problem.
**Implemented:**
- `SuggestionRail.tsx` — the stack reports how far it reaches (`onExtent`),
computed in the same pass that resolves the collision-avoided tops.
- `EditorCore.tsx` — the wrapper takes `minHeight: railExtent + 24`, so the
space the cards occupy becomes real, scrollable page. `minHeight` never
shrinks the column, so a rail that fits beside its text changes nothing.
- The prose moved into its own box, pinned with `position: sticky` while the
stack overhangs it, so scrolling down to reach the lower cards no longer
carries every sentence off the top. The offset is `min(0, port content)`:
prose shorter than the viewport pins at the top; **taller prose pins by its
bottom edge**, so the last lines — the ones the overhanging cards flag —
stay visible rather than the first.
- The extent is cleared when the last card goes, or the window narrows past
the rail's threshold; otherwise the column keeps the height of a stack that
no longer exists.
**A trap worth recording.** That prose box must be left at its natural height.
The first version kept the existing `h-full`, so it measured the wrapper — which
this change had just grown to the stack's height — and reported the cards'
height back as the text's own. `railExtent > contentH` was then never true and
the pin could never trip. It typechecked, looked right, and did nothing; only
measuring the running page caught it (`proseHeight: 1424` for a two-line
document).
**Verified in a real browser at the review's own 1517×810**, driving the local
build with the rule pack from item 3b — which needs no model, so eight cards
appear offline in one paragraph. All three branches exercised:
- *Overhang, short prose* — 8 cards, stack 1400 px, prose 95 px. Page gained
675 px of scroll where it previously had none; scrolled to the end, the last
card sits fully in view (770926) **and the prose is still on screen** (80175).
- *Overhang, tall prose* — port 225 px, prose 347 px → `top: 146px`. Ordinary
scrolling is untouched (at `scrollTop` 200 the text moves normally with the
page); only at the overhang does it pin, bottom-anchored, last lines visible.
- *No overhang* — the port stays unscrollable and nothing moves.
Hover-linking re-checked on the last card, the one this fix made reachable at
all: it glows the right span ("It make"), the span is on screen, the card lifts.
**Known limit, not fixed.** The overlays anchored in wrapper coordinates (gloss
tip, selection bubble, word/misspell cards, confetti) rely on the invariant
noted at `recomputeRail` — "stable under scroll since text and wrapper scroll
together" — which the pin breaks. They are still placed correctly when opened,
because their coordinates come from live rects; they drift only if she scrolls
*while one is open* *and* the column is pinned, i.e. inside the overhang. Left
alone rather than papered over; if it ever bites, the fix is to close or
re-anchor them on scroll.
**Deliberately not done:** no compaction of the cards. Making crowded cards
drop to a one-line form is the obvious way to shorten the stack, and it is
wrong here — the explanation *is* the teaching, and hiding it from an ESL
writer to save vertical space trades the product's purpose for tidiness. Ten
cards cannot sit beside four lines of text; the answer is to make the overhang
navigable, not to shrink what each card says.
## 5. Mixed-language spans: offer translation, don't ignore
**Status (follow-up session): premise partly wrong — re-scope before
@@ -289,6 +478,23 @@ 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.)*
*(Fourth session: item 2 done — see the subsection under it. **Neither 3b
nor 2 is deployed yet**: both sit on unmerged topic branches
(`feat/instant-local-rules`, then `feat/stable-suggestions` stacked on it)
and `main` is still at `ba06d90`. Untouched: 4, 6, 7, 8. Item 3's
incremental-surfacing half is now cheap — the chunking it was waiting on
exists — but it needs streaming, which the current `/check` shape doesn't
do.)*
*(Fifth session: item 4 done — see the subsection under it. Still nothing
deployed: `main` remains at `ba06d90`, and 3b → 2 → 4 are now three stacked
topic branches. **Merging and deploying that stack is the obvious next move**
— three sessions of work she hasn't seen. Untouched: 6, 7, 8, item 3's
incremental half, and item 5's re-scoped Translate card type.)*
**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:
+26
View File
@@ -497,6 +497,32 @@ CREATE INDEX idx_suggestions_resolved ON suggestions(status, resolved_at);
stmt: `
ALTER TABLE suggestions ADD COLUMN source TEXT NOT NULL DEFAULT 'llm';
UPDATE suggestions SET source = 'local' WHERE type = 'mechanics';
`,
},
{
// Sentence-level identity, so a re-check stops regenerating the world.
// Every pass used to delete its whole family and re-insert it, which
// meant accepting one edit gave every other card a new id and a newly
// worded explanation — the rail visibly emptied and refilled, and the
// model was asked again about sentences nobody had touched.
//
// `chunk_hash` records which sentence a suggestion belongs to, and
// checked_chunks records which sentences a family has already read. A
// re-check then asks only about the difference and keeps the rest of
// the rows exactly as they are, id and wording included.
//
// Existing rows get '' — "sentence unknown", which reads as in-play, so
// they are simply reconciled on the next pass like any fresh finding.
name: "0014_suggestion_chunk_hash",
stmt: `
ALTER TABLE suggestions ADD COLUMN chunk_hash TEXT NOT NULL DEFAULT '';
CREATE TABLE checked_chunks (
doc_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE,
family TEXT NOT NULL,
hash TEXT NOT NULL,
PRIMARY KEY (doc_id, family, hash)
);
`,
},
}
+145
View File
@@ -0,0 +1,145 @@
package suggestions
import (
"crypto/sha256"
"encoding/hex"
"strings"
"unicode"
)
// Chunking splits a document into sentence-sized units so a re-check can ask the
// model only about the sentences that actually changed. Accepting one edit used
// to re-run the whole document: every card vanished, came back with a new id and
// a freshly-worded explanation, and spans re-merged into different shapes. The
// sentences she didn't touch have nothing new to say about themselves, so their
// suggestions are simply kept (see reconcilePending).
//
// A chunk's identity is its hash, not its position — she inserts a paragraph at
// the top and every sentence below keeps its suggestions.
// chunk is one sentence of the document, with the hash that identifies it.
type chunk struct {
text string
hash string
}
// asciiTerminators end a sentence only when whitespace (or the end of the text)
// follows, so "3.5" and "Ms." don't split mid-word — a wrong split costs only a
// slightly smaller chunk, but a split inside a number would churn its hash on
// every keystroke around it.
const asciiTerminators = ".!?"
// cjkTerminators end a sentence outright: Chinese runs sentences together with
// no space after 。, and she writes in both languages in one document.
const cjkTerminators = "。!?"
// closers are swallowed into the sentence they close, so the quote mark travels
// with the sentence rather than opening the next one.
const closers = `)]}"'’”」』`
// splitChunks divides text into sentences, dropping whitespace-only runs.
// Newlines always break a chunk, so a list or a line of dialogue is its own unit.
//
// `salt` distinguishes two *readings* of the same sentence. The grammar
// checkpoint's advice depends on the document's tone — the same line gets
// different notes as an academic essay than as a journal entry — so switching
// tone must re-open every sentence rather than serve back advice written for the
// old register.
func splitChunks(text, salt string) []chunk {
var out []chunk
runes := []rune(text)
start := 0
add := func(end int) {
if s := string(runes[start:end]); strings.TrimSpace(s) != "" {
out = append(out, chunk{text: s, hash: hashChunk(s, salt)})
}
start = end
}
for i := 0; i < len(runes); i++ {
r := runes[i]
if r == '\n' {
add(i + 1)
continue
}
cjk := strings.ContainsRune(cjkTerminators, r)
if !cjk && !strings.ContainsRune(asciiTerminators, r) {
continue
}
// Swallow a run of terminators ("?!", "…") and any closing punctuation.
j := i + 1
for j < len(runes) && (strings.ContainsRune(asciiTerminators+cjkTerminators+closers, runes[j])) {
j++
}
if cjk || j >= len(runes) || unicode.IsSpace(runes[j]) {
add(j)
i = j - 1
}
}
if start < len(runes) {
add(len(runes))
}
return out
}
// hashChunk identifies a sentence by its content under the same normalization
// the suppression logic uses: quote style and whitespace runs churn constantly
// (the editor rewrites quotes as she types, a paragraph reflows) and none of
// that changes what the sentence says, so none of it should cost a re-check.
func hashChunk(s, salt string) string {
sum := sha256.Sum256([]byte(salt + "\x00" + normalizeForDedup(s)))
return hex.EncodeToString(sum[:])[:16]
}
// hashSet indexes chunks by hash — "is this sentence in the document?"
func hashSet(chunks []chunk) map[string]bool {
out := make(map[string]bool, len(chunks))
for _, c := range chunks {
out[c.hash] = true
}
return out
}
// changedChunks returns the chunks whose hash wasn't in the last checked set,
// in document order and deduplicated — a sentence repeated verbatim is one
// question, not two.
func changedChunks(chunks []chunk, checked map[string]bool) []chunk {
seen := make(map[string]bool, len(chunks))
var out []chunk
for _, c := range chunks {
if checked[c.hash] || seen[c.hash] {
continue
}
seen[c.hash] = true
out = append(out, c)
}
return out
}
// joinChunks renders a chunk set as the text to hand the model: one sentence per
// line, so two sentences pulled from opposite ends of the document don't read as
// one run-on.
func joinChunks(chunks []chunk) string {
parts := make([]string, 0, len(chunks))
for _, c := range chunks {
parts = append(parts, strings.TrimSpace(c.text))
}
return strings.Join(parts, "\n")
}
// chunkFor names the sentence a suggestion belongs to: the first chunk whose
// text contains the flagged span. Returns "" when the span straddles a sentence
// boundary or the model paraphrased what it quoted — such a row is re-examined
// on every pass rather than cached, which is the safe direction.
func chunkFor(original string, chunks []chunk) string {
o := normalizeForDedup(original)
if o == "" {
return ""
}
for _, c := range chunks {
if strings.Contains(normalizeForDedup(c.text), o) {
return c.hash
}
}
return ""
}
+118
View File
@@ -0,0 +1,118 @@
package suggestions
import (
"strings"
"testing"
)
func texts(chunks []chunk) []string {
out := make([]string, 0, len(chunks))
for _, c := range chunks {
out = append(out, strings.TrimSpace(c.text))
}
return out
}
func TestSplitChunks(t *testing.T) {
cases := []struct {
name string
in string
want []string
}{
{
name: "plain sentences",
in: "I has two apple. She go to market yesterday! Why?",
want: []string{"I has two apple.", "She go to market yesterday!", "Why?"},
},
{
// A decimal must not split, or the sentence's identity would churn
// while she types the number.
name: "decimals stay whole",
in: "It costs 3.50 today. Tomorrow, more.",
want: []string{"It costs 3.50 today.", "Tomorrow, more."},
},
{
name: "closing quote travels with its sentence",
in: `He said "early," and left. She stayed.`,
want: []string{`He said "early," and left.`, "She stayed."},
},
{
// Chinese runs sentences together with no space after 。 — she writes
// in both languages in one document.
name: "cjk terminators split without a space",
in: "我想说这句话。但是不知道用英语怎么说。",
want: []string{"我想说这句话。", "但是不知道用英语怎么说。"},
},
{
name: "newlines break chunks",
in: "A list item\nAnother item\n",
want: []string{"A list item", "Another item"},
},
{
name: "blank runs are dropped",
in: "\n\n \nOnly this.\n\n",
want: []string{"Only this."},
},
{
name: "trailing fragment is its own chunk",
in: "Done. Still writing",
want: []string{"Done.", "Still writing"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := texts(splitChunks(tc.in, ""))
if len(got) != len(tc.want) {
t.Fatalf("want %q, got %q", tc.want, got)
}
for i := range got {
if got[i] != tc.want[i] {
t.Fatalf("chunk %d: want %q, got %q", i, tc.want[i], got[i])
}
}
})
}
}
// A sentence's identity survives the churn that doesn't change what it says:
// the editor rewrites quotes as she types, and a paragraph reflows.
func TestChunkIdentityIgnoresCosmeticChurn(t *testing.T) {
a := splitChunks(`She said "hello" softly.`, "")
b := splitChunks("She said “hello” softly.", "")
if len(a) != 1 || len(b) != 1 {
t.Fatalf("want one chunk each, got %d and %d", len(a), len(b))
}
if a[0].hash != b[0].hash {
t.Fatalf("quote/whitespace churn changed the sentence's identity")
}
if same := splitChunks(`She said "hello" softly.`, "academic"); same[0].hash == a[0].hash {
t.Fatalf("a different tone must be a different reading of the sentence")
}
}
func TestChangedChunksAndLookup(t *testing.T) {
chunks := splitChunks("One thing. Another thing. One thing.", "")
if len(chunks) != 3 {
t.Fatalf("want 3 chunks, got %d", len(chunks))
}
// A repeated sentence is one question, not two.
if got := changedChunks(chunks, nil); len(got) != 2 {
t.Fatalf("want 2 distinct changed chunks, got %d", len(got))
}
checked := hashSet(chunks[:1])
changed := changedChunks(chunks, checked)
if len(changed) != 1 || strings.TrimSpace(changed[0].text) != "Another thing." {
t.Fatalf("want only the unread sentence, got %q", texts(changed))
}
if chunkFor("Another", chunks) != chunks[1].hash {
t.Fatalf("span was attributed to the wrong sentence")
}
// A span the document doesn't contain has no sentence, so it is never cached.
if chunkFor("nowhere in here", chunks) != "" {
t.Fatalf("unanchorable span should have no chunk")
}
}
+126 -77
View File
@@ -160,16 +160,21 @@ func (h *Handler) mechanics(w http.ResponseWriter, r *http.Request) {
httputil.WriteJSON(w, http.StatusOK, out)
}
// replaceMechanics swaps the document's pending offline rows for the supplied
// findings in one transaction, leaving the LLM families and actioned rows
// untouched. Findings the user already accepted or dismissed are suppressed (the
// detector has no memory between runs), and malformed spans are skipped.
// replaceMechanics brings the document's pending offline rows in line with the
// supplied findings in one transaction, leaving the LLM families and actioned
// rows untouched. Findings the user already accepted or dismissed are suppressed
// (the detector has no memory between runs), and malformed spans are skipped.
//
// The DELETE is scoped by *source*, not by type: the rule pack owns both the
// mechanics family and its share of the collocation family, and every run is a
// full recompute of the document, so everything it wrote last time goes. Scoping
// by type instead would strand offline collocations the current text no longer
// warrants — the one row nobody would ever replace.
// A finding the detector still reports keeps its existing row — same id, same
// created_at — and only its offsets move. This pass fires 250 ms after a
// keystroke, so deleting and re-inserting the family would hand every card a new
// identity several times a sentence: the rail would remount, a card expanded for
// Ask Petal would collapse under her, and the arrival chime would re-fire.
//
// The scope is *source*, not type: the rule pack owns both the mechanics family
// and its share of the collocation family, and every run is a full recompute of
// the document. Scoping by type instead would strand offline collocations the
// current text no longer warrants — the one row nobody would ever replace.
func (h *Handler) replaceMechanics(docID string, findings []mechanicsFinding) error {
tx, err := h.DB.Begin()
if err != nil {
@@ -177,18 +182,18 @@ func (h *Handler) replaceMechanics(docID string, findings []mechanicsFinding) er
}
defer tx.Rollback()
if _, err := tx.Exec(
`DELETE FROM suggestions WHERE doc_id = ? AND status = ? AND source = ?`,
docID, db.SuggestionStatusPending, db.SuggestionSourceLocal,
); err != nil {
existing, err := loadPending(tx, docID, "source = '"+db.SuggestionSourceLocal+"'")
if err != nil {
return err
}
index := indexByEdit(existing)
sup, err := buildSuppressor(tx, docID)
if err != nil {
return err
}
kept := make(map[string]bool, len(existing))
for _, f := range findings {
if f.From < 0 || f.To <= f.From || strings.TrimSpace(f.Original) == "" {
continue // malformed span — the client re-anchors by string anyway
@@ -196,16 +201,34 @@ func (h *Handler) replaceMechanics(docID string, findings []mechanicsFinding) er
if sup.suppressed(f.Original, f.Replacement) {
continue
}
typ := localType(f.Type)
if row, ok := index.take(f.Original, f.Replacement, f.From); ok {
kept[row.id] = true
if err := reposition(tx, row, f.From, f.To, ""); err != nil {
return err
}
continue
}
if _, err := tx.Exec(
`INSERT INTO suggestions (doc_id, from_pos, to_pos, original, replacement, explanation, type, source)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
docID, f.From, f.To, f.Original, f.Replacement, f.Explanation,
localType(f.Type), db.SuggestionSourceLocal,
typ, db.SuggestionSourceLocal,
); err != nil {
return err
}
}
// Whatever the detector no longer reports, she has fixed.
for _, row := range existing {
if kept[row.id] {
continue
}
if _, err := tx.Exec(`DELETE FROM suggestions WHERE id = ?`, row.id); err != nil {
return err
}
}
return tx.Commit()
}
@@ -258,11 +281,64 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R
return
}
// Nothing to analyze on an empty document — skip the LLM round-trip.
// Nothing to analyze on an empty document — skip the LLM round-trip. The
// family's rows go with the text they were about.
if strings.TrimSpace(contentText) == "" {
httputil.WriteJSON(w, http.StatusOK, []db.Suggestion{})
if err := h.reconcilePending(docID, contentText, nil, scope, nil, nil, false); err != nil {
httputil.ServerError(w, err)
return
}
out, err := h.fetchPending(userID, docID)
if err != nil {
httputil.ServerError(w, err)
return
}
httputil.WriteJSON(w, http.StatusOK, out)
return
}
// Decide what to ask about before spending anything: a chunked pass asks only
// about the sentences that changed since it last read the document, and when
// none did it doesn't call the model at all — nor consume its rate-limit slot,
// so the next real edit isn't throttled by a check that had nothing to do.
//
// Only a chunked pass consults that record, so only it needs the tone folded
// into a sentence's identity.
salt := ""
if scope.chunked {
salt = tone
}
chunks := splitChunks(contentText, salt)
askText, fresh := contentText, chunks
if scope.chunked {
checked, err := h.checkedChunks(docID, scope.family)
if err != nil {
httputil.ServerError(w, err)
return
}
changed := changedChunks(chunks, checked)
if len(changed) == 0 {
// Every sentence has already been read. Drop the rows whose sentence is
// gone, keep the rest exactly as they are, and answer immediately.
if err := h.reconcilePending(docID, contentText, nil, scope, chunks, nil, false); err != nil {
httputil.ServerError(w, err)
return
}
out, err := h.fetchPending(userID, docID)
if err != nil {
httputil.ServerError(w, err)
return
}
httputil.WriteJSON(w, http.StatusOK, out)
return
}
// When every sentence is new — a first pass, a paste, a tone switch — hand
// over the document verbatim so the model reads it with its paragraphing
// intact. Otherwise send just the delta, one sentence per line.
if len(changed) < len(hashSet(chunks)) {
askText, fresh = joinChunks(changed), changed
}
}
ok, _, slotAt := limiter.Allow(docID)
if !ok {
@@ -277,7 +353,7 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R
return
}
raw, err := run(r.Context(), h.Client, contentText, tone, llm.LangFor(pairLang))
raw, err := run(r.Context(), h.Client, askText, tone, llm.LangFor(pairLang))
if err != nil {
// Allow ran before the model call, so a failed pass would otherwise hold
// the per-document slot for the full interval — stranding the frontend's
@@ -287,7 +363,9 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R
return
}
if err := h.replacePending(docID, contentText, raw, scope); err != nil {
// A whole-document pass re-read everything, so every one of its rows is up for
// re-proposal; a chunked pass only puts the sentences it asked about in play.
if err := h.reconcilePending(docID, contentText, raw, scope, chunks, fresh, !scope.chunked); err != nil {
httputil.ServerError(w, err)
return
}
@@ -308,78 +386,49 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R
// inserts. The grammar checkpoint and voice pass each own a disjoint family, so
// running one never disturbs the other's pending flags.
type pendingScope struct {
deleteWhere string // extra WHERE clause scoping the DELETE to this family
deleteWhere string // extra WHERE clause scoping this pass to its own family
forceType string // if set, every inserted row gets this type; else normalizeType
// family keys the sentences this pass has already read (see checked_chunks).
family string
// chunked passes re-read only the sentences that changed. True for the typing-
// cadence grammar checkpoint, which fires constantly and must feel still;
// false for the explicit whole-document passes, where she pressed a button
// asking for a fresh read of everything.
chunked bool
}
// Every scope below is confined to source='llm'. The offline rule pack replaces
// its own rows wholesale on each edit (see replaceMechanics) and its findings
// Every scope below is confined to source='llm'. The offline rule pack owns its
// own rows and recomputes them on each edit (see replaceMechanics); its findings
// must survive all three model passes — including the collocation coach, which
// now shares the collocation family with it.
var (
// grammarScope owns the grammar/phrasing/idiom/clarity flags — everything but
// the other self-owned families (voice, collocation), which run on their own
// cadence/pass and must survive a grammar checkpoint. Notably the offline pass
// writes its rows in the same /check request just before this DELETE runs, so
// the source clause is also what keeps them alive.
grammarScope = pendingScope{deleteWhere: "source = 'llm' AND type NOT IN ('voice','collocation')", forceType: ""}
// voiceScope owns the model's voice flags only.
voiceScope = pendingScope{deleteWhere: "source = 'llm' AND type = 'voice'", forceType: db.SuggestionTypeVoice}
// writes its rows in the same /check request just before this pass reconciles,
// so the source clause is also what keeps them alive.
grammarScope = pendingScope{
deleteWhere: "source = 'llm' AND type NOT IN ('voice','collocation')",
family: "grammar",
chunked: true,
}
// voiceScope owns the model's voice flags only. Voice is a property of the
// document as a whole — a sentence isn't inconsistent with itself — so this
// pass always reads everything.
voiceScope = pendingScope{
deleteWhere: "source = 'llm' AND type = 'voice'",
forceType: db.SuggestionTypeVoice,
family: "voice",
}
// collocationScope owns the model's collocation flags only — the rule pack's
// share of the same family is left standing.
collocationScope = pendingScope{deleteWhere: "source = 'llm' AND type = 'collocation'", forceType: db.SuggestionTypeCollocation}
collocationScope = pendingScope{
deleteWhere: "source = 'llm' AND type = 'collocation'",
forceType: db.SuggestionTypeCollocation,
family: "collocation",
}
)
// replacePending swaps a document's pending suggestions within one family for a
// fresh batch in a single transaction. Accepted/rejected suggestions and the
// other family's pending rows are left untouched.
//
// Suggestions touching a sentence the user already settled are suppressed from
// the fresh batch (see suppressor): not just the identical edit re-proposed, but
// reversals and re-polishing of the model's own just-accepted output — the
// "fickle, keeps going back and forth on a few sentences" behavior. The model has
// no memory between passes, so without this it re-opens resolved sentences every
// checkpoint.
func (h *Handler) replacePending(docID, contentText string, raw []llm.RawSuggestion, scope pendingScope) error {
tx, err := h.DB.Begin()
if err != nil {
return err
}
defer tx.Rollback()
if _, err := tx.Exec(
`DELETE FROM suggestions WHERE doc_id = ? AND status = ? AND `+scope.deleteWhere,
docID, db.SuggestionStatusPending,
); err != nil {
return err
}
sup, err := buildSuppressor(tx, docID)
if err != nil {
return err
}
for _, s := range raw {
if sup.suppressed(s.Original, s.Replacement) {
continue
}
typ := scope.forceType
if typ == "" {
typ = normalizeType(s.Type)
}
from, to := locate(contentText, s.Original)
if _, err := tx.Exec(
`INSERT INTO suggestions (doc_id, from_pos, to_pos, original, replacement, explanation, type, source)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
docID, from, to, s.Original, s.Replacement, s.Explanation, typ, db.SuggestionSourceLLM,
); err != nil {
return err
}
}
return tx.Commit()
}
// dedupQuoteReplacer folds every straight/curly single- and double-quote variant
// (and backtick/acute accent) onto one canonical character. The editor and the
// model both rewrite quotes between passes — a sentence accepted with "…" comes
+27 -2
View File
@@ -20,10 +20,17 @@ import (
type stubClient struct {
response string
calls int
// The full prompt of the most recent call, so a test can assert which
// sentences a chunked pass actually asked about.
lastPrompt string
}
func (s *stubClient) Complete(_ context.Context, _ llm.CompletionRequest) (string, error) {
func (s *stubClient) Complete(_ context.Context, req llm.CompletionRequest) (string, error) {
s.calls++
s.lastPrompt = ""
for _, m := range req.Messages {
s.lastPrompt += m.Content + "\n"
}
return s.response, nil
}
@@ -64,6 +71,17 @@ func newTestServer(t *testing.T, client llm.LLMClient) (http.Handler, string, *H
return authed, docID, h
}
// setDocText rewrites the seeded document, standing in for the writer editing.
// The grammar checkpoint only asks the model about sentences that changed since
// it last read the document, so a test that wants a second real pass has to
// change something first — as she always has.
func setDocText(t *testing.T, h *Handler, docID, text string) {
t.Helper()
if _, err := h.DB.Exec(`UPDATE documents SET content_text = ? WHERE id = ?`, text, docID); err != nil {
t.Fatalf("update doc text: %v", err)
}
}
func do(t *testing.T, srv http.Handler, method, path, body string) *httptest.ResponseRecorder {
t.Helper()
var r *http.Request
@@ -183,6 +201,7 @@ func TestFickleEditsSuppressed(t *testing.T) {
]}`}
srv, docID, h := newTestServer(t, client)
h.Limit = llm.NewRateLimiter(0)
setDocText(t, h, docID, `He left "early," because of the rain. The cat always have a calm face.`)
rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
var got []db.Suggestion
@@ -194,6 +213,10 @@ func TestFickleEditsSuppressed(t *testing.T) {
do(t, srv, http.MethodPost, "/suggestions/"+s.ID+"/accept", "")
}
// Both edits are now in the document, which is what re-opens those sentences
// for a second reading.
setDocText(t, h, docID, `He left "early," due to the rain. The cat always has a calm face.`)
// Reversal of the first accept (note the " → ' quote churn) and a re-polish of
// the second accept must both be dropped; only the unrelated edit survives.
client.response = `{"suggestions":[
@@ -314,7 +337,9 @@ func TestCollocationPassCoexists(t *testing.T) {
t.Fatalf("collocation response should carry all three families, got %+v", got)
}
// A grammar checkpoint must NOT wipe the voice or collocation flags.
// A grammar checkpoint must NOT wipe the voice or collocation flags. She fixes
// the flagged sentence, so its own grammar row goes and nothing replaces it.
setDocText(t, h, docID, "I have two apples.")
client.response = `{"suggestions":[]}`
rec = do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
if rec.Code != http.StatusOK {
+293
View File
@@ -0,0 +1,293 @@
package suggestions
import (
"database/sql"
"gitea.parodia.dev/drwily/petal/internal/db"
"gitea.parodia.dev/drwily/petal/internal/llm"
)
// Reconciliation replaces the old "delete the family, insert the new batch"
// shape of every pass. A suggestion the pass proposes again is the *same*
// suggestion: it keeps its row, and therefore its id, its created_at and — most
// visibly — the explanation it was first given. The model re-words its reasoning
// every time it is asked, so re-inserting meant one unchanged mistake carried
// three different explanations in a single sitting.
//
// The id is what the frontend keys its cards on, so a stable id is also what
// keeps the rail from emptying and refilling, a card from collapsing mid-read,
// and the arrival chime from re-firing for advice she has already seen.
// pendingRow is the part of an existing pending suggestion reconciliation cares
// about.
type pendingRow struct {
id string
original string
replacement string
chunkHash string
from int
}
// loadPending reads the pending rows a pass owns. `where` is the pass's own
// scoping clause (by source, and for the model passes by family) — the same
// fragment that used to scope its DELETE.
func loadPending(tx *sql.Tx, docID, where string) ([]pendingRow, error) {
rows, err := tx.Query(
`SELECT id, original, replacement, chunk_hash, from_pos FROM suggestions
WHERE doc_id = ? AND status = ? AND `+where,
docID, db.SuggestionStatusPending,
)
if err != nil {
return nil, err
}
defer rows.Close()
var out []pendingRow
for rows.Next() {
var r pendingRow
if err := rows.Scan(&r.id, &r.original, &r.replacement, &r.chunkHash, &r.from); err != nil {
return nil, err
}
out = append(out, r)
}
return out, rows.Err()
}
// editKey identifies an edit by what it proposes, not where: "this exact change
// to this exact text". Normalized like the suppression comparisons, so the
// editor's quote rewriting and a reflowed paragraph don't read as a new edit.
func editKey(original, replacement string) string {
return normalizeForDedup(original) + "\x00" + normalizeForDedup(replacement)
}
// editIndex matches freshly proposed edits against the rows already standing.
type editIndex struct {
rows []pendingRow
used []bool
byKey map[string][]int
}
func indexByEdit(rows []pendingRow) *editIndex {
idx := &editIndex{rows: rows, used: make([]bool, len(rows)), byKey: map[string][]int{}}
for i, r := range rows {
k := editKey(r.original, r.replacement)
idx.byKey[k] = append(idx.byKey[k], i)
}
return idx
}
// take claims the standing row for this edit, if there is one. When a document
// repeats the same mistake, `near` (the fresh span's start) picks the closest
// standing row, so two identical cards keep their own identities instead of
// trading them whenever the text between them grows.
func (i *editIndex) take(original, replacement string, near int) (pendingRow, bool) {
best, bestDist := -1, 0
for _, n := range i.byKey[editKey(original, replacement)] {
if i.used[n] {
continue
}
d := i.rows[n].from - near
if d < 0 {
d = -d
}
if best < 0 || d < bestDist {
best, bestDist = n, d
}
}
if best < 0 {
return pendingRow{}, false
}
i.used[best] = true
return i.rows[best], true
}
// reposition updates the advisory offsets (and the sentence a row belongs to)
// without touching anything the writer can see. The frontend re-anchors by
// string at render time, so these only matter for the local-vs-model span
// arbitration in dedupeSpans.
func reposition(tx *sql.Tx, row pendingRow, from, to int, chunkHash string) error {
if row.from == from && row.chunkHash == chunkHash {
return nil
}
_, err := tx.Exec(
`UPDATE suggestions SET from_pos = ?, to_pos = ?, chunk_hash = ? WHERE id = ?`,
from, to, chunkHash, row.id,
)
return err
}
// reconcilePending brings a model pass's family in line with what it just
// proposed, sentence by sentence:
//
// - A row on a sentence this pass didn't ask about is kept untouched — that
// is the whole point of chunking. Only its offsets are refreshed.
// - A row on a sentence that no longer exists in the document is dropped: she
// rewrote or deleted it.
// - A row on a sentence the pass *did* ask about survives only if the model
// proposed the same edit again, in which case it keeps its identity.
//
// `fresh` names the sentences the model was asked about (nil when it wasn't
// called at all). inPlayAll marks the whole-document passes — voice and the
// collocation coach — where every row is up for re-proposal because the model
// just re-read everything.
func (h *Handler) reconcilePending(
docID, contentText string,
raw []llm.RawSuggestion,
scope pendingScope,
chunks, fresh []chunk,
inPlayAll bool,
) error {
tx, err := h.DB.Begin()
if err != nil {
return err
}
defer tx.Rollback()
existing, err := loadPending(tx, docID, scope.deleteWhere)
if err != nil {
return err
}
present := hashSet(chunks)
asked := hashSet(fresh)
modelRan := inPlayAll || fresh != nil
// Sentences to hand back to the model next time, because a row we were
// caching on them turned out to be unanchorable (see below).
reopen := map[string]bool{}
var inPlay []pendingRow
for _, r := range existing {
switch {
// A row whose sentence we can't name is never cached — it is re-examined
// whenever the model speaks, and left alone when it doesn't.
case inPlayAll, r.chunkHash == "" && modelRan, asked[r.chunkHash]:
inPlay = append(inPlay, r)
case r.chunkHash != "" && !present[r.chunkHash]:
if _, err := tx.Exec(`DELETE FROM suggestions WHERE id = ?`, r.id); err != nil {
return err
}
default:
// Untouched sentence: keep the card exactly as she last saw it.
from, to := locate(contentText, r.original)
if from < 0 {
// The sentence is unchanged in substance but the quoted span no
// longer matches byte for byte — a quote mark the editor rewrote
// inside it, say. The frontend anchors by that string, so this card
// can't be shown; drop it and let the sentence be read again rather
// than cache advice nobody can see.
reopen[r.chunkHash] = true
if _, err := tx.Exec(`DELETE FROM suggestions WHERE id = ?`, r.id); err != nil {
return err
}
continue
}
if err := reposition(tx, r, from, to, r.chunkHash); err != nil {
return err
}
}
}
for h := range reopen {
delete(present, h)
}
sup, err := buildSuppressor(tx, docID)
if err != nil {
return err
}
index := indexByEdit(inPlay)
kept := make(map[string]bool, len(inPlay))
for _, s := range raw {
if sup.suppressed(s.Original, s.Replacement) {
continue
}
from, to := locate(contentText, s.Original)
// Attribute the finding to a sentence the model was actually shown before
// falling back to the whole document: a short span ("the the") can occur in
// two sentences, and crediting it to the cached one would drop it as advice
// we already have.
hash := chunkFor(s.Original, fresh)
if hash == "" {
hash = chunkFor(s.Original, chunks)
}
// A sentence we didn't ask about already has whatever advice it deserves.
// The model can't normally quote one — it was only shown the delta — but if
// it wanders there anyway, the cached card stands rather than gaining a
// twin.
if !inPlayAll && hash != "" && present[hash] && !asked[hash] {
continue
}
if row, ok := index.take(s.Original, s.Replacement, from); ok {
kept[row.id] = true
if err := reposition(tx, row, from, to, hash); err != nil {
return err
}
continue
}
typ := scope.forceType
if typ == "" {
typ = normalizeType(s.Type)
}
if _, err := tx.Exec(
`INSERT INTO suggestions (doc_id, from_pos, to_pos, original, replacement, explanation, type, source, chunk_hash)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`,
docID, from, to, s.Original, s.Replacement, s.Explanation, typ, db.SuggestionSourceLLM, hash,
); err != nil {
return err
}
}
// Asked about and not proposed again: the model has changed its mind, or she
// has fixed it.
for _, r := range inPlay {
if kept[r.id] {
continue
}
if _, err := tx.Exec(`DELETE FROM suggestions WHERE id = ?`, r.id); err != nil {
return err
}
}
// Record the sentences this family has now read. Every sentence still in the
// document has been read by *some* pass: the ones just asked about now, the
// rest in an earlier round.
if scope.chunked {
if _, err := tx.Exec(
`DELETE FROM checked_chunks WHERE doc_id = ? AND family = ?`, docID, scope.family,
); err != nil {
return err
}
for h := range present {
if _, err := tx.Exec(
`INSERT INTO checked_chunks (doc_id, family, hash) VALUES (?, ?, ?)`,
docID, scope.family, h,
); err != nil {
return err
}
}
}
return tx.Commit()
}
// checkedChunks loads the sentences a family read on its last pass.
func (h *Handler) checkedChunks(docID, family string) (map[string]bool, error) {
rows, err := h.DB.Query(
`SELECT hash FROM checked_chunks WHERE doc_id = ? AND family = ?`, docID, family,
)
if err != nil {
return nil, err
}
defer rows.Close()
out := map[string]bool{}
for rows.Next() {
var hash string
if err := rows.Scan(&hash); err != nil {
return nil, err
}
out[hash] = true
}
return out, rows.Err()
}
+191
View File
@@ -0,0 +1,191 @@
package suggestions
import (
"encoding/json"
"net/http"
"strings"
"testing"
"gitea.parodia.dev/drwily/petal/internal/db"
"gitea.parodia.dev/drwily/petal/internal/llm"
)
// byOriginal indexes a pending set by the text each card flags.
func byOriginal(in []db.Suggestion) map[string]db.Suggestion {
out := map[string]db.Suggestion{}
for _, s := range in {
out[s.Original] = s
}
return out
}
// TestUntouchedSentencesKeepTheirCards is the heart of the stability work: she
// edits one sentence, and the cards on every other sentence stay exactly as they
// were — same id (so the rail keeps the card instead of remounting it), same
// explanation (the model re-words its reasoning every time it is asked, and one
// unchanged mistake used to carry three different explanations in a sitting).
// The model is only asked about the sentence that changed.
func TestUntouchedSentencesKeepTheirCards(t *testing.T) {
client := &stubClient{response: `{"suggestions":[
{"original":"I has two apple","replacement":"I have two apples","explanation":"first wording","type":"grammar"},
{"original":"She go to market","replacement":"She goes to market","explanation":"agreement","type":"grammar"}
]}`}
srv, docID, h := newTestServer(t, client)
h.Limit = llm.NewRateLimiter(0)
setDocText(t, h, docID, "I has two apple. She go to market yesterday.")
rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
var first []db.Suggestion
if err := json.Unmarshal(rec.Body.Bytes(), &first); err != nil {
t.Fatalf("decode: %v", err)
}
if len(first) != 2 {
t.Fatalf("first pass: want 2, got %d: %+v", len(first), first)
}
kept := byOriginal(first)["I has two apple"]
// She fixes only the second sentence. The model, asked again, re-words its
// reasoning about the first — which it must never get the chance to do.
setDocText(t, h, docID, "I has two apple. She goes to market yesterday.")
client.response = `{"suggestions":[
{"original":"I has two apple","replacement":"I have two apples","explanation":"REWORDED","type":"grammar"}
]}`
rec = do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
var second []db.Suggestion
if err := json.Unmarshal(rec.Body.Bytes(), &second); err != nil {
t.Fatalf("decode: %v", err)
}
if strings.Contains(client.lastPrompt, "I has two apple") {
t.Fatalf("untouched sentence was sent to the model:\n%s", client.lastPrompt)
}
if !strings.Contains(client.lastPrompt, "She goes to market") {
t.Fatalf("edited sentence was not sent to the model:\n%s", client.lastPrompt)
}
now := byOriginal(second)["I has two apple"]
if now.ID != kept.ID {
t.Fatalf("card was remounted: id %q became %q", kept.ID, now.ID)
}
if now.Explanation != "first wording" {
t.Fatalf("explanation drifted: %q", now.Explanation)
}
// The fixed sentence's card is gone, and the model's stray re-proposal for the
// cached sentence did not become a second card.
if len(second) != 1 {
t.Fatalf("want exactly one card left, got %d: %+v", len(second), second)
}
}
// TestUnchangedDocumentSkipsTheModel proves a check with nothing new to read
// costs nothing: no model call, and every card left standing untouched. This is
// the doc-open and tone-less re-check path.
func TestUnchangedDocumentSkipsTheModel(t *testing.T) {
client := &stubClient{response: `{"suggestions":[
{"original":"I has","replacement":"I have","explanation":"agreement","type":"grammar"}
]}`}
srv, docID, h := newTestServer(t, client)
h.Limit = llm.NewRateLimiter(0)
rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
var first []db.Suggestion
_ = json.Unmarshal(rec.Body.Bytes(), &first)
if len(first) != 1 || client.calls != 1 {
t.Fatalf("first pass: %d cards, %d calls", len(first), client.calls)
}
rec = do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
var second []db.Suggestion
if err := json.Unmarshal(rec.Body.Bytes(), &second); err != nil {
t.Fatalf("decode: %v", err)
}
if client.calls != 1 {
t.Fatalf("re-checking an unedited document called the model %d times", client.calls)
}
if len(second) != 1 || second[0].ID != first[0].ID {
t.Fatalf("card did not survive an idle re-check: %+v", second)
}
}
// TestDeletedSentenceDropsItsCard covers the other half of the skip path: she
// removes a flagged sentence outright, so nothing changed that the model could
// be asked about — but its card must still go.
func TestDeletedSentenceDropsItsCard(t *testing.T) {
client := &stubClient{response: `{"suggestions":[
{"original":"I has","replacement":"I have","explanation":"agreement","type":"grammar"}
]}`}
srv, docID, h := newTestServer(t, client)
h.Limit = llm.NewRateLimiter(0)
do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
setDocText(t, h, docID, "")
rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
var got []db.Suggestion
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
t.Fatalf("decode: %v", err)
}
if len(got) != 0 {
t.Fatalf("card outlived its sentence: %+v", got)
}
}
// TestToneChangeReopensEverySentence: the checkpoint's advice is written for the
// document's tone, so switching from a journal to an academic essay has to
// re-read sentences that haven't changed a character.
func TestToneChangeReopensEverySentence(t *testing.T) {
client := &stubClient{response: `{"suggestions":[
{"original":"I has","replacement":"I have","explanation":"agreement","type":"grammar"}
]}`}
srv, docID, h := newTestServer(t, client)
h.Limit = llm.NewRateLimiter(0)
do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
if _, err := h.DB.Exec(`UPDATE documents SET tone = 'academic' WHERE id = ?`, docID); err != nil {
t.Fatalf("set tone: %v", err)
}
do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "")
if client.calls != 2 {
t.Fatalf("tone change did not re-read the document: %d model calls", client.calls)
}
}
// TestMechanicsFindingsKeepTheirRows: the rule pack re-runs 250 ms after every
// keystroke. A finding it still reports must keep its row, or the rail would
// remount several times a sentence — collapsing a card she has open, and
// re-firing the arrival chime for advice she is already reading.
func TestMechanicsFindingsKeepTheirRows(t *testing.T) {
srv, docID, _ := newTestServer(t, &stubClient{response: `{"suggestions":[]}`})
body := `{"findings":[
{"from":0,"to":5,"original":"I has","replacement":"I have","explanation":"agreement"},
{"from":6,"to":15,"original":"two apple","replacement":"two apples","explanation":"plural"}
]}`
rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/mechanics", body)
var first []db.Suggestion
if err := json.Unmarshal(rec.Body.Bytes(), &first); err != nil {
t.Fatalf("decode: %v", err)
}
if len(first) != 2 {
t.Fatalf("want 2 rows, got %d", len(first))
}
// She types elsewhere: same findings, shifted spans, one of them now fixed.
rec = do(t, srv, http.MethodPost, "/docs/"+docID+"/mechanics", `{"findings":[
{"from":20,"to":25,"original":"I has","replacement":"I have","explanation":"agreement"}
]}`)
var second []db.Suggestion
if err := json.Unmarshal(rec.Body.Bytes(), &second); err != nil {
t.Fatalf("decode: %v", err)
}
if len(second) != 1 {
t.Fatalf("want 1 row, got %d: %+v", len(second), second)
}
if second[0].ID != byOriginal(first)["I has"].ID {
t.Fatalf("surviving finding was given a new identity: %+v", second[0])
}
if second[0].FromPos != 20 {
t.Fatalf("span did not follow the text: %+v", second[0])
}
}
+20 -8
View File
@@ -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<Set<string>>(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,
)
@@ -509,7 +518,10 @@ export default function App() {
<>
<div
onMouseDown={handleChromeDown}
className="flex flex-1 flex-col overflow-y-auto px-6 py-8"
// `petal-scrollport` marks this as the editor's scrolling
// ancestor; EditorCore measures it to pin the text column while
// the suggestion rail's overhang is scrolled.
className="petal-scrollport flex flex-1 flex-col overflow-y-auto px-6 py-8"
>
<div ref={canvasRef} className="mx-auto flex w-full max-w-[720px] flex-1 flex-col">
{/* Title, then the three chrome pills. Their labels are
+68 -1
View File
@@ -15,7 +15,7 @@ import TableHeader from '@tiptap/extension-table-header'
import TableCell from '@tiptap/extension-table-cell'
import { FontSize } from './FontSize'
import type { EditorView } from '@tiptap/pm/view'
import { useCallback, useEffect, useRef, useState } from 'react'
import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'
import { Toolbar } from '../Toolbar/Toolbar'
import { SuggestionCard } from './SuggestionCard'
import { SuggestionRail, type RailItem } from './SuggestionRail'
@@ -34,6 +34,10 @@ import { speak, speechSupported } from '../../audio/speech'
import type { SpellChecker } from '../../hooks/useSpellChecker'
import { usePack } from '../../i18n'
// Breathing room left below the last suggestion card when the rail's stack is what
// defines the column's height, so the bottom card doesn't sit flush on the edge.
const RAIL_TAIL = 24
export interface EditorChange {
content: string // Tiptap JSON, stringified
content_text: string // flattened plain text for the LLM
@@ -229,6 +233,9 @@ export function EditorCore({
// own name, so it says "português" rather than "pt-PT".
const pack = usePack()
const wrapperRef = useRef<HTMLDivElement>(null)
// The text column itself, measured separately from its wrapper: the wrapper is
// grown to cover the card stack, so only this reports the height of the prose.
const contentRef = useRef<HTMLDivElement>(null)
const [hover, setHover] = useState<HoverState | null>(null)
// The open spelling popover (click a red-underlined word), or null.
const [misspell, setMisspell] = useState<MisspellState | null>(null)
@@ -266,6 +273,15 @@ export function EditorCore({
// `activeId` is the suggestion currently emphasized (hovered text or card).
const [railItems, setRailItems] = useState<RailItem[]>([])
const [railEnabled, setRailEnabled] = useState(false)
// How far the resolved card stack reaches below the wrapper's top, reported by
// the rail. Cards are absolutely positioned and so contribute no layout height:
// without this the column below the last line of text isn't scrollable and any
// card that lands there is unreachable, not merely far from its sentence.
const [railExtent, setRailExtent] = useState(0)
// Sticky offset for the text column, or null when it should sit in normal flow.
// Set only while the stack overhangs the text: scrolling down to reach the lower
// cards would otherwise carry every sentence off the top of the screen.
const [stickTop, setStickTop] = useState<number | null>(null)
const [railExpandedId, setRailExpandedId] = useState<string | null>(null)
const [activeId, setActiveId] = useState<string | null>(null)
// A stable handle to the latest recompute so the editor's onUpdate (captured
@@ -444,6 +460,42 @@ export function EditorCore({
}
}, [recomputeRail])
// The rail only reports its extent while it's mounted, so clear it when the last
// card goes (accepted the lot, or the window narrowed past the rail's threshold)
// — otherwise the column keeps the height of a stack that no longer exists.
useEffect(() => {
if (!railEnabled || railItems.length === 0) setRailExtent(0)
}, [railEnabled, railItems.length])
// Decide whether the text column has to be pinned. The rail's cards hang off an
// absolutely-positioned column, so when several suggestions share one short
// paragraph the stack runs far past the last line of text. Growing the wrapper to
// `railExtent` makes that space scrollable (item 4: the lower cards were simply
// unreachable); pinning the prose inside it means scrolling down to read those
// cards keeps the sentences on screen instead of scrolling them away.
//
// The offset is `min(0, port - content)`: prose shorter than the viewport sticks
// at the top, taller prose sticks by its *bottom* edge, so its last lines — the
// ones the overhanging cards flag — stay visible rather than the first.
useLayoutEffect(() => {
const wrapper = wrapperRef.current
const content = contentRef.current
if (!wrapper || !content || !railEnabled || railExtent <= 0) {
setStickTop(null)
return
}
const contentH = content.offsetHeight
// Only pin when the stack actually overhangs the prose; a rail that fits
// beside its text needs nothing, and pinning it would be a change for free.
if (railExtent <= contentH) {
setStickTop(null)
return
}
const port = wrapper.closest('.petal-scrollport')
const portH = port ? port.clientHeight : window.innerHeight
setStickTop(Math.min(0, portH - contentH - RAIL_TAIL))
}, [railEnabled, railExtent, railItems])
// Emphasize the flagged text for the active suggestion, mirroring the rail
// card ↔ text link both ways. Driven through the decoration plugin (not an
// imperative DOM class) so it survives the repaints that fire on every edit.
@@ -1082,6 +1134,10 @@ export function EditorCore({
<div
ref={wrapperRef}
className="relative flex-1"
// Grown to cover the card stack when it overhangs the prose, so the space
// those cards occupy is actually scrollable. `minHeight` never shrinks the
// column, so a rail that fits beside its text changes nothing.
style={railEnabled && railExtent > 0 ? { minHeight: railExtent + RAIL_TAIL } : undefined}
onMouseOver={handleMouseOver}
onMouseOut={handleMouseOut}
onMouseMove={handleMouseMove}
@@ -1092,8 +1148,18 @@ export function EditorCore({
onTouchStart={handleTouchStart}
onTouchMove={cancelLongPress}
onTouchEnd={cancelLongPress}
>
{/* The prose sits in its own box so it can be measured (and pinned)
independently of the wrapper, which the rail may have grown. The box is
deliberately left at its natural height: sized to the wrapper it would
report the stack's height back as the text's own, and the pin below
could never trip. */}
<div
ref={contentRef}
style={stickTop === null ? undefined : { position: 'sticky', top: stickTop }}
>
<EditorContent editor={editor} className="h-full" />
</div>
{findOpen && editor && <FindReplace editor={editor} onClose={() => setFindOpen(false)} />}
{confetti && <Confetti top={confetti.top} left={confetti.left} />}
{gloss && (
@@ -1165,6 +1231,7 @@ export function EditorCore({
onHover={setActiveId}
onActivate={activateRailCard}
onToggleExpand={toggleRailExpand}
onExtent={setRailExtent}
/>
)}
</div>
@@ -28,6 +28,12 @@ interface Props {
// A card's body was clicked — scroll its highlight into view and toggle expand.
onActivate: (id: string) => void
onToggleExpand: (id: string) => void
// How far down the resolved stack reaches (px below the wrapper's top). Cards
// are absolutely positioned, so they add nothing to layout height — a cluster of
// errors in one short paragraph can pile cards hundreds of px past the end of the
// text, with no scrollable space to reach them. The editor uses this to grow the
// column so every card can at least be scrolled to.
onExtent: (bottom: number) => void
}
// SuggestionRail is the right-margin "comment column": every outstanding
@@ -44,6 +50,7 @@ export function SuggestionRail({
onHover,
onActivate,
onToggleExpand,
onExtent,
}: Props) {
// Measured resolved tops keyed by suggestion id (after collision avoidance).
const [tops, setTops] = useState<Record<string, number>>({})
@@ -65,13 +72,16 @@ export function SuggestionRail({
const layoutKey = ordered.map((i) => `${i.suggestion.id}:${Math.round(i.anchorTop)}`).join('|')
useLayoutEffect(() => {
let cursor = -Infinity
let bottom = 0
const next: Record<string, number> = {}
for (const { suggestion, anchorTop } of ordered) {
const h = cardRefs.current.get(suggestion.id)?.offsetHeight ?? 96
const top = Math.max(anchorTop, cursor)
next[suggestion.id] = top
cursor = top + h + CARD_GAP
bottom = top + h
}
onExtent(bottom)
setTops((prev) => {
const ids = Object.keys(next)
if (ids.length === Object.keys(prev).length && ids.every((id) => prev[id] === next[id])) return prev
Binary file not shown.