feat(annotator): a verb for finishing a frame — Save and next takes the filled slot - #384
Merged
Merged
Conversation
…he filled slot The top bar #368 left had no control for the commonest move in the product: after annotating a frame, store it and go to the next one. The navigator's `›` is chrome rather than a verb, so Skip inherited prominence by vacuum on exactly the frames somebody had just done work on, and the explicit save lived two clicks deep in the overflow. Save and next is the one filled control now, carrying ↵, with Skip beside it carrying X — two ways to resolve a frame, both advancing, neither collapsible. It is `go(1)`, the navigator's own save-first advance, so there is one save pipeline; it reads `Next` when no save will happen. On the last frame it is not rendered and Finish job takes the filled slot, which is the only place the two could have contended. The review move becomes outline and gains a tooltip; Save and stay returns as a ghost button; the progress dot gains its word. `enter` now means *finish*: the ring close while a shape is in progress, the flow verb otherwise — substituted in the adapter, which is the only layer holding the interaction state. `x` is a new row in the default binding table.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #383.
The bar #368 left had no control for the commonest move in the product. After annotating a frame the right action is store this and show me the next one, and the only thing that advanced was the navigator's
›chevron — chrome, not a verb — so Skip was the most prominent thing to press on work somebody had just done, and the explicit save was two clicks deep in the overflow.What the right zone renders now
n/m annotated· Save and stay (ghost, ⌘S) · the review move (outline) · Skip/Un-skip (outline,X) · Finish job · Save and next (filled,↵) · overflow⋯.Save and nextisgo(1)— the navigator's own save-first advance, reused rather than reimplemented, so there is one save pipeline and one place principle 10 is enforced. The settle toannotatedis not sent from here either:progress_after_annotatingmakes that move in the same transaction as the write.Nextwhen no save will happen, so the button never promises one it will not perform.Finish jobtakes the filled slot. Everywhere else Finish job keeps WS2's outline-and-disabled-with-reason treatment.submit_for_review, elseaccept) is outline and carries decision 6's tooltip.Save and stayis back as a ghost button.● annotated · Saved.Stop-and-flag: enumerated, and it did not fire
The condition was two filled candidates co-declaring. Enumerated from
capabilities.pyrather than reasoned about —asset_actionsandjob_actions, batchin_annotation(nothing declares anything in any other batch state):completedeclared?unannotatedannotate,skipannotatedannotate,skip,submit_for_reviewskippedrestorereview_pendingaccept,return_to_annotatorreview_pendingblocks)Next)acceptedNext)Nothing contends. The filled slot is chosen by
assetIndex >= assetCount - 1, which is this page's own arithmetic and not a declaration anything can co-claim; every wire-declared control on the bar is outline or ghost.test("leaves exactly one filled control in every progress, crossed with the last frame")sweeps all ten cells againstbg-primary.The second stop-and-flag — advancing past the last frame has no defined behaviour — also did not fire:
go()clamps and returns without committing when the move is a no-op, and decision 3 removes the button there anyway.Three findings, each a departure worth naming
1.
enterwas already taken, and the decision record could not have known.DEFAULT_BINDINGSbindsentertosend commit— v1's polygon ring close, the one close a keyboard can always reach, asserted by six e2e scenarios. Binding the flow verb toenterin that table would have shadowed it, because the fold is last-wins.The resolution keeps both:
entermeans finish — finish the shape while one is being drawn, finish the frame otherwise. The two never overlap, because outsidedrawing-polygonthe machine has no row for a commit and the press is silently swallowed today. The substitution is inAnnotatorCanvas.handleKeyDown, before the read-only branch, because the deciding fact is the interaction state and that is the adapter's — core's table stays honest and thehostkind is already inREAD_ONLY_KINDS, so↵advances a frame nobody may edit exactly as the button beside it does. Mutation-verified: dropping theidlepredicate makes the ring close stop working and turns the named scenario red.2.
Xwas bound to nothing at all. The brief says the chips are visual and "the bindings themselves stay in the existing input layers" —mod+swas there,xwas not, so the chip would have named a key that does nothing. Soxis a new row inDEFAULT_BINDINGS(a bare letter onc's andv's terms), reaching a newSKIP_FRAMEhost action. That is a secondcore/touch beyond the brief's literal carve-out, taken because the alternative was a chip that lies.3. Decision 3 read as a promotion, not an appearance. "
Finish jobtakes the filled slot on the last frame" could mean it renders only there; it does not, and the deciding evidence is the shipped contract — an existing e2e scenario finishes a job from frame 1 of 2, and a job whose every frame is settled can legitimately be finished from anywhere in it. So Finish job stays visible on every frame with the WS2 amendment's disabled-with-reason, and what changes on the last frame is its variant. The parenthetical "(still disabled-with-reason otherwise, per the WS2 amendment)" reads as confirming exactly that.Two smaller readings, stated rather than buried:
Nextdegradation also fires on a read-only frame. Decision 2's key is no annotations and no unsaved changes; a settled frame has annotations, cannot be dirty, and cannot be written to by anyone — so the stated key alone would putSave and nextwhere no save is reachable at all, which is the thing decision 2 exists to prevent. One extra disjunct,!readOnly.xlforSave and stayandlgfor the review move, each overflow item carrying the exact inverse class so the control exists in exactly one place at any width. The trigger decision 4 names is a container collision with the class field, which would need aResizeObserveron the bar; the breakpoints are a proxy, and the order is the one the decision fixes. Both thresholds are reachable aboveANNOTATOR_MIN_VIEWPORT_PX(768).WORKFLOW_PRIMARIESis renamedREVIEW_ACTIONS— the brief's "or its successor" — because the list is no longer the primary and a name that says otherwise is the kind of thing a reader trusts.Found, not fixed
Nothing. No unrelated defect surfaced.
Test plan
New: 12 vitest cases in
topBar.test.tsx(right-zone composition across all five progress states crossed with last-frame, the filled-slot sweep, theNextdegradation, the ↵ andXchords with their focus and wire gates, the microtext) and 4 Playwright scenarios inannotate.spec.ts— the save-first ordering, the label's two halves,enter's two meanings in one test, and the last frame's slot handover. The ordering claim is in the browser deliberately: making a document dirty means drawing, and jsdom'sgetBoundingClientRectreturns all zeros, so a component test clicking the button over a clean document would pass with the commit deleted.Eight mutations, each confirmed to turn a named test red, each applied with its anchor asserted present-exactly-once before and its replacement asserted present after, each reverted by its own file with the tree verified clean:
{!lastFrame && (→ always rendervariant={lastFrame ? …}→ alwayssecondaryflowLabel→ always"Save and next"declares(asset, skip)from theXrow{word}from the progress dotSAVE_AND_NEXThost rowinteractionNow.current.type === "idle"One of those runs came back green on the first attempt — the tooltip mutation changed a half of the string the test does not assert. Re-run against the asserted half, it turned red. Recorded because a green mutation run that is really a bad anchor is exactly the false calm the protocol's harness rules are about.
Gate run in stages under the harness ceiling, exit codes verbatim:
check.sh pythoncheck.sh frontendpnpm test:scriptsdocs_linksandwire_rosterscheck.sh generatedcheck.sh browserNo CI job added, renamed or removed, so the
mainruleset is untouched. No kernel change, no migration, no wire change —openapi.jsonand the generated client are byte-still.DESIGN.md's top-bar spec andCHANGELOG.mdare updated in this PR;_frame.ts'sexpectProgressdocstring no longer claims the word lives in an accessible name.