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
41 changes: 41 additions & 0 deletions frontend/app/cycle/cycle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,47 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa
});
});

await test.step("the completed batch reopens as a viewer, from the tile and the address bar", async () => {
/*
* The step the walk never took (#423): every earlier read-only claim ran
* against stubs, so nothing proved that a *real* completed batch — whose
* asset declarations the kernel computes — reopens as a viewer. The two
* entries below are the two roads in: the gallery tile, and a reload of the
* job URL with no cache to inherit.
*/
await page.getByTestId("open-batch-cycle-batch").click();
await expect(page.getByTestId("gallery")).toBeVisible();
const tiles = page.getByTestId(/^tile-/);
await expect(tiles).toHaveCount(3);
const assetId = (await tiles.first().getAttribute("data-testid"))!.replace("tile-", "");
await tiles.first().getByTestId(`open-${assetId}`).click();
await expect(page.getByTestId("annotation-page")).toBeVisible();

await expect(page.getByTestId("readonly-banner")).toContainText(/viewing only/i);
await expect(page.getByTestId("banner-create-correction")).toBeVisible();
await expect(page.getByTestId("tool-palette")).toHaveCount(0);
await expect(page.getByTestId("class-add")).toBeDisabled();

// A full draw gesture writes nothing and dirties nothing.
const canvas = page.getByTestId("annotator-canvas");
const box = (await canvas.boundingBox())!;
await page.getByTestId("annotator-root").focus();
await page.keyboard.press("1");
await page.mouse.move(box.x + box.width * 0.3, box.y + box.height * 0.3);
await page.mouse.down();
await page.mouse.move(box.x + box.width * 0.6, box.y + box.height * 0.6, { steps: 8 });
await page.mouse.up();
await expect(page.getByTestId("save-state")).toContainText("Saved");

// The address bar is the second entry: a cold load of the same job answers
// the same mode, with nothing inherited from the session above.
await page.reload();
await expect(page.getByTestId("readonly-banner")).toContainText(/viewing only/i);

await openProject(page, PROJECT, "batches");
await expect(page.getByTestId("batches-table")).toBeVisible();
});

