Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,14 @@ market segment and the domain does not hold one. The single map is
quietly out of a list; headings are non-selectable `SelectLabel`s and a category with
nothing under it renders none.

**An option that is an identifier plus the facts about it takes two lines** (#472,
2026-08-09) — the identifier at the label role, the facts beneath it at the meta role in
`muted-foreground`. It is `SelectItem`'s `meta` prop, so the closed trigger shows the same
two lines the open list does; the trigger is `min-h-9` rather than `h-9` and grows to fit,
which leaves every one-line select on the contract's 36px. **Nothing truncates**: an
identifier cut off in the middle is not an identifier, so a long one wraps. The specimen is
on the styleguide page.

### Lists and filtering

Any list that can exceed ~20 rows carries a filter input. Filtering is client-side and
Expand Down
18 changes: 16 additions & 2 deletions docs/ui.md
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,18 @@ because the model reads the whole image once; refining after it is quick.
**The proposal is not an annotation until it is accepted.** It is drawn faintly
with a dashed outline, carries its class and the model's confidence beside it, and
is in neither the document nor the undo history. `Esc` is its undo. Switching
tools, switching frames or leaving the page discards it, and nothing is written.
class, switching frames or leaving the page discards it, and nothing is written.

**The tool stays armed while you change class.** Arming it is a decision about how
to work, and picking the class to work on is the next thing you do — so a class
switch ends the proposal on screen and not the tool. The next click asks under the
new class, in its geometry and its colour. Only pressing the button again, or
moving to another frame, puts the tool away.

Land on a class that can hold no proposal — a tag, a lane — and the tool **parks**
rather than switching itself off: the button dims and says why, the panel says what
to pick, and the canvas goes back to drawing that class normally. Choose a box or a
polygon class again and the tool carries on, with nothing to press.

Accepting creates one ordinary annotation, in one undo step, carrying
`provenance: model`, the `model_ref` the answer named and its `confidence` — the
Expand All @@ -444,7 +455,10 @@ frame settles the same way.
**The tool is offered only for a class that can hold the answer.** The proposal
comes back as a polygon for a polygon class and as the shape's bounding box for a
box class; a schema whose classes are tag-only or lane-only gets no button at all,
because there is no kind the answer could be expressed in.
because there is no kind the answer could be expressed in. That is the *project*
answered; the parked state above is the same question asked of the class you are
holding, and it dims the button rather than removing it because the answer changes
again the moment you pick another class.

Arming it with no usable connection shows an in-editor panel saying what is
missing — none configured, or configured with its weights not yet downloaded — and
Expand Down
8 changes: 8 additions & 0 deletions frontend/annotator/src/adapters/react/paint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,14 @@ describe("a pending suggestion, drawn as a proposal (#424)", () => {
expect(paintSuggestion(cleared(shown()), SIGN)).toBeNull();
});

it("draws nothing for a parked session, which has no class to draw it in (#472)", () => {
// Constructed, because a parked session cannot reach `shown` — the same
// belt-and-braces as the kind check below, for a function exported from the
// package root that a caller could hand anything the type permits.
const parked: SuggestionState = { ...shown(), labelClass: null };
expect(paintSuggestion(parked, SIGN)).toBeNull();
});

