1057 lines
61 KiB
Markdown
1057 lines
61 KiB
Markdown
# Petal UX review — 2026-07-27 (implementation doc)
|
||
|
||
**Origin:** a hands-on browser session against the live VPS deploy
|
||
(petal.parodia.dev) on a 1517×810 desktop viewport, deliberately writing
|
||
ESL-style English, accepting/undoing suggestions, using Ask Petal, the
|
||
right-click dictionary, the Garden, History, and the doc-type menu. Goal:
|
||
close the gap to Grammarly's *feel* (latency, stability, proximity of
|
||
feedback) without its cost or surveillance.
|
||
|
||
**How to use this doc:** each item has symptom/repro, likely code location,
|
||
proposed fix, and acceptance criteria. Items are ordered by
|
||
value-for-effort. Where the session couldn't confirm root cause, that is
|
||
said explicitly — verify before building. The product *why* behind Petal
|
||
lives in `SUGGESTIONS.md`; execution phases in `BUILD_PLAN.md`. Nothing here
|
||
contradicts them; this is polish on the existing loop.
|
||
|
||
**What was verified working well (don't regress):** error coverage and
|
||
explanation quality across two check rounds; bilingual (en+zh) card
|
||
explanations; Ask Petal follow-up answers; instant right-click dictionary
|
||
with TTS + slow-replay; Garden sprouting; autosave + History snapshots;
|
||
kitten cheers on accept; clean console throughout.
|
||
|
||
---
|
||
|
||
## 0. DONE this session — kitten yields to cards
|
||
|
||
The corner mascot's halo sat on top of the bottom rail cards, History-panel
|
||
footer controls, and the Garden counter, blocking reading and clicks.
|
||
|
||
**Implemented (this commit):** when any `.petal-rail-card` overlaps the
|
||
mascot badge, the kitten fades to 15% opacity, shrinks 10% toward its
|
||
corner (standalone `scale` property, so it composes with the bob
|
||
animation's `transform`), and goes `pointer-events: none` so clicks pass
|
||
through. It wakes (full size/opacity) while its speech bubble or the
|
||
companion picker is open, or when cards no longer overlap.
|
||
|
||
- `web/src/components/Companion/useCardOverlap.ts` — rect-intersection
|
||
hook, rAF-throttled on scroll/resize + 500 ms poll.
|
||
- `web/src/components/Companion/PetalCompanion.tsx` — `faded` wiring.
|
||
- `web/src/index.css` — `.petal-companion-faded`, transition.
|
||
|
||
**Follow-up — DONE.** The overlap check now watches
|
||
`.petal-rail-card, [role="dialog"][aria-modal="true"]`. Matching the modal
|
||
role rather than each panel's class covers History and Garden today (both
|
||
already render `role="dialog" aria-modal="true"` drawers) and any future
|
||
drawer for free, with no selector list to keep in sync.
|
||
|
||
Two refinements the original note didn't anticipate:
|
||
|
||
- The hook now reports `{ cards, modal }` separately. A card overlap still
|
||
lets the kitten wake for a bubble; a **modal overlap yields
|
||
unconditionally** — a cheer isn't worth covering the panel she just
|
||
opened on purpose.
|
||
- The speech bubble is its own layer, so fading the badge didn't hide it.
|
||
It's now held back explicitly while a panel is open, and reappears when
|
||
she closes it (`useCompanion` keeps it in state).
|
||
|
||
Verified against the live build: with History open, the drawer overlaps the
|
||
badge rect and the "📜 写作证明 · Writing passport" control sits under the
|
||
mascot — the exact defect. The new selector trips on it.
|
||
|
||
- `useCardOverlap.ts` — `CardOverlap` return type, modal selector, identity
|
||
guard so the 500 ms poll doesn't re-render on every tick.
|
||
- `PetalCompanion.tsx` — `crowded.modal || (crowded.cards && …)`, bubble gate.
|
||
|
||
---
|
||
|
||
## 1. Bug: redo does not re-apply an accepted suggestion — NOT REPRODUCIBLE
|
||
|
||
**Status (2026-07-27, follow-up session):** investigated against the live
|
||
VPS build with the ProseMirror history plugin state read directly
|
||
(`done.eventCount` / `undone.eventCount`) and `view.dispatch` hooked to log
|
||
every transaction. **Redo works in every path tried** — including the exact
|
||
repro below, on the same Idiom card. Closing this unless it resurfaces with
|
||
a tighter repro.
|
||
|
||
Paths tried, all correct (accept → `done` +1; undo → `done` −1, `undone` 1,
|
||
text reverts; redo → replacement restored):
|
||
|
||
- accept → undo → redo pressed immediately
|
||
- accept → undo → **18 s pause so the full re-check lands** → redo
|
||
- accept with the editor never focused → undo → redo
|
||
|
||
The doc's hypothesis — that the re-check wipes the redo stack — was tested
|
||
directly and is **false**: across the pause, the only doc-changing
|
||
transaction was the undo itself (`metaHistory: true`); the re-check's
|
||
transactions are all decoration-only (`docChanged: false`), which
|
||
prosemirror-history ignores. `canRedo` stayed `true` throughout.
|
||
|
||
One real trap that likely explains the original report: **keyboard undo
|
||
only works when focus is in the editor.** Clicking Accept in the rail moves
|
||
focus to the Accept button; `handleAccept`'s `chain().focus()` normally
|
||
returns it, but that only runs when `findRange` locates the original span —
|
||
if the span isn't found, focus stays on the button and Ctrl+Z/Ctrl+Shift+Z
|
||
silently do nothing. Worth hardening regardless of this item.
|
||
|
||
**Still real, found while investigating:** after undo, the suggestion is
|
||
already marked accepted server-side, so the card doesn't reliably come back
|
||
for the text that's now showing again. In one run the "for buy some apple"
|
||
card was gone while the erroneous text was visible; in another the card did
|
||
return (re-merged into a wider span). This is the second half of this item's
|
||
own acceptance criterion and belongs with item 2's stable identity work.
|
||
|
||
**Original repro (could not reproduce):** accept a suggestion (text
|
||
updates), press Ctrl+Z (text reverts — correct), press Ctrl+Shift+Z →
|
||
nothing happens. Observed on the Idiom card "by foots → on foot".
|
||
|
||
**Where to look:** `EditorCore.handleAccept`
|
||
(`web/src/components/Editor/EditorCore.tsx` ~line 556) applies via
|
||
`editor.chain().focus().insertContentAt(range, s.replacement).run()`, which
|
||
*is* a normal history transaction — so the break is probably downstream:
|
||
after the undo, the parent `onAccept`/re-check flow may dispatch a
|
||
transaction that clears the redo stack (any doc-touching tr wipes redo), or
|
||
the accepted suggestion's server-side state makes the recheck rewrite
|
||
content. Root cause was **not** confirmed in the session — instrument
|
||
first.
|
||
|
||
**Acceptance:** accept → undo → redo restores the replacement; the
|
||
suggestion card state stays consistent with whichever text is showing.
|
||
|
||
## 2. Suggestion stability: stop regenerating the world on every accept
|
||
|
||
The biggest feel gap vs Grammarly. Today every accept (and every edit)
|
||
triggers a full-document `POST /api/docs/:id/check`; all remaining cards
|
||
vanish and re-arrive seconds later, spans re-merge into different shapes,
|
||
and the LLM re-words every explanation each round (the "weather were" card
|
||
carried three different explanations in one session). It reads as
|
||
instability, doubles the pause after each accept, and burns qwen3.5 tokens.
|
||
|
||
**Proposed fix (server + client):**
|
||
- Split the doc into sentences (or paragraphs) and hash each. On re-check,
|
||
send only chunks whose hash changed since the last check; suggestions on
|
||
unchanged chunks are returned from cache byte-identical, including the
|
||
explanation text. `internal/suggestions/handlers.go` is the entry point.
|
||
- Give suggestions stable identity across checks: key on
|
||
(chunk hash, original span, replacement) so an untouched suggestion keeps
|
||
its `id`, and the client keeps the existing card DOM instead of
|
||
remounting (no vanish/reappear).
|
||
- Client: on accept, remove that one card optimistically and leave the rest
|
||
untouched while the changed-chunk recheck runs.
|
||
|
||
**Acceptance:** accepting one suggestion never changes the text, wording,
|
||
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 ~8–15 s from typing-stop to cards, with only a small "Checking…"
|
||
in the status bar. Two independent levers, both worth doing:
|
||
|
||
- **Incremental surfacing.** Stream/deliver per-chunk results as each
|
||
sentence finishes checking instead of one batch at the end (pairs
|
||
naturally with item 2's chunking). Status bar shows a live count:
|
||
"Found 3 so far…".
|
||
- **Instant local rules layer.** A tiny deterministic pass that underlines
|
||
the classics with zero network: a/an before vowel sound, plural after
|
||
some/many/three…, he/she/it + verb-s, common mass nouns ("an
|
||
information"). Petal's ethos (see `SUGGESTIONS.md`: LLM is garnish, plain
|
||
code essential) fits this exactly. Note `grammarLite.test.ts` already
|
||
exists under `web/src/components/Companion/` — check whether a rules
|
||
engine is already half-built before writing a new one. Local hits render
|
||
immediately with a modest style, then get confirmed/enriched (or
|
||
withdrawn) when the LLM pass lands.
|
||
|
||
**Acceptance:** an obvious error like "a apple" underlines in <100 ms
|
||
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
|
||
lower cards scrolls the document text fully off-screen, severing the
|
||
card↔sentence connection.
|
||
|
||
**Proposed fix:** keep the editor column sticky/pinned while the rail
|
||
(`web/src/components/Editor/SuggestionRail.tsx`, `.petal-rail` in
|
||
`index.css`) scrolls in its own `overflow-y: auto` container. Preserve the
|
||
existing anchor-to-highlight layout for cards that fit; the container only
|
||
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 (770–926) **and the prose is still on screen** (80–175).
|
||
- *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
|
||
building.** On the live build the same sentence *does* now produce a card:
|
||
a **Clarity** card reading 我想说这句话但是不知道用英语怎么说。 → "I want to
|
||
say this sentence but do…". So the span is detected and an English
|
||
rendering is already generated; what's missing is only the *framing* — it's
|
||
labeled Clarity rather than a first-class 翻译 · Translate type, so the
|
||
flagship moment reads as a tidy-up. Re-scope this item from "detect and
|
||
translate" to "give it its own type, label, and card treatment", which is
|
||
much cheaper than the plan below. The original observation follows.
|
||
|
||
Typed mid-document: 我想说这句话但是不知道用英语怎么说。 ("I want to say
|
||
this but don't know how in English") — Petal produced **no card at all**.
|
||
The pair model (`SUGGESTIONS.md` §1: user may type in either language,
|
||
Petal infers direction) says this should be the flagship moment.
|
||
|
||
**Proposed fix:** during chunking (item 2), detect spans in the pair's X
|
||
language inside an English context (CJK detection already exists for
|
||
spellcheck exclusion — see `web/src/components/Editor/SpellCheck.ts`).
|
||
Emit a new suggestion type `translate` whose replacement is the English
|
||
rendering, card labeled 翻译 · Translate, with the usual
|
||
Accept / Ask Petal. Accept replaces the span (keep the original as the
|
||
card's strikethrough line so she can still see what she wrote).
|
||
|
||
**Acceptance:** a Chinese sentence inside an English doc yields a Translate
|
||
card within one check cycle; accepting swaps in the English; an
|
||
English-only doc and a Chinese-only doc are unaffected.
|
||
|
||
### 5 — DONE (sixth session). The label was the whole gap, and it isn't the model's to give.
|
||
|
||
The re-scoped premise held: Petal already found the span and already rendered
|
||
it into English. Only the type was wrong. So the work was to decide that type
|
||
*structurally* rather than ask for it — a model that re-reasons every pass
|
||
would drift between labels for a sentence that hadn't changed, which is exactly
|
||
the instability item 2 just spent a session removing.
|
||
|
||
**Implemented:**
|
||
|
||
- `language.go` — `isTranslation(original, replacement, pairLang)`. Both halves
|
||
must hold: the span reads as her language *and* what Petal offers back reads
|
||
as English. The second half is not decoration — a Chinese span rewritten into
|
||
different Chinese is something else, and Petal has no business calling it a
|
||
translation.
|
||
- `reconcile.go` — the promotion sits at the single point where a type is
|
||
stamped, and only on the open-typed grammar checkpoint. A pass with a
|
||
`forceType` owns its family outright: voice reads paragraphs for tone and its
|
||
rows carry no replacement, so a "translation" there would be a card offering
|
||
nothing to accept. There is a test for exactly that.
|
||
- `normalizeType` still refuses `translate` from the model, deliberately. A
|
||
model that volunteers the label lands on grammar and is then promoted — or
|
||
not — on the evidence.
|
||
- Migration `0015` rebuilds the suggestions table for the extended `type` CHECK
|
||
(SQLite can't ALTER one), as `0005` and `0008` did before it.
|
||
- Client: `--color-jade`, the `translate` entry in `TYPE_META`, and
|
||
`.petal-suggestion-translate` for the inline underline.
|
||
- The pill is the one bilingual type name in the rail — `翻译 · Translate`,
|
||
`Tradução · Translate`, `Traduction · Translate`, from the pack. Every other
|
||
type stays English on purpose: those are the terms she is learning, and she is
|
||
learning them in English. This card's whole subject is her own language.
|
||
- The rail card stops truncating its two lines for a translation. Elsewhere the
|
||
original and replacement differ by a word and the explanation below is what
|
||
she reads; here the two lines are a whole sentence in each language and *are*
|
||
the card.
|
||
|
||
**The pair families need different tests, and pretending otherwise was the
|
||
trap.** The item's plan says to reuse the CJK detection from spellcheck — which
|
||
works, for zh, because it is a different script. It gives nothing for pt-PT, fr
|
||
or es, where no such signal exists. Those fall back to function words, and need
|
||
two distinct markers before Petal will claim anything; the lists deliberately
|
||
omit every word that is *also* English (`do`, `con`, `ya`, `todo`, and the
|
||
pan-Romance shorts), even where that costs a very common one. A single marker is
|
||
never enough, so a one-word Portuguese span won't trip it — a single word is a
|
||
vocabulary question, not a translation. The whole heuristic is justified by how
|
||
cheap its failure is: a wrong answer changes a coloured pill and nothing else,
|
||
because the replacement, the explanation and the Accept button are identical
|
||
either way.
|
||
|
||
**Two things only the browser could have told me.**
|
||
|
||
- **The inline underline was invisible.** The decoration plugin emits a
|
||
per-type class, and `.petal-suggestion` sets `border-bottom: 2px solid
|
||
transparent` — so a type with no colour rule renders with no underline at
|
||
all. Every test passed; the flagship span simply had no mark under it on
|
||
screen. Found by looking at the page, and it is the reason this doc keeps
|
||
insisting on that.
|
||
- **At her viewport there is no rail.** `railEnabled` needs 348 px beside the
|
||
editor, and at 1517×810 with the document list open the margin is 258 — so
|
||
the card she actually gets is the inline hover panel, not a rail card. Worth
|
||
knowing before item 7 is scoped: that item assumes the rail is what she sees
|
||
and treats the anchored popover as the missing half. On this screen it is the
|
||
other way round. (The fifth session measured rail cards at the same width, so
|
||
some state does reach it; not chased here.)
|
||
|
||
**Verified against the running binary**, with a stand-in model server so no VPN
|
||
or GPU was involved. The stub types the Chinese finding `"clarity"` — exactly
|
||
what the live build did — so the label on screen can only have come from Petal's
|
||
own detection. Through the real `/check`: `grammar` for the English sentence,
|
||
`translate` for the Chinese one. In the browser: the jade underline distinct
|
||
from grammar's mint, and the card showing the `翻译 · Translate` pill, the
|
||
Chinese struck through and wrapped over two lines rather than clipped, the
|
||
English rendering, the bilingual explanation, and Accept.
|
||
|
||
**Not verified in a browser:** the *rail* card's version of the same thing. The
|
||
rail is unreachable on this 1517 px display (above), and forcing it by hand kept
|
||
being overridden by React's own layout. Its label goes through the same
|
||
`typeLabel` call the hover card just proved, and its diff differs only by a
|
||
class toggle — but that is a reading, not a measurement, and it is written down
|
||
as one.
|
||
|
||
Coverage: `language_test.go` (the flagship sentence; a lone Han rune; one
|
||
Chinese word inside English prose; Chinese→Chinese; a Chinese span on the wrong
|
||
pair; all three Latin pairs; an English sentence stuffed with pan-Romance
|
||
lookalikes; French elision; unknown and absent pairs), `translate_type_test.go`
|
||
(the type through the real `/check`, an English correction keeping its own type,
|
||
and voice unable to mint one), `suggestionMeta.test.ts` (every type has a colour
|
||
and a name, translate's colour is its own, the pill is bilingual per pair and
|
||
every other pill isn't), and `db_test.go`'s
|
||
`TestTranslateTypeMigrationPreservesRows` — 0015 rebuilds the table, so it is
|
||
the first migration here that could silently *drop her rows*; every column,
|
||
both timestamps and both indexes are asserted across it.
|
||
|
||
One test changed contract: the pt-PT pack's Brazilian-forms grep searched
|
||
`JSON.stringify(pack)`, which includes field *names* — and duly failed on
|
||
`translateLabel`, since it lowercases to "transla·tela·bel" and so "contains"
|
||
the pt-BR *tela*. It now searches the pack's copy only, with two canaries, since
|
||
every assertion in it is a negative and a haystack that quietly went empty would
|
||
make the whole test pass by having nothing to search.
|
||
|
||
## 6. Ask Petal answers: bilingual, and room to read
|
||
|
||
The card's *explanation* is bilingual, but the Ask Petal *answer* came back
|
||
English-only, rendered in a small scrollable box inside the card.
|
||
|
||
**Proposed fix:** prompt the answer path
|
||
(`internal/suggestions/chat*.go` / `AskPetal.tsx`) to reply in both pair
|
||
languages (native first, mirroring the bubble pattern in
|
||
`PetalCompanion.tsx`); let the answer area grow to the card's width with a
|
||
sane max-height (~50vh) before scrolling.
|
||
|
||
**Acceptance:** an Ask Petal answer shows 中文 + English; a 3-paragraph
|
||
answer is readable without scrolling a ~100 px box.
|
||
|
||
### 6 — DONE (eighth session). The answer was English-only on purpose, and the box was the smaller half of the item.
|
||
|
||
**The first half was one sentence in a prompt.** `askPetalSystemTemplate` said
|
||
*"Detect the language of the user's message and respond in that same language…
|
||
Never mix languages in a single response."* Self-consistent, and it made the
|
||
English-only answer inevitable: ask in English — which she does, because she is
|
||
practising — and the explanation that goes deepest into the "why" is the one
|
||
surface that gives her nothing in her own language. It now asks for both halves
|
||
every time, pair language first, and the old sentence is gone (a model handed
|
||
both instructions picks one at random).
|
||
|
||
**Which half is the lesson is not Petal's to assume.** The first draft of this
|
||
justified the change as "her language is the safety net, English is what she's
|
||
learning" — wrong, and wrong in a way the code would have carried for good. The
|
||
pair is (English + X) and Petal is used from both ends: an English speaker
|
||
learning French needs the French half for exactly the reason a Mandarin speaker
|
||
learning English needs the English one. So the prompt asks for both and says it
|
||
does not know which way round, and nothing in the wording, the rendering or the
|
||
comments assigns the halves a role. The ordering still holds either way — the
|
||
pair language leads, English follows, which is the pack's order everywhere else.
|
||
|
||
**The blank line between the halves is a contract, and a soft one.**
|
||
`bilingualReply.ts` splits on the first blank line to render the two halves the
|
||
way the companion renders its two lines. It is deliberately forgiving because
|
||
the reply streams in token by token from a small local model: a half-arrived
|
||
reply is all "native" and the English simply appears beneath it when the break
|
||
lands; a model that ignores the instruction renders as one ordinary block. The
|
||
one thing it will never do is drop text. A separator with nothing on one side of
|
||
it is a stray newline, not a split, and is kept whole.
|
||
|
||
- Petal's bubbles now take the card's full width. Two languages in the 85% a
|
||
chat reserves to show who is talking wrapped a sentence into a paragraph, and
|
||
the tint and alignment already say who is talking.
|
||
- `chatFailed` moved into the packs. It is the only message the panel writes
|
||
without the model, and it was English-only — telling the half of the pair that
|
||
can't read English nothing at all, in the one situation where nothing else is
|
||
on screen. It is written blank-line separated, so it renders through the same
|
||
two-half bubble as a real reply.
|
||
|
||
**The height half was the larger one, and the first fix was wrong.** The item
|
||
asks for ~50vh. Because the anchored card opens under the flagged word and never
|
||
flips above it, the first version took the *smaller* of 50vh and the room left
|
||
below the card, so it could never overhang the screen. Measured on the running
|
||
build, that gave **176 px against a 442 px answer** — worse than the 220 px it
|
||
replaced. The card's own pill, diff, explanation and action row already spend
|
||
~290 px of an 810 px window: "fits below the word" and "room to read" are not
|
||
both available, and the clamp silently chose the wrong one.
|
||
|
||
So the ceiling is flat 50vh and the overhang is made navigable instead — item 4's
|
||
answer to the same conflict, in its own words: *make the overhang navigable, not
|
||
shrink what each card says*. Both surfaces that host the panel now report their
|
||
reach, so the column grows and the page can scroll to what hangs below:
|
||
|
||
- `SuggestionCard.tsx` — `onExtent`, a ResizeObserver rather than a one-shot
|
||
measure, because the card grows twice after it mounts: the panel opens, and
|
||
then the reply streams into it. It reports 0 as it unmounts.
|
||
- `EditorCore.tsx` — `cardExtent` beside `railExtent`, resolved to one
|
||
`overhang` (whichever reaches lower) that feeds the wrapper's `minHeight`. The
|
||
rail's contribution stays conditional on the rail being mounted; the card's
|
||
does not, because it withdraws its own.
|
||
- The rail needed nothing: it already re-measures on `expandedId` and a
|
||
per-card ResizeObserver, so a rail card whose conversation grows reports it.
|
||
|
||
**Verified in a real browser at the review's own 1517×810**, against a stand-in
|
||
model server (no VPN, no GPU) returning a deliberately long three-paragraph
|
||
bilingual reply:
|
||
|
||
- *Anchored card.* Box **176 → 362 px** (50vh), card overhangs by 165 px, the
|
||
column gained 209 px of scroll where the same card previously had 20, and
|
||
Accept is fully on screen after scrolling to it. No horizontal overflow.
|
||
- *Rail card.* Box 334 px, overhang 104 px, 204 px of scroll room, Accept
|
||
reachable. The rail's own extent pipeline covered it, as read.
|
||
- *On a taller window* the whole 494 px answer fits with no scrollbar at all.
|
||
|
||
**Honest limit:** at 810 px a genuinely long answer still scrolls — 362 px of
|
||
442. The item's "readable without scrolling a ~100 px box" is met in the sense
|
||
that mattered (the box is no longer a peephole and the rest is a short scroll,
|
||
not a hunt), but a three-paragraph *bilingual* answer is roughly twice the text
|
||
the item imagined, and half a small screen does not hold it.
|
||
|
||
**A trap worth recording, because it cost two false measurements.** The restart
|
||
script used `pkill -f 'scratchpad/petal$'`, which never matched: the process was
|
||
started as `./petal` after a `cd`, so its command line doesn't contain the path.
|
||
Every "restart" after the first therefore failed to bind the port and died
|
||
quietly, while the *original* binary kept serving the *original* bundle — and
|
||
the page loaded fine, the app worked, and the numbers looked plausible. Two
|
||
rounds of "the fix didn't take" were measurements of code that was never
|
||
running. What caught it was the inline `max-height` reading `176.417px`, a value
|
||
the new code cannot produce. **Check the served bundle hash, not that the page
|
||
loads** (`curl -s localhost:PORT/ | grep -o 'index-[A-Za-z0-9_-]*\.js'` against
|
||
`web/dist/index.html`).
|
||
|
||
**Deliberately not done:**
|
||
|
||
- No auto-scroll to the card when the panel opens. The conversation starts short
|
||
and grows; scrolling the page out from under her the moment she clicks Ask
|
||
Petal would move the sentence she is reading about, to solve a problem she
|
||
does not have yet.
|
||
- The seed bubble stays single-language. It is the pair-language rendering of
|
||
the English explanation printed directly above it in the same card — the card
|
||
is already bilingual across those two lines, and repeating the English inside
|
||
the bubble would be the "same text twice" the seed exists to avoid.
|
||
|
||
Coverage: `bilingualReply.test.ts` (both scripts, mid-stream, one-language,
|
||
extra blank lines, whitespace-only separator, empty, single newlines inside a
|
||
half), `lang_test.go`'s `TestAskPetalAnswersInBothLanguages` (all four pairs ask
|
||
for both languages, name the pair language first, keep the separator, and no
|
||
longer carry the sentence forbidding it), and an `i18n.test.ts` case that every
|
||
pack's `chatFailed` has two non-empty halves and keeps its English one in
|
||
English. **The extent wiring has no unit test**, for the reason item 7 recorded:
|
||
jsdom has no layout, every rect is zero, and a test there would pass whatever
|
||
the code did. It is browser-verified only, and is written down as such.
|
||
|
||
## 7. Inline popover at the underline (verify, then strengthen)
|
||
|
||
Grammarly's core gesture is click-the-word → popup at the word.
|
||
`EditorCore.tsx` already has a `hover` card anchored to highlights (see
|
||
`handleAccept`'s fallback burst position), and the rail wires
|
||
`activeId`/`onHover` both ways — so part of this exists. The session
|
||
experience on a wide (1517 px) screen was still: click underline → the
|
||
*far-right* rail card expands, ~400 px of eye travel.
|
||
|
||
**Task:** confirm the hover card appears on click as well as hover, that it
|
||
offers Accept + a one-line reason + "more" (expanding the rail card), and
|
||
that hover-linking (card ↔ span glow) works in both directions. Fix
|
||
whichever half is missing.
|
||
|
||
**Acceptance:** clicking an underline shows an anchored mini-popover with
|
||
Accept, without needing the rail; hovering a rail card glows its span and
|
||
vice versa.
|
||
|
||
### 7 — DONE (seventh session). The rail isn't a screen-size fact; it's a mode.
|
||
|
||
The item says to confirm first and fix whichever half is missing. Confirming
|
||
first is what mattered, because the interesting defect wasn't either half.
|
||
|
||
**Settled first: the contradiction items 4 and 5 left behind.** The fifth
|
||
session measured rail cards at 1517px; the sixth found no rail at all at the
|
||
same width and wrote down a margin of 258. Both were right. The editor is a
|
||
fixed 720px column centred in the pane, and the doc-list sidebar is 280px, so
|
||
at 1517px the right margin is **258 with the sidebar open and 406 without** —
|
||
either side of the rail's 348 threshold. What moves between them is
|
||
distraction-free mode, which engages *on its own* the moment the editor takes
|
||
focus. So the rail is not a property of her screen. It appears when she starts
|
||
writing and disappears when she stops, and both sessions had simply caught it
|
||
in different states.
|
||
|
||
**The bug that fell out of that.** `recomputeRail` was triggered by a
|
||
ResizeObserver on the wrapper, a window `resize`, or a change to the suggestion
|
||
set. Entering or leaving distraction-free mode is none of the three: the
|
||
wrapper is a fixed 720px column, so re-centring it changes its *position* and
|
||
never its *size*, and a ResizeObserver reports only size. `railEnabled` therefore
|
||
kept whatever value it last had.
|
||
|
||
Leaving distraction-free with the rail up is the bad direction, and it is not
|
||
subtle — measured in Chrome at 1517×810: the 300px column stayed mounted in the
|
||
266px margin the restored sidebar left behind, **overhanging the viewport by
|
||
66px**, cards clipped mid-sentence ("use "an": "a…"), and the page grew a
|
||
horizontal scrollbar it never has otherwise. The other direction is only a loss:
|
||
she starts typing, the margin opens to 406, and no rail arrives. Both persisted
|
||
indefinitely — dispatching a lone `resize` event was enough to correct either,
|
||
which is what proved the measurement was the only thing missing.
|
||
|
||
**Implemented:**
|
||
|
||
- `EditorCore.tsx` — the ResizeObserver now watches `.petal-scrollport` as well
|
||
as the wrapper. The scrollport spans the pane, so it resizes whenever the
|
||
chrome around the editor does; the wrapper, being fixed-width, never does. It
|
||
is the element the sticky-pin code already reaches for, so it needed no new
|
||
handle, and unlike threading `focusMode` down as a prop it also covers any
|
||
future chrome that moves the editor.
|
||
- `railFit.ts` — `RAIL_MIN_MARGIN` and `railFitsBeside` lifted out of the
|
||
measurement callback. A bare `>=` doesn't need a name; this one earns it,
|
||
because the number picks between two entirely different suggestion surfaces
|
||
and the margin it reads moves for reasons unrelated to window size.
|
||
- `EditorCore.tsx` — **clicking a highlight now opens the anchored card even
|
||
when the rail is up**, which is the item's own acceptance criterion and was
|
||
previously false by design. Hover still defers to the rail: an unbidden
|
||
floating card next to a margin card saying the same thing is noise, and that
|
||
earlier reasoning was about hover and still holds. A click isn't. The rail card
|
||
glows instead of expanding, so the suggestion is never open in two places, and
|
||
a click-opened card keeps its glow after the pointer leaves (it closes on a
|
||
click away) so the margin and the open card don't disagree about what she's
|
||
reading.
|
||
|
||
**Measured, not estimated.** The item guessed ~400px of eye travel from
|
||
underline to rail card. At 1517px in distraction-free mode the real distance
|
||
from the first flagged span's right edge to its card is **651px**. After the
|
||
change the card lands 6px under the word.
|
||
|
||
**Verified in a real browser at the review's own 1517×810**, driving the local
|
||
build with the rule pack from item 3b so no model or VPN was involved:
|
||
|
||
- *Rail follows the mode, with no resize event anywhere.* Click into the prose →
|
||
sidebar collapses, margin 406, rail mounts with its cards, no overflow. Escape
|
||
→ sidebar restores, margin 258, rail unmounts, no overflow, no horizontal
|
||
scroll. Re-focus → it comes back. Re-run after the `railFit` extraction.
|
||
- *Click with the rail up.* Popover opens flush under "a apple" (6px gap, left
|
||
edges aligned), carrying the type pill, the diff, the full explanation, Ask
|
||
Petal, Accept and Dismiss; it fits the viewport; exactly one rail card glows
|
||
and **none is expanded**.
|
||
- *Accept from that popover.* Text became "an apple", the popover closed, the
|
||
rail went 6 cards → 5, and the other four kept their id, position and wording
|
||
— item 2's stability holding under a path it hadn't been exercised on.
|
||
- *The two halves the item asked about were already fine.* Span hover lights its
|
||
rail card, card hover lights its span (both directions, checked via the
|
||
`-active` classes). And with the rail off, clicking an underline already gave
|
||
an anchored popover — richer than the item's "one-line reason + more", since
|
||
it carries the whole explanation and Ask Petal. Nothing to build there.
|
||
|
||
**Deliberately not done:** no "more" affordance linking the popover to a rail
|
||
card. The item imagined the popover as a teaser for the rail's fuller version;
|
||
there is no fuller version — both surfaces render the same explanation, and the
|
||
popover has Ask Petal too. Adding a control that expands a second copy of what
|
||
she is already reading would be the redundancy the hover rule exists to avoid.
|
||
|
||
Coverage: `railFit.test.ts` pins the threshold to the margins actually measured
|
||
in Chrome — 406 fits, 258 and the mid-animation 266 don't, the bound is
|
||
inclusive, 1920-with-sidebar fits, narrow windows never do. **The observer wiring
|
||
itself has no unit test and can't have a useful one**: jsdom has no layout, so
|
||
every `getBoundingClientRect()` is zero, `railFitsBeside(0, 0)` is false, and the
|
||
rail branch is unreachable there. That half is browser-verified only, and is
|
||
written down as such rather than covered by a test that would pass regardless.
|
||
|
||
## 8. Smaller items (each small, do opportunistically)
|
||
|
||
- **Accept All per category.** Five tense fixes = five clicks today. Add
|
||
"Accept all Grammar (5)" per category header in the rail, one undo step
|
||
for the batch. Acceptance: batch-accept applies all, single Ctrl+Z
|
||
reverts the batch.
|
||
- **Keyboard flow.** Tab/Shift+Tab (or n/p) cycles underlines with the
|
||
popover open; Enter accepts, Esc dismisses popover. Acceptance: a doc
|
||
can be fully triaged without the mouse.
|
||
- **Dismissal persistence.** Untested in session: does a dismissed (✕)
|
||
suggestion stay dismissed after the next full re-check? With item 2's
|
||
stable identity, store dismissed keys per doc and filter server-side.
|
||
Acceptance: dismiss → edit elsewhere → recheck → the dismissed card does
|
||
not return.
|
||
- **Status-bar summary.** "3 petals to polish 🌸 · 三片花瓣待打磨" next to
|
||
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)
|
||
|
||
- No document score/grade, no streaks-pressure — the Garden and kitten
|
||
already carry motivation warmly.
|
||
- No browser-extension-style everywhere-checking; Petal is the writing
|
||
place.
|
||
- The doc-type dropdown's translucent look during open was **animation
|
||
mid-fade, not a bug** — leave it.
|
||
|
||
---
|
||
|
||
## Handoff — state as of 2026-07-27, second session
|
||
|
||
**Shipped and live.** `main` is at the merge
|
||
`Merge fix/companion-yields-to-cards`, pushed to gitea, and the VPS is
|
||
rebuilt on it (`git pull && docker compose up -d --build`, all five
|
||
containers healthy). Working tree clean.
|
||
|
||
Note that deploy carried **two** commits: the item 0 fade (written in the
|
||
first session, never deployed) and this session's panel follow-up. So the
|
||
kitten only started yielding to rail cards in production with this push —
|
||
if you're comparing against memory of the live site, that's why.
|
||
|
||
**Done:** item 0 and its follow-up. **Closed without code:** item 1 (not
|
||
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.)*
|
||
|
||
*(Sixth session: **the stack is merged and live.** Items 3b, 2 and 4 went to
|
||
`main` as one `--no-ff` merge (`963fc17`), pushed to gitea, and the VPS is
|
||
rebuilt on it — all five containers healthy, frontend build
|
||
`014b43cdf44f`, migration `0014` (`checked_chunks`) applied cleanly on the
|
||
real database. Green before merging: `go test ./...`, `tsc --noEmit`, 195
|
||
vitest tests. So the three sessions of work she hasn't seen, she can now
|
||
see. Untouched: 6, 7, 8, item 3's incremental half, item 5's re-scoped
|
||
Translate card type.)*
|
||
|
||
*(Sixth session, second half: item 5 done — see the subsection under it —
|
||
**merged and live** (`b23c5a9`). Two findings there are worth reading before
|
||
picking the next item: the inline underline needs a per-type CSS rule or it
|
||
renders invisibly, and at her actual viewport the rail is disabled — the inline
|
||
hover card is what she sees, which inverts item 7's premise. Untouched: 6, 7, 8,
|
||
item 3's incremental half.)*
|
||
|
||
*(Seventh session: item 7 done — see the subsection under it. Two things there
|
||
are worth carrying forward. First, **the rail is a mode, not a screen size**:
|
||
distraction-free engages by itself on editor focus and moves the margin across
|
||
the rail's threshold, so "does she see the rail?" has no fixed answer at a given
|
||
width — items 4 and 5 disagreed only because they caught it in different states.
|
||
Second, the layout invariant that bit here is the same shape as the one item 4
|
||
recorded: **a fixed-width column that gets re-centred changes position without
|
||
changing size**, and neither a ResizeObserver on it nor a window resize will say
|
||
so. Untouched: 6, 8, item 3's incremental half.)*
|
||
|
||
*(Eighth session: item 6 done — see the subsection under it. Two things to carry
|
||
forward. First, **the pair is symmetric**: Petal is used from both ends, so
|
||
"her language" and "the language being learned" are not interchangeable terms,
|
||
and any copy or prompt that assigns the two halves a role is wrong for half the
|
||
users — the wording here was corrected mid-session for exactly that. Second,
|
||
**check the served bundle hash before believing a browser measurement**: a
|
||
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. **Not pushed and not deployed**, by choice:
|
||
`main` is at the merge `feat/settled-spans` **locally only**, and both gitea and
|
||
the VPS are still at `1aa3a14`. Nothing about it needs a migration or a backup —
|
||
it adds one read-only endpoint and touches no schema — so deploying it is
|
||
`git push` then `git pull && docker compose up -d --build`, whenever it's
|
||
wanted. 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
|
||
`data/backups/pre-0015-translate-20260728T073254Z/`), because stopping the
|
||
container does **not** checkpoint: `petal.db` was still four hours stale while
|
||
2 MB of her writing sat in the `-wal`. A lone `cp data/petal.db` would have been
|
||
a backup of the wrong day. Worth remembering for the next migration.
|
||
|
||
After the rebuild, on the live database: `integrity_check ok`, still 112
|
||
suggestions across 9 documents, both indexes recreated, `'translate'` in the
|
||
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 (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 —
|
||
it is the last small one, it is self-contained (prompt the answer path to reply
|
||
in both pair languages, then let the answer area grow to ~50vh instead of a
|
||
~100px scrollbox), and item 7 just made the surface it lives on more prominent:
|
||
Ask Petal now opens inside a card anchored at the word in *both* layouts, so a
|
||
cramped English-only answer is more visible than when the review was written.
|
||
Item 8's four are all still small and independent; **dismissal persistence** is
|
||
the one with real value now that item 2 gives suggestions stable identity across
|
||
checks. Item 3's incremental-surfacing half still needs streaming, which the
|
||
current `/check` response shape doesn't do — it remains the largest of what's
|
||
left.
|
||
|
||
*(Superseded, kept for the reading list: the sixth session recommended item 5's
|
||
Translate card type, which is now done and live.)*
|
||
|
||
*The advice below was written for the second session and is kept for its
|
||
reading list, not its recommendation: item 3b is done and deployed.* It was
|
||
**largely already built, in `main`**, and the item as written didn't know
|
||
that. Before touching the rules engine, read:
|
||
|
||
- `web/src/components/Companion/prose.ts` — the deterministic rules engine,
|
||
client-side, already the single source of non-LLM detection. Sibling
|
||
tests: `prose.test.ts`, `grammarLite.test.ts`.
|
||
- Commit `96f68a9` ("Add deterministic mechanics suggestion family,
|
||
rule-based, no LLM"). Applyable rules already emit exact-span
|
||
`original -> replacement` fixes that surface as suggestion cards, under
|
||
a `mechanics` family persisted via `POST /docs/{id}/mechanics`;
|
||
awareness-only rules (run-ons, splices) stay companion bubbles, and the
|
||
companion hides fix-bearing hints so a span is never both.
|
||
- Commit `9d2501a` ("Suppress fickle re-edits of sentences the user already
|
||
settled") — a partial item 2 that also already shipped.
|
||
|
||
So item 3b's real remaining work is probably not "write the rules" but
|
||
"make the existing local hits render *immediately*, before the LLM pass,
|
||
with a modest style" — i.e. the latency/ordering half of the item, plus
|
||
whatever rules `prose.ts` is missing. Scope it against the code, not
|
||
against the item text.
|
||
|
||
Item 5 likewise: `internal/suggestions/translate.go` already exists — read
|
||
it before designing a `translate` suggestion type.
|
||
|
||
(An earlier draft of this handoff claimed those two commits sat on
|
||
unmerged branches. They don't; both are in `main`. That came from
|
||
misreading `git branch -vv` tracking info as merge status.)
|
||
|
||
**Verification technique, for whoever picks up items 2/3/7.** Claims about
|
||
editor behaviour in this doc should be checked against the running build,
|
||
not reasoned about from source — item 1 looked airtight on paper and was
|
||
wrong. Against a production bundle there's no exposed editor handle, so:
|
||
walk up from `.ProseMirror` to the nearest `__reactFiber$` key, breadth-
|
||
first through the fiber tree for an object with `.view`/`.state`/
|
||
`.commands`/`.schema` — that's the Tiptap editor. From there
|
||
`editor.can().redo()`, the prosemirror-history plugin state
|
||
(`done.eventCount` / `undone.eventCount`), and a wrapper around
|
||
`view.dispatch` logging `tr.docChanged` are enough to settle most
|
||
"does the editor really do X" questions in a couple of minutes.
|
||
|
||
**Housekeeping:** every topic branch in the repo was fully merged into
|
||
`main` and they have all been deleted, locally and on origin. Testing
|
||
item 1 meant accepting suggestions on her live document;
|
||
the one residual edit was reverted ("on foot" → "by foots") and the doc
|
||
text is as it was found, but the History panel now shows several extra
|
||
auto-snapshots from that session.
|