Skip to content

fix: one setting, one wait, and a box over the whole object - #562

Merged
JArmandoAnaya merged 4 commits into
mainfrom
fix/autolabel-corrections
Aug 12, 2026
Merged

fix: one setting, one wait, and a box over the whole object#562
JArmandoAnaya merged 4 commits into
mainfrom
fix/autolabel-corrections

Conversation

@JArmandoAnaya

Copy link
Copy Markdown
Contributor

Seven items were reported against the suggest tool after dogfooding. Three of the reported
mechanisms turned out to be wrong
, and the diagnosis is the more useful half of this change:
two of the three "regressions" are behaviours that already work, and the thing actually making
the tool slow and unpredictable was a control nobody could see was firing requests.

Closes #557.

What was actually wrong, mechanism by mechanism

A. A spinner at the cursor — real, and removed

A ring at the click point and a progress cursor, both from #543. Removed:
SuggestHalo, its constants, the cursor and the prop plumbing that carried it, and
useReducedMotion, which had no other caller. The panel's Looking at that… card is now the
only report of a wait. The 1.5s cold-start sentence keeps its clock, which is what that clock
is now for.

B. "Every click pays full inference cost" — the stated mechanism is falsified

There is an embedding cache, and nothing removed or bypassed it: a BoundedCache[UUID, …] of
capacity 8 on the process-wide provider pool, keyed on asset_id, with an encodes counter
that exists for exactly this question and a test that asserts it
(test_a_second_click_on_the_same_asset_decodes_without_encoding_again). Every line of it is a
+ from b89c88b (#448) with no - in any later commit:

git log -p --follow -- src/visionset/inference/sam_provider.py \
  | grep -E "^(commit |[-+].*(_embedding|BoundedCache|get_image_embeddings|_encodes))"
commit 49a3765  commit 127bdf5  commit ac0c645  commit 5a0968e  commit b89c88b
+        self._embeddings: BoundedCache[UUID, tuple[Any, tuple[int, int]]] = BoundedCache(
+        held = self._embeddings.get(target.asset_id)
+        self._encodes += 1

49a3765's whole change to providers.py is a return-type widening. The cache is untouched
by this PR.

What did get heavier on the decode side is post-processing, which 49a3765 moved above the
port. Two costs were addressable and both are taken here:

  • The closing reach grew without bound with the object's area — radius 22 on a 4K frame,
    which is 44 bitset passes to bridge gaps that were never there. Now capped.
  • A box paid for the close and threw the result away. A close cannot move an extent, which
    is what makes skipping it safe and what makes it invisible to any assertion about the box.
    The geometry branch moved ahead of the close, and a polygon now runs it on the one piece it
    is about to trace rather than on every piece that survived.

Morphological passes per click, counted rather than timed — the numbers are deterministic and
hardware-independent, which wall clock on this machine emphatically is not (see the gate note):

asset polygon, before → after box, before → after
640×480 8 → 8 8 → 0
1920×1440 26 → 12 26 → 0
3840×2160 44 → 12 44 → 0

This does not make repeat clicks light, and the PR does not claim it does. The dominant
per-click term is the adapter marshalling a full-resolution mask into Python lists — measured
at roughly the same order as the whole pipeline at 4K — and it predates #551. It is out of this
change's scope (the pipeline's math is), and it is filed as #561.

C. "Adjusting detail re-runs inference" — falsified both ways

Neither variant holds. The contour reaches the client (AnnotationPage.tsx reads
region.contour), and applyDetail → withDetail → polygonAt is local arithmetic with no
mutate anywhere in it. annotate.spec.ts already asserted a flat network log across [ and
] and still does.

The control that re-ran the model is Close gaps. It is a range input whose React
onChange fires on every step of a drag, and each one sent a fresh suggest request — up to ten
per drag, undebounced. That control is removed by E, which is the fix; the diagnosis is
recorded here rather than a debounce added to a path that no longer exists.

D. The preview drew no vertices — half-shipped

The dashed outline already shipped (SUGGESTION_DASH = "10 6", opacity 0.6). What was missing
was the vertices, so detail moved a number and nothing anybody could see. The preview is now
a third visual state with its own rule — not a shape marked selected, because selection
carries the panel row, the delete key and keyboard behaviour a proposal must not have.
DESIGN.md's "vertices render only while selected" is amended in the same change, so the prose
contract cannot age into contradiction with the code.

E. Two settings out, and a box becomes a union

fill_holes and fragments leave the wire and the editor; both behaviours stay at fixed
defaults beside the pipeline. A box class now declares no parameters, which is how a client
is told to render no adjustments section — the editor still contains no condition mentioning a
box.

The bbox rule changed: after noise filtering the box is the union of every surviving piece.
A point prompt means this object, and a mask arriving in pieces is nearly always one object
seen around an occlusion; largest-only cuts it off at the occlusion and one-box-per-piece
annotates it twice.

F. detail as a slider

A three-position native range with a live Fine · 41 pts label. Pressing it never moves focus
off the canvas — every chord in the editor is a keydown on the annotator's root — while Tab
still reaches it, so keyboard operation stays a deliberate arrival rather than a side effect of
pointing at it.

G. No pointer leak exists, and the guard passed on the first run

The card is pointer-events-auto inside a pointer-events-none column and is a sibling of
the canvas, so there is no DOM path from a press on it into the <svg>. A new browser spec
presses every control on the card and asserts zero suggest requests and an unmoved seed point:
it passed before any change was made to that path, and it is a regression guard rather than a
red-before-green reproduction.
Said plainly because a green test presented as a fix is worse
than no test.

What the report describes is C seen from the other side: pressing Close gaps did fire a new
suggest, legitimately, because that is what the control did. Removing it removes that too.

Red-before-green

Acceptance criterion 7 asks for A, B, C and G to fail on pre-fix HEAD. Only A can, and the
reason is the diagnosis above: B's cache and C's local re-simplification already work, and G's
leak does not exist. Claiming three red baselines here would mean writing tests against
mechanisms that are not there.

  • Asays so on the card and puts nothing at all on the canvas fails on pre-fix HEAD,
    and the mutation below restores the busy cursor to prove it.
  • B — the addressable half is a performance property with no behavioural signature, so it
    is asserted structurally (test_a_box_never_pays_for_the_close_at_all) and measured by pass
    count. The cache half needs no test; it has one already, passing, on main.
  • C — covered by the existing zero-request browser spec, extended to the slider.
  • G — guard only, as above.

Mutation verification

Work committed first. One mutation at a time, anchor asserted present-and-unique before and
replacement asserted present after, reverted by its own recorded patch, each step an
unconditional statement, runner output to a file. Every row turned a named test red.

mutation named test that went red
_union_of's right takes the first box test_a_box_is_one_union_over_every_surviving_piece
noise floor → 0.0 test_the_union_leaves_out_the_specks_the_noise_filter_dropped
closing radius uncapped test_the_reach_stops_at_the_cap_however_large_the_piece
detail declared for BBOX too test_a_box_class_is_told_nothing_applies
pieces sorted by size, ignoring the prompt test_the_piece_under_the_click_leads_even_when_it_is_not_the_biggest
the close moved back ahead of the branch test_a_box_never_pays_for_the_close_at_all
slider always reports balanced reports a step through the door that needs no request
onMouseDown dropped from the slider does not let a press on the slider take focus off the canvas
the label drops its vertex count names the step and what it costs in one label
the busy cursor restored says so on the card and puts nothing at all on the canvas
preview vertices given an empty list the preview draws its vertices, and a committed shape does not

Three of these lied before they told the truth, and the harness caught each one.

  • _union_of's left = min(...) mutated to boxes[0].x came back green: the union has
    four sites, and in a fixture whose pointed-at piece is already the leftmost that spelling is
    a no-op. Mutating right instead reddens the named test. Stopping at the first green would
    have reported the rule unverifiable.
  • The first preview-vertices mutation deleted the whole element, which orphaned its import
    and failed tsc, so the browser suite never started — a broken harness, not a covered rule.
    Redone type-validly (points={[]}) it reddens exactly the named test.
  • The e2e command cds into frontend/app, so git apply -R ran from the wrong directory and
    silently did not revert. The harness's own dirty-tree check caught it on the next case;
    reverted by hand and the tree verified clean before continuing.

The gate

Staged, because the harness kills any command at about ten minutes. Every stage's exit code.

architecture 0   formats 0     mcp 0         server 0
cli 0            inference 0   packaging 0   test_versioning.py 0
examples 0       jobs 0        scripts 0
fixtures 0       kernel 0
bash scripts/check.sh frontend    exit=0
  frontend/annotator test:   Test Files  36 passed (36) / Tests  1000 passed (1000)
  frontend/ui-core test:     Test Files  50 passed (50) / Tests   963 passed (963)
  frontend build, frontend lint: All checks passed.

bash scripts/check.sh generated   exit=0
  ==> openapi drift / generated client drift / mcp tool reference drift / version sync

bash scripts/check.sh docs        exit=0
  ==> docs site build / docs projection is deterministic / docs site internal links

bash scripts/check.sh browser     exit=0
  annotator e2e (chromium):        256 passed (2.3m)
  browser cycle, real server:        1 passed (32.4s)

ruff check .            All checks passed!
ruff format --check .   380 files already formatted
mypy src/visionset      Success: no issues found in 159 source files
lint-imports            Contracts: 4 kept, 0 broken.

#550's two flaky scenarios passed this time, at the same worker count. Nothing here touches
them; noted so the run is not read as evidence they are fixed.

A note on the timing numbers. This machine's wall clock swung 4× between identical runs
(the same benchmark on unmodified main measured 296 ms and then 1227 ms), and CPU time swung
with it under SMT contention. That is why the performance claim above is a count of
morphological passes
rather than a duration: it is exact, reproducible and independent of the
box. A real wall-clock number wants your GPU machine, and the command is in the flags below.

Found, not fixed

Flags for Armando

  • Three of the seven reported mechanisms were wrong (B, C, G), and one was half-shipped
    (D). Worth knowing where the reports came from, because the pattern is consistent: all three
    are things a person would infer from watching the tool rather than from reading it, and the
    single real cause underneath two of them was one undebounced range input.
  • A wall-clock "first click heavy, later clicks light" measurement is still owed, and this
    box cannot produce it — no torch installed, no CUDA, and the load makes timing meaningless.
    On your GPU machine:
    uv run pytest tests/inference/test_sam_provider.py -q for the encoder count, then two
    consecutive suggest clicks on one 4K frame with the network panel open.
  • MAXIMUM_CLOSING_RADIUS = 6 is the one number here without a prior calibration behind it.
    It is chosen so a 4K object closes gaps of a few pixels rather than 22, which is the scale of
    the artefact rather than of the object. If real masks want more reach, it is one constant.
  • Every loading affordance found in this flow, since the dispatch asked for the list:
    the halo (TransientLayer.tsx, feat(annotator): in-flight indicator for interactive auto-labeling #543), the busy cursor (AnnotatorCanvas.tsx, feat(annotator): in-flight indicator for interactive auto-labeling #543), the
    panel's asking card (SuggestPanel.tsx, Interactive auto-labeling — click-to-suggest in the editor canvas #424) and its cold-start sentence (feat(annotator): in-flight indicator for interactive auto-labeling #543). The
    first two are gone; the last two stay and are the whole report now. A fifth, the checking
    spinner on the connection-list blocker, is about a different request and is untouched.

Two of the three suggestion settings did nothing to an ordinary single clean
mask — every position produced an identical shape — so they read as controls
wired to nothing, and could only be got wrong on the unusual mask. Both come
off the wire. Both behaviours stay, at fixed defaults beside the pipeline.

- `SuggestParameter` is `detail` alone, declared for polygons only. A box class
  declares nothing, which is how a client is told to render no adjustments.
- Specks under a twentieth of the largest piece are dropped unconditionally,
  and `components` now leads with the piece the prompt points at.
- A box is the union of every surviving piece. A point prompt means *this
  object*, and a mask in several pieces is nearly always one object seen around
  an occlusion; largest-only cuts it off and one-box-per-piece doubles it.
- The closing reach is capped, and a box skips the close entirely because it
  cannot move an extent. Per click at 4K: 44 morphological passes become 12 for
  a polygon and 0 for a box.

cf. #557
…vertices

- The ring at the click point and the busy cursor are gone. An indicator
  sitting on the picture beside the pointer reads as the machine having seized
  rather than as work in progress, and the card already says so in words. The
  panel's message is the only report; the cold-start sentence keeps its clock.
- A suggestion preview is a third visual state, not a shape marked selected:
  its vertices are drawn the whole time it is up, undecimated at every step,
  because where precision was gained or lost is what the setting is about.
- `detail` becomes a three-position slider with a live `Fine · 41 pts` label,
  replacing three text labels that did not read as pressable. Pressing it never
  moves focus off the canvas, so the chords keep working.
- The two mask settings and the re-ask path they drove are deleted. That path
  sent one inference request per input event of a drag.

cf. #557
The skip is a performance rule with no behavioural signature — a close cannot
move an extent, which is what makes it safe to skip and what makes it invisible
to any assertion about the box. Asserted structurally instead.

cf. #557
@JArmandoAnaya
JArmandoAnaya force-pushed the fix/autolabel-corrections branch from 4ead019 to 1af10df Compare August 12, 2026 18:57
@JArmandoAnaya

Copy link
Copy Markdown
Contributor Author

Rebased onto 031e0a1#560 landed while this was in review and the two changes overlap on seven files, including all three generated artifacts. A textual merge of two independently regenerated specs is how main ends up with a contract matching neither app, so this was rebased rather than left to merge.

export_openapi.py + generate:client after the rebase produced zero further changes, so the merge was correct — verified rather than assumed. Whole-tree ruff, mypy and lint-imports re-run per the post-rebase rule; generated, frontend and the inference/kernel/server suites all green on the new base.

Left open, as the dispatch asks.

@JArmandoAnaya
JArmandoAnaya merged commit 8d0be3d into main Aug 12, 2026
15 checks passed
@JArmandoAnaya
JArmandoAnaya deleted the fix/autolabel-corrections branch August 12, 2026 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interactive auto-labeling: a cursor spinner, a per-click cost, a slider that re-asks, and four corrections

1 participant