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
27 changes: 18 additions & 9 deletions docs/ui.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,10 @@ one guards emitting an **incomplete copy of something that stays intact**. The
kernel never catches the three together and neither does the UI: three dialogs,
three questions.

There is no pre-export validation route, so consent is the schema editor's shape:
attempt → read `LOSSY_EXPORT_NOT_CONSENTED` off the 409 → ask → retry with the flag.
There is no pre-export validation route, so consent here is attempt-shaped: attempt → read
`LOSSY_EXPORT_NOT_CONSENTED` off the 409 → ask → retry with the flag. The schema editor does
not have this shape - it previews first - and the difference is exactly the routed preview
that export lacks.
`FormatOut.lossy` makes the question predictable in advance, because lossiness is
declared by the **format** - a bbox-only format loses a polygon whether or not
today's dataset holds one.
Expand Down Expand Up @@ -844,13 +846,20 @@ which is the lost update `STALE_WRITE` exists to prevent, and "Close" would leav
editor showing a draft the server no longer recognises. Reloading is the only remedy that
does not either lose work or leave the screen lying.

`SchemaService.preview` **is** routed, at `POST .../schema/preview` - it answers both gates
the publish itself would, `is_destructive` and `is_refused`, without writing anything. The
editor does not call it: nothing is locked between a preview and a publish, so somebody
could label a class in the gap and turn a preview that looked safe into a refusal, and a
call the editor never makes cannot disagree with the publish that follows it. The publish's
own 409 is what actually decides, and making that refusal legible - not pre-empting it - is
the editor's whole job here, exactly as `SchemaEditor`'s own docstring states it.
`SchemaService.preview` is routed at `POST .../schema/preview` - it answers both gates the
publish itself would, `is_destructive` and `is_refused`, without writing anything - and
**the editor calls it**, once before a class leaves the draft and once before a publish.
Both are therefore answered before any publish is sent. A class that already
carries labels never reaches a publish request at all: it gets one terminal dialog naming
the annotations and assets that block it, counted, and no button that starts a save the
dialog knows will be refused. A class that carries none gets one confirmation, which names
how many classes narrow and states that nothing already labeled is invalidated.

That does not demote the 409. Nothing is locked between a preview and the publish, so
somebody can label a class in the gap and turn a preview that looked safe into a refusal -
which is why the publish's own refusal stays authoritative, and why it renders through the
same blocker view the preview feeds rather than through a second presentation that could
drift from it. The preview removes a doomed round trip; it does not decide.

