diff --git a/docs/annotations.md b/docs/annotations.md index af99bba9..02c7517a 100644 --- a/docs/annotations.md +++ b/docs/annotations.md @@ -150,6 +150,13 @@ be constructed, so it never reaches a service to be reported. That is the divisi [schemas.md](schemas.md) already draws: per-value validity is pydantic's, validity that needs another object is the service's. +An annotation accepted from the editor's suggest tool is written this way and no other: an +ordinary `add` carrying `provenance="model"`, the `model_ref` the suggestion named, and its +`confidence`. There is no separate route for it, no relaxed validation, and no link back to the +connection — the model's identity is **copied** at write time, so deleting the connection later +leaves the record intact. The gesture is in [ui.md](ui.md); what it proposes is in +[inference.md](inference.md). + ## `delete` has no `confirm=` The one exception to the rule in [projects.md](projects.md) and [batches.md](batches.md). diff --git a/docs/ui.md b/docs/ui.md index 129486bd..41ff7092 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -408,6 +408,47 @@ is distance-to-segment with a zoom-independent tolerance, and it is only worth solving beside the tool that edits the result), and it is **not draggable**. The object list is how a lane is selected, which is a real affordance rather than a gap. +#### Suggesting a shape from a click + +The sparkles button — hotkey `S` — arms the **suggest tool**: click the thing you +want and a segmentation model proposes its shape. It runs through a model +connection (`docs/inference.md`), and the server side of it is +`POST /inference/suggest`. + +The gesture: + +| Press | What it does | +| --- | --- | +| left-click | adds a point on the object, and asks again | +| alt-click | adds a point that is **not** on the object, and asks again | +| `↵` | accepts the proposal as an annotation | +| `Esc` | clears the points; pressing it again puts the tool away | + +Every click sends **all** the points placed so far — the route is stateless — and +the answer replaces the preview. The first click on a frame is the slow one, +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. + +Accepting creates one ordinary annotation, in one undo step, carrying +`provenance: model`, the `model_ref` the answer named and its `confidence` — the +same write path a hand-drawn shape takes, so the same schema rules apply and the +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. + +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 +one action to fix it. Nothing navigates away, and no exit loses work. A refusal +from the server is rendered where the panel is, in the server's own words, which +is what carries the install command when the optional runtime is absent. + ### The annotation side panel `AnnotatorPanel` — Objects and Labels — lives in **`ui-core`**, not in the diff --git a/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx b/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx index b3607c28..9476c189 100644 --- a/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx +++ b/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx @@ -138,6 +138,8 @@ import { NO_TARGET } from "../../core/interaction/target"; import { toolFor } from "../../core/interaction/tool"; import type { Tool } from "../../core/interaction/tool"; import { + ACCEPT_SUGGESTION, + DISCARD_SUGGESTION, READ_ONLY_KINDS, RESET_ZOOM, SAVE_AND_NEXT, @@ -150,6 +152,8 @@ import { runAction, } from "../../core/input"; import type { Action, Binding, InputHost } from "../../core/input"; +import { hasPending, isAcceptable } from "../../core/interaction/suggestion"; +import type { Polarity, SuggestionState } from "../../core/interaction/suggestion"; import { createClipboard } from "../../core/interaction/clipboard"; import type { Clipboard } from "../../core/interaction/clipboard"; import type { IdFactory } from "../../core/ids"; @@ -172,7 +176,7 @@ import type { Viewport } from "../viewport"; import { AnnotationLayer } from "./AnnotationLayer"; import { useAnnotatorSnapshot } from "./hooks"; import { digitFromCode, isComposing, isTextEntry } from "./keyboard"; -import { classColor, editedId, paintAnnotation } from "./paint"; +import { classColor, editedId, paintAnnotation, paintSuggestion } from "./paint"; import { stageScreenSizes } from "./Shapes"; import { withoutHidden } from "./visibility"; import { TransientLayer } from "./TransientLayer"; @@ -362,6 +366,42 @@ export interface AnnotatorCanvasProps { * deciding a host's policy. This governs *input*. */ readonly readOnly?: boolean; + /** + * The suggest session, or `null`/absent when the tool is not armed (#424). + * + * **The host holds it**, for the reason `core/interaction/suggestion.ts` gives + * at length: every transition but the first is driven by a server's answer, and + * this adapter deliberately fetches nothing. What arrives here is the state to + * draw and to route presses into — not a channel to change it. + * + * Its presence is a **mode**, and it is exactly the mode `state.ts`'s pan + * contract already describes from the other side: *"while panning or pinching, + * the adapter does not forward pointer events to the machine; if a gesture was + * in flight when the pan began, it sends `pointer-cancel` first."* An armed + * suggest tool is the second occupant of that rule, and it is honoured in the + * same two places — the effect below arms it, `handlePointerDown` diverts. + * + * That is also why the suggest tool is not a `Tool`. `tool.ts` derives the tool + * from the active class and stores nothing, and it is emphatic about why; a + * fifth variant there would be a stored mode wearing a derived one's name, and + * `toolFor` would have nowhere to derive it from. The class stays what it was — + * a suggestion is labelled with it — and this is a mode over the top. + */ + readonly suggestion?: SuggestionState | null; + /** + * A press while the suggest tool is armed, in asset pixels. + * + * Alt-click is the negative point, which is D2's *"left-click adds a positive + * point, alt/right-click a negative point"* with the alt half taken and the + * right half left alone: a secondary press is a pan on this canvas, and #380's + * `contextmenu` note explains why taking it back is not free. Alt is the + * spelling that costs no existing gesture. + * + * Absent, with a session armed, means a host that armed a tool it cannot serve + * — so the press is swallowed rather than falling through to a drawing gesture, + * which would draw a shape somebody was trying to point at. + */ + readonly onSuggestPoint?: (point: Point, polarity: Polarity) => void; } /** What a host can do to the stage. Read the position through `onViewChange`. */ @@ -389,6 +429,8 @@ export function AnnotatorCanvas({ className, viewRef, readOnly = false, + suggestion = null, + onSuggestPoint, }: AnnotatorCanvasProps): JSX.Element { const snapshot = useAnnotatorSnapshot(store); const { asset, schema } = snapshot.document; @@ -514,6 +556,32 @@ export function AnnotatorCanvas({ dispatch({ type: "tool-changed" }); }, [tool, dispatch]); + /** + * Arming the suggest tool interrupts whatever the pointer was doing (#424). + * + * The pan contract's second occupant, discharged the way the first one is: a + * gesture in flight when the mode begins is cancelled, and after that the + * presses simply do not arrive. Without this, arming mid-drag would leave the + * machine in `moving` with a staged preview nothing will ever commit — and the + * next `pointer-up`, which this component no longer forwards, would never come + * to clear it. + * + * Keyed on **armed-ness** rather than on the session object, which changes on + * every click and every answer: re-cancelling an already-idle machine on each + * refine is harmless and re-running this effect that often is not what it is + * for. Disarming needs nothing — there is no gesture to interrupt on the way + * out, and the machine has been idle throughout. + */ + const armedSuggest = suggestion !== null; + const armedNow = useRef(armedSuggest); + useEffect(() => { + if (armedSuggest === armedNow.current) return; + armedNow.current = armedSuggest; + if (armedSuggest && interactionNow.current.type !== "idle") { + dispatch({ type: "pointer-cancel" }); + } + }, [armedSuggest, dispatch]); + const announced = useRef({ document: snapshot.document, selection: snapshot.selection }); useEffect(() => { const seen = announced.current; @@ -602,10 +670,26 @@ export function AnnotatorCanvas({ * same frame as a pointer event must read the state that event left behind. */ const finishing = resolved.kind === "send" && resolved.event.type === "commit"; + /** + * `escape` means **take back**, and a pending suggestion is the most recent + * thing there is to take back (#424, D4: *"Esc is the preview's undo"*). + * + * The same substitution `enter` has had since #383, on the other chord and + * for the same reason: the deciding fact is state the adapter holds. It + * outranks the machine's cancel while something is pending and disappears + * the moment nothing is — so every cancel row in `machine.ts` still reads + * exactly as written, and a second Escape clears the selection as it always + * did. + */ + const escaping = resolved.kind === "send" && resolved.event.type === "cancel"; const action: Action = - finishing && interactionNow.current.type === "idle" - ? { kind: "host", name: SAVE_AND_NEXT } - : resolved; + suggestion !== null && escaping && hasPending(suggestion) + ? { kind: "host", name: DISCARD_SUGGESTION } + : suggestion !== null && finishing && isAcceptable(suggestion) + ? { kind: "host", name: ACCEPT_SUGGESTION } + : finishing && interactionNow.current.type === "idle" + ? { kind: "host", name: SAVE_AND_NEXT } + : resolved; // (3) The guard, with Escape surviving it. v1 ran Escape *before* its `inInput` // check, deliberately, so Escape blurs a field; that ordering is easy to lose @@ -617,10 +701,12 @@ export function AnnotatorCanvas({ // on a read-only page unable to paste into an ordinary input. Nothing else // changes hands — the branch below still swallows a claimed chord that // reached the canvas. + // Read off `resolved`, never off `action`: Escape blurs a field because it is + // Escape, and the substitution above must not be able to take that away by + // turning the chord into a host row. const target = event.target instanceof HTMLElement ? event.target : null; - const cancelling = action.kind === "send" && action.event.type === "cancel"; if (isTextEntry(target)) { - if (!cancelling) return; + if (!escaping) return; target?.blur(); } @@ -697,6 +783,20 @@ export function AnnotatorCanvas({ const point = imagePoint(event); if (point === null) return; + + // The suggest mode: this press is a prompt point, and the machine hears + // nothing at all. Placed after the pan branch so a secondary press still + // pans — a person refining a suggestion needs to move around the picture — + // and before the dispatch, because a press that reached the machine would + // start drawing the very box the model is being asked for. + // + // A session with no handler swallows the press rather than falling through, + // for the reason the prop's docstring gives. + if (suggestion !== null) { + onSuggestPoint?.(point, event.altKey ? "negative" : "positive"); + return; + } + dispatch({ type: "pointer-down", point, button, modifiers: modifiersOf(event) }); // After the dispatch, and only for a drag — see the note above. if (DRAG_STATES.has(interactionNow.current.type)) { @@ -803,6 +903,19 @@ export function AnnotatorCanvas({ activeClass === null ? undefined : schema.classes.find((row) => row.name === activeClass); const drawColor = activeClass === null ? "#8a8a93" : classColor(declared, activeClass); + // The session's own class rather than `activeClass`: they agree today, because + // arming the tool activates a class that can hold a suggestion — but the + // session captured its class at arming time, the way `drawing-bbox` captures + // its `labelClass` at the press, and the preview must be labelled with what it + // will actually be written as. + const painted = + suggestion === null + ? null + : paintSuggestion( + suggestion, + schema.classes.find((row) => row.name === suggestion.labelClass), + ); + return (
diff --git a/frontend/annotator/src/adapters/react/TransientLayer.tsx b/frontend/annotator/src/adapters/react/TransientLayer.tsx index c95a891e..9e2a680e 100644 --- a/frontend/annotator/src/adapters/react/TransientLayer.tsx +++ b/frontend/annotator/src/adapters/react/TransientLayer.tsx @@ -23,9 +23,16 @@ import type { JSX } from "react"; import { polygonCloseAttempt } from "../../core/geometry/hitTest"; import type { Target } from "../../core/interaction/target"; import type { InteractionState } from "../../core/interaction/state"; +import type { PromptPoint } from "../../core/interaction/suggestion"; import type { AssetDescriptor, Point } from "../../core/types"; -import { pendingPolygon, rubberBand, screenPx } from "./paint"; -import type { PaintedAnnotation } from "./paint"; +import { + SUGGESTION_DASH, + SUGGESTION_OPACITY, + pendingPolygon, + rubberBand, + screenPx, +} from "./paint"; +import type { PaintedAnnotation, PaintedSuggestion } from "./paint"; import { AnnotationShape, HANDLE_PX, @@ -47,6 +54,24 @@ export interface TransientLayerProps { /** The pointer, in asset pixels, when a drawing tool wants a crosshair. */ readonly crosshair: Point | null; readonly asset: AssetDescriptor; + /** + * The proposal waiting to be accepted, if one is showing (#424, D2). + * + * Here rather than in `AnnotationLayer` because it is exactly what this layer + * is for: something on screen that is not in the document. It never enters + * `AnnotatorStore`, so the committed layer has nothing to draw it from — which + * is D4's ephemerality made visible rather than merely promised. + */ + readonly suggestion?: PaintedSuggestion | null; + /** + * Every click of the suggest session so far, drawn whatever the answer is + * doing — including while one is in flight and after a refusal. + * + * Separate from `suggestion` for that reason: the dots are what makes a refine + * click legible, and blanking them while the server thinks would make each + * press look like it had been dropped. + */ + readonly promptPoints?: readonly PromptPoint[]; } const DASH = "6 4"; @@ -60,6 +85,8 @@ export function TransientLayer({ closeRing, crosshair, asset, + suggestion = null, + promptPoints, }: TransientLayerProps): JSX.Element { const band = rubberBand(state); const pending = pendingPolygon(state); @@ -106,11 +133,134 @@ export function TransientLayer({ /> )} + {suggestion !== null && } + + {promptPoints !== undefined && promptPoints.length > 0 && ( + + )} + ); } +/** + * A proposal, drawn as a proposal: reduced opacity, a dashed outline, and its + * class and confidence beside it (#424, D2). + * + * Both signals together, and `paint.ts` states why neither is enough alone. The + * label carries the confidence because that is the one fact a person needs to + * decide whether to look closely before pressing Enter, and it has nowhere else + * to be — the panel lists annotations, and this is not one yet. + * + * The stroke is one step **wider** than a committed shape's, which sounds + * backwards for something drawn faintly and is not: opacity takes contrast away, + * and a hairline at 0.6 alpha over a photograph disappears. The two adjustments + * are one decision. + */ +function SuggestedShape({ + suggestion, + zoom, +}: { + readonly suggestion: PaintedSuggestion; + readonly zoom: number; +}): JSX.Element { + const { geometry, color, label } = suggestion; + const stroke = screenPx(STROKE_PX + 1, zoom); + const anchor: Point = + geometry.type === "bbox" + ? [geometry.x, geometry.y] + : (geometry.points.reduce( + (best, point) => (point[1] < best[1] ? point : best), + geometry.points[0] ?? [0, 0], + )); + return ( + + {geometry.type === "bbox" ? ( + + ) : ( + `${point[0]},${point[1]}`).join(" ")} + fill={color} + fillOpacity={0.14} + stroke={color} + strokeWidth={stroke} + strokeDasharray={SUGGESTION_DASH} + strokeLinejoin="round" + /> + )} + {/* `ShapeLabel`'s treatment, at the meta size — the halo is what keeps it + readable over a photograph, and the lift rides the CSS custom property + the stage publishes so this element carries no zoom of its own. */} + + {label} + + + ); +} + +/** + * Where the user clicked, and what each click meant. + * + * Filled for a positive point and hollow for a negative one, on top of the + * colour difference rather than instead of it: "this is the thing" and "this is + * not the thing" are opposite instructions, and a person who cannot tell green + * from red must still be able to see which of their four clicks was the + * subtraction. + */ +function PromptPoints({ + points, + zoom, +}: { + readonly points: readonly PromptPoint[]; + readonly zoom: number; +}): JSX.Element { + const radius = screenPx(VERTEX_PX + 1, zoom); + return ( + + {points.map((placed, index) => ( + + ))} + + ); +} + /** * A shape mid-session: the vertices placed, the rubber band to the cursor, and — * for a polygon — the ring around vertex zero that says where a click would close diff --git a/frontend/annotator/src/adapters/react/index.ts b/frontend/annotator/src/adapters/react/index.ts index a4f3503b..fe508841 100644 --- a/frontend/annotator/src/adapters/react/index.ts +++ b/frontend/annotator/src/adapters/react/index.ts @@ -29,14 +29,19 @@ export { Vertices, } from "./Shapes"; export { + SUGGESTION_DASH, + SUGGESTION_OPACITY, classColor, + confidenceLabel, editedId, paintAnnotation, paintDocument, + paintSuggestion, pendingPolygon, rubberBand, screenPx, type PaintedAnnotation, + type PaintedSuggestion, type PendingPolygon, } from "./paint"; export { diff --git a/frontend/annotator/src/adapters/react/paint.test.ts b/frontend/annotator/src/adapters/react/paint.test.ts index 1b35dde4..42021e6d 100644 --- a/frontend/annotator/src/adapters/react/paint.test.ts +++ b/frontend/annotator/src/adapters/react/paint.test.ts @@ -12,14 +12,26 @@ import { createDocument } from "../../core/state/document"; import { EMPTY_SELECTION, selectionOf } from "../../core/state/selection"; import type { Annotation } from "../../core/types"; import { + SUGGESTION_DASH, + SUGGESTION_OPACITY, classColor, + confidenceLabel, editedId, paintAnnotation, paintDocument, + paintSuggestion, pendingPolygon, rubberBand, screenPx, } from "./paint"; +import { + answered, + armed, + cleared, + refused, + withPoint, +} from "../../core/interaction/suggestion"; +import type { Suggestion, SuggestionState } from "../../core/interaction/suggestion"; function tag(id: string): Annotation { return { ...annotation(id), geometry: { type: "classification_tag" } }; @@ -233,3 +245,73 @@ describe("the shape under construction", () => { } }); }); + +describe("a pending suggestion, drawn as a proposal (#424)", () => { + const SIGN = SCHEMA.classes.find((declared) => declared.name === "sign"); + const A_BOX = { type: "bbox", x: 10, y: 20, width: 30, height: 40 } as const; + + function proposal(confidence: number | null = 0.871): Suggestion { + return { geometry: A_BOX, confidence, modelRef: "facebook/sam2-hiera-base-plus@main" }; + } + + function shown(suggestion: Suggestion = proposal()): SuggestionState { + const asked = withPoint(armed("sign"), [100, 120], "positive"); + return answered(asked, asked.serial, suggestion); + } + + it("is told apart by two signals, and never by colour alone", () => { + // The shape draws in its class's own colour — that is the point of it — so + // hue cannot be what distinguishes it from a stored annotation. Both of these + // are visible to somebody who cannot tell the two hues apart. + expect(SUGGESTION_OPACITY).toBeLessThan(1); + expect(SUGGESTION_DASH).not.toBe(""); + expect(paintSuggestion(shown(), SIGN)?.color).toBe(classColor(SIGN, "sign")); + }); + + it("carries the geometry, the class and the confidence", () => { + const painted = paintSuggestion(shown(), SIGN); + expect(painted?.geometry).toEqual(A_BOX); + expect(painted?.label).toBe("sign 87%"); + }); + + it("carries the clicks that produced it, so a refine is legible", () => { + const refined = withPoint(shown(), [200, 210], "negative"); + const settled = answered(refined, refined.serial, proposal()); + expect(paintSuggestion(settled, SIGN)?.points).toEqual([ + { point: [100, 120], polarity: "positive" }, + { point: [200, 210], polarity: "negative" }, + ]); + }); + + it("names the class alone when the model reported no confidence", () => { + expect(confidenceLabel("sign", null)).toBe("sign"); + expect(paintSuggestion(shown(proposal(null)), SIGN)?.label).toBe("sign"); + }); + + it("rounds the confidence to whole percent at both ends", () => { + expect(confidenceLabel("sign", 0)).toBe("sign 0%"); + expect(confidenceLabel("sign", 1)).toBe("sign 100%"); + expect(confidenceLabel("sign", 0.005)).toBe("sign 1%"); + }); + + it("draws nothing for every status but `shown` — each of those is a sentence", () => { + const asked = withPoint(armed("sign"), [100, 120], "positive"); + expect(paintSuggestion(armed("sign"), SIGN)).toBeNull(); + expect(paintSuggestion(asked, SIGN)).toBeNull(); + expect(paintSuggestion(answered(asked, asked.serial, null), SIGN)).toBeNull(); + expect(paintSuggestion(refused(asked, asked.serial, "not here yet"), SIGN)).toBeNull(); + expect(paintSuggestion(cleared(shown()), 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 + // and a caller could hand it anything the type permits. + const path = shown({ + geometry: { type: "polyline", points: [[0, 0], [1, 1]] }, + confidence: 0.5, + modelRef: "m@1", + }); + expect(paintSuggestion(path, SIGN)).toBeNull(); + }); +}); diff --git a/frontend/annotator/src/adapters/react/paint.ts b/frontend/annotator/src/adapters/react/paint.ts index d5a5880a..f2ded7b9 100644 --- a/frontend/annotator/src/adapters/react/paint.ts +++ b/frontend/annotator/src/adapters/react/paint.ts @@ -45,6 +45,7 @@ import { annotationsInDrawOrder } from "../../core/state/document"; import type { AnnotationDocument } from "../../core/state/document"; import type { Selection } from "../../core/state/selection"; import type { InteractionState } from "../../core/interaction/state"; +import type { PromptPoint, SuggestionState } from "../../core/interaction/suggestion"; import type { Annotation, BboxGeometry, @@ -239,3 +240,76 @@ export function pendingPolygon(state: InteractionState): PendingPolygon | null { closable: state.type === "drawing-polygon", }; } + +/** + * How a proposal is told apart from a stored annotation (#424, D2). + * + * **Two signals, and never colour alone.** A suggestion is drawn in its class's + * own colour — that is the point of it, since the class is what it will be + * labelled — so hue cannot be what distinguishes it. Reduced opacity plus a + * dashed stroke are both visible to somebody who cannot tell the two hues apart, + * and the dash is the one that survives a screenshot at any zoom. + * + * The dash is deliberately **not** `TransientLayer`'s `"6 4"`: that one is a + * rubber band the pointer is dragging, and this is a shape waiting to be + * accepted. A longer dash at the same stroke width reads as a different kind of + * provisional rather than as the same one. + */ +export const SUGGESTION_OPACITY = 0.6; + +/** The preview's stroke pattern — see `SUGGESTION_OPACITY`. */ +export const SUGGESTION_DASH = "10 6"; + +/** A pending suggestion, ready to draw. */ +export interface PaintedSuggestion { + /** Never a tag or a path: the two kinds `SUGGESTIBLE_GEOMETRY_TYPES` names. */ + readonly geometry: BboxGeometry | PolygonGeometry; + readonly color: string; + /** The class, and the model's confidence when it reported one. */ + readonly label: string; + /** The clicks that produced it, so the preview shows what it was asked. */ + readonly points: readonly PromptPoint[]; +} + +/** + * The pending suggestion as a draw list, or `null` when there is nothing to draw. + * + * `null` covers every status but `shown` — an armed tool nobody has clicked with, + * a first ask still in flight, an answer with nothing in it, a refusal. Each of + * those is a *sentence*, and a sentence is the host's to render: this package + * ships no chrome, and `AnnotatorPanel`'s argument applies to a spinner and an + * error message exactly as it does to a toolbar. + * + * The **points are carried whatever the status**, which is why the caller checks + * for them separately: the dots are what makes a refine click legible, and they + * must stay on screen while the answer to the click that placed them is still + * coming back. + */ +export function paintSuggestion( + state: SuggestionState, + declared: LabelClass | undefined, +): PaintedSuggestion | null { + const suggestion = state.suggestion; + if (state.status !== "shown" || suggestion === 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), + points: state.points, + }; +} + +/** + * `class 87%`, or the bare class when the model reported no confidence. + * + * Rounded to whole percent, because a suggestion's confidence is a rough signal + * about whether to look closely and two decimal places would suggest a precision + * the number does not have. A confidence outside `[0, 1]` cannot arrive — the + * kernel's `PredictedRegion` refuses one — so nothing is clamped here. + */ +export function confidenceLabel(labelClass: string, confidence: number | null): string { + if (confidence === null) return labelClass; + return `${labelClass} ${Math.round(confidence * 100)}%`; +} diff --git a/frontend/annotator/src/core/input/actions.ts b/frontend/annotator/src/core/input/actions.ts index f7b38566..15492131 100644 --- a/frontend/annotator/src/core/input/actions.ts +++ b/frontend/annotator/src/core/input/actions.ts @@ -205,3 +205,53 @@ export const SAVE_AND_NEXT = "save-and-next"; * `x` is unclaimed. */ export const SKIP_FRAME = "skip-frame"; + +/** + * Arm or disarm the suggest tool — `s`, a bare letter on `c`'s, `v`'s and `x`'s + * terms (#424, slice 3b). + * + * A host row, and it could be nothing else: a suggestion comes from a model + * behind an HTTP route, and this package has no HTTP and never will. What core + * owns is the *shape* of the session — `interaction/suggestion.ts` — the same + * split `SAVE` has, where the meaning is here and the request is the host's. + * + * It takes an ordinary row in `DEFAULT_BINDINGS` because `s` is unclaimed; + * `mod+s` is `SAVE` and the two are different chords. Claimed even on a host that + * offers no suggestions, for the reason every host row is: the registry is the + * list of keystrokes the annotator takes away from the browser, and a host that + * cannot serve it answers `false` and the chord falls through. + */ +export const TOGGLE_SUGGEST = "toggle-suggest"; + +/** + * Turn the pending suggestion into an annotation — `enter` (#424, D4). + * + * `SAVE_AND_NEXT`'s sibling and its neighbour in the same substitution. + * `enter` already means *finish*, and this is the third thing it can finish: + * a shape being drawn (the commit row), a suggestion being previewed (this), + * or the frame (`SAVE_AND_NEXT`). All three are decided from state the adapter + * holds and this table does not, so all three are read off `enter` in + * `AnnotatorCanvas.handleKeyDown` rather than bound here — a second `enter` row + * would shadow the commit, because the fold is last-wins. + * + * A host row rather than an `add` effect, because the annotation an accepted + * suggestion becomes carries a `model_ref` and a `confidence` that arrived over + * a wire. `acceptedAnnotation` builds it; the host is what holds the answer. + */ +export const ACCEPT_SUGGESTION = "accept-suggestion"; + +/** + * Throw the pending suggestion away — `escape` (#424, D4: *"Esc is the preview's + * undo"*). + * + * The same substitution as `ACCEPT_SUGGESTION`, on the other chord, and it is + * **first**: a preview is the most recent thing a person put on screen, so an + * Escape while one is showing is about that and not about the selection. With + * nothing pending the chord falls through to `send cancel` untouched, so every + * cancel rule in `machine.ts` still reads exactly as it did. + * + * It is the preview's undo rather than the command log's for the structural + * reason `suggestion.ts` gives: nothing about a pending suggestion is in the + * document, so there is nothing for `mod+z` to step back to. + */ +export const DISCARD_SUGGESTION = "discard-suggestion"; diff --git a/frontend/annotator/src/core/input/bindings.test.ts b/frontend/annotator/src/core/input/bindings.test.ts index 26cb79a0..8dc1d889 100644 --- a/frontend/annotator/src/core/input/bindings.test.ts +++ b/frontend/annotator/src/core/input/bindings.test.ts @@ -26,7 +26,14 @@ import { pressOf, wideSchema, } from "./_palette"; -import { FOCUS_CLASS_FIELD, RESET_ZOOM, SAVE, SKIP_FRAME, TOGGLE_HELP } from "./actions"; +import { + FOCUS_CLASS_FIELD, + RESET_ZOOM, + SAVE, + SKIP_FRAME, + TOGGLE_HELP, + TOGGLE_SUGGEST, +} from "./actions"; import type { Action } from "./actions"; import { CLASS_HOTKEY_DIGITS, @@ -87,6 +94,10 @@ const DISPATCH: readonly DispatchRow[] = [ // the ring close above, substituted by the adapter when nothing is being drawn, // so a second `enter` row here would shadow the commit. { chord: "x", key: "x", action: { kind: "host", name: SKIP_FRAME } }, + // #424. `enter` and `escape` are absent from this trio for `x`'s neighbour's + // reason: accepting and discarding a suggestion are substitutions the adapter + // makes over the two rows above, and a row here would shadow both. + { chord: "s", key: "s", action: { kind: "host", name: TOGGLE_SUGGEST } }, { chord: "v", key: "v", action: { kind: "activate-class", labelClass: null } }, ]; @@ -150,6 +161,17 @@ describe("the default shortcut table", () => { } }); + it("keeps the suggest tool and the save chord apart, though both are `s` (#424)", () => { + // The same claim `mod+c`/`c` makes above, on the pair that arrived last: the + // modifier is part of the chord string, so claiming `s` did not shadow + // `mod+s` and nothing had to be re-bound to make room for it. + expect(resolve(DEFAULTS, keystroke("s"))).toEqual({ + kind: "host", + name: TOGGLE_SUGGEST, + }); + expect(resolve(DEFAULTS, keystroke("s", MOD))).toEqual({ kind: "host", name: SAVE }); + }); + it("answers null for a chord nobody bound", () => { expect(resolve(DEFAULTS, keystroke("q"))).toBeNull(); expect(resolve(DEFAULTS, keystroke("Escape", { altKey: true }))).toBeNull(); diff --git a/frontend/annotator/src/core/input/bindings.ts b/frontend/annotator/src/core/input/bindings.ts index 0f71afb8..d8e2e9af 100644 --- a/frontend/annotator/src/core/input/bindings.ts +++ b/frontend/annotator/src/core/input/bindings.ts @@ -123,7 +123,14 @@ import { isTaggableClass } from "../interaction/tags"; import type { AnnotationSchema } from "../types"; -import { FOCUS_CLASS_FIELD, RESET_ZOOM, SAVE, SKIP_FRAME, TOGGLE_HELP } from "./actions"; +import { + FOCUS_CLASS_FIELD, + RESET_ZOOM, + SAVE, + SKIP_FRAME, + TOGGLE_HELP, + TOGGLE_SUGGEST, +} from "./actions"; import type { Action } from "./actions"; import { chordOf } from "./keys"; import type { Keystroke } from "./keys"; @@ -178,6 +185,11 @@ export const DEFAULT_BINDINGS: readonly Binding[] = [ // so an unbound one would be a chip that lies. This is the half that fits in the // table; its sibling rides `enter`, for the reason above. { chord: "x", action: { kind: "host", name: SKIP_FRAME } }, + // `s`, one more bare letter (#424). Not `mod+s`, which is `SAVE` — the two are + // different chords and the fold never sees them collide. A host with no model + // connection answers `false` and the chord falls through, which is what makes + // claiming it safe on a build that cannot serve it. + { chord: "s", action: { kind: "host", name: TOGGLE_SUGGEST } }, { chord: "v", action: { kind: "activate-class", labelClass: null } }, ]; diff --git a/frontend/annotator/src/core/input/index.ts b/frontend/annotator/src/core/input/index.ts index 575ef4d0..f143de09 100644 --- a/frontend/annotator/src/core/input/index.ts +++ b/frontend/annotator/src/core/input/index.ts @@ -78,6 +78,8 @@ export { type ModifierState, } from "./keys"; export { + ACCEPT_SUGGESTION, + DISCARD_SUGGESTION, FOCUS_CLASS_FIELD, READ_ONLY_KINDS, RESET_ZOOM, @@ -85,6 +87,7 @@ export { SAVE_AND_NEXT, SKIP_FRAME, TOGGLE_HELP, + TOGGLE_SUGGEST, type Action, type ActionKind, type KeyIntent, diff --git a/frontend/annotator/src/core/interaction/suggestion.test.ts b/frontend/annotator/src/core/interaction/suggestion.test.ts new file mode 100644 index 00000000..a7e63521 --- /dev/null +++ b/frontend/annotator/src/core/interaction/suggestion.test.ts @@ -0,0 +1,324 @@ +/** + * The suggest session, driven directly — no store, no adapter, no DOM. + * + * The lifecycle claims D4 makes are the subject: a preview replaced by a refine, + * discarded by Escape, and accepted into exactly one annotation carrying where it + * came from. The two that are structural rather than behavioural — nothing enters + * the command log, nothing enters the document — are asserted here against a real + * `AnnotatorStore`, because "it is ephemeral" is a claim about `canUndo` and about + * `document.annotations`, and only a store can answer either. + */ + +import { describe, expect, it } from "vitest"; + +import { AnnotatorStore } from "../state/store"; +import { annotationsInDrawOrder, createDocument } from "../state/document"; +import type { AnnotationDocument } from "../state/document"; +import { addAnnotationCommand } from "../state/commands"; +import type { AnnotationSchema, AssetDescriptor, Geometry, LabelClass } from "../types"; +import { + SUGGESTIBLE_GEOMETRY_TYPES, + acceptedAnnotation, + allowedGeometriesFor, + answered, + armed, + cleared, + hasPending, + isAcceptable, + isSuggestibleClass, + promptOf, + refused, + schemaCanSuggest, + suggestClassFor, + withPoint, +} from "./suggestion"; +import type { Suggestion, SuggestionState } from "./suggestion"; + +const ASSET: AssetDescriptor = { id: "asset-424", width: 800, height: 600 }; + +function classOf(name: string, geometry: LabelClass["geometry"]): LabelClass { + return { name, geometry, color: null, attributes: [] }; +} + +const CAR = classOf("car", "bbox"); +const ROAD = classOf("road", "polygon"); +const LANE = classOf("lane", "polyline"); +const WEATHER = classOf("weather", "classification_tag"); + +function schemaOf(...classes: readonly LabelClass[]): AnnotationSchema { + return { + project_id: "project-424", + version: 4, + classes, + description: null, + created_at: null, + provenance: null, + }; +} + +function documentOf(schema: AnnotationSchema = schemaOf(CAR, ROAD)): AnnotationDocument { + return createDocument(ASSET, schema, []); +} + +const A_BOX: Geometry = { type: "bbox", x: 10, y: 20, width: 30, height: 40 }; +const A_POLYGON: Geometry = { + type: "polygon", + points: [ + [0, 0], + [10, 0], + [10, 10], + ], +}; + +function proposal(geometry: Geometry = A_BOX, confidence: number | null = 0.87): Suggestion { + return { geometry, confidence, modelRef: "facebook/sam2-hiera-base-plus@main" }; +} + +/** A session that has asked once and been answered. The commonest starting point. */ +function showing(suggestion: Suggestion = proposal()): SuggestionState { + const asked = withPoint(armed("car"), [100, 120], "positive"); + return answered(asked, asked.serial, suggestion); +} + +describe("which classes the tool is offered for", () => { + it("names exactly the two kinds a mask can be narrowed into", () => { + expect(SUGGESTIBLE_GEOMETRY_TYPES).toEqual(["bbox", "polygon"]); + }); + + it("accepts a box class and a polygon class", () => { + expect(isSuggestibleClass(CAR)).toBe(true); + expect(isSuggestibleClass(ROAD)).toBe(true); + }); + + it("refuses a lane and a tag — an open path and no coordinates at all", () => { + expect(isSuggestibleClass(LANE)).toBe(false); + expect(isSuggestibleClass(WEATHER)).toBe(false); + }); + + it("answers the allowed kinds as the class's own, and nothing else", () => { + expect(allowedGeometriesFor(CAR)).toEqual(["bbox"]); + expect(allowedGeometriesFor(ROAD)).toEqual(["polygon"]); + }); + + it("answers no allowed kinds for a class that can hold neither", () => { + expect(allowedGeometriesFor(LANE)).toEqual([]); + expect(allowedGeometriesFor(WEATHER)).toEqual([]); + }); + + it("says a lane-and-tag schema cannot reach the tool at all — D3's third case", () => { + expect(schemaCanSuggest(schemaOf(LANE, WEATHER))).toBe(false); + expect(suggestClassFor(schemaOf(LANE, WEATHER), "lane")).toBe(null); + }); + + it("keeps a held class that can already hold a suggestion", () => { + expect(suggestClassFor(schemaOf(CAR, ROAD), "road")).toBe("road"); + }); + + it("falls back to the first suggestible class when the held one is not", () => { + expect(suggestClassFor(schemaOf(LANE, ROAD, CAR), "lane")).toBe("road"); + expect(suggestClassFor(schemaOf(LANE, ROAD, CAR), null)).toBe("road"); + }); +}); + +describe("the preview lifecycle", () => { + it("arms with no points, nothing asked and nothing to take back", () => { + const state = armed("car"); + expect(state.points).toEqual([]); + expect(state.status).toBe("idle"); + expect(state.suggestion).toBe(null); + expect(hasPending(state)).toBe(false); + }); + + it("turns a click into an ask", () => { + const state = withPoint(armed("car"), [40, 50], "positive"); + expect(state.status).toBe("asking"); + expect(state.points).toEqual([{ point: [40, 50], polarity: "positive" }]); + expect(hasPending(state)).toBe(true); + }); + + it("shows the answer to that ask", () => { + const state = showing(); + expect(state.status).toBe("shown"); + expect(state.suggestion?.geometry).toEqual(A_BOX); + expect(isAcceptable(state)).toBe(true); + }); + + it("replaces the preview when a refine click is answered", () => { + const refined = withPoint(showing(), [200, 210], "negative"); + // The old shape stays up while the next answer is in flight — a refine that + // blanked the canvas would flicker on every press. + expect(refined.status).toBe("asking"); + expect(refined.suggestion?.geometry).toEqual(A_BOX); + + const next = answered(refined, refined.serial, proposal(A_POLYGON, 0.42)); + expect(next.status).toBe("shown"); + expect(next.suggestion?.geometry).toEqual(A_POLYGON); + expect(next.points).toHaveLength(2); + }); + + it("sends the accumulated points, split by what each one meant", () => { + const one = withPoint(armed("car"), [10, 10], "positive"); + const two = withPoint(one, [20, 20], "negative"); + const three = withPoint(two, [30, 30], "positive"); + expect(promptOf(three)).toEqual({ + positive: [ + [10, 10], + [30, 30], + ], + negative: [[20, 20]], + }); + }); + + it("treats an answer with nothing in it as an answer, not as an idle tool", () => { + const asked = withPoint(armed("car"), [1, 1], "positive"); + const none = answered(asked, asked.serial, null); + expect(none.status).toBe("none"); + expect(none.suggestion).toBe(null); + expect(isAcceptable(none)).toBe(false); + // Still pending, so Escape has something to take back — which is what tells + // "asked and got nothing" apart from "not asked". + expect(hasPending(none)).toBe(true); + }); + + it("holds the server's prose on a refusal and drops the stale preview", () => { + const asked = withPoint(showing(), [2, 2], "positive"); + const stopped = refused(asked, asked.serial, "The weights are not here yet."); + expect(stopped.status).toBe("refused"); + expect(stopped.refusal).toBe("The weights are not here yet."); + expect(stopped.suggestion).toBe(null); + expect(isAcceptable(stopped)).toBe(false); + }); + + it("clears a refusal when the next click retries", () => { + const asked = withPoint(armed("car"), [1, 1], "positive"); + const stopped = refused(asked, asked.serial, "Not here yet."); + expect(withPoint(stopped, [2, 2], "positive").refusal).toBe(null); + }); +}); + +describe("a late answer never wins", () => { + it("drops an answer that names a superseded ask, by identity", () => { + const first = withPoint(armed("car"), [10, 10], "positive"); + const second = withPoint(first, [20, 20], "positive"); + const late = answered(second, first.serial, proposal(A_POLYGON)); + expect(late).toBe(second); + }); + + it("drops a late refusal too", () => { + const first = withPoint(armed("car"), [10, 10], "positive"); + const second = withPoint(first, [20, 20], "positive"); + expect(refused(second, first.serial, "too late")).toBe(second); + }); + + it("keeps counting across a clear, so an in-flight answer cannot repaint", () => { + const asked = withPoint(armed("car"), [10, 10], "positive"); + const wiped = cleared(asked); + const next = withPoint(wiped, [20, 20], "positive"); + // The ask Escape interrupted still names serial 1; the fresh one names 2. + expect(answered(next, asked.serial, proposal())).toBe(next); + }); +}); + +describe("Escape is the preview's undo", () => { + it("takes the points and the preview back to an armed tool", () => { + const wiped = cleared(showing()); + expect(wiped.points).toEqual([]); + expect(wiped.status).toBe("idle"); + expect(wiped.suggestion).toBe(null); + expect(hasPending(wiped)).toBe(false); + // Armed, not off: the class survives so the next click starts a fresh + // session with the same label rather than disarming the tool. + expect(wiped.labelClass).toBe("car"); + }); +}); + +describe("acceptance", () => { + it("carries provenance, the model and the confidence", () => { + const accepted = acceptedAnnotation(documentOf(), showing(), () => "id-1"); + expect(accepted).not.toBe(null); + expect(accepted?.provenance).toBe("model"); + expect(accepted?.model_ref).toBe("facebook/sam2-hiera-base-plus@main"); + expect(accepted?.confidence).toBe(0.87); + expect(accepted?.label_class).toBe("car"); + expect(accepted?.geometry).toEqual(A_BOX); + }); + + it("carries a null confidence through rather than inventing one", () => { + const accepted = acceptedAnnotation( + documentOf(), + showing(proposal(A_BOX, null)), + () => "id-1", + ); + expect(accepted?.confidence).toBe(null); + expect(accepted?.provenance).toBe("model"); + }); + + it("seeds the class's attribute defaults, exactly as a drawn shape does", () => { + const withDefault: LabelClass = { + ...CAR, + attributes: [ + { name: "occluded", kind: "boolean", required: false, options: null, default: false }, + ], + }; + const accepted = acceptedAnnotation( + documentOf(schemaOf(withDefault)), + showing(), + () => "id-1", + ); + expect(accepted?.attributes).toEqual({ occluded: false }); + }); + + it("refuses to build anything from a session with no preview showing", () => { + const mint = (): string => "id-1"; + expect(acceptedAnnotation(documentOf(), armed("car"), mint)).toBe(null); + const asked = withPoint(armed("car"), [1, 1], "positive"); + expect(acceptedAnnotation(documentOf(), asked, mint)).toBe(null); + expect(acceptedAnnotation(documentOf(), answered(asked, asked.serial, null), mint)).toBe(null); + }); + + it("refuses when the schema no longer declares the session's class", () => { + expect(acceptedAnnotation(documentOf(schemaOf(ROAD)), showing(), () => "id-1")).toBe(null); + }); +}); + +describe("nothing about a pending suggestion is in the document or the history", () => { + it("leaves the store untouched through a whole click-refine-discard session", () => { + const store = new AnnotatorStore(documentOf()); + const before = store.document; + + let session = armed("car"); + session = withPoint(session, [10, 10], "positive"); + session = answered(session, session.serial, proposal()); + session = withPoint(session, [20, 20], "negative"); + session = answered(session, session.serial, proposal(A_POLYGON)); + expect(cleared(session).points).toEqual([]); + + expect(store.document).toBe(before); + expect(store.canUndo).toBe(false); + expect(store.canRedo).toBe(false); + expect(annotationsInDrawOrder(store.document)).toHaveLength(0); + }); + + /** + * **The mutation test for D4.** Turn `acceptedAnnotation`'s output into a + * command and the log moves by exactly one; the preview that produced it never + * did. A design that staged the preview into the store would make `canUndo` + * true before this line, and this is the assertion that would turn red. + */ + it("adds exactly one history entry when — and only when — it is accepted", () => { + const store = new AnnotatorStore(documentOf()); + const session = showing(); + expect(store.canUndo).toBe(false); + + const accepted = acceptedAnnotation(store.document, session, () => "id-1"); + expect(accepted).not.toBe(null); + store.execute(addAnnotationCommand(accepted!)); + + expect(store.canUndo).toBe(true); + expect(annotationsInDrawOrder(store.document)).toHaveLength(1); + + store.undo(); + expect(annotationsInDrawOrder(store.document)).toHaveLength(0); + expect(store.canUndo).toBe(false); + }); +}); diff --git a/frontend/annotator/src/core/interaction/suggestion.ts b/frontend/annotator/src/core/interaction/suggestion.ts new file mode 100644 index 00000000..f37ffbc5 --- /dev/null +++ b/frontend/annotator/src/core/interaction/suggestion.ts @@ -0,0 +1,329 @@ +/** + * The suggest gesture's state: points placed, an answer pending, a preview + * showing — and none of it in the document or in the command log (D4 on #424). + * + * ## Why this is beside `machine.ts` and not inside it + * + * `InteractionState` is what the *pointer* is in the middle of, and `machine.ts` + * types its table as total over that union: every state owes a row, and every row + * answers a pointer event synchronously. A suggestion is neither. It outlives the + * press that asked for it, it is resolved by a server rather than by a + * pointer-up, and it survives pointer-cancel, blur and a hundred pointer-moves. + * A variant there would have to answer eight events it has no opinion about, and + * the one event it *does* care about — an answer arriving — is not a pointer + * event at all. + * + * So it is a second, small state machine living beside the first, in the same + * directory for the same reason: interaction state that is not document state. + * `AnnotatorCanvas` holds `InteractionState` in a `useState` and this is held the + * same way, one layer further out — see "who holds it" below. + * + * ## Nothing here is in the undo stack, and that is structural rather than a rule + * + * The only way into `AnnotatorStore` is an `Effect`, and this module produces + * none. It cannot add, stage, replace or commit; it has no store, no + * `IdFactory` and no `Annotation`. Accepting is a separate, ordinary + * `draftAnnotation` + `add`, which is one history entry like any other drawn + * shape — `acceptedAnnotation` below is that one function, and it is the only + * thing here that has ever heard of the document. + * + * Escape is the preview's undo, and it is `cleared()`: a pure transition back to + * armed-with-nothing. `canUndo` never moves for a suggestion nobody accepted. + * + * ## Who holds it, and why not the adapter + * + * The **host** holds it. `AnnotatorCanvas` deliberately never fetches anything + * ("the adapter never fetches anything, which is the 'no HTTP' half of the + * embeddable contract"), and every transition here except `withPoint` is driven + * by a response. A canvas holding this state would need a channel to ask for the + * answer and a second to receive it, which is a host prop in both directions and + * an extra copy of the state in between. + * + * What the adapter does own is the two things it is uniquely able to do: turning + * a client position into an asset pixel, and drawing the preview. Both take the + * state as a prop. + * + * ## `serial`, and the answer that arrives too late + * + * Clicks refine, so a second click leaves while the first answer is still in + * flight — and a slow first answer landing after a fast second one would replace + * a three-point preview with a one-point one. Every ask stamps a serial, and + * `answered`/`refused` drop anything that does not name the current one. + * + * It is a plain counter rather than a timestamp because `Date.now` is a host + * global this package cannot name, and because a counter is what makes a test + * assert staleness by construction instead of by sleeping. + * + * ## What the server is asked, and what it is not + * + * The **accumulated** points go every time, never a diff: the route is stateless + * by design, so "the model already knows about my first click" is not a thing + * 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. + */ + +import { classNamed } from "../state/document"; +import type { AnnotationDocument } from "../state/document"; +import type { IdFactory } from "../ids"; +import type { Annotation, AnnotationSchema, Geometry, GeometryType, LabelClass, Point } from "../types"; +import { draftAnnotation } from "./draft"; + +/** + * The kinds a segmenter's answer can be narrowed into, and therefore the classes + * the tool is offered for (D3). + * + * A mask becomes an outline or its extent, and nothing else: a `polyline` is an + * open path, and answering one from a closed region would be inventing a lane + * out of a silhouette. A `classification_tag` has no coordinates at all. + * + * The server holds the same list from the other end — a region it cannot express + * in the kinds it was given comes back as `region: null` — so this is what stops + * the product asking a question whose only honest answer is "nothing". + */ +export const SUGGESTIBLE_GEOMETRY_TYPES = ["bbox", "polygon"] as const satisfies readonly GeometryType[]; + +/** One of the two kinds a suggestion can be answered in. */ +export type SuggestibleGeometryType = (typeof SUGGESTIBLE_GEOMETRY_TYPES)[number]; + +/** Whether a class can hold anything a segmenter is able to propose. */ +export function isSuggestibleClass(labelClass: LabelClass): boolean { + return (SUGGESTIBLE_GEOMETRY_TYPES as readonly string[]).includes(labelClass.geometry); +} + +/** + * The kinds the answer may come back in, for the class a suggestion will carry. + * + * A **list of one** for every class this build has, because `LabelClass.geometry` + * is singular — `types.ts`: *"`geometry` is singular, and that is the rule an + * annotator is built around"*. It is still a list, because that is the shape the + * route takes and because the day a class declares a set, this function is the + * only thing that changes. + * + * Empty for a class that can hold neither, which is the same fact + * `isSuggestibleClass` reports and the reason the tool is not offered there. + */ +export function allowedGeometriesFor( + labelClass: LabelClass, +): readonly SuggestibleGeometryType[] { + return isSuggestibleClass(labelClass) ? [labelClass.geometry as SuggestibleGeometryType] : []; +} + +/** + * The class a press on the suggest tool should arm, or `null` when this schema + * has none. + * + * The tool strip's own rule, applied: a press activates the class that derives + * the tool asked for, and a press whose tool is already reachable moves nothing. + * So an active class that can already hold a suggestion is kept — swapping it for + * the schema's first suggestible class would silently change what the next shape + * is labelled, which is `ToolPalette`'s consequence (1) exactly. + */ +export function suggestClassFor( + schema: AnnotationSchema, + activeClass: string | null, +): string | null { + const held = schema.classes.find((declared) => declared.name === activeClass); + if (held !== undefined && isSuggestibleClass(held)) return held.name; + return schema.classes.find(isSuggestibleClass)?.name ?? null; +} + +/** Whether this schema can reach the tool at all — what the strip button asks. */ +export function schemaCanSuggest(schema: AnnotationSchema): boolean { + return schema.classes.some(isSuggestibleClass); +} + +/** What a click told the model: this is the thing, or this is not the thing. */ +export type Polarity = "positive" | "negative"; + +/** One click, in asset pixels, and what it meant. */ +export interface PromptPoint { + readonly point: Point; + readonly polarity: Polarity; +} + +/** What the model proposed, already narrowed to a kind the class can hold. */ +export interface Suggestion { + readonly geometry: Geometry; + /** `null` when the model reported none — a real answer, not a missing field. */ + readonly confidence: number | null; + /** The model that proposed it, carried onto the annotation if it is accepted. */ + readonly modelRef: string; +} + +/** + * Where the session is. + * + * Five, and each is a different thing on screen: `idle` is an armed tool nobody + * has clicked with, `asking` is a request in flight, `shown` has a preview, + * `none` is a successful answer with nothing to propose, and `refused` is a + * server refusal with prose to render. + * + * `none` is deliberately not folded into `idle`. They differ in exactly the way a + * user cares about: one has been asked and answered, the other has not been + * asked. Folding them would make a click on a patch of sky look like a click that + * never happened. + */ +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; + /** Every click so far, in the order they were placed. */ + readonly points: readonly PromptPoint[]; + readonly status: SuggestionStatus; + /** The preview, when there is one. Kept across `asking` so it does not flicker. */ + readonly suggestion: Suggestion | null; + /** What the server refused with, in prose. Non-null only while `refused`. */ + readonly refusal: string | null; + /** Which ask the state is waiting on — see the module note on staleness. */ + readonly serial: number; +} + +/** What the route is sent: the accumulated points, split by what they meant. */ +export interface Prompt { + readonly positive: readonly Point[]; + readonly negative: readonly Point[]; +} + +/** A freshly armed session: this class, no points, nothing asked. */ +export function armed(labelClass: string): SuggestionState { + return { + labelClass, + points: [], + status: "idle", + suggestion: null, + refusal: null, + serial: 0, + }; +} + +/** + * A click: one more point, and a new ask. + * + * The previous `suggestion` is **kept** while the next answer is in flight. A + * refine click that blanked the canvas and then repainted it would flicker on + * every press, and the shape on screen is still the best answer anyone has until + * a better one arrives. + * + * A previous refusal is dropped, because it was about the ask that is now + * superseded. A person who clicks again is retrying, and leaving the old sentence + * up would make the retry look like it had failed too. + */ +export function withPoint( + state: SuggestionState, + point: Point, + polarity: Polarity, +): SuggestionState { + return { + ...state, + points: [...state.points, { point, polarity }], + status: "asking", + refusal: null, + serial: state.serial + 1, + }; +} + +/** The points as the route wants them: two lists, each in placement order. */ +export function promptOf(state: SuggestionState): Prompt { + const positive: Point[] = []; + const negative: Point[] = []; + for (const placed of state.points) { + (placed.polarity === "positive" ? positive : negative).push(placed.point); + } + return { positive, negative }; +} + +/** + * An answer arrived. `null` is a successful answer with nothing to propose. + * + * A `serial` that is not the one being waited on is **dropped whole**, state + * returned by identity — so a caller can compare with `toBe` and a slow first + * answer cannot overwrite a fast second one. + */ +export function answered( + state: SuggestionState, + serial: number, + suggestion: Suggestion | null, +): SuggestionState { + if (serial !== state.serial) return state; + if (suggestion === null) { + return { ...state, status: "none", suggestion: null, refusal: null }; + } + return { ...state, status: "shown", suggestion, refusal: null }; +} + +/** + * The ask refused. The prose is the server's; this module invents no sentences. + * + * The stale preview goes with it. A refusal beside a shape from two clicks ago + * would invite somebody to accept a suggestion the points on screen no longer + * describe. + */ +export function refused( + state: SuggestionState, + serial: number, + prose: string, +): SuggestionState { + if (serial !== state.serial) return state; + return { ...state, status: "refused", suggestion: null, refusal: prose }; +} + +/** + * Escape: the preview's undo. + * + * Back to armed-with-nothing rather than off, and the serial **keeps counting**. + * A cleared session is still a session, and an answer to the ask that was in + * flight when Escape was pressed must not be able to repaint the preview that was + * just discarded — which is exactly what resetting the serial to zero would + * allow the next click to do. + */ +export function cleared(state: SuggestionState): SuggestionState { + return { ...state, points: [], status: "idle", suggestion: null, refusal: null }; +} + +/** Whether there is anything for Escape to take back. */ +export function hasPending(state: SuggestionState): boolean { + return state.points.length > 0 || state.status !== "idle"; +} + +/** Whether Enter would commit something. Only a shown suggestion can be accepted. */ +export function isAcceptable(state: SuggestionState): boolean { + return state.status === "shown" && state.suggestion !== null; +} + +/** + * The annotation an accepted suggestion becomes: an ordinary drawn shape, + * carrying where it came from. + * + * Built on `draftAnnotation` rather than beside it, so a suggestion inherits the + * class's attribute defaults, the document's asset and the provisional + * `schema_version` exactly as a hand-drawn shape does — and so the two cannot + * come to disagree about what a new annotation is. What it overrides is the three + * fields that make it a model's proposal a human accepted (D4): `provenance`, + * `model_ref` and `confidence`. + * + * `draftAnnotation` keeps its own `provenance: "human"` and its docstring's claim + * that *"every caller is a gesture or a keystroke"* stays true: this is a + * keystroke, and the acceptance is the human act the provenance records. + * + * `null` when there is nothing to accept, so a caller cannot commit a preview + * that is not showing. The class is looked up only to fail honestly: a schema + * that lost the class mid-session has nothing to write. + */ +export function acceptedAnnotation( + document: AnnotationDocument, + state: SuggestionState, + mint: IdFactory, +): Annotation | null { + if (!isAcceptable(state) || state.suggestion === null) return null; + if (classNamed(document, state.labelClass) === undefined) return null; + const drawn = draftAnnotation(document, state.labelClass, state.suggestion.geometry, mint); + return { + ...drawn, + provenance: "model", + model_ref: state.suggestion.modelRef, + confidence: state.suggestion.confidence, + }; +} diff --git a/frontend/annotator/src/index.ts b/frontend/annotator/src/index.ts index 7a957711..04e449c9 100644 --- a/frontend/annotator/src/index.ts +++ b/frontend/annotator/src/index.ts @@ -145,6 +145,31 @@ export { type Cursor, } from "./core/interaction/affordance"; export { draftAnnotation } from "./core/interaction/draft"; +// The suggest session (#424) — ephemeral by construction: no effects, no store, +// nothing in the command log until `acceptedAnnotation` is added like any shape +export { + SUGGESTIBLE_GEOMETRY_TYPES, + acceptedAnnotation, + allowedGeometriesFor, + answered, + armed, + cleared, + hasPending, + isAcceptable, + isSuggestibleClass, + promptOf, + refused, + schemaCanSuggest, + suggestClassFor, + withPoint, + type Polarity, + type Prompt, + type PromptPoint, + type Suggestion, + type SuggestibleGeometryType, + type SuggestionState, + type SuggestionStatus, +} from "./core/interaction/suggestion"; export { TRANSITIONS, transition, @@ -172,8 +197,10 @@ export { } from "./core/interaction/tags"; // The input layer (#46) — a press, a chord, the map, and what carries a row out export { + ACCEPT_SUGGESTION, CLASS_HOTKEY_DIGITS, DEFAULT_BINDINGS, + DISCARD_SUGGESTION, FOCUS_CLASS_FIELD, READ_ONLY_KINDS, RESET_ZOOM, @@ -181,6 +208,7 @@ export { SAVE_AND_NEXT, SKIP_FRAME, TOGGLE_HELP, + TOGGLE_SUGGEST, chordOf, classAction, classHotkeys, @@ -241,17 +269,21 @@ export { PolygonShape, SELECTED_STROKE_PX, STROKE_PX, + SUGGESTION_DASH, + SUGGESTION_OPACITY, ShapeLabel, TransientLayer, VERTEX_PX, Vertices, classColor, + confidenceLabel, digitFromCode, editedId, isComposing, isTextEntry, paintAnnotation, paintDocument, + paintSuggestion, pendingPolygon, rubberBand, screenPx, @@ -262,6 +294,7 @@ export { type AnnotatorView, type CompositionProbe, type PaintedAnnotation, + type PaintedSuggestion, type PendingPolygon, type TextEntryProbe, type TransientLayerProps, diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index e60c9266..302f9aec 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -73,7 +73,9 @@ */ import { + ACCEPT_SUGGESTION, AnnotatorCanvas, + DISCARD_SUGGESTION, MAX_ZOOM, MIN_ZOOM, FOCUS_CLASS_FIELD, @@ -81,18 +83,36 @@ import { SAVE_AND_NEXT, SKIP_FRAME, TOGGLE_HELP, + TOGGLE_SUGGEST, + acceptedAnnotation, + addAnnotationCommand, + allowedGeometriesFor, + answered, + armed, atZoomCeiling, atZoomFloor, + cleared, createClipboard, defaultRegistry, annotationsInDrawOrder, documentFromWire, + hasPending, + parseGeometry, + promptOf, + randomUuid, + refused, selectOnly, + suggestClassFor, toolFor, useAnnotatorSnapshot, + withPoint, type AnnotatorStore, type AnnotatorView, type Clipboard, + type Point, + type Polarity, + type Suggestion, + type SuggestionState, type Viewport, } from "@visionset/annotator"; import { AnnotatorStore as Store } from "@visionset/annotator"; @@ -174,6 +194,8 @@ import { } from "./jobQueries"; import { AddClassDialog, runAddClass } from "./AddClassDialog"; import { FrameGallery } from "./FrameGallery"; +import { SuggestPanel } from "./SuggestPanel"; +import { useInferenceConnections, useSuggestRegion, usableConnection } from "./inferenceQueries"; import { PROGRESS_LABEL, outstandingWork, progressDotClass, progressTone } from "../screens/batchState"; import type { LabelClassBody, SchemaDiff, SchemaVersion } from "../screens/queries"; import { @@ -227,6 +249,38 @@ export const REVIEW_ACTIONS: readonly { /** One notch, matching what a wheel step feels like on the same stage. */ const ZOOM_STEP = 1.25; +/** + * The wire's suggestion as the engine's, or `null` for an answer with nothing in + * it (#424). + * + * `parseGeometry` rather than a cast: it is the annotator's own *"unknown in, + * typed out"* door, and a suggestion arrives on the same wire an annotation does. + * A shape this build cannot read is treated as no suggestion rather than crashing + * a render — the same call `paintAnnotation` makes when the document moves under + * it — because the alternative is a `WireFormatError` thrown out of a mutation + * callback, where nothing is listening. + * + * `region` is optional in the generated type (the field carries a default), so + * `?? null` is what turns "absent" and "explicitly null" into the one answer they + * both are. + */ +function readSuggestion(answer: { + readonly model_ref: string; + readonly region?: { readonly geometry: unknown; readonly confidence: number | null } | null; +}): Suggestion | null { + const region = answer.region ?? null; + if (region === null) return null; + try { + return { + geometry: parseGeometry(region.geometry), + confidence: region.confidence, + modelRef: answer.model_ref, + }; + } catch { + return null; + } +} + /** * A hotkey on a button, in the spelling the shortcut sheet uses. * @@ -314,6 +368,16 @@ export interface AnnotationPageProps { * *invisibly*, leaving the address bar naming a frame nobody was looking at. */ readonly onAssetChange?: (assetId: string) => void; + /** + * Where somebody goes to set up a model connection, if the app has such a + * screen (#424, D6). + * + * Optional, and it is expected to be absent for now: the Inference surface + * waits on #421's open rail question, and `ui-core` imports no router. Absent, + * the suggest tool's panel still says what is missing and simply renders no + * control — a host that cannot honour one renders none rather than a dead one. + */ + readonly onConfigureInference?: () => void; } export function AnnotationPage(props: AnnotationPageProps): JSX.Element { @@ -384,6 +448,7 @@ function JobScreen({ initialAssetId, onOpenGallery, onAssetChange, + onConfigureInference, }: AnnotationPageProps): JSX.Element { const job = useJob(jobId); const batch = useBatchOf(job.data?.batch_id); @@ -522,6 +587,7 @@ function JobScreen({ activeClass={activeClass} onActivateClass={activateClass} onNavigate={setChosen} + {...(onConfigureInference === undefined ? {} : { onConfigureInference })} {...(onOpenGallery === undefined ? {} : { @@ -596,6 +662,8 @@ interface WorkspaceProps { readonly onActivateClass: (labelClass: string | null) => void; readonly onNavigate: (index: number) => void; readonly onOpenGallery?: () => void; + /** #424's D6 destination, if the host has one. See `AnnotationPageProps`. */ + readonly onConfigureInference?: () => void; } /** @@ -639,6 +707,7 @@ function Workspace({ onActivateClass: activateClass, onNavigate, onOpenGallery, + onConfigureInference, }: WorkspaceProps): JSX.Element { const store = useMemo( () => @@ -696,6 +765,146 @@ function Workspace({ */ const [stage, setStage] = useState(null); + /** + * The suggest session (#424, D4) — **here, and outside the store on purpose.** + * + * The whole of ephemerality is where this lives. `AnnotatorStore` is the + * document and its history; a pending suggestion is neither, so it is held as + * ordinary component state beside `activeClass` and `hiddenIds`. Nothing stages + * it, nothing commits it, and `canUndo` cannot move for it — accepting is a + * separate `addAnnotationCommand` like any drawn shape, and Escape is the + * preview's undo. + * + * In `Workspace` rather than in `JobScreen`, which is the opposite call from + * `activeClass` and `clipboard` — and the difference is exactly what those two + * are for. They live one level up so they *survive* the per-asset remount; a + * suggestion must not. D2 says switching assets discards, and the `key={asset.id}` + * remount is that rule enforced by construction rather than by an effect + * somebody has to remember to write. + */ + const [session, setSession] = useState(null); + + /** + * The connection list, fetched **only once the tool is armed**. + * + * A job nobody suggests on makes no inference request at all, which is the same + * discipline `useActiveSchema` follows two blocks down: a read that only one + * surface needs is enabled by that surface. The cost is a moment where the + * answer is not known yet, and `usableConnection` names it (`checking`) rather + * than leaving a click to vanish into it. + */ + const connections = useInferenceConnections(session !== null); + const { connection, blocker } = usableConnection(connections.data); + const suggestRegion = useSuggestRegion(); + + /** + * Arming and disarming — and arming activates a class, exactly as every other + * button on the strip does. + * + * `suggestClassFor` keeps a held class that can already hold a suggestion and + * otherwise moves to the schema's first one, which is `ToolPalette`'s own rule: + * a press moves the active class to one that derives the tool asked for, and a + * press that would change *which* class without changing the tool changes + * nothing. + */ + function toggleSuggest(): void { + if (readOnly) return; + if (session !== null) { + setSession(null); + return; + } + const labelClass = suggestClassFor(store.document.schema, activeClass); + if (labelClass === null) return; + activateClass(labelClass); + setSession(armed(labelClass)); + } + + /** + * A click on the canvas while the tool is armed: one more point, one more ask. + * + * The **accumulated** points go every time — the route is stateless and says + * so — and the serial the transition stamped is captured here so a slow first + * answer cannot overwrite a fast second one. Both callbacks fold through + * `setSession`'s updater rather than through the closed-over `session`, because + * by the time an answer lands the session has usually moved. + */ + function suggestAt(point: Point, polarity: Polarity): void { + if (session === null || connection === null) return; + const declared = store.document.schema.classes.find( + (candidate) => candidate.name === session.labelClass, + ); + if (declared === undefined) return; + + const next = withPoint(session, point, polarity); + setSession(next); + const asked = next.serial; + const prompt = promptOf(next); + suggestRegion.mutate( + { + projectId, + assetId: asset.id, + connectionId: connection.id, + positive: prompt.positive, + negative: prompt.negative, + allowedGeometries: allowedGeometriesFor(declared), + }, + { + onSuccess: (answer) => { + setSession((live) => + live === null ? live : answered(live, asked, readSuggestion(answer)), + ); + }, + onError: (error: unknown) => { + setSession((live) => + live === null ? live : refused(live, asked, refusalProse(error)), + ); + }, + }, + ); + } + + /** + * Accept: one ordinary annotation, one history entry, `provenance: model`. + * + * Through `addAnnotationCommand` — the same command a finished draw produces — + * so the write path, the undo step and the save diff are all the ones that + * already exist. The frame enters at `annotated` through the normal settle, + * which is the Decision (Armando, 2026-08-07) on #424: an interactively + * accepted suggestion is not a *silent* write, so #418's `review_pending` + * constraint governs unattended batch prediction and not this. + * + * The session is cleared rather than disarmed: somebody who accepted one shape + * is usually about to click the next thing. + */ + function acceptSuggestion(): void { + if (session === null || readOnly) return; + const drawn = acceptedAnnotation(store.document, session, randomUuid); + if (drawn === null) return; + store.execute(addAnnotationCommand(drawn)); + store.select(selectOnly(drawn.id)); + setSession(cleared(session)); + } + + /** Escape: clear what is pending, or — with nothing pending — put the tool away. */ + function discardSuggestion(): void { + if (session === null) return; + setSession(hasPending(session) ? cleared(session) : null); + } + + /** + * Switching tools discards (D2), and *switching tools* here means the active + * class moving off the one the session captured. + * + * The strip's other buttons, the panel's list and every digit hotkey all end at + * `activateClass`, so this one effect covers all of them — where a handler on + * each would be four places to add the fifth door to. Arming is unaffected: it + * activates the class first and opens the session with the same name, so the + * two agree by the time this runs. + */ + useEffect(() => { + setSession((live) => (live === null || live.labelClass === activeClass ? live : null)); + }, [activeClass]); + /** * The one capability the canvas hands out rather than owning (#189). * @@ -740,6 +949,23 @@ function Workspace({ if (declares(asset, ASSET_ACTION.skip) && !setProgress.isPending) settle("skipped"); return true; } + // `s` (#424). Claimed even where it does nothing — a read-only frame still + // has to swallow the chord rather than let a bare letter reach the page + // around the canvas, which is why the registry claims it at all. + if (name === TOGGLE_SUGGEST) { + toggleSuggest(); + return true; + } + // `↵` and `Esc`, substituted by the adapter only while a session is live, so + // neither reaches here unless there is something to accept or take back. + if (name === ACCEPT_SUGGESTION) { + acceptSuggestion(); + return true; + } + if (name === DISCARD_SUGGESTION) { + discardSuggestion(); + return true; + } return false; } @@ -1069,6 +1295,17 @@ function Workspace({ const canAnnotate = declares(asset, ASSET_ACTION.annotate); const readOnly = !canAnnotate; + /** + * The session, gated on the mode rather than torn down by an effect (#424). + * + * A frame that becomes a viewer under somebody — `ui-capabilities`: *"read-only + * is a transition, not only an entry state"* — must not keep a preview offering + * a write that is now refused. Deriving it here rather than clearing the state + * means the mode arrives in place, on the same render, with no `setState` + * mirror of the rule to keep in step. + */ + const suggesting = readOnly ? null : session; + /** * Why it is read-only, in the words a person can act on. * @@ -1991,6 +2228,11 @@ function Workspace({ store.select(selectOnly(annotationId)); setReclassing(annotationId); }} + // The suggest mode (#424). Its presence diverts every primary + // press away from the interaction machine, which is what stops a + // click meant for the model from drawing a box instead. + suggestion={suggesting} + onSuggestPoint={suggestAt} /> )} @@ -2048,6 +2290,33 @@ function Workspace({ onUndo: () => store.undo(), onRedo: () => store.redo(), }} + // #424. The strip hides it on a schema no class of which could + // hold the answer; this page offers it because it has an API + // behind it, which the showcase does not. + suggest={{ active: suggesting !== null, onToggle: toggleSuggest }} + /> + )} + + {/* + The suggest tool's own voice (#424, D6) — a sibling of the canvas for + `ToolPalette`'s reason, and in the one corner the editor does not + already occupy. + + Rendered for the whole session rather than only for its refusals: the + asking state, the found-nothing state and the accept affordance are + the same question answered differently, and scattering them would + leave a person assembling one answer from three places. + */} + {suggesting !== null && ( + )} diff --git a/frontend/ui-core/src/annotator/SuggestPanel.tsx b/frontend/ui-core/src/annotator/SuggestPanel.tsx new file mode 100644 index 00000000..30944c4f --- /dev/null +++ b/frontend/ui-core/src/annotator/SuggestPanel.tsx @@ -0,0 +1,307 @@ +/** + * The suggest tool's one voice: what it is doing, or why it cannot (#424, D6). + * + * ## An in-editor panel, and never a navigation + * + * `DESIGN.md` principle 10 — *the annotation workspace is self-sufficient*, + * ratified 2026-08-05 and marked immovable — is the whole shape of this + * component. Somebody who arms a tool over an unconfigured workspace must be + * told, on the canvas, without leaving work behind. So this is a card floating + * over the picture: not a toast (which disappears while somebody is reading it), + * not a redirect (which loses the frame), and not a modal (which stops the + * gesture the page exists for). + * + * ## One panel for five states, because they are one question + * + * "What is the suggest tool doing" has five honest answers, and each of them is a + * sentence somewhere on this card: waiting for a click, asking, showing something + * to accept, having found nothing, or refusing. The alternative — a spinner in + * one corner, an error surface in another, an empty state somewhere else — + * scatters one answer across three places and leaves the person to assemble it. + * + * Principle 9, *never disable without explanation*, is what makes the refusal + * cases carry a remedy rather than a state: "not configured" names the thing to + * make, "not ready" names the download, and a server refusal is quoted **as the + * server wrote it**, because those messages carry the exact install command + * (`_extra.py`: *"the message is the remedy"*). + * + * ## The action is a callback, and its absence renders nothing + * + * `ui-core` imports no router — `information-architecture` states it — so where + * "set one up" goes is the host's. A host that has nowhere to send somebody + * passes no callback and gets the explanation with **no control at all**, which + * is `onOpenGallery`'s established rule: a host that cannot honour a control + * renders no control rather than a dead one. The sentence is useful on its own, + * and a button that did nothing would be worse than no button. + */ + +import { + isAcceptable, + hasPending, + type SuggestionState, +} from "@visionset/annotator"; +import { Check, Loader2, Sparkles, TriangleAlert, X } from "lucide-react"; +import type { JSX, ReactNode } from "react"; + +import { Button } from "../primitives/Button"; +import type { SuggestBlocker } from "./inferenceQueries"; + +export interface SuggestPanelProps { + /** The session, whose status decides which sentence this card carries. */ + readonly session: SuggestionState; + /** + * Why the tool cannot run at all, from the connection list — or `null` when it + * can, and `undefined` while the list is still loading. + */ + readonly blocker: SuggestBlocker | null | undefined; + /** + * The server's own words for a refusal, when one happened. + * + * Quoted rather than restated: `LOCAL_INFERENCE_UNAVAILABLE` and + * `INFERENCE_CONNECTION_NOT_RUNNABLE` are both `expose_message=True` precisely + * so the install command reaches a person, and a sentence written here would + * throw that away. `refusalProse` is what turns the rest into prose. + */ + readonly refusal: string | null; + /** Where a person goes to make or finish a connection, if the host has one. */ + readonly onConfigure?: () => void; + readonly onAccept: () => void; + readonly onDiscard: () => void; +} + +/** + * What the two blockers say, and what each one's action means. + * + * A record rather than a ternary chain so the pair is readable as a table and a + * third blocker cannot be added to the copy without an entry — the same reason + * `ToolPalette`'s `TOOL_LABELS` is total over what `drawableGeometry` answers. + */ +const BLOCKER_COPY: Readonly< + Record< + SuggestBlocker, + { + readonly title: string; + readonly body: string; + /** What the way out is called, or `null` where there is nothing to press. */ + readonly action: string | null; + readonly tone: "calm" | "warn"; + } + > +> = { + // Not a warning: nothing is wrong, the answer is simply not back. A red card + // for a request in flight would teach somebody to distrust a working tool. + checking: { + title: "Getting the model ready…", + body: "Checking which model connection this workspace can suggest through.", + action: null, + tone: "calm", + }, + "no-connections": { + title: "No model connection yet", + body: "Suggesting a shape runs a segmentation model through a connection, and this workspace has none configured. Set one up once and every job can use it.", + action: "Set up a connection", + tone: "warn", + }, + "not-ready": { + title: "The model is not downloaded yet", + body: "This workspace has a connection, but its weights are not on this machine. Downloading them is a one-time step and runs in the background.", + action: "Finish setting it up", + tone: "warn", + }, +}; + +export function SuggestPanel({ + session, + blocker, + refusal, + onConfigure, + onAccept, + onDiscard, +}: SuggestPanelProps): JSX.Element { + // The blocker outranks everything: a session over a workspace with no usable + // connection has nothing to report about a request it never made. + if (blocker !== null && blocker !== undefined) { + const copy = BLOCKER_COPY[blocker]; + return ( + + ) : ( + + ) + } + > +

+ {copy.title} +

+

{copy.body}

+ {/* The action's *destination* is the host's, so its absence removes the + control and leaves the explanation — never a dead button. */} + {copy.action !== null && onConfigure !== undefined && ( + + )} +
+ ); + } + + if (session.status === "refused") { + return ( + }> +

That suggestion could not be made

+ {/* The server's sentence, verbatim. It is the one that carries the + install command when the cause is a missing extra. */} +

+ {refusal ?? session.refusal} +

+

+ Your clicks are still here — press Esc to clear them, or click again to retry. +

+
+ ); + } + + if (session.status === "asking") { + return ( + }> +

+ Looking at that… +

+ {/* The route's own note, said where it matters: the first click on a frame + pays for reading the whole image and every later one is nearly free. */} +

+ The first click on a frame is the slow one — refining after it is quick. +

+
+ ); + } + + if (session.status === "none") { + return ( + }> +

+ Nothing to suggest there +

+

+ Click nearer the middle of the object, or add another point. Alt-click marks + something that is not part of it. +

+ +
+ ); + } + + if (isAcceptable(session)) { + return ( + }> +

+ A shape for “{session.labelClass}” +

+

+ Click again to refine it — alt-click to take a part away. +

+
+ + +
+
+ ); + } + + return ( + }> +

