Skip to content

fix(annotator): the suggest tool survives a class switch; a select option is two lines - #473

Merged
JArmandoAnaya merged 2 commits into
mainfrom
fix/suggest-tool-ux
Aug 9, 2026
Merged

fix(annotator): the suggest tool survives a class switch; a select option is two lines#473
JArmandoAnaya merged 2 commits into
mainfrom
fix/suggest-tool-ux

Conversation

@JArmandoAnaya

Copy link
Copy Markdown
Contributor

Closes #472.

Two things a person had to work around: a select that squashed the label it was showing, and a
tool that switched itself off when you picked a class to use it on. Frontend only — no wire, no
route, no kernel, no docker.

A select option is two lines when it is an identifier plus its facts

SelectItem takes a meta. The children stay the identifier at the label role; meta goes
underneath at the meta role in muted-foreground. Radix renders the selected item's own
ItemText into the trigger, so the closed control and the open list are the same two lines by
construction
— there is no second copy of the layout to keep in step, which is the reason this
is a primitive variant rather than one screen's styling.

The trigger is min-h-9 with py-1 instead of a fixed h-9. A one-line option measures exactly
36px still (22.4px of text plus 8px of padding plus the border is under the floor), so every
select that shipped before this is unmoved; a two-line one grows. Nothing truncates and nothing
ellipsises — half a model id is not a model id, so a long one wraps.

Custom model… gets a meta line too, so no row in the list is a different height from its
neighbours.

The styleguide gains the specimen and DESIGN.md gains the rule, next to the geometry picker's
grouping rule it sits beside.

The suggest tool survives a class switch

This is #451's behaviour, deliberately reversed, and that is the part worth reading. Slice 3b
held the mode beside the active class and discarded it whenever that class moved, on D2's
"switching tools discards" — moving the active class being how this build spells switching
tools. Directed (Armando, 2026-08-09): arming is a decision about how to work and picking a class
is the next thing somebody does, so a class switch ends what is pending and not the session.

withClass in core/interaction/suggestion.ts is the whole rule, and it is one function because
the three readings are one transition over a different argument:

The class moves to What happens
another suggestible class, nothing pending swapped in; the next click asks under it, in its geometry and its colour
another suggestible class, a preview showing the preview and its clicks are discarded; the tool stays armed
a class that can hold nothing the session parks: labelClass is null, no press is diverted, the armed intent is kept
— asset switch — disarms and discards, through the key={asset.id} remount, unchanged

The preview is discarded because it was answered under the old class's allowed_geometries;
accepting it under the new one could write a kind that class does not admit, which is the same
argument D3 makes about which classes may be asked in the first place.

The serial keeps counting across all three, for cleared's reason: an answer to the ask that
was in flight when the class moved must not repaint a preview belonging to a class nobody is on.
withClass returns the state by identity when the class did not really move, so the page's effect
folds through unconditionally without a render or a discarded preview per re-selection.

