fix(annotator): asset-bounded suggest clicks, confidence only in the preview, selected-only labels - #514
Merged
Merged
Conversation
The pane spans the whole stage on purpose, so a press in the margin around the
picture reaches the adapter with a coordinate outside the frame. A drag wants
that — "make the box this big" survives leaving the picture. A prompt point
cannot use it: there is nothing under the margin to segment. Measured before
the fix, a click at (900, 700) on a 640x480 asset sent
positive: [{"x":900,"y":700}], painted a dot there, and drew the answer.
`withinBounds` joins `clampPoint` in core/geometry/primitives — the two
questions a frame is asked, and the difference is whether a stray coordinate is
work to be salvaged or an instruction that was never given. The adapter drops
the press before the host hears about it, so no point is recorded, no request
leaves and no preview moves: one guarantee rather than three. Asked in asset
pixels, so zoom and pan are already accounted for.
cf. #451.
… picked Two rules on one element. The class label renders **only while its shape is selected** — a frame of forty boxes drew forty class names over the picture at all times, which hid the asset behind the annotations of it. The panel is the full inventory; the canvas answers what *this* one is. That rule is DESIGN.md's own, written under the v1 metrics kept as the reference; the React adapter has rendered the label unconditionally since the layer was first written, so this is the adapter starting to obey a rule that was already recorded. And the label says the class and nothing else. A confidence tells somebody whether to accept a proposal; once accepted the shape is a label like any other, so the number stays on the live suggestion preview and leaves the rest of the editor — no percentage on a canvas box, on a panel row, or in a tooltip. The Sparkles glyph is now the only provenance signal, and its tooltip carries the model_ref alone. Nothing is discarded: confidence and model_ref are stored unchanged, and the number's home is the batch review loop. `PaintedAnnotation` gives back the two fields it gained for the old label. A projected field with no reader is what a renderer starts writing next; absence is the rule, enforced where a component cannot reach around it. `confidencePercent` stays exported — one consumer today, and it is the spelling whoever shows the number next must import rather than respell. cf. #417, #425, #451, #512.
The itemised count is what the scene test exists to hold, and a selected-only label moves it: two elements per annotation on a frame nobody has picked a shape on rather than three. The `<text>` was the most expensive of the three — a stroke, a paint order and a translate each — so a legibility decision pays here as well.
This was referenced Aug 10, 2026
JArmandoAnaya
added a commit
that referenced
this pull request
Aug 21, 2026
…preview, selected-only labels (#514) * fix(annotator): a suggest click outside the asset is not a prompt The pane spans the whole stage on purpose, so a press in the margin around the picture reaches the adapter with a coordinate outside the frame. A drag wants that — "make the box this big" survives leaving the picture. A prompt point cannot use it: there is nothing under the margin to segment. Measured before the fix, a click at (900, 700) on a 640x480 asset sent positive: [{"x":900,"y":700}], painted a dot there, and drew the answer. `withinBounds` joins `clampPoint` in core/geometry/primitives — the two questions a frame is asked, and the difference is whether a stray coordinate is work to be salvaged or an instruction that was never given. The adapter drops the press before the host hears about it, so no point is recorded, no request leaves and no preview moves: one guarantee rather than three. Asked in asset pixels, so zoom and pan are already accounted for. cf. #451. * fix(annotator): the canvas says the class, and only for the shape you picked Two rules on one element. The class label renders **only while its shape is selected** — a frame of forty boxes drew forty class names over the picture at all times, which hid the asset behind the annotations of it. The panel is the full inventory; the canvas answers what *this* one is. That rule is DESIGN.md's own, written under the v1 metrics kept as the reference; the React adapter has rendered the label unconditionally since the layer was first written, so this is the adapter starting to obey a rule that was already recorded. And the label says the class and nothing else. A confidence tells somebody whether to accept a proposal; once accepted the shape is a label like any other, so the number stays on the live suggestion preview and leaves the rest of the editor — no percentage on a canvas box, on a panel row, or in a tooltip. The Sparkles glyph is now the only provenance signal, and its tooltip carries the model_ref alone. Nothing is discarded: confidence and model_ref are stored unchanged, and the number's home is the batch review loop. `PaintedAnnotation` gives back the two fields it gained for the old label. A projected field with no reader is what a renderer starts writing next; absence is the rule, enforced where a component cannot reach around it. `confidencePercent` stays exported — one consumer today, and it is the spelling whoever shows the number next must import rather than respell. cf. #417, #425, #451, #512. * test(annotator): the committed layer's element budget is 440, not 660 The itemised count is what the scene test exists to hold, and a selected-only label moves it: two elements per annotation on a frame nobody has picked a shape on rather than three. The `<text>` was the most expensive of the three — a stroke, a paint order and a translate each — so a legibility decision pays here as well.
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 #513.
Three editor-surface corrections. Frontend only — no kernel, no server, no wire.
openapi.jsonandfrontend/ui-core/src/generated/are untouched andcheck.sh generatedsays so. Nothing in
frontend/annotator/src/core/gained a React import, a DOM global or aDOM type in a signature; all three headless gates are green.
1. A suggest click outside the asset is a no-op
What it did before, measured rather than reasoned about
Driven through
AnnotationPagewith a stubbed route table, a press at (900, 700) on a640 × 480 asset, suggest armed:
So yes: the request left carrying coordinates outside the asset in both axes, a prompt dot
was painted out in the margin, and whatever came back was drawn. Nothing client-side
refused it, and nothing server-side does either —
SuggestPointis two barefloats. Afollow-up on what the server should do with an out-of-frame prompt is drafted in the
session report and not filed; this PR does not touch the server.
What ships
withinBoundsjoinsclampPointincore/geometry/primitives.ts— the two questions aframe is asked, and the difference between them is the whole design. A drag that left the
picture still means make the box this big, so it clamps. A click in the margin is not a
click on anything, and clamping it would put a prompt point on the asset's edge that nobody
placed there. Inclusive at both ends, matching
clampPoint's own range, because the lastrow of pixels is part of the asset.
The gate is in
AnnotatorCanvas.handlePointerDown, beforeonSuggestPointfires: apoint the host never hears about records no click, sends no request and moves no preview —
one guarantee instead of three. It is asked in asset pixels, so
imagePointhas alreadyaccounted for zoom and pan and there is no second transform to keep in step with the first.
Clicks inside the asset are unchanged, including on top of an existing shape: the suggest
branch still sits after the pan branch and before the machine dispatch, exactly where #451
put it.
2. Confidence renders only on the live preview
vehicle 91%confidenceLabel, cf. #451)vehicle · 62%vehicle✨ 62%✨model_ref…, confidence 62%Model-produced by <model_ref>The boundary, stated once: the preview shows the number because that is where the
accept-or-reject decision is made; a committed annotation does not, because by then the
decision has been made. Nothing is discarded —
confidenceandmodel_refare stored andtravel on the wire exactly as before.
PaintedAnnotationgives back the two fields it gained in #512. They now have zeroconsumers, and their docstring justified them by the label suffix that is gone. A projected
field with no reader is what a renderer starts writing next; making the absence structural
is what
paint.test.ts› hands the renderer neither the score nor who produced it pins.confidencePercentstays exported. One consumer today — the preview, throughconfidenceLabel— and that is recorded in its docstring rather than inlined away: it isthe spelling, and whoever shows the number next imports it instead of writing a second
Math.round.Every deleted assertion, named
The #473 precedent — a pinned test that encoded the rule this PR was asked to change is
named here so its removal is a decision rather than a disappearance.
paint.test.tspaint.test.tspaint.test.tscanvasLabel.test.tsx› writes no score over a model's shapepaint.test.tscanvasLabel.test.tsx› writes no provenance mark eitherpaint.test.tspaint.test.tspanel.test.tsxconfidence 62%half of says in words what the glyph says in a picturepanel.test.tsxpanel.test.tsxobject-confidence-1assertion in marks only the rows that are the model'spanel.test.tsxobject-model-1present, row text free of41labelTextand theobject-confidence-<n>testid are gone;git grep -nwE labelTextandgit grep -n object-confidenceare both empty.Null-confidence-is-absent-never-zero stays tested where confidence still renders:
paint.test.ts› names the class alone when the model reported no confidence, androunds the confidence to whole percent at both ends beside it.
3. The canvas label renders only while its shape is selected
Where the divergence came from: it was never introduced — the React adapter has rendered
ShapeLabelunconditionally sinceAnnotationShapewas first written, in the commit thatbrought the adapter itself.
DESIGN.mdhas recorded the opposite the whole time, underv1's shape metrics, kept as the reference: "the class label renders only while selected,
… anchored at the first vertex, never a pointer target." So this is the adapter starting to
obey a rule that was already written down, not a regression being backed out. Nothing in the
tracker records a reversal, and no toggle exists, so the flag-and-stop branch of the brief
does not apply.
Metrics are untouched — same size, weight, anchor, lift, and still never a pointer
target. Only the visibility condition moved. (Note: the shipped size is
LABEL_PX = 12andno explicit weight, where DESIGN.md's reference line says 11 / 700. That gap predates this
PR and changing it is out of scope; recorded in the session report.)
The read-only mode is unaffected in the way that matters:
Shapes.tsxalready documentedselection there as "the stroke and the label", and a viewer paints no grips, so the label is
now doing more of that work rather than less.
Mutation verification
Eight mutations. Applied to the committed tree; anchor asserted present exactly once
before and the replacement asserted present after; each reverted by
git apply -Ron itsown recorded diff with
dirty=0confirmed after every one; green restored on a clean treeat the end.
suggestFlow.test.tsx, led by sends nothing and shows nothing>/<)primitives.test.ts+ 1 insuggestFlow.test.tsx› counts the asset's own edge as insideprimitives.test.ts+ 1 › records no point, so the next real click is still the first onecanvasLabel.test.tsx, led by writes nothing over an unselected shapecanvasLabel.test.tsxpanel.test.tsx, led by shows no confidence anywhere on the rowprovenance/confidenceagainpaint.test.ts, led by hands the renderer neither the score nor who produced itM1 was also run against the browser suite on its own: without the check, the new scenario
fails at a suggest click in the margin around the picture asks nothing, at any zoom —
the margin press leaves
suggest-idle. So the browser assertion is not vacuous either.A note on the harness, because it nearly cost a run. The first battery chained
mutate && run && revertwith the test output piped throughhead.headcloses the pipe,the runner takes SIGPIPE,
pipefailmakes the chain non-zero, and four reverts silentlynever executed — so mutations stacked and the next run's red read like a broken
implementation. It was recoverable only because the work was committed before the first
mutation, which is exactly what that rule is for. The rewritten harness is unconditional,
asserts a clean tree before every mutation, and refuses an empty patch.
Test plan
New:
primitives.test.ts(+5, the predicate and its edges),suggestFlow.test.tsx(+6, thegate end to end through
AnnotationPageagainst a stubbed route table),canvasLabel.test.tsx(new file, 7 — the label's visibility and its content),panel.test.tsx(+1 and 4 rewritten),
paint.test.ts(4, replacing 7),annotate.spec.ts(+1 browser).Where the browser was used, and why only there. One new browser assertion: the surround
does not exist in jsdom — every rectangle is zero, so a component test about "the margin
around the picture" would be asserting about nothing. It measures both rectangles, proves
the margin is real, presses in it, and then presses on the picture so the absence is a rule
and not a broken fixture — all at a non-default zoom, since a rule written against
screen coordinates would pass at the fitted scale and refuse half the picture at any other.
Everything else here is jsdom-complete: whether a
<text>node exists and whether a requestleft are not things a browser knows better.
serveApiinannotate.spec.tsgains asuggestibleflag (off by default, so every otherscenario keeps the empty connection list that its panel copy is about) and a
POST /inference/suggeststub. The stub is written against the realSuggestionOut—{ model_ref, region: { geometry, confidence } }. A flatter shape is refused by thegenerated runtime check and surfaces as "the server answered with something this app does
not recognise", which looks nothing like a stub bug; that is the test-double-fidelity rule
earning itself again.
Gate
bash scripts/check.shrun in stages under the harness's ~10-minute command ceiling,with pytest split by the directories
ls tests/reported at run time. Machine was free(load average 3.84 on 20 cores), so this is the full gate split and not the saturation
fallback. Every exit code verbatim.
pytest tests/architecturepytest tests/clipytest tests/examplespytest tests/formatspytest tests/inferencepytest tests/jobspytest tests/kernelpytest tests/mcppytest tests/packagingpytest tests/serverpytest tests/test_versioning.pyruff check .ruff format --check .mypy src/visionsetlint-importscheck.sh frontend(build, tests, lint)check.sh generated(openapi, client, mcp reference, version sync)playwright test(annotator + app e2e, chromium)playwright test -c playwright.cycle.config.tstests/scriptsholds thenode --testgates, which thefrontendgroup runs, so it isabsent above rather than reported as exit 5. No baseline-proof exception was used or
needed.
The flaky one is
e2e/gallery.spec.ts› tiles never overlap, at any density, which passedon retry — cf. #511. This diff touches no gallery code.
One suite failed first and it was mine, which is the point of it:
e2e/perf.spec.ts›the benchmark scene is 220 annotations… holds an itemised SVG element budget, and a
selected-only label moves it — 660 → 440 on the 220-annotation bench scene, two elements per
annotation rather than three. Its own comment says a deliberate rendering change should move
these numbers. Updated, with the reason recorded beside it. The
<text>was the mostexpensive of the three (a stroke, a paint order and a translate each), so the legibility
decision pays here as well.
Information architecture
No route, tab, screen, nav entry or entry point moves. The sitemap is unchanged.
Found, not fixed
SuggestPointaccepts any float. The server takes prompt coordinates outside theasset's frame and hands them to the runtime; nothing refuses or clamps them. This PR
closes the client half only. Drafted, not filed, per the brief.
LABEL_PX = 12with no explicit weight, where DESIGN.md's reference line says11 / 700. Predates this change; the brief scoped section 3 to visibility.
cf. #417, #425, #451, #473, #511, #512.