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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 30 additions & 16 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -1008,22 +1008,36 @@ gallery badges (#55) — and it **already exists, shipped and unit-tested**:
outline; the class label renders only while selected, 11px / 700, anchored at the
first vertex, never a pointer target.

**A model's work is marked; a person's is not.** `provenance: "model"` earns a mark on the
canvas label (`class · 62%`, and `class · model` where the model recorded no score) and a
`Sparkles` glyph with the score on the side-panel row, whose accessible name carries the
claim in words and whose tooltip carries the full `model_ref`. Never colour alone — class
colour is already user data and cannot also mean provenance. **Absence is the human case**:
no "manual" badge, no mark on the common path, because the row a reviewer sees a thousand
times is the one that must stay quiet. `import` provenance is unmarked until there is an
importer whose mark would mean something.

**Confidence has one spelling, and it is whole percent.** `confidencePercent` in
`frontend/annotator/src/adapters/react/paint.ts` — the same shared-helper rule as
`classColor` directly above, and for the same reason: a live suggestion, a committed
annotation's canvas label and its panel row all show the same number, and two notations for
it is a number that disagrees with itself. Two decimals would claim a precision a
confidence does not have. A `null` confidence reads as absent — never as `0`, never as a
low score.
**The canvas label is part of what selection looks like.** A frame carrying forty boxes drew
forty class names over the picture at all times, which hides the asset behind the annotations
of it. The panel is the full inventory; the canvas answers *what is this one* for the shape
somebody picked, and an unselected shape is its box alone. This is also how a viewer's
selection reads, since a read-only frame paints no grips.

**A model's work is marked; a person's is not.** `provenance: "model"` earns a `Sparkles`
glyph on the side-panel row, whose accessible name carries the claim in words and whose
tooltip carries the full `model_ref`. That glyph is the *only* provenance signal in the
editor — the canvas label is the class and nothing else. Never colour alone — class colour is
already user data and cannot also mean provenance. **Absence is the human case**: no "manual"
badge, no mark on the common path, because the row a reviewer sees a thousand times is the one
that must stay quiet. `import` provenance is unmarked until there is an importer whose mark
would mean something.

**Confidence renders on the live suggestion preview and nowhere else in the editor.** The
number tells somebody whether to accept a proposal; once accepted, the shape is a label like
any other and the score is decoration on every subsequent reading of it. So no percentage on a
canvas box, on a panel row, or in a tooltip. It is not discarded — `confidence` and
`model_ref` are stored unchanged — and its home is the batch review loop, where a surface
showing it must also name *what it measures*: a point-prompted mask score and a detection's
prompt affinity are different quantities on different scales and cannot be pooled, thresholded
or sorted together.

**Where it is shown, it has one spelling, and that is whole percent.** `confidencePercent` in
`frontend/annotator/src/adapters/react/paint.ts` — the same shared-helper rule as `classColor`
above: whoever shows the number imports it rather than respelling it, because two notations
for one quantity is a number that disagrees with itself. Two decimals would claim a precision
a confidence does not have. A `null` confidence reads as absent — never as `0`, never as a low
score.

## Libraries

Expand Down
15 changes: 15 additions & 0 deletions frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ import type {
} from "react";

import { topmostAnnotationAt } from "../../core/geometry/hitTest";
import { withinBounds } from "../../core/geometry/primitives";
import { assetTolerances } from "../../core/geometry/tolerance";
import { affordanceAt, viewerAffordanceAt } from "../../core/interaction/affordance";
import { transition } from "../../core/interaction/machine";
Expand Down Expand Up @@ -825,6 +826,20 @@ export function AnnotatorCanvas({
// A session with no handler swallows the press rather than falling through,
// for the reason the prop's docstring gives.
if (suggestion !== null) {
// The stage surround is not the asset. The pane spans the whole viewport
// on purpose (see the input surface's note below), so `point` here can be
// negative or past the edge — which a *drag* wants, because "make the box
// this big" survives leaving the picture, and which a prompt point cannot
// use: there is nothing under the margin for a segmenter to segment. So
// an out-of-frame press is dropped whole rather than clamped onto the
// edge, and dropped **here**, before the host is told: a point the host
// never hears about records no click, sends no request and moves no
// preview, which is one guarantee instead of three.
//
// Asked in asset pixels rather than in screen ones, so zoom and pan are
// already accounted for by `imagePoint` and there is no second transform
// to keep in step with the first.
if (!withinBounds(point, asset)) return;
onSuggestPoint?.(point, event.altKey ? "negative" : "positive");
return;
}
Expand Down
42 changes: 18 additions & 24 deletions frontend/annotator/src/adapters/react/Shapes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import type {
PolygonGeometry,
PolylineGeometry,
} from "../../core/types";
import { confidencePercent, screenPx } from "./paint";
import { screenPx } from "./paint";
import type { PaintedAnnotation } from "./paint";

/** Stroke thickness, in screen pixels, for an unselected shape. */
Expand Down Expand Up @@ -273,29 +273,16 @@ export function Vertices({ points, color, zoom, hotIndex }: {
);
}

/**
* What a label says: the class, and for a model's work what the model claimed.
*
* A suffix rather than a second element, so the shipped label metrics are
* untouched — same size, same weight, same anchor, same lift. The label only
* gets longer, and only on a selected shape, which is the one place it renders
* at all.
*
* A model-produced annotation always carries the separator, because
* `confidence` is optional on one and a shape marked only when the model
* happened to score itself is not a mark a reviewer can trust the absence of.
* `import` provenance reads as a person's for now — the importers that would
* make that distinction mean something are not built.
*/
export function labelText(shape: PaintedAnnotation): string {
if (shape.provenance !== "model") return shape.labelClass;
const score = shape.confidence === null ? "model" : confidencePercent(shape.confidence);
return `${shape.labelClass} · ${score}`;
}

/**
* The class name, above the shape.
*
* **The class and nothing else.** A confidence is a decision aid for
* accept-or-reject and stops being one the moment a shape is accepted, so the
* number lives on the live suggestion preview and nowhere else in the editor;
* which model produced a stored shape is the side panel's mark. A canvas that
* wrote either would be answering, on every selected shape, a question a
* reviewer asks about one.
*
* `y` is the anchor itself — a plain asset coordinate — and the screen-pixel lift
* above it rides on the CSS `translate` property instead of being subtracted here.
* That is what keeps every one of this element's attributes free of the zoom:
Expand All @@ -318,7 +305,7 @@ export function ShapeLabel({ shape }: { readonly shape: PaintedAnnotation }): JS
translate: "0 var(--vs-label-lift)",
}}
>
{labelText(shape)}
{shape.labelClass}
</text>
);
}
Expand Down Expand Up @@ -352,7 +339,14 @@ export function PolylineShape({ geometry, color, hot, selected }: {
}

/**
* One committed annotation: its shape, its label, and its grips when selected.
* One committed annotation: its shape, and — while it is selected — its label
* and its grips.
*
* **The label is part of what selection looks like.** A frame carrying forty
* boxes drew forty class names over the picture at all times, which hid the
* asset behind the annotations of it; the panel is the full inventory, and the
* canvas answers "what is *this* one" for the shape somebody picked. It is also
* how a viewer's selection reads, since a read-only frame paints no grips.
*
* Grips and vertices are drawn only for a selected shape, which mirrors
* `resolveTarget` — it looks for a handle or a vertex only among the *selected*
Expand Down Expand Up @@ -386,7 +380,7 @@ export function AnnotationShape({ shape, zoom, handles }: ShapeProps): JSX.Eleme
selected={shape.selected}
/>
)}
<ShapeLabel shape={shape} />
{shape.selected && <ShapeLabel shape={shape} />}
{handles && shape.selected && shape.geometry.type === "bbox" && (
<Grips geometry={shape.geometry} color={shape.color} zoom={zoom} hotHandle={null} />
)}
Expand Down
73 changes: 37 additions & 36 deletions frontend/annotator/src/adapters/react/paint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { ASSET, SCHEMA, annotation } from "../../core/state/_sample";
import { createDocument } from "../../core/state/document";
import { EMPTY_SELECTION, selectionOf } from "../../core/state/selection";
import type { Annotation } from "../../core/types";
import { labelText } from "./Shapes";
import {
SUGGESTION_DASH,
SUGGESTION_OPACITY,
Expand Down Expand Up @@ -327,7 +326,15 @@ describe("a pending suggestion, drawn as a proposal (#424)", () => {
});
});

describe("what a model produced, projected so the label can say so", () => {
/**
* Where a model's confidence lives, and where the draw list refuses to carry it.
*
* The preview shows the number because it is what an accept-or-reject decision
* is made on; a committed annotation does not, because by then the decision has
* been made. The projection is what enforces the second half: a renderer cannot
* write a score it was never given.
*/
describe("a committed annotation's confidence is not the canvas's business", () => {
function predicted(overrides: Partial<Annotation> = {}): Annotation {
return {
...annotation("m1"),
Expand All @@ -349,45 +356,39 @@ describe("what a model produced, projected so the label can say so", () => {
return painted;
}

it("carries both fields onto the draw list", () => {
const shape = paintOne(predicted());
expect(shape.provenance).toBe("model");
expect(shape.confidence).toBe(0.62);
});

it("carries a person's own values too, so the label can tell them apart", () => {
const shape = paintOne(annotation("h1"));
expect(shape.provenance).toBe("human");
expect(shape.confidence).toBeNull();
});

it("writes the model's score beside the class, at the shipped spelling", () => {
expect(labelText(paintOne(predicted()))).toBe("sign · 62%");
it("hands the renderer neither the score nor who produced it", () => {
// Absence in the projection, not a rule in the component: a `<text>` that
// has never been given a confidence cannot start writing one.
const shape: Record<string, unknown> = { ...paintOne(predicted()) };
expect("confidence" in shape).toBe(false);
expect("provenance" in shape).toBe(false);
});

it("still marks a model's work when the model reported no score", () => {
// `confidence` is optional on a model-produced annotation, and a mark that
// appeared only when a model happened to score itself is a mark whose
// absence says nothing.
expect(labelText(paintOne(predicted({ confidence: null })))).toBe("sign · model");
it("paints a model's shape exactly as it paints a person's", () => {
// Every field the two share, and there is now nothing else: the canvas
// cannot tell them apart, which is the point.
const mine = paintOne(annotation("m1"));
const theirs = paintOne(predicted());
expect(theirs).toEqual({ ...mine, id: theirs.id });
});

it("leaves a person's label exactly as it shipped", () => {
expect(labelText(paintOne(annotation("h1")))).toBe("sign");
});

it("leaves an imported label alone, having no mark that would mean anything", () => {
const imported = predicted({ provenance: "import", model_ref: null, confidence: null });
expect(labelText(paintOne(imported))).toBe("sign");
it("keeps the number on the live preview, which is where the decision is", () => {
// The one surface in the editor that shows it, and `paintSuggestion` is
// what puts it there — see the `#424` block above for the full range.
const asked = withPoint(armed("sign"), [100, 120], "positive");
const preview = answered(asked, asked.serial, {
geometry: { type: "bbox", x: 10, y: 20, width: 30, height: 40 },
confidence: 0.62,
modelRef: "facebook/sam2-hiera-base-plus@main",
});
const declared = SCHEMA.classes.find((one) => one.name === "sign");
expect(paintSuggestion(preview, declared)?.label).toBe("sign 62%");
});

it("spells the score the way the suggestion overlay already does", () => {
// One quantity, one spelling. `confidenceLabel` is the canvas's live
// suggestion; `labelText` is the committed annotation; a reader looking at
// both at once must not see two notations for the same number.
expect(labelText(paintOne(predicted({ confidence: 0.87 })))).toContain(
confidencePercent(0.87),
);
expect(confidenceLabel("sign", 0.87)).toContain(confidencePercent(0.87));
it("spells it one way, so no second surface can disagree with the first", () => {
// `confidencePercent` is exported for this: whoever shows the number
// imports it. Two `Math.round`s would be the same quantity in two
// notations, which is the defect the shared helper exists to prevent.
expect(confidenceLabel("sign", 0.87)).toBe(`sign ${confidencePercent(0.87)}`);
});
});
28 changes: 11 additions & 17 deletions frontend/annotator/src/adapters/react/paint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import type {
Point,
PolygonGeometry,
PolylineGeometry,
Provenance,
} from "../../core/types";

/** A shape whose class the schema declares, ready to draw. */
Expand All @@ -66,17 +65,6 @@ export interface PaintedAnnotation {
/** Under the pointer, or held by the drag in flight. */
readonly hot: boolean;
readonly color: string;
/**
* Who drew it, projected so the label can say so.
*
* Carried rather than derived at the label because the draw list is where a
* renderer's questions are already answered — a `<text>` that reached back
* into the document for provenance would be the one component that needs a
* document, and the only reason for it would be four characters of suffix.
*/
readonly provenance: Provenance;
/** How sure the model was, and `null` for every label a person drew. */
readonly confidence: number | null;
}

/**
Expand Down Expand Up @@ -188,8 +176,6 @@ function painted(
selected: selection.has(annotation.id),
hot: annotation.id === hotId,
color: classColor(declared, annotation.label_class),
provenance: annotation.provenance,
confidence: annotation.confidence,
};
}

Expand Down Expand Up @@ -328,9 +314,17 @@ export function paintSuggestion(
* have. A confidence outside `[0, 1]` cannot arrive — the kernel's
* `PredictedRegion` refuses one — so nothing is clamped here.
*
* Exported for the same reason `classColor` is: the panel shows the same
* quantity as the canvas, and a second spelling of it in `ui-core` would be a
* number that disagrees with itself across two surfaces.
* **One consumer today** — the live suggestion preview, through
* `confidenceLabel`. Exported anyway, for the reason `classColor` is exported:
* this is the spelling, and the surfaces that will show a confidence next are
* outside this package. A second `Math.round` in `ui-core` would be the same
* number disagreeing with itself across two screens, which is the defect the
* shared helper exists to make impossible rather than merely unlikely.
*
* The editor deliberately shows it nowhere else. A confidence helps somebody
* decide whether to accept a proposal; once accepted, the shape is a label like
* any other, and the number would be decoration on every subsequent reading of
* it.
*/
export function confidencePercent(confidence: number): string {
return `${Math.round(confidence * 100)}%`;
Expand Down
45 changes: 43 additions & 2 deletions frontend/annotator/src/core/geometry/primitives.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* The four primitives, and the one claim about `Bounds` a reader should not have
* to take on faith: that an `AssetDescriptor` is already a frame.
* The primitives, and the one claim about `Bounds` a reader should not have to
* take on faith: that an `AssetDescriptor` is already a frame.
*/

import { describe, expect, it } from "vitest";
Expand All @@ -10,6 +10,7 @@ import {
clampPoint,
closestPointOnSegment,
distance,
withinBounds,
type Bounds,
} from "./primitives";

Expand Down Expand Up @@ -58,6 +59,46 @@ describe("a point pushed back inside the frame", () => {
});
});

/**
* The other question about a frame: not *where does this point go* but *was it
* in there at all*. A caller that must not salvage a stray coordinate asks this
* one — see the docstring for why the two exist side by side.
*/
describe("whether a point is inside the frame", () => {
const frame: Bounds = { width: 100, height: 50 };

it("says yes for a point in the middle", () => {
expect(withinBounds([40, 20], frame)).toBe(true);
});

it("counts the edges and the corners as inside, exactly as the clamp does", () => {
// The same range, stated twice: a point `clampPoint` would leave untouched
// is a point this must call inside, or the last row of pixels becomes a
// place where a press silently stops working.
for (const corner of [[0, 0], [100, 0], [0, 50], [100, 50]] as const) {
expect(withinBounds(corner, frame)).toBe(true);
expect(clampPoint(corner, frame)).toEqual([...corner]);
}
});

it("says no past each of the four edges, one axis at a time", () => {
expect(withinBounds([-0.001, 20], frame)).toBe(false);
expect(withinBounds([100.001, 20], frame)).toBe(false);
expect(withinBounds([40, -0.001], frame)).toBe(false);
expect(withinBounds([40, 50.001], frame)).toBe(false);
});

it("says no for a coordinate that is not a number", () => {
expect(withinBounds([Number.NaN, 20], frame)).toBe(false);
expect(withinBounds([40, Number.NaN], frame)).toBe(false);
});

it("takes an asset descriptor as its frame, like everything else here", () => {
expect(withinBounds([ASSET.width, ASSET.height], ASSET)).toBe(true);
expect(withinBounds([ASSET.width + 1, ASSET.height], ASSET)).toBe(false);
});
});

describe("a frame is a width and a height", () => {
it("accepts an asset descriptor without conversion", () => {
// The assignment is the assertion: if `AssetDescriptor` ever stopped
Expand Down
Loading
Loading