Stopping the container does not checkpoint the WAL: petal.db on disk was four hours stale while 2 MB of her writing sat in petal.db-wal. A lone `cp data/petal.db` would have backed up the wrong day — and 0015 is the first migration here that rebuilds a table, so that backup was the one that mattered. Recorded for the next one. Claude-Session: https://claude.ai/code/session_016y6gyuHkQXPiEuW8RGQyua
685 lines
38 KiB
Markdown
685 lines
38 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.
|
||
|
||
## 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.
|
||
|
||
## 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.
|
||
|
||
---
|
||
|
||
## 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.)*
|
||
|
||
**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 (sixth session onward):** items 6, 7, 8 are all untouched
|
||
and all small; item 5's re-scoped Translate card type is the cheapest
|
||
*visible* win (see its Status note — the span is already detected and
|
||
already rendered in English, it's only mislabeled as Clarity), and
|
||
`internal/suggestions/translate.go` already exists — read it before
|
||
designing a new type. Item 3's incremental-surfacing half now has the
|
||
chunking it was waiting on, but still needs streaming, which the current
|
||
`/check` response shape doesn't do.
|
||
|
||
*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.
|