+ Click the thing you want +

+

+ One click proposes a shape for “{session.labelClass}”. Alt-click marks something + that is not part of it. +

+ {hasPending(session) && } +
+ ); +} + +/** The take-back, where a state has something to take back and nothing to accept. */ +function Discard({ onDiscard }: { readonly onDiscard: () => void }): JSX.Element { + return ( + + ); +} + +/** `AnnotationPage`'s chord chip, at this card's scale. Visual only. */ +function Chip({ children }: { readonly children: ReactNode }): JSX.Element { + return ( + + {children} + + ); +} + +/** + * The card itself: bottom-right of the stage, clear of the tool strip. + * + * Bottom-**right** rather than beside the strip, because the strip is top-left + * and the object counter is bottom-left: this is the one corner the editor does + * not already occupy, and a panel that covered the tools would hide the button + * that arms it. + */ +function Card({ + testId, + tone, + icon, + children, +}: { + readonly testId: string; + readonly tone: "calm" | "warn"; + readonly icon: ReactNode; + readonly children: ReactNode; +}): JSX.Element { + return ( +
+ +
{children}
+
+ ); +} diff --git a/frontend/ui-core/src/annotator/ToolPalette.tsx b/frontend/ui-core/src/annotator/ToolPalette.tsx index bae6b3fc..39933c98 100644 --- a/frontend/ui-core/src/annotator/ToolPalette.tsx +++ b/frontend/ui-core/src/annotator/ToolPalette.tsx @@ -43,6 +43,21 @@ * is disabled-with-reason, never absent* — is the part worth keeping, and `mask` * and `keypoints` are still in that position the day a schema declares one. * + * ## The suggest button is the one control here that is a mode (#424) + * + * Every other button on this strip *reports* a derived tool and moves the active + * class to derive it. Suggest cannot: it is not a `Tool`, because `tool.ts` + * derives the tool from the class and there is no class that means "ask a model". + * So it is a mode held beside the class — the host arms it, the host lights it — + * and it arrives as its own prop rather than as a row in `toolChoices`, so that + * the list stays exactly what its type says it is. + * + * It is **absent** rather than disabled on a schema whose classes can hold + * neither a box nor a polygon (D3), which is the same rule that keeps a + * `classification_tag` off the strip: a disabled control has to be explicable in + * principle 9's terms, and "no class in this project could accept the answer" is + * a fact about the schema rather than a capability that is coming. + * * ## Why this is a second component rather than the showcase's, moved * * `@visionset/app`'s `demo/ToolStrip.tsx` is the same rule with inline styles from @@ -66,6 +81,7 @@ import { drawableGeometry, hotkeyForClass, + schemaCanSuggest, type AnnotationSchema, type Tool, } from "@visionset/annotator"; @@ -74,6 +90,7 @@ import { MousePointer2, Plus, Redo2, + Sparkles, Spline, Square, Undo2, @@ -179,6 +196,29 @@ export interface ToolPaletteProps { readonly tool: Tool; readonly onActivateClass: (labelClass: string | null) => void; readonly onToggleHelp: () => void; + /** + * The suggest tool (#424), or absent where the host cannot serve one. + * + * A prop rather than a row in `toolChoices`, because it is not a `Tool` and + * must not be made to look like one: `tool.ts` derives the tool from the active + * class and stores nothing, while suggest is a *mode* held beside the class it + * borrows. Folding it into the list would put a stored mode in a derived one's + * row and force `toolFor` to answer for something it cannot see. + * + * **Hidden, not disabled, when this schema declares no class that can hold a + * suggestion** (D3's third case) — the strip's own rule for a tool the schema + * cannot reach, and the same mechanism that keeps a `classification_tag` off it. + * A disabled sparkle over a tag-only schema would be promising a capability + * that does not apply to this project rather than one that is coming. + * + * Absent as a whole for the `onOpenGallery` reason: a host with no API behind + * it — the showcase — renders no control rather than a dead one. + */ + readonly suggest?: { + /** Whether the tool is armed. Held by the host, like the active class. */ + readonly active: boolean; + readonly onToggle: () => void; + }; /** * Open the add-a-class dialog (#233), or absent where there is nowhere to add * one — the demo has no project behind it. The `onOpenGallery` rule: a host @@ -211,6 +251,7 @@ export function ToolPalette({ onToggleHelp, onAddClass, history, + suggest, }: ToolPaletteProps): JSX.Element { /** * The canvas keeps the focus. @@ -250,6 +291,21 @@ export function ToolPalette({ ))} + {/* After the drawing tools and before the `+`, because it is a way of + drawing rather than a way of managing the schema — and a mode, so it is + the one control here whose `active` is not `tool === choice.tool`. */} + {suggest !== undefined && schemaCanSuggest(schema) && ( + + + + )} + {/* Beside the tools, because "the class I need is not here" is a thought somebody has while looking at this strip — and a digit hotkey for the new class arrives free, since the palette *is* the hotkey order (#46). */} diff --git a/frontend/ui-core/src/annotator/inferenceQueries.ts b/frontend/ui-core/src/annotator/inferenceQueries.ts new file mode 100644 index 00000000..e2f08a28 --- /dev/null +++ b/frontend/ui-core/src/annotator/inferenceQueries.ts @@ -0,0 +1,163 @@ +/** + * The two calls the suggest tool makes: which connections exist, and what the + * model proposes for a click (#424, slice 3b). + * + * ## Why the connection list is a *read* the editor does + * + * D5 puts the model behind a connection and D6 says the editor must explain + * itself when there is no usable one — and "usable" is three different states + * (`setup_state`, and whether any row exists at all) that only the list can + * answer. So the panel's copy is derived from the same read the request uses to + * pick a connection, rather than from a refusal discovered after the click. + * + * The list is **workspace-scoped**, not project-scoped: `/inference/connections` + * takes no project, because a model is a machine's capability and not a + * project's property. That is why the key below sits at the root. + * + * ## Which connection, and the limit that is stated rather than hidden + * + * The **first `ready` one, in the list's own order**. There is no picker, because + * the surface that would hold one is #421's and waits on its open rail question — + * so this is a deliberate limit rather than a design: a workspace with two ready + * connections always suggests through the older of them, and choosing is what + * #421 adds. + * + * It is not a hand-mirrored capability table. `setup_state` is the wire's own + * field and this reads it; the *legality* of the call is the server's answer, and + * a connection this picks that the server then refuses renders its refusal like + * any other. `ui-capabilities`' rule is that the client may not compute what is + * legal, and picking which of several offered rows to send is not that. + * + * ## The suggestion is a mutation, and it writes nothing + * + * `useMutation` over `useQuery` for a POST that has no cache to hold: the same + * points sent twice answer the same way, but the ask is an *event* — somebody + * clicked — and caching it would make a refine click on the same pixel silently + * skip the request. Nothing here invalidates anything, because nothing is + * written: acceptance is a separate, ordinary annotation create. + */ + +import type { GeometryType } from "@visionset/annotator"; +import { useMutation, useQuery, type UseQueryResult } from "@tanstack/react-query"; + +import { useApiClient } from "../data/ApiProvider"; +import { unwrap } from "../data/errors"; +import { checkListInferenceConnections, checkSuggestRegion } from "../generated/checks"; +import type { components } from "../generated/api"; + +export type Connection = components["schemas"]["ConnectionOut"]; + +/** + * The suggest route's answer, declared structurally rather than imported. + * + * `WireAnnotation`'s precedent, one route along and for the identical reason: the + * spec spells a polygon's points with `prefixItems`, `openapi-typescript` honours + * it as a **tuple**, and the value `openapi-fetch` hands back widens to + * `number[][]`. The two are the same JSON and TypeScript will not unify them, so + * naming the shape this module actually reads is the honest answer. + * + * `geometry` stays `unknown` on top of that, because it is about to go through + * `parseGeometry` — the annotator's *"unknown in, typed out"* door — and a type + * the caller then re-narrows anyway would be a second mirror of a kernel shape, + * which `annotator-core` forbids in so many words. + */ +export interface SuggestedRegion { + readonly geometry: unknown; + readonly confidence: number | null; +} + +export interface SuggestionOut { + readonly model_ref: string; + readonly region?: SuggestedRegion | null; +} + +export const inferenceKeys = { + connections: () => ["inference", "connections"] as const, +}; + +/** + * Every connection this workspace has, `ready` or not. + * + * Unfiltered on purpose: "none configured" and "one configured but its weights + * are not here" are different sentences with different remedies, and a filtered + * list would make them look identical. + */ +export function useInferenceConnections( + enabled = true, +): UseQueryResult { + const client = useApiClient(); + return useQuery({ + queryKey: inferenceKeys.connections(), + enabled, + queryFn: async () => + unwrap(await client.GET("/inference/connections", {}), checkListInferenceConnections) + .items, + }); +} + +/** What a suggest call needs beyond the points: whose asset, and through what. */ +export interface SuggestInput { + readonly projectId: string; + readonly assetId: string; + readonly connectionId: string; + /** Every positive click so far, in placement order. */ + readonly positive: readonly (readonly [number, number])[]; + readonly negative: readonly (readonly [number, number])[]; + /** + * The kinds the active class can hold — the caller's schema, not a preference. + * + * The route's own docstring is emphatic about this: an answer in a kind the + * schema would refuse is a suggestion that cannot be accepted, so the server + * narrows or answers nothing rather than proposing something unusable. + */ + readonly allowedGeometries: readonly GeometryType[]; +} + +/** Ask the model. Nothing is written and nothing is remembered. */ +export function useSuggestRegion() { + const client = useApiClient(); + return useMutation({ + mutationFn: async (input: SuggestInput): Promise => + unwrap( + await client.POST("/inference/suggest", { + body: { + project_id: input.projectId, + asset_id: input.assetId, + connection_id: input.connectionId, + positive: input.positive.map(([x, y]) => ({ x, y })), + negative: input.negative.map(([x, y]) => ({ x, y })), + allowed_geometries: [...input.allowedGeometries], + } as never, + }), + checkSuggestRegion, + ), + }); +} + +/** + * Why the suggest tool cannot run *yet*, when it cannot. `null` when it can. + * + * `checking` is one of them deliberately. The list is only fetched once the tool + * is armed — a job that never suggests makes no inference request at all — so + * there is a real moment where the answer is not known, and a click landing in it + * must be told something rather than vanishing. Three states, one union, so the + * panel's copy is total over them. + */ +export type SuggestBlocker = "checking" | "no-connections" | "not-ready"; + +/** + * The connection a click should go through, and why there is none. + * + * One function rather than two, because the answers are exclusive and the panel + * needs whichever it is: a `connection` to send to, or a `blocker` to explain. + */ +export function usableConnection(connections: readonly Connection[] | undefined): { + readonly connection: Connection | null; + readonly blocker: SuggestBlocker | null; +} { + if (connections === undefined) return { connection: null, blocker: "checking" }; + if (connections.length === 0) return { connection: null, blocker: "no-connections" }; + const ready = connections.find((row) => row.setup_state === "ready"); + if (ready === undefined) return { connection: null, blocker: "not-ready" }; + return { connection: ready, blocker: null }; +} diff --git a/frontend/ui-core/src/annotator/suggestFlow.test.tsx b/frontend/ui-core/src/annotator/suggestFlow.test.tsx new file mode 100644 index 00000000..a5440408 --- /dev/null +++ b/frontend/ui-core/src/annotator/suggestFlow.test.tsx @@ -0,0 +1,473 @@ +/** + * The suggest gesture wired end to end (#424, slice 3b): arm, click, preview, + * accept, save. + * + * Driven through `AnnotationPage` rather than through the parts, on `topBar`'s + * argument: every claim here is about how the pieces are wired — that a press + * reaches the route instead of the drawing tool, that the preview is outside the + * command log until somebody accepts it, and that the annotation which finally + * leaves carries where it came from. None of the three is visible from a + * component in isolation. + * + * The wire is stubbed with a route table rather than mocked at the hook, so what + * is asserted is **the request that actually leaves**. + * + * jsdom has no layout, so every rectangle is zero and a click lands at the + * asset's origin. That is fine and is the reason no assertion here is about + * *where* the shape came out: the coordinates a press converts to are a browser + * claim and belong in `e2e/`. What is testable here is the wiring. + */ + +import { QueryClient } from "@tanstack/react-query"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { userEvent } from "@testing-library/user-event"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { JSX, ReactNode } from "react"; + +import { ApiProvider } from "../data/ApiProvider"; +import { writeToken } from "../data/session"; +import { AnnotationPage } from "./AnnotationPage"; +import { TooltipProvider } from "../primitives/Menu"; +import { assetActions, batchActions, jobActions } from "../testing/wire.fixtures.js"; + +const API = "http://visionset.test"; +const PROJECT = "11111111-1111-4111-8111-111111111111"; +const BATCH = "22222222-2222-4222-8222-222222222222"; +const JOB = "33333333-3333-4333-8333-333333333333"; +const ASSET = "44444444-4444-4444-8444-444444444444"; +const CONNECTION = "66666666-6666-4666-8666-666666666666"; +const MODEL_REF = "facebook/sam2-hiera-base-plus@main"; + +const SCHEMA = { + project_id: PROJECT, + version: 1, + description: null, + created_at: null, + provenance: "curated", + classes: [ + { name: "vehicle", geometry: "bbox", color: "#3355ff", attributes: [] }, + { name: "lane-area", geometry: "polygon", color: null, attributes: [] }, + ], +}; + +interface Sent { + readonly method: string; + readonly path: string; + readonly body: string; +} + +const sent: Sent[] = []; + +/** The connection rows the workspace answers with. Replaced per test. */ +let connections: readonly Record[] = []; +/** What `POST /inference/suggest` answers, or the refusal it answers with. */ +let suggestion: Record | null = null; +let suggestRefusal: { status: number; code: string; message: string } | null = null; + +function connectionRow(setup: "ready" | "not_set_up"): Record { + return { + id: CONNECTION, + name: "local sam", + connection_type: "local", + model_id: "facebook/sam2-hiera-base-plus", + model_revision: "main", + device: "cuda", + precision: "fp16", + endpoint_url: null, + setup_state: setup, + allowed_actions: [], + created_at: "2026-08-08T00:00:00Z", + updated_at: "2026-08-08T00:00:00Z", + }; +} + +function answer(path: string): unknown { + if (path === "/inference/connections") { + return { items: connections, total: connections.length }; + } + if (path === `/jobs/${JOB}`) { + return { + id: JOB, + batch_id: BATCH, + state: "in_progress", + asset_count: 1, + allowed_actions: jobActions("in_progress", { settled: false }), + }; + } + if (path === `/batches/${BATCH}`) { + return { + id: BATCH, + project_id: PROJECT, + name: "drive-01", + state: "in_annotation", + schema_version: 1, + asset_count: 1, + allowed_actions: batchActions("in_annotation"), + promoted_asset_count: 0, + parent_batch_id: null, + progress: { + unannotated: 1, + annotated: 0, + skipped: 0, + review_pending: 0, + accepted: 0, + total: 1, + }, + }; + } + if (path.endsWith("/schema/versions/1") || path.endsWith("/schema")) return SCHEMA; + if (path.endsWith("/assets")) { + return { + items: [ + { + id: ASSET, + project_id: PROJECT, + modality: "image", + content_hash: "abcdef0".padEnd(64, "0"), + width: 640, + height: 480, + format: "png", + thumbnail_hash: null, + frame_index: null, + frame_timestamp: null, + source_id: null, + ingested_at: null, + job_id: JOB, + progress: "unannotated", + allowed_actions: assetActions("unannotated", { batchState: "in_annotation" }), + }, + ], + total: 1, + }; + } + return { items: [], total: 0 }; +} + +beforeEach(() => { + sent.length = 0; + connections = [connectionRow("ready")]; + suggestion = { + model_ref: MODEL_REF, + region: { + geometry: { type: "bbox", x: 12, y: 34, width: 56, height: 78 }, + confidence: 0.9125, + }, + }; + suggestRefusal = null; + writeToken("a-token"); + vi.stubGlobal("matchMedia", (query: string) => ({ + media: query, + matches: true, + addEventListener: () => {}, + removeEventListener: () => {}, + })); + vi.stubGlobal("fetch", async (request: Request) => { + const path = new URL(request.url).pathname; + if (request.method !== "GET") { + sent.push({ method: request.method, path, body: await request.clone().text() }); + if (path === "/inference/suggest") { + if (suggestRefusal !== null) { + return new Response( + JSON.stringify({ + code: suggestRefusal.code, + message: suggestRefusal.message, + }), + { status: suggestRefusal.status, headers: { "content-type": "application/json" } }, + ); + } + return new Response(JSON.stringify(suggestion), { + status: 200, + headers: { "content-type": "application/json" }, + }); + } + return new Response(JSON.stringify({}), { + status: 200, + headers: { "content-type": "application/json" }, + }); + } + return new Response(JSON.stringify(answer(path)), { + status: 200, + headers: { "content-type": "application/json" }, + }); + }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + globalThis.sessionStorage.clear(); +}); + +function mount(node: ReactNode): JSX.Element { + return ( + + {node} + + ); +} + +async function open(onConfigureInference?: () => void): Promise { + render( + mount( + , + ), + ); + await screen.findByTestId("annotation-page"); +} + +/** Arm the suggest tool from the strip, and wait for the panel to answer. */ +async function arm(): Promise { + await userEvent.click(screen.getByTestId("tool-suggest")); + await screen.findByTestId("suggest-panel"); +} + +/** One press on the canvas. `alt` is D2's negative point. */ +function clickCanvas(alt = false): void { + fireEvent.pointerDown(screen.getByTestId("annotator-pane"), { + button: 0, + clientX: 100, + clientY: 100, + altKey: alt, + pointerId: 1, + }); +} + +/** Every suggest request that has left, newest last. */ +function asks(): readonly Record[] { + return sent + .filter((row) => row.path === "/inference/suggest") + .map((row) => JSON.parse(row.body) as Record); +} + +describe("arming the tool", () => { + it("asks for the connection list only once somebody arms it", async () => { + await open(); + // A job nobody suggests on makes no inference request at all. + expect(screen.getByTestId("tool-suggest")).toBeTruthy(); + + await arm(); + + await waitFor(() => { + expect(screen.getByTestId("suggest-idle")).toBeTruthy(); + }); + }); + + it("activates a class that can hold the answer, so the shape has a label", async () => { + await open(); + await arm(); + // Nothing was armed before; the strip's own rule moves the active class to + // one that derives the tool asked for — here, the first suggestible class. + expect(screen.getByTestId("class-row-vehicle").getAttribute("data-selected")).toBe("true"); + }); + + it("disarms when another tool moves the active class (D2)", async () => { + await open(); + await arm(); + await userEvent.click(screen.getByTestId("class-row-lane-area")); + expect(screen.queryByTestId("suggest-panel")).toBeNull(); + }); +}); + +describe("a click asks the model", () => { + it("sends the asset, the connection and the class's own geometry kinds", async () => { + await open(); + await arm(); + clickCanvas(); + + await waitFor(() => expect(asks()).toHaveLength(1)); + const ask = asks()[0]; + expect(ask["project_id"]).toBe(PROJECT); + expect(ask["asset_id"]).toBe(ASSET); + expect(ask["connection_id"]).toBe(CONNECTION); + // The caller's schema, not a preference: `vehicle` is a bbox class, so a + // polygon answer would be a suggestion that could not be accepted. + expect(ask["allowed_geometries"]).toEqual(["bbox"]); + expect(ask["positive"]).toHaveLength(1); + expect(ask["negative"]).toEqual([]); + }); + + it("draws the answer as a preview, dashed and faint", async () => { + await open(); + await arm(); + clickCanvas(); + + const shape = await screen.findByTestId("suggestion-shape"); + expect(shape.getAttribute("stroke-dasharray")).toBeTruthy(); + expect( + screen.getByTestId("suggestion-preview").getAttribute("opacity"), + ).toBe("0.6"); + expect(screen.getByTestId("suggestion-label").textContent).toBe("vehicle 91%"); + }); + + it("sends the accumulated points on a refine, never a diff", async () => { + await open(); + await arm(); + clickCanvas(); + await waitFor(() => expect(asks()).toHaveLength(1)); + + clickCanvas(true); + await waitFor(() => expect(asks()).toHaveLength(2)); + + const second = asks()[1]; + // The route is stateless, so "the model already knows about my first click" + // is not a thing that can be true. + expect(second["positive"]).toHaveLength(1); + expect(second["negative"]).toHaveLength(1); + }); + + it("does not draw a shape — the press never reaches the interaction machine", async () => { + await open(); + await arm(); + clickCanvas(); + fireEvent.pointerUp(screen.getByTestId("annotator-pane"), { + button: 0, + clientX: 140, + clientY: 140, + pointerId: 1, + }); + + await waitFor(() => expect(asks()).toHaveLength(1)); + expect(screen.getByTestId("object-total").textContent).toBe("0 objects"); + }); +}); + +describe("the preview is outside the document and outside the history", () => { + /** + * **The mutation test for D4.** Turn a suggestion into a `stage` or an `add` + * the moment it arrives — rather than when somebody accepts it — and this is + * what turns red: the undo control lights up over work nobody committed. + */ + it("leaves undo untouched while a suggestion is only showing", async () => { + await open(); + await arm(); + clickCanvas(); + await screen.findByTestId("suggestion-shape"); + + expect(screen.getByTestId("tool-undo").getAttribute("aria-disabled")).toBe("true"); + expect(screen.getByTestId("object-total").textContent).toBe("0 objects"); + }); + + it("discards on Esc without touching the document", async () => { + await open(); + await arm(); + clickCanvas(); + await screen.findByTestId("suggestion-shape"); + + await userEvent.click(screen.getByTestId("suggest-discard")); + + expect(screen.queryByTestId("suggestion-shape")).toBeNull(); + expect(screen.getByTestId("tool-undo").getAttribute("aria-disabled")).toBe("true"); + // Armed still, not put away: somebody who cleared a preview is about to + // click again. + expect(screen.getByTestId("suggest-panel")).toBeTruthy(); + }); +}); + +describe("acceptance", () => { + it("adds exactly one object and exactly one undo step", async () => { + await open(); + await arm(); + clickCanvas(); + await screen.findByTestId("suggestion-shape"); + + await userEvent.click(screen.getByTestId("suggest-accept")); + + expect(screen.getByTestId("object-total").textContent).toBe("1 object"); + expect(screen.getByTestId("tool-undo").getAttribute("aria-disabled")).toBeNull(); + // The preview is gone the moment it became an annotation — there is no + // second copy of the shape on the canvas. + expect(screen.queryByTestId("suggestion-shape")).toBeNull(); + }); + + /** + * **The mutation test for D4's provenance.** Drop the three overrides in + * `acceptedAnnotation` and this is what turns red: the annotation that leaves + * claims a human drew it, and the model that proposed it is unrecorded. + */ + it("writes it through the ordinary create path, carrying where it came from", async () => { + await open(); + await arm(); + clickCanvas(); + await screen.findByTestId("suggestion-shape"); + await userEvent.click(screen.getByTestId("suggest-accept")); + + await userEvent.click(screen.getByTestId("save-and-stay")); + + await waitFor(() => { + expect(sent.some((row) => row.path.endsWith("/annotations"))).toBe(true); + }); + const write = sent.find((row) => row.path.endsWith("/annotations")); + const body = JSON.parse(write?.body ?? "[]") as readonly Record[]; + expect(body).toHaveLength(1); + expect(body[0]["provenance"]).toBe("model"); + expect(body[0]["model_ref"]).toBe(MODEL_REF); + expect(body[0]["confidence"]).toBe(0.9125); + expect(body[0]["label_class"]).toBe("vehicle"); + expect(body[0]["geometry"]).toEqual({ type: "bbox", x: 12, y: 34, width: 56, height: 78 }); + }); +}); + +describe("when there is nothing to suggest through (D6)", () => { + it("says nothing is configured, and renders no control without a destination", async () => { + connections = []; + await open(); + await arm(); + + await screen.findByTestId("suggest-no-connections"); + expect(screen.queryByTestId("suggest-configure")).toBeNull(); + }); + + it("offers the way out when the host wires one", async () => { + connections = []; + const onConfigureInference = vi.fn(); + await open(onConfigureInference); + await arm(); + + await userEvent.click(await screen.findByTestId("suggest-configure")); + expect(onConfigureInference).toHaveBeenCalledTimes(1); + }); + + it("tells an undownloaded connection apart from no connection", async () => { + connections = [connectionRow("not_set_up")]; + await open(); + await arm(); + await screen.findByTestId("suggest-not-ready"); + }); + + it("sends nothing while the tool is blocked", async () => { + connections = []; + await open(); + await arm(); + await screen.findByTestId("suggest-no-connections"); + clickCanvas(); + + expect(asks()).toHaveLength(0); + }); +}); + +describe("a refusal", () => { + it("renders the server's own words, which carry the install command", async () => { + suggestRefusal = { + status: 500, + code: "LOCAL_INFERENCE_UNAVAILABLE", + message: + "running a model locally needs the 'local-inference' extra, and 'torch' is not " + + 'installed here. Install it with: pip install "visionset[local-inference]"', + }; + await open(); + await arm(); + clickCanvas(); + + const prose = await screen.findByTestId("suggest-refusal"); + expect(prose.textContent).toContain("visionset[local-inference]"); + // No 500 page, no toast, no raw code — the editor stays where it is and the + // work on it is untouched (principle 10). + expect(screen.getByTestId("annotation-page")).toBeTruthy(); + expect(screen.getByTestId("object-total").textContent).toBe("0 objects"); + }); +}); diff --git a/frontend/ui-core/src/annotator/suggestPanel.test.tsx b/frontend/ui-core/src/annotator/suggestPanel.test.tsx new file mode 100644 index 00000000..46b6a195 --- /dev/null +++ b/frontend/ui-core/src/annotator/suggestPanel.test.tsx @@ -0,0 +1,202 @@ +/** + * The suggest tool's panel: the five things it can be saying, and the one rule + * about its action (#424, D6). + * + * The three *blocked* readings are the issue's own list — none configured, none + * ready, and the server refusing because this build cannot run the model — and + * each is asserted to carry a remedy rather than a state. The fourth claim is the + * structural one: with no callback there is an explanation and **no control**, + * never a dead button. + */ + +import { render, screen } from "@testing-library/react"; +import { userEvent } from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; +import type { JSX } from "react"; + +import { answered, armed, refused, withPoint } from "@visionset/annotator"; +import type { Suggestion, SuggestionState } from "@visionset/annotator"; + +import { SuggestPanel } from "./SuggestPanel"; +import { usableConnection, type Connection } from "./inferenceQueries"; + +const A_BOX = { type: "bbox", x: 10, y: 20, width: 30, height: 40 } as const; + +function proposal(): Suggestion { + return { geometry: A_BOX, confidence: 0.9, modelRef: "facebook/sam2-hiera-base-plus@main" }; +} + +function asked(): SuggestionState { + return withPoint(armed("vehicle"), [100, 120], "positive"); +} + +function shown(): SuggestionState { + const session = asked(); + return answered(session, session.serial, proposal()); +} + +function mount(overrides: Partial[0]> = {}): JSX.Element { + return ( + + ); +} + +/** A connection row, in whichever setup state a case needs. */ +function connection(setup: Connection["setup_state"]): Connection { + return { + id: "c1", + name: "local sam", + connection_type: "local", + model_id: "facebook/sam2-hiera-base-plus", + model_revision: "main", + device: "cuda", + precision: "fp16", + endpoint_url: null, + setup_state: setup, + allowed_actions: [], + created_at: "2026-08-08T00:00:00Z", + updated_at: "2026-08-08T00:00:00Z", + } as Connection; +} + +describe("which connection a click goes through", () => { + it("is none, and says why, when the workspace has configured none", () => { + expect(usableConnection([])).toEqual({ connection: null, blocker: "no-connections" }); + }); + + it("is none, and says why, when one exists but its weights are not here", () => { + expect(usableConnection([connection("not_set_up")])).toEqual({ + connection: null, + blocker: "not-ready", + }); + }); + + it("is the first ready one, in the list's own order", () => { + const ready = connection("ready"); + const answer = usableConnection([connection("not_set_up"), ready, connection("ready")]); + expect(answer.connection).toBe(ready); + expect(answer.blocker).toBe(null); + }); + + it("names the loading window rather than pretending it is a working tool", () => { + // The list is only fetched once the tool is armed, so this window is real — + // and a click landing in it must be told something rather than vanishing. + expect(usableConnection(undefined)).toEqual({ connection: null, blocker: "checking" }); + }); +}); + +describe("the no-connection panel (D6)", () => { + it("says what is missing when nothing is configured, and offers the way out", async () => { + const onConfigure = vi.fn(); + render(mount({ blocker: "no-connections", onConfigure })); + + expect(screen.getByTestId("suggest-no-connections")).toBeTruthy(); + expect(screen.getByTestId("suggest-panel").getAttribute("data-tone")).toBe("warn"); + await userEvent.click(screen.getByTestId("suggest-configure")); + expect(onConfigure).toHaveBeenCalledTimes(1); + }); + + it("tells a configured-but-undownloaded connection apart from no connection", () => { + render(mount({ blocker: "not-ready", onConfigure: vi.fn() })); + expect(screen.getByTestId("suggest-not-ready")).toBeTruthy(); + expect(screen.queryByTestId("suggest-no-connections")).toBeNull(); + // Two states, two sentences: one is a thing to make and the other is a + // download, and a shared message would send somebody to the wrong remedy. + expect(screen.getByTestId("suggest-panel").textContent).toContain("not on this machine"); + }); + + it("is not a warning while it is merely checking", () => { + render(mount({ blocker: "checking" })); + expect(screen.getByTestId("suggest-checking")).toBeTruthy(); + expect(screen.getByTestId("suggest-panel").getAttribute("data-tone")).toBe("calm"); + }); + + /** + * The structural claim. `ui-core` imports no router, so where "set one up" + * goes is the host's — and a host that has nowhere to send somebody must get + * the explanation with nothing to press, never a control that does nothing. + */ + it("renders the explanation and no control when the host wires no destination", () => { + render(mount({ blocker: "no-connections" })); + expect(screen.getByTestId("suggest-no-connections")).toBeTruthy(); + expect(screen.queryByTestId("suggest-configure")).toBeNull(); + }); + + it("never renders an action for the checking state, callback or not", () => { + render(mount({ blocker: "checking", onConfigure: vi.fn() })); + expect(screen.queryByTestId("suggest-configure")).toBeNull(); + }); + + it("outranks whatever the session was doing", () => { + // A session over a workspace with no usable connection has nothing to report + // about a request it never made. + render(mount({ session: shown(), blocker: "no-connections" })); + expect(screen.getByTestId("suggest-no-connections")).toBeTruthy(); + expect(screen.queryByTestId("suggest-accept")).toBeNull(); + }); +}); + +describe("what the panel says while the tool is working", () => { + it("invites the first click", () => { + render(mount()); + expect(screen.getByTestId("suggest-idle")).toBeTruthy(); + expect(screen.getByTestId("suggest-panel").textContent).toContain("vehicle"); + }); + + it("says a request is in flight, in the async vocabulary and not a new spinner", () => { + render(mount({ session: asked() })); + expect(screen.getByTestId("suggest-asking")).toBeTruthy(); + expect(screen.queryByTestId("suggest-accept")).toBeNull(); + }); + + it("offers accept and discard once something is showing", async () => { + const onAccept = vi.fn(); + const onDiscard = vi.fn(); + render(mount({ session: shown(), onAccept, onDiscard })); + + expect(screen.getByTestId("suggest-shown")).toBeTruthy(); + await userEvent.click(screen.getByTestId("suggest-accept")); + expect(onAccept).toHaveBeenCalledTimes(1); + await userEvent.click(screen.getByTestId("suggest-discard")); + expect(onDiscard).toHaveBeenCalledTimes(1); + }); + + it("treats an answer with nothing in it as an answer, and says what to try", () => { + const session = asked(); + render(mount({ session: answered(session, session.serial, null) })); + expect(screen.getByTestId("suggest-none")).toBeTruthy(); + expect(screen.queryByTestId("suggest-accept")).toBeNull(); + }); +}); + +describe("a refusal", () => { + /** + * The one that matters most: `LOCAL_INFERENCE_UNAVAILABLE` is + * `expose_message=True` precisely so the install command reaches a person, and + * a sentence written in the client would throw it away. + */ + it("shows the server's own words, including the install command", () => { + const session = asked(); + const prose = + "running a model locally needs the 'local-inference' extra, and 'torch' is not " + + 'installed here. Install it with: pip install "visionset[local-inference]"'; + render(mount({ session: refused(session, session.serial, prose), refusal: prose })); + + expect(screen.getByTestId("suggest-refusal").textContent).toBe(prose); + expect(screen.getByTestId("suggest-panel").getAttribute("data-tone")).toBe("warn"); + }); + + it("says the clicks survive it, because they do", () => { + const session = asked(); + render(mount({ session: refused(session, session.serial, "nope"), refusal: "nope" })); + expect(screen.getByTestId("suggest-panel").textContent).toContain("Esc"); + expect(screen.queryByTestId("suggest-accept")).toBeNull(); + }); +}); diff --git a/frontend/ui-core/src/annotator/toolPalette.test.tsx b/frontend/ui-core/src/annotator/toolPalette.test.tsx index 8b21b6f4..493f0b6e 100644 --- a/frontend/ui-core/src/annotator/toolPalette.test.tsx +++ b/frontend/ui-core/src/annotator/toolPalette.test.tsx @@ -256,3 +256,56 @@ describe("adding a class from the palette (#233)", () => { expect(document.activeElement).not.toBe(button); }); }); + +describe("the suggest tool (#424)", () => { + it("is absent without the prop — a host with no model behind it renders none", () => { + render(mount()); + expect(screen.queryByTestId("tool-suggest")).toBeNull(); + }); + + it("is offered when the schema declares a class that can hold the answer", () => { + render(mount({ suggest: { active: false, onToggle: vi.fn() } })); + expect(screen.getByTestId("tool-suggest")).toBeTruthy(); + expect(screen.getByTestId("tool-suggest").getAttribute("aria-label")).toBe("Suggest (S)"); + }); + + it("is hidden, not disabled, on a schema no class of which could hold one", () => { + // D3's third case. A disabled sparkle here would be promising a capability + // that does not apply to this project rather than one that is coming — which + // is the distinction `PENDING_TOOLS` exists for and this is not. + const tagsOnly = { + ...(SCHEMA as unknown as { classes: unknown[] }), + classes: [ + { name: "daytime", geometry: "classification_tag", color: null, attributes: [] }, + { name: "kerb", geometry: "polyline", color: null, attributes: [] }, + ], + } as unknown as Parameters[0]; + render(mount({ schema: tagsOnly, suggest: { active: false, onToggle: vi.fn() } })); + expect(screen.queryByTestId("tool-suggest")).toBeNull(); + }); + + it("is lit from the host's mode rather than from the derived tool", () => { + // The one control on this strip whose `active` is not `tool === choice.tool`: + // suggest is a mode held beside the class, and the class still derives `bbox`. + render(mount({ tool: "bbox", suggest: { active: true, onToggle: vi.fn() } })); + expect(screen.getByTestId("tool-suggest").getAttribute("data-active")).toBe("true"); + expect(screen.getByTestId("tool-bbox").getAttribute("data-active")).toBe("true"); + }); + + it("toggles on a press, and does not move the active class itself", async () => { + const onToggle = vi.fn(); + const onActivateClass = vi.fn(); + render(mount({ onActivateClass, suggest: { active: false, onToggle } })); + await userEvent.click(screen.getByTestId("tool-suggest")); + expect(onToggle).toHaveBeenCalledTimes(1); + // Arming *does* activate a class — but that is the page's decision, made from + // `suggestClassFor`, not this strip reaching for one. + expect(onActivateClass).not.toHaveBeenCalled(); + }); + + it("keeps the canvas's focus, like every other button here", () => { + render(mount({ suggest: { active: false, onToggle: vi.fn() } })); + const press = fireEvent.mouseDown(screen.getByTestId("tool-suggest")); + expect(press).toBe(false); + }); +}); diff --git a/frontend/ui-core/src/index.ts b/frontend/ui-core/src/index.ts index c18baf70..fc0d6c62 100644 --- a/frontend/ui-core/src/index.ts +++ b/frontend/ui-core/src/index.ts @@ -323,6 +323,18 @@ export { type AssetProgress, type SavePlan, } from "./annotator/jobQueries.js"; +export { SuggestPanel, type SuggestPanelProps } from "./annotator/SuggestPanel.js"; +export { + inferenceKeys, + usableConnection, + useInferenceConnections, + useSuggestRegion, + type Connection, + type SuggestBlocker, + type SuggestInput, + type SuggestionOut, + type SuggestedRegion, +} from "./annotator/inferenceQueries.js"; export { queryKeys, useActiveSchema,