it("draws nothing for a kind that is not one of the two suggestible ones", () => {
// Unreachable through the route, which narrows to `allowed_geometries` — and
// still refused here, because this function is exported from the package root
Expand Down
9 changes: 7 additions & 2 deletions frontend/annotator/src/adapters/react/paint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,17 @@ export function paintSuggestion(
): PaintedSuggestion | null {
const suggestion = state.suggestion;
if (state.status !== "shown" || suggestion === null) return null;
// A parked session (#472) has no class, so there is no colour to draw it in and
// no label to write on it. It also cannot be `shown`, so this is the same kind
// of guard as the one above: what the type allows, not what the machine does.
const labelClass = state.labelClass;
if (labelClass === null) return null;
const geometry = suggestion.geometry;
if (geometry.type !== "bbox" && geometry.type !== "polygon") return null;
return {
geometry,
color: classColor(declared, state.labelClass),
label: confidenceLabel(state.labelClass, suggestion.confidence),
color: classColor(declared, labelClass),
label: confidenceLabel(labelClass, suggestion.confidence),
points: state.points,
};
}
Expand Down
97 changes: 97 additions & 0 deletions frontend/annotator/src/core/interaction/suggestion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@ import {
cleared,
hasPending,
isAcceptable,
isParked,
isSuggestibleClass,
promptOf,
refused,
schemaCanSuggest,
suggestClassFor,
suggestibleClassIn,
withClass,
withPoint,
} from "./suggestion";
import type { Suggestion, SuggestionState } from "./suggestion";
Expand Down Expand Up @@ -118,6 +121,100 @@ describe("which classes the tool is offered for", () => {
expect(suggestClassFor(schemaOf(LANE, ROAD, CAR), "lane")).toBe("road");
expect(suggestClassFor(schemaOf(LANE, ROAD, CAR), null)).toBe("road");
});

it("reads the held class without the fallback, which is the parked question (#472)", () => {
const schema = schemaOf(LANE, ROAD, CAR);
expect(suggestibleClassIn(schema, "car")).toBe("car");
// The very case `suggestClassFor` answers `road` for. Moving somebody off the
// class they just picked is not this function's business.
expect(suggestibleClassIn(schema, "lane")).toBe(null);
expect(suggestibleClassIn(schema, null)).toBe(null);
expect(suggestibleClassIn(schema, "not-a-class")).toBe(null);
});
});

/**
* The class moving under an armed session (#472) — the behaviour #451 shipped, and
* the direction it was deliberately turned around in.
*/
describe("the active class moves and the session goes with it", () => {
it("returns the state by identity when the class did not really move", () => {
const state = showing();
// Not merely equal: the host folds this through on every render of the class
// it is already on, so a fresh object here would be a re-render per keystroke
// and, worse, a discarded preview.
expect(withClass(state, "car")).toBe(state);
});

it("stays armed on the new class, with nothing pending carried over", () => {
const state = withClass(armed("car"), "road");
expect(state.labelClass).toBe("road");
expect(state.status).toBe("idle");
expect(isParked(state)).toBe(false);
expect(hasPending(state)).toBe(false);
});

it("discards a preview the new class may not be able to hold", () => {
const shown = showing();
expect(shown.suggestion).not.toBe(null);

const moved = withClass(shown, "road");

// The shape was answered under `car`'s `allowed_geometries`; accepting it
// under `road` could write a kind that class does not admit.
expect(moved.suggestion).toBe(null);
expect(moved.points).toEqual([]);
expect(moved.status).toBe("idle");
expect(isAcceptable(moved)).toBe(false);
// And the tool is still armed, which is the whole of the change.
expect(moved.labelClass).toBe("road");
});

it("keeps the serial counting, so the answer in flight cannot repaint", () => {
const asked = withPoint(armed("car"), [10, 10], "positive");
const moved = withClass(asked, "road");

expect(moved.serial).toBe(asked.serial);
// The ask that was in flight lands under the old serial and is dropped whole.
expect(answered(moved, asked.serial - 1, proposal())).toBe(moved);

// And the next click on the new class cannot be answered by it either: the
// serial moves on rather than being handed back out.
const again = withPoint(moved, [20, 20], "positive");
expect(again.serial).toBe(asked.serial + 1);
});

it("parks on a class that can hold nothing, rather than ending", () => {
const state = withClass(showing(), null);
expect(isParked(state)).toBe(true);
expect(state.labelClass).toBe(null);
expect(state.suggestion).toBe(null);
expect(state.points).toEqual([]);
});

it("re-arms from parked on the class that unparked it, with no second press", () => {
const parked = withClass(armed("car"), null);
const back = withClass(parked, "road");

expect(isParked(back)).toBe(false);
// `road`, not `car`: a parked session resumes on the class the person has just
// picked, and nothing here remembers the one they left.
expect(back.labelClass).toBe("road");
expect(back.status).toBe("idle");
});

it("stays parked while the active class moves between classes that hold nothing", () => {
const parked = withClass(armed("car"), null);
expect(withClass(parked, null)).toBe(parked);
});

it("writes nothing while parked, whatever a caller believes about the status", () => {
const document = documentOf();
// Constructed, not reachable: a parked session cannot be `shown`. The guard is
// the guarantee — no class, no annotation — and not a formality.
const impossible: SuggestionState = { ...showing(), labelClass: null };
expect(acceptedAnnotation(document, impossible, () => "id-1")).toBe(null);
});
});