Parked is a state with a sentence, not a control going quiet

  • The strip button is dimmed with its reason and stays lit — both are true at once: the tool
    is armed, and it cannot act. Hidden is the schema answer (D3's third case, unchanged);
    disabled-with-reason is the class answer, because that one comes back the moment the active
    class moves, and a button that vanished and returned as somebody worked down the class list
    would be describing a project that keeps changing.
  • The panel gains a sixth reading: it names the class, says what to pick, says the tool is still
    armed, and carries the way out — since the control it would otherwise be is the dimmed one. No
    Esc chip beside it, because the chord is a substitution the canvas makes while something is
    pending and a parked session has nothing pending; printing the key would be printing one that
    does nothing.
  • Parked outranks the connection blocker. "Getting the model ready…" over a class that could
    never hold the answer reports progress towards something that is not going to happen and hides
    the one choice the person can change.
  • The canvas is handed null while parked. The class that parked it may still be drawable — a
    lane is a polyline — and a tool that swallowed those presses would have stopped being parked
    and started being broken. There is a test that draws one.

Tests

Core (suggestion.test.ts, +10): identity when the class did not move; armed survives the swap;
the preview discards; the serial keeps counting and the in-flight answer is dropped; parking;
re-arming on the class that unparked it, not the one left behind; parked-to-parked is identity;
nothing is written without a class. paint.test.ts: a parked session draws nothing.

Adapter/product: suggestFlow.test.tsx gains the whole class-switch block driven through
AnnotationPage with a stubbed route table — including the asset switch, which needed a second
frame in the fixture, and a polyline class, which is what parks the tool. suggestPanel.test.tsx
gains the parked reading, its way out, its precedence over the blocker, and the no-class-at-all
sentence. toolPalette.test.tsx gains dimmed-with-reason and the press it refuses.
primitives.test.tsx gains the two-line option; inference.test.tsx gains it on the real form,
trigger and list; styleguide.spec.ts measures in a browser what jsdom cannot — the one-line
trigger on 36px and the two-line one taller.

The pinned test that moved. suggestFlow.test.tsxdisarms when another tool moves the
active class (D2)
is gone, replaced by disarms when it is pressed again plus the block above.
It encoded the rule this PR was asked to change, and it is named here so the removal is a decision
rather than a disappearance.

Mutation verification

Each mutation applied to the committed tree, its anchor asserted present before and the
replacement after, run, then reverted by its exact edit with a clean tree confirmed.

Mutation Result
The page's effect goes back to setSession(null) on a class switch 6 red in suggestFlow.test.tsx, led by stays armed on a class switch
withClass keeps the pending state — { ...state, labelClass } instead of { ...cleared(state), labelClass } 2 red in suggestion.test.ts + 1 red in suggestFlow.test.tsx, all discards a preview…
SelectTrigger back to a fixed h-9 1 red: grows rather than clipping, and leaves a one-line option where it was
The canvas is handed the parked session (suggesting, not diverting) 2 red, led by leaves the canvas alone while parked

The two the issue asked to be named are the first two rows.

Gate

Run in stages under the harness's ~10-minute command ceiling; pytest split by the directories
ls tests/ reported at run time. The machine was free (load average 1.45 on 20 cores), so this is
the full gate split and not the saturation fallback. Every exit code verbatim.

Stage Exit
pytest tests/architecture 0
pytest tests/cli 0
pytest tests/examples 0
pytest tests/formats 0
pytest tests/inference 0
pytest tests/jobs 0
pytest tests/kernel 0
pytest tests/mcp 0
pytest tests/packaging 0
pytest tests/server tests/test_versioning.py 0
ruff check . 0
ruff format --check . 0
mypy src/visionset 0
lint-imports (4 kept, 0 broken) 0
pnpm -r build 0
pnpm test (node script gates + vitest) 0
pnpm -r lint (incl. all three annotator boundary gates) 0
check.sh generated (openapi drift, client drift, mcp reference, version sync) 0
playwright test (annotator + app e2e, chromium) 0 — 234 passed, no flakes
playwright test -c playwright.cycle.config.ts 0 — 1 passed

Frontend totals: annotator 914 passed (34 files), ui-core 844 passed (46 files).
tests/scripts holds only .mjs node gates, which run under pnpm test, so it is absent above
rather than reported as exit 5. No baseline-proof exception was used or needed.

Boundaries

Nothing in frontend/annotator/src/core/ gained a React import, a DOM global or a DOM type in a
signature — the mode change lives in the core because that is where the mode lives, and all three
gates are green. No route, tab, screen, nav entry or entry point moves, so the sitemap is
unchanged. openapi.json and the generated client are untouched, and the drift gates say so.

Found, not fixed

  • The connection list is still fetched the moment the tool is armed, which now includes a session
    that parks before it ever asks. It is one request against a workspace-scoped list, and
    un-parking is one click away, so prefetching is the better of the two — recorded because the
    "a job nobody suggests on makes no inference request at all" discipline it sits under is worth
    keeping visible.
  • #466 (the four inference request models do not forbid unknown fields) is untouched.

…ts facts

The curated model select put the id, the download size and the hint on one
line, inside a trigger measured for one. The text wrapped and read squashed.

`SelectItem` takes a `meta`: the children stay the identifier at the label
role, `meta` goes underneath at the meta role. Radix renders the selected
item's own `ItemText` into the trigger, so the closed control and the open
list are the same two lines by construction rather than by a second copy of
the layout at the call site.

The trigger is `min-h-9` with `py-1` instead of a fixed `h-9`: a one-line
option still measures exactly 36px, so every select that shipped before this
is unmoved, and a two-line one grows to fit. Nothing truncates and nothing
ellipsises — half a model id is not a model id.

The styleguide gains the specimen, and its e2e measures what jsdom cannot:
the one-line trigger on 36px, the two-line one taller.
…ather than dying

Selecting a different class silently disarmed the tool. That was an artifact
of #451, which read "switching tools discards" (D2) and treated moving the
active class as switching tools. Directed (Armando, 2026-08-09): arming is a
decision about how to work and picking a class is the next thing somebody
does, so a class switch ends what is pending and not the session.

`withClass` in the core is the whole rule, and it is one function because the
three readings are one transition over a different argument:

- nothing pending — the class is swapped in, and the next click asks under it;
- a preview showing — it is discarded with the clicks that produced it, since
  it was answered under the old class's `allowed_geometries` and accepting it
  under the new one could write a kind that class does not admit;
- a class that can hold nothing — the session parks, `labelClass` is `null`,
  no press is diverted, and the armed intent is kept so that returning to a
  class which can hold an answer picks up with no second press.

The serial keeps counting across all three, so an answer to the ask that was
in flight when the class moved cannot repaint a preview nobody is on.

Parked is a state with a sentence, not a control that goes quiet: the strip
button is dimmed with its reason (lit and dimmed at once — both are true),
and the panel names the class, says what to pick, says the tool is still
armed, and carries the way out, since the button it would otherwise be is the
one that is dimmed. The canvas is handed `null` while parked, because the
class that parked it may still be drawable and a tool that swallowed those
presses would have stopped being parked.

Switching assets still disarms and discards, through the `key={asset.id}`
remount, unchanged.
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.

Suggest tool: model select squashes long labels; mode drops on class switch

1 participant