await test.step("promote the completed batch into the trunk", async () => {
// The trunk carries assets only, and promotion is a **union** against current
// membership — idempotent, with no log entry when nothing changed.
Expand Down
53 changes: 53 additions & 0 deletions frontend/app/e2e/annotate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,59 @@ test("a completed batch's canvas cannot be drawn on, however hard it is asked",
expect(sent.filter((r) => r.method() === "POST" && r.url().includes("/annotations"))).toEqual([]);
});

/**
* The doors #422 left open (#423). The classes region renders in the read-only
* mode — which classes exist stays true there — but its create paths are writes:
* each one opens the add-a-class dialog, and the dialog publishes a real schema
* version from a page that has just said it is a viewer.
*/
test("a completed batch's viewer leaves no door into the add-a-class dialog", async ({ page }) => {
const sent: Request[] = [];
await openJob(page, sent, undefined, { batch: "completed", job: "completed" });
await expect(page.getByTestId("readonly-banner")).toBeVisible();

// The `+` stays on screen — hiding it would be a control that comes and goes —
// but it is a refusal, and it says why.
const add = page.getByTestId("class-add");
await expect(add).toBeDisabled();
await expect(add).toHaveAttribute("title", /completed/i);

// Nothing matches what was typed: the `Create class` row must not appear, and
// the Enter fallthrough that would have created it must be dead too.
await page.getByTestId("class-filter").fill("a-class-nobody-declared");
await expect(page.getByTestId("class-create")).toHaveCount(0);
await page.getByTestId("class-filter").press("Enter");
await expect(page.getByTestId("add-class-dialog")).toHaveCount(0);
});

/**
* The one frame where the read-only mode said nothing at all (#423): the banner
* rendered only while the frame was not skipped — a guard older than the
* correction link that now lives inside it — so a skipped frame in a completed
* batch showed no "viewing only", no route onward, and a skipped notice whose
* "Un-skip it" names a move the wire withholds there.
*/
test("a skipped frame in a completed batch still says viewing only, and names the correction path", async ({
page,
}) => {
const sent: Request[] = [];
await openJob(page, sent, progressStore({ "asset-1": "skipped", "asset-2": "annotated" }), {
batch: "completed",
job: "completed",
});

const banner = page.getByTestId("readonly-banner");
await expect(banner).toBeVisible();
await expect(banner).toContainText(/viewing only/i);
await expect(page.getByTestId("banner-create-correction")).toBeVisible();

// The notice's remedy is not available here, so the banner is the one surface
// that speaks — two banners saying different things about one frame is how a
// person learns to trust neither.
await expect(page.getByTestId("skipped-notice")).toHaveCount(0);
await expect(page.getByTestId("unskip")).toBeDisabled();
});

/**
* The founder's decision in #123, and the half no unit test can reach: **the
* clipboard survives moving to the next frame.**
Expand Down
17 changes: 12 additions & 5 deletions frontend/ui-core/src/annotator/AnnotationPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1763,11 +1763,15 @@ function Workspace({
fail" is what shipped, and what it looked like from the other side was a
working editor that lost your work.

Not rendered for a skipped frame — the notice below is the same fact with
the remedy attached, and two banners saying one thing is how a person
learns to ignore both.
Not rendered for a skipped frame **in an open batch** — the notice below
is the same fact with the remedy attached, and two banners saying one
thing is how a person learns to ignore both. In a closed batch the yield
runs the other way (#423): the notice's Un-skip is a move the wire
withholds there, so this banner — and the correction route it carries —
is the one surface that can still say something actionable. The old
guard predated the correction link and hid it on exactly that frame.
*/}
{readOnly && !skipped && (
{readOnly && (!skipped || closedBecause !== null) && (
<p
className="flex shrink-0 items-center gap-2 border-b border-border bg-muted px-3 py-1.5 text-meta text-muted-foreground"
data-testid="readonly-banner"
Expand Down Expand Up @@ -1807,7 +1811,10 @@ function Workspace({
</p>
)}

{skipped && (
{/* Only while the batch is open: "Un-skip it" is this notice's whole
remedy, and in a closed batch the wire withholds that move — the
read-only banner above speaks for that frame instead (#423). */}
{skipped && closedBecause === null && (
<p
className="flex shrink-0 items-center gap-2 border-b border-destructive/30 bg-destructive/5 px-3 py-1.5 text-meta text-destructive"
data-testid="skipped-notice"
Expand Down
14 changes: 12 additions & 2 deletions frontend/ui-core/src/annotator/ClassRegion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,16 @@ export function ClassRegion({
* create row would put a schema change one stray Enter away from somebody who
* was picking a class. An exact match suppresses it too, so the row never sits
* under the very class it offers to add.
*
* Null while refusing, with the header's `+`: the create paths are writes the
* same way arming is — each ends in a published schema version — so a region
* that refuses its rows and still offers to create is a read-only mode with a
* hole in it (#423).
*/
const creatable =
onAddClass === undefined || query === "" || shown.length > 0 ? null : filter.trim();
onAddClass === undefined || refusal !== undefined || query === "" || shown.length > 0
? null
: filter.trim();

/**
* Enter takes the first match, which is the typeahead the top-bar field had.
Expand Down Expand Up @@ -167,7 +174,10 @@ export function ClassRegion({
className="size-6"
aria-label="Add a class"
data-testid="class-add"
disabled={onAddClass === undefined}
// Refusing closes this door too, with the rows' own sentence on it —
// hiding it would be a control that comes and goes between frames.
disabled={onAddClass === undefined || refusal !== undefined}
{...(refusal === undefined ? {} : { title: refusal })}
onClick={() => onAddClass?.("")}
>
<Plus className="size-4" />
Expand Down
15 changes: 15 additions & 0 deletions frontend/ui-core/src/annotator/classRegion.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,19 @@ describe("the region's refusals", () => {
expect(screen.getByTestId("classes-empty")).toBeDefined();
expect(screen.queryByTestId("class-list")).toBeNull();
});

it("closes every door into the add-a-class dialog while it is refusing", async () => {
// The create paths are writes the same way arming is: a viewer that can
// publish a schema version is a read-only mode with a hole in it (#423).
const onAddClass = vi.fn();
render(mount(3, { refusal: "This batch is completed.", onAddClass }));

const add = screen.getByTestId("class-add");
expect(add.hasAttribute("disabled")).toBe(true);
expect(add.getAttribute("title")).toBe("This batch is completed.");

await userEvent.type(screen.getByTestId("class-filter"), "nothing-declared{Enter}");
expect(screen.queryByTestId("class-create")).toBeNull();
expect(onAddClass).not.toHaveBeenCalled();
});
});
Loading