Files
petal/UX_REVIEW_2026-07-27.md
T
prosolis c33de1175b Render deterministic rule hits instantly, not on the LLM's clock
The rule pack in prose.ts already found "a apple" — articles,
pluralAfterNumber, subjectVerbAgreement, uncountables are all there, and
they already surface as real mechanics cards. But mechanicsFindings only
ran inside runCheck, behind the same 4s checkpoint debounce as the model,
and only reached the screen via the server's reply. A free, instant,
offline-capable detection was being delivered on an LLM-shaped delay.

The rule pack now runs on its own 250ms fuse and renders its findings with
no network at all, as provisional cards. The mechanics submit follows; its
reply is authoritative and clears them. If the reply never comes — offline,
server down — the cards simply stay, which is the whole point of having
rules that need no model.

Provisional cards are keyed by wording rather than position, so one 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, so accepting inside that window still records the keep and
plants its word in the garden instead of being quietly dropped; null means
there is no row and the edit has landed regardless. Findings she actions
while provisional are remembered client-side, because the detector has no
memory between runs. runCheck no longer re-submits what the fast pass
already filed — it's the catch-up path for when that submit failed.

The arrival chime keys rule-pack cards by wording too, so a finding doesn't
chime once as provisional and again as persisted.

Not done, deliberately: no distinct style for unconfirmed local hits. The
rail renders both engines identically on purpose, and a provisional card now
lives for one LAN round-trip.

Claude-Session: https://claude.ai/code/session_016y6gyuHkQXPiEuW8RGQyua
2026-07-27 22:28:37 -07:00

388 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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.
## 3. Perceived latency: mask the LLM round-trip
Measured ~815 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.
## 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.
## 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.)*
**Suggested next:** item 3b, the instant local rules layer — but it is
**largely already built, in `main`**, and the item as written doesn't know
that. Before writing any rules engine, read:
- `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.