describe("the preview lifecycle", () => {
Expand Down
94 changes: 92 additions & 2 deletions frontend/annotator/src/core/interaction/suggestion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,33 @@
* that can be true. `promptOf` is the projection, and it is here rather than in
* the host so that the ordering rule — positives and negatives in the order they
* were placed, each list on its own — has one owner.
*
* ## The class can move under a session, and the session survives it (#472)
*
* #451 held the mode *beside* the active class and discarded it whenever that
* class moved, on the reading that moving the active class is how this build
* spells switching tools. In use that is wrong: arming the tool is a decision
* about how to work, and picking the class to work on is the next thing somebody
* does. Directed (Armando, 2026-08-09): a session ends when the user puts the
* tool away or the asset changes, and a class switch is neither.
*
* {@link withClass} is that whole rule, and it is one function because the three
* readings are one transition:
*
* - **Nothing pending.** The class is swapped in; the next click asks under it.
* - **A preview showing.** It is discarded with the clicks that produced it. It
* was answered under the old class's `allowed_geometries`, so accepting it
* under the new one could write a shape that class does not admit — the same
* argument D3 makes about which classes may be asked in the first place.
* - **A class that can hold nothing** — a tag, a path. The session **parks**:
* `labelClass` is `null`, no click is diverted, and the armed intent is kept so
* that returning to a class which can hold an answer picks up where it left
* off. Parking rather than disarming is what stops a capability disappearing
* without a sentence (principle 9); the sentence itself is the host's.
*
* 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
* that belongs to a class nobody is on any more.
*/

import { classNamed } from "../state/document";
Expand Down Expand Up @@ -133,6 +160,28 @@ export function schemaCanSuggest(schema: AnnotationSchema): boolean {
return schema.classes.some(isSuggestibleClass);
}

/**
* The active class if a suggestion could be labelled with it, and `null` if not.
*
* What {@link withClass} is fed when the active class moves, so `null` is the
* parked reading rather than an error: a schema that can suggest can still be
* sitting on a class that cannot, and that is a state to describe rather than one
* to prevent.
*
* Deliberately **not** {@link suggestClassFor}, which falls back to the schema's
* first suggestible class. That fallback is right when somebody presses the tool
* — they asked for it, so take them somewhere it works — and wrong here, where
* they asked for a *class*: moving the active class out from under a person who
* just picked one is the consequence `ToolPalette` refuses to have.
*/
export function suggestibleClassIn(
schema: AnnotationSchema,
activeClass: string | null,
): string | null {
const held = schema.classes.find((declared) => declared.name === activeClass);
return held !== undefined && isSuggestibleClass(held) ? held.name : null;
}

/** What a click told the model: this is the thing, or this is not the thing. */
export type Polarity = "positive" | "negative";

Expand Down Expand Up @@ -168,8 +217,14 @@ export type SuggestionStatus = "idle" | "asking" | "shown" | "none" | "refused";

/** The whole of a suggest session. `null`, in a host, is a tool that is not armed. */
export interface SuggestionState {
/** The class the accepted annotation will carry. Fixed for the session. */
readonly labelClass: string;
/**
* The class the accepted annotation will carry.
*
* `null` is a **parked** session: armed, but sitting on a class that can hold
* nothing a segmenter proposes, so there is nothing to ask about. It is not an
* absent value to be defaulted — see {@link withClass} and {@link isParked}.
*/
readonly labelClass: string | null;
/** Every click so far, in the order they were placed. */
readonly points: readonly PromptPoint[];
readonly status: SuggestionStatus;
Expand Down Expand Up @@ -283,6 +338,37 @@ export function cleared(state: SuggestionState): SuggestionState {
return { ...state, points: [], status: "idle", suggestion: null, refusal: null };
}

/**
* The active class moved. The tool stays armed; what was pending does not.
*
* One function for the three readings the module note sets out — swap, discard,
* park — because they are one transition over a different argument. A class that
* did not actually move returns the state **by identity**, so a host can fold
* this through unconditionally without a render or a discarded preview for a
* re-selection of the class already held.
*
* `null` parks. Re-arming is this same call with a class that can hold an answer,
* which is why nothing here remembers *which* class was parked from: the class a
* parked session resumes on is the one the user has just picked, not the one they
* left.
*/
export function withClass(state: SuggestionState, labelClass: string | null): SuggestionState {
if (labelClass === state.labelClass) return state;
return { ...cleared(state), labelClass };
}

/**
* Whether the session is armed over a class that can hold nothing.
*
* The one thing a host must not do with a parked session is divert a press into
* it: the class the user is on 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.
*/
export function isParked(state: SuggestionState): boolean {
return state.labelClass === null;
}

/** Whether there is anything for Escape to take back. */
export function hasPending(state: SuggestionState): boolean {
return state.points.length > 0 || state.status !== "idle";
Expand Down Expand Up @@ -318,6 +404,10 @@ export function acceptedAnnotation(
mint: IdFactory,
): Annotation | null {
if (!isAcceptable(state) || state.suggestion === null) return null;
// A parked session cannot reach `shown`, so this is unreachable by the state
// machine — and it is the guarantee, not a formality: nothing may be written
// without a class, whatever a caller believes about how it got here.
if (state.labelClass === null) return null;
if (classNamed(document, state.labelClass) === undefined) return null;
const drawn = draftAnnotation(document, state.labelClass, state.suggestion.geometry, mint);
return {
Expand Down
3 changes: 3 additions & 0 deletions frontend/annotator/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,14 @@ export {
cleared,
hasPending,
isAcceptable,
isParked,
isSuggestibleClass,
promptOf,
refused,
schemaCanSuggest,
suggestClassFor,
suggestibleClassIn,
withClass,
withPoint,
type Polarity,
type Prompt,
Expand Down
27 changes: 27 additions & 0 deletions frontend/app/e2e/styleguide.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,30 @@ test("the tab bar sits one rhythm step above its content", async ({ page }) => {
expect(panel).not.toBeNull();
expect(panel!.y - (list!.y + list!.height)).toBeCloseTo(12, 0);
});

/**
* #472's claim, which is a claim about pixels.
*
* `primitives.test.tsx` asserts the *structure* — two elements, the meta in the
* muted role, the id keeping its own line — and jsdom computes no layout, so the
* one thing it cannot see is the thing that was reported: a two-line value inside
* a control measured for one line. A revert to `h-9` leaves every unit test green
* and fails here.
*/
test("a two-line option grows its trigger, and a one-line one stays on the contract's 36px", async ({
page,
}) => {
const plain = await page.getByLabel("Geometry").boundingBox();
const stacked = await page.getByLabel("Model").boundingBox();
expect(plain).not.toBeNull();
expect(stacked).not.toBeNull();

// Unmoved: every select that shipped before the variant is still exactly 36px.
expect(plain!.height).toBeCloseTo(36, 0);
// Grown, not squashed: the second line is inside the box rather than over it.
expect(stacked!.height).toBeGreaterThan(plain!.height);

// And the identifier is whole — no ellipsis, no clipped end.
const id = page.getByLabel("Model").locator("span", { hasText: "facebook/sam2.1-hiera-base-plus" }).first();
await expect(id).toHaveCSS("text-overflow", "clip");
});
Loading
Loading