A whole category accepted in one click, and one undo
Five article fixes were five clicks, five confetti bursts and five undo steps. "Accept all Tidy-up (5)" makes them one of each. The single undo decided the implementation: every replacement goes into one Tiptap chain, which applies as one transaction and so undoes as one history event. That only works if the spans can't move under each other, so the plan resolves every span against the document as it stands and applies them last-first. Three outcomes rather than one, because a batch that quietly dropped a card would be reporting edits it never made: a span she already fixed herself is settled without an edit (what a single Accept does too), and a card quoting the same words as one already taken is left on screen, since findRange would resolve both to the same place. The control sits on the first card of its kind — the rail can't carry a category header, its cards are anchored to their own sentences — and only when the category has company. It is outlined rather than filled: it acts on cards she can't see from where she's standing.
This commit is contained in:
+123
-4
@@ -861,6 +861,105 @@ one from many.
|
||||
**Still open in item 8:** Accept All per category, and the keyboard triage
|
||||
flow. Both untouched.
|
||||
|
||||
### 8 — Accept All per category DONE (tenth session). The undo step decided the shape.
|
||||
|
||||
The item's one design question — the batch must be a single undo — turned out to
|
||||
decide the implementation rather than follow it. Five accepts is five
|
||||
transactions, and prosemirror-history would group them only by *timing*, which is
|
||||
not a guarantee. So every replacement goes into one Tiptap `chain()`: a chain is
|
||||
applied as a single transaction, and the history plugin undoes a transaction as
|
||||
one event. That is the whole mechanism, and the rest of the work is making a set
|
||||
of spans safe to apply in one go.
|
||||
|
||||
**Implemented:**
|
||||
|
||||
- `acceptBatch.ts` — `planBatch(list, resolve)`, pure, takes the span resolver as
|
||||
an argument so the arithmetic can be tested without a ProseMirror document.
|
||||
Every span is resolved against the doc as it stands *before* any edit, and the
|
||||
steps are returned **last-span-first**: applying from the end backwards means no
|
||||
earlier position can be shifted by a later replacement, which is what lets them
|
||||
share one transaction with no position mapping.
|
||||
- Three outcomes, not one. A card whose span is **gone** (she fixed it herself) is
|
||||
settled without an edit — exactly what a single Accept already does with an
|
||||
unresolvable span. A card that **overlaps** one already taken is left on screen:
|
||||
`findRange` resolves to the *first* occurrence, so two cards quoting the same
|
||||
words would have the second overwrite the first. A batch that silently dropped
|
||||
either would be reporting edits it never made.
|
||||
- `EditorCore.handleAcceptAll(type)` — builds the plan, applies the chain, fires
|
||||
**one** confetti burst over the topmost changed span (read before the edit
|
||||
removes the highlight), and hands the settled cards to the parent.
|
||||
`burstAt`/`showConfetti` were lifted out of `handleAccept`, which now shares
|
||||
them.
|
||||
- `App.handleAcceptMany` — the bookkeeping only. Each row is filed individually
|
||||
(there is no batch endpoint, and each accept plants its own word in the garden)
|
||||
but the kitten cheers **once**: five cheers for one click would read as five
|
||||
separate congratulations for a decision she made once.
|
||||
- `batchLeaders` — which card carries the control. The item asks for a category
|
||||
*header* in the rail, and the rail cannot have one: cards are anchored to their
|
||||
own sentence, so a type's cards are scattered down the column. The closest
|
||||
honest stand-in is the first card of its kind. The first version put the button
|
||||
on every card of the type, which in the browser was five identical buttons in
|
||||
one column saying one thing.
|
||||
- The control appears only at **two or more** — a lone Grammar card doesn't grow a
|
||||
second button saying the same thing as the first — and on both surfaces, since
|
||||
item 7 made the anchored popover primary in both layouts. It is outlined in the
|
||||
category's colour rather than filled like Accept: it acts on cards she can't see
|
||||
from where she's standing, and an equally loud button would invite the click she
|
||||
meant to give the one suggestion in front of her.
|
||||
- The label stays English (`Accept all Tidy-up (4)`) even on a translation card,
|
||||
whose pill is bilingual. The pill names the kind of advice she is reading; this
|
||||
names an action over the rest of the queue, and every other word in that row —
|
||||
Accept, Dismiss, Ask Petal — is English. A control that changed language between
|
||||
cards would read as a different control.
|
||||
|
||||
**Verified in a real browser at the review's own 1517×810**, against a stub model
|
||||
server (no VPN, no GPU) that adds two `grammar` findings so the document carries
|
||||
two categories at once:
|
||||
|
||||
- *Single undo, measured.* `view.dispatch` hooked to count doc-changing
|
||||
transactions: accepting five Tidy-up cards produced **one** transaction and took
|
||||
the history's `done` count from 1 to 2. One Ctrl+Z restored all five originals;
|
||||
one Ctrl+Shift+Z put all five corrections back.
|
||||
- *Categories don't touch each other.* With three Tidy-up and one Grammar card,
|
||||
"Accept all Tidy-up (3)" fixed exactly its three; the Grammar span stayed
|
||||
underlined and its card stayed put.
|
||||
- *One control per category*, on the first card of its kind, on the rail — and the
|
||||
Grammar card, alone in its type, offered none on either surface.
|
||||
- *Both surfaces click.* The rail's button and the anchored popover's own button
|
||||
were each clicked and each applied the whole category, closing the card and
|
||||
leaving one cheer behind.
|
||||
|
||||
**A measurement trap that cost an hour, and it is the sixth and ninth sessions'
|
||||
trap in a third disguise.** After the accept the rail appeared to keep its
|
||||
accepted cards on screen indefinitely — server said nothing pending, the status
|
||||
bar agreed, the DOM still had five cards. It is not a bug: **`recomputeRail` does
|
||||
its work inside `requestAnimationFrame`, and Chrome does not fire rAF in a hidden
|
||||
tab.** Driving the page through the JS tool leaves the window backgrounded, so
|
||||
the rail is simply frozen; every screenshot (which forces a paint) showed it
|
||||
correctly empty. Anything measured through rAF is invalid unless the tab is
|
||||
foreground — check `document.visibilityState` before believing a rail reading.
|
||||
|
||||
And the bundle-hash check caught its third variant: **the Go binary embeds
|
||||
`web/dist`**, so rebuilding the frontend alone changes nothing. `npm run build`
|
||||
without `go build` served the previous bundle from a server that had just been
|
||||
restarted and looked entirely healthy.
|
||||
|
||||
Coverage: `acceptBatch.test.ts` — the ordering property asserted by actually
|
||||
applying the plan to a string and checking the result; document order rather than
|
||||
card order; the missing-span case; two cards quoting the same words; adjacent
|
||||
spans not counting as overlapping; an awareness-only card with nothing to insert;
|
||||
and `batchLeaders` (one leader per category, follows the stack order it is given,
|
||||
empty stack). **The wiring itself has no unit test**, for the reason items 6 and 7
|
||||
recorded: jsdom has no layout, and a rail test there would pass whatever the code
|
||||
did. It is browser-verified only, and is written down as such.
|
||||
|
||||
**Deliberately not done:** no "accept everything" across all categories. The
|
||||
categories are the unit she can reason about — five article fixes are one
|
||||
decision, but her whole queue is not — and a single button that rewrites the
|
||||
document in one press is the opposite of a tool that teaches.
|
||||
|
||||
**Still open in item 8:** the keyboard triage flow.
|
||||
|
||||
---
|
||||
|
||||
## Explicit non-goals (from this review)
|
||||
@@ -960,6 +1059,18 @@ 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.)*
|
||||
|
||||
*(Tenth session: item 8's Accept All per category done — see the subsection under
|
||||
item 8. **Still not pushed and not deployed**, and the undeployed stack is now
|
||||
four commits: the ninth session's settled-spans work plus this. Neither needs a
|
||||
migration and neither touches the schema, so deploying is still `git push` then
|
||||
`git pull && docker compose up -d --build`. Two things to carry forward. First,
|
||||
**a hidden Chrome tab fires no `requestAnimationFrame`**, and `recomputeRail`
|
||||
lives inside one — so the rail freezes under JS-tool automation and every reading
|
||||
of it is stale until a screenshot forces a paint. Second, **the binary embeds
|
||||
`web/dist`**: rebuild the frontend and you must rebuild the binary, or the
|
||||
bundle-hash check will (rightly) fail. Untouched: item 8's 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
|
||||
@@ -974,10 +1085,18 @@ 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
|
||||
**Suggested next (tenth session onward):** two things remain in the whole review.
|
||||
**Keyboard triage** is the one to take: it is the last of item 8, and Accept All
|
||||
just built half of what it needs — a category is now a thing the UI can act on in
|
||||
one step, so "triage without the mouse" is mostly about driving the anchored
|
||||
popover between spans. **Item 3's incremental surfacing** is the last item of any
|
||||
size, and still needs a streaming `/check`. It remains the only one left that
|
||||
changes how the app *feels* rather than what it can do.
|
||||
|
||||
*(Superseded, kept for the reading list: the ninth session's advice.)* Three
|
||||
things remained. **Accept All per category** was 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
|
||||
|
||||
Reference in New Issue
Block a user