`compare` **is** routed since #231, and it answers the neighbouring question - what
two *published* versions did to each other. The version navigator uses it, and never
Expand Down
4 changes: 3 additions & 1 deletion frontend/ui-core/src/screens/DatasetScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
* and the UI keeps them apart too: the delete dialog, the schema dialog and this
* one are three.
*
* There is no pre-export validation route, so consent is the schema editor's shape:
* There is no pre-export validation route, so consent here is attempt-shaped:
* attempt, read `LOSSY_EXPORT_NOT_CONSENTED` off the 409, ask, retry with the flag.
* The schema editor does not have this shape — it previews first — and the
* difference is exactly the routed preview that export lacks.
* `FormatOut.lossy` is what makes the question predictable — it is declared by the
* *format*, because a bbox-only format loses a polygon whether or not today's
* dataset holds one.
Expand Down
48 changes: 36 additions & 12 deletions frontend/ui-core/src/screens/SchemaEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,19 @@
* one is not — so a TypeScript copy would drift, and the drift would read as a
* screen calling a change safe that the API then refuses.
*
* ## A preview exists, and it is advisory rather than authoritative
* ## The preview leads, and the publish still decides
*
* `compare` answers what two *published* versions did to each other. `POST
* .../schema/preview` answers the harder question — what publishing *this*
* draft would do — and is routed: `SchemaService.preview` is not the missing
* method a comment here used to claim. Routing it does not remove the need for
* the refusal surface below, though: a preview is advisory because nothing is
* locked between it and a publish, so the two can disagree by the time Save is
* actually pressed. The publish's own 409 stays the authoritative answer, and
* making that refusal legible — not pre-empting it — is the editor's actual job.
* .../schema/preview` answers the harder question — what publishing *this* draft
* would do — and the editor calls it twice: before a class leaves the draft, and
* before a publish. That is what removes the round trip that was doomed before it
* was sent, and it is why a class already carrying labels is refused in one
* terminal dialog rather than after two confirmations that contradict it.
*
* It does not make the preview authoritative. Nothing is locked between a preview
* and the publish, so somebody can label a class in the gap; the publish's own 409
* is still the answer, and it renders through the same blocker view the preview
* feeds, so the two paths cannot drift apart.
*
* **Two 409s, and only one is retryable.** This is the exact case `docs/api.md`
* exists for:
Expand Down Expand Up @@ -1250,7 +1253,26 @@ function describeDestructiveClasses(classes: readonly string[]): string {
return `${named.slice(0, -1).join(", ")}, and ${named.at(-1) ?? "another class"}`;
}

/** Retryable: a publishable preview says the change narrows the contract. */
/**
* Retryable: a publishable preview says the change narrows the contract.
*
* The counts are the point. A confirmation that asks "are you sure?" without
* saying what yes costs is a speed bump, and the two numbers worth saying are
* both already measured: how many classes narrow, and how many annotations are
* at risk. The second is always zero here — a preview carrying blockers is
* refused outright and never reaches this dialog — and a measured zero is the
* reason the publish is offered at all, so it is stated rather than implied.
*
* It says nothing about what becomes of annotations *after* the publish. An
* open batch pinned to the outgoing version can still write the removed class,
* and whether that is tolerated or forbidden is not something this dialog is
* entitled to answer.
*
* A shape removed from a class and the class itself removed are indistinguishable
* here: the wire's change record carries the class name but not the geometry, so
* the copy counts classes, which is true of both, rather than guessing which
* happened.
*/
function DestructiveDialog({
preview,
pending,
Expand All @@ -1268,11 +1290,13 @@ function DestructiveDialog({
<DialogContent data-testid="destructive-dialog">
<DialogTitle>This narrows the schema</DialogTitle>
<DialogDescription>
This change narrows the schema for {describeDestructiveClasses(destructiveClasses)}.
<span className="tabular-nums">{formatCount(destructiveClasses.length)}</span>{" "}
{destructiveClasses.length === 1 ? "class narrows" : "classes narrow"}:{" "}
{describeDestructiveClasses(destructiveClasses)}.
</DialogDescription>
<DialogDescription>
Existing annotations are not touched. Saving anyway publishes the new version and leaves
earlier ones exactly as they are — a version is immutable.
No existing annotation becomes invalid — that is why this can be published at all.
Publishing adds a new version; earlier versions keep what they declared.
</DialogDescription>
<DialogFooter>
<Button variant="secondary" onClick={onCancel}>
Expand Down
43 changes: 43 additions & 0 deletions frontend/ui-core/src/screens/screens.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,14 @@ describe("the schema editor", () => {
const dialog = await screen.findByTestId("destructive-dialog");
expect(dialog.textContent).toContain("lane");
expect(dialog.textContent).not.toContain("internal wording must not appear");
// The blast radius, counted: DESIGN.md requires a confirmation to name what
// it costs. `blockers` is empty on every preview that reaches this dialog,
// so the zero is measured rather than assumed.
expect(dialog.textContent).toContain("1 class narrows");
expect(dialog.textContent).toContain("No existing annotation becomes invalid");
// The removed promise. It described what happens to annotations *after* the
// publish, which the kernel has not decided, so the dialog stops claiming it.
expect(dialog.textContent).not.toContain("Existing annotations are not touched");
expect(
sent.some(
(request) =>
Expand Down Expand Up @@ -550,6 +558,41 @@ describe("the schema editor", () => {
expect(new URL(publishRequest?.url ?? "").searchParams.get("allow_destructive")).toBe("true");
});

it("counts every narrowing class in the confirmation", async () => {
projectWithSchema();
handlers.push((request) => {
if (request.method !== "POST" || !new URL(request.url).pathname.endsWith("/schema/preview")) {
return undefined;
}
return {
status: 200,
body: {
is_refused: false,
blockers: [],
diff: {
is_destructive: true,
destructive_classes: ["lane", "sign"],
changes: [
{ kind: "destructive", label_class: "lane", attribute: null, detail: "class 'lane' removed" },
{ kind: "destructive", label_class: "sign", attribute: null, detail: "class 'sign' removed" },
],
},
},
};
});

render(mount(<ProjectScreen projectId={PROJECT} tab="schema" />));
await screen.findByTestId("schema-editor");
await removeClass(1);
await userEvent.click(screen.getByTestId("save-schema"));

const dialog = await screen.findByTestId("destructive-dialog");
expect(dialog.textContent).toContain("2 classes narrow");
expect(dialog.textContent).toContain("lane");
expect(dialog.textContent).toContain("sign");
expect(dialog.textContent).toContain("No existing annotation becomes invalid");
});

it("prevents repeated confirmation while its second preview is pending", async () => {
projectWithSchema();
let previews = 0;
Expand Down
Loading