fix(schemas): publishing the contract already in force writes nothing

docs/schemas.md

@@ -69,13 +69,41 @@ was created under, so a version read three milestones from now still says exactl
 said when the label was made. A version that could be edited would make that record a
 guess.
 
-Creating a version identical to the active one is allowed. Versions are cheap, and refusing
-a no-op would need an equality rule that then has to defend itself against reordered
-attributes and changed colors.
-
 Schema rows are deleted only as part of deleting their project, through the database's
 `ON DELETE CASCADE` - see [projects.md](projects.md).
 
+## Publishing the contract already in force writes nothing
+
+`create_version` compares the classes it is given against the active version, and when they
+are identical it returns that version and inserts no row:
+
+```python
+first = schemas.create_version(project.id, [SIGN])
+again = schemas.create_version(project.id, [SIGN])
+assert again == first  # one version, not two
+```
+
+It is not a refusal - the call succeeds, and the version the caller holds afterwards is the
+one in force, which is the only thing it asked for. Over HTTP the answer is `201` either
+way: the API declares one 2xx response per operation, and a client that branched on "did
+this succeed" would see no difference in any case.
+
+**Identical means the classes compare equal** - names, geometries, colours, attributes and
+order. That is deliberately *not* the same question as an empty diff:
+[additive versus destructive](#additive-versus-destructive) classifies whether existing
+annotations survive and ignores `color` on purpose, so gating this on the diff would answer
+"saved" to somebody who changed a swatch and then throw the swatch away. Equality implies an
+empty diff and never the reverse, so the diff stays the one definition of *changed in a way
+that matters*.
+
+Only the **active** version is compared. Re-publishing an older version's classes is a real
+change - it is what a revert is - and answering it with that old version would leave the
+newer one in force.
+
+`description` and `provenance` are not part of the comparison, because they are not part of
+the contract. A save that changes only the commit message is a no-op, and the message is not
+recorded: there is no version for it to describe.
+
 ## A version says why it exists, and when
 
 ```python

frontend/ui-core/src/generated/api.ts

@@ -1691,6 +1691,13 @@ export interface paths {
          *
          *     The body is the whole proposed version; versions are never edited in place.
          *
+         *     **Sending the classes that are already in force writes nothing.** The answer
+         *     is the version that was already active, and it is not an error: the version
+         *     a client holds afterwards is the one in force either way, which is the only
+         *     thing it asked for. Identical means the classes match exactly — names,
+         *     geometries, colours, attributes and order — so a colour change is a change
+         *     and does publish a version.
+         *
          *     `description` is this version's commit message — written once, here, and
          *     never afterwards, because a version is immutable and there is no route that
          *     edits one. Blank is legal and comes back as null. `created_at` is stamped by

frontend/ui-core/src/screens/SchemaEditor.tsx

@@ -168,9 +168,41 @@ export interface SchemaEditorProps {
   readonly onDraftChange: (draft: SchemaDraft | null) => void;
 }
 
-/** The comparison `dirty` has always used — deep, and cheap at a schema's size. */
+/**
+ * Two sets of classes describing the same contract, compared as contracts.
+ *
+ * Not `JSON.stringify` over the objects, which is what this used to be. The draft
+ * and the wire describe one class with differently *shaped* objects: a class
+ * added here is a literal in this file's key order, and one that came off the
+ * wire carries every optional field `LabelClassBody` declares — a hand-added
+ * attribute has no `options` key at all where the server sends `null`. Stringify
+ * calls those unequal, and a comparison that answers "changed" about an
+ * unchanged contract is exactly the bug this comparison exists to prevent.
+ *
+ * So the projection, in `LabelClassBody`'s own field order, with every optional
+ * field defaulted the way the wire defaults it. Cheap at a schema's size.
+ */
 function same(a: readonly LabelClassBody[], b: readonly LabelClassBody[]): boolean {
-  return JSON.stringify(a) === JSON.stringify(b);
+  return canonical(a) === canonical(b);
+}
+
+function canonical(classes: readonly LabelClassBody[]): string {
+  return JSON.stringify(
+    classes.map((declared) => [
+      declared.name,
+      declared.geometry,
+      declared.color ?? null,
+      (declared.attributes ?? []).map((attribute) => [
+        attribute.name,
+        attribute.kind,
+        attribute.required ?? false,
+        // Order is authored and part of the contract, so options are compared as
+        // given rather than sorted.
+        attribute.options ?? null,
+        attribute.default ?? null,
+      ]),
+    ]),
+  );
 }
 
 export function SchemaEditor({
@@ -223,7 +255,24 @@ export function SchemaEditor({
   const classes = showing.classes;
   const note = showing.note;
   const failure = publish.isError ? asApiError(publish.error) : null;
-  const dirty = !same(classes, showing.seed);
+  /**
+   * Whether saving would change anything — measured against **the version in
+   * force**, not against the snapshot the draft was seeded from.
+   *
+   * That is the same question `SchemaService.create_version` now answers, so the
+   * two cannot disagree: what the client declines to send is exactly what the
+   * kernel would decline to write.
+   *
+   * Measuring it against `seed` was the defect. The re-base that refreshes
+   * `seed` after a save rides on the callback passed to `publish.mutate`, and
+   * TanStack drops those when the observer's component unmounts — which is what
+   * happens on a project that had no schema, because the invalidated 404 goes
+   * back to `pending` and `SchemaSection` swaps this editor for a loading state
+   * while the refetch flies. The draft came back holding an empty `seed`, read
+   * as dirty, and a second press published a version identical to the first.
+   * `active` is a prop and cannot be missed that way.
+   */
+  const dirty = !same(classes, active?.classes ?? []);
   /**
    * The version that arrived while this draft was being written, or `null`.
    *

frontend/ui-core/src/screens/schemaDraft.test.tsx

@@ -28,6 +28,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import type { JSX, ReactNode } from "react";
 
 import { ApiProvider } from "../data/ApiProvider";
+import { Toaster } from "../primitives/Feedback";
 import { writeToken } from "../data/session";
 import { ProjectScreen } from "./ProjectScreen";
 
@@ -45,7 +46,7 @@ let activeSchema: { project_id: string; version: number; classes: unknown[] };
 /** How many times it was asked, so "the refetch fired" is observed rather than assumed. */
 let schemaReads = 0;
 
-type Answer = { status: number; body?: unknown };
+type Answer = { status: number; body?: unknown; delay?: number };
 let handlers: ((request: Request) => Answer | undefined)[] = [];
 
 beforeEach(() => {
@@ -115,11 +116,18 @@ const DATASET = {
 };
 
 function respond(answer: Answer): Promise<Response> {
-  return Promise.resolve(
+  if (answer.delay !== undefined) {
+    return new Promise((resolve) => setTimeout(() => resolve(built(answer)), answer.delay));
+  }
+  return Promise.resolve(built(answer));
+}
+
+function built(answer: Answer): Response {
+  return (
     new Response(answer.status === 204 ? null : JSON.stringify(answer.body ?? null), {
       status: answer.status,
       headers: { "content-type": "application/json" },
-    }),
+    })
   );
 }
 
@@ -291,5 +299,143 @@ describe("the schema draft survives a version published underneath", () => {
   });
 });
 
+/**
+ * The third way a draft goes wrong, and the only one that reaches the wire.
+ *
+ * The two above are about a draft being *lost*. This one is about a draft whose
+ * baseline is stale: the editor believes there is still something to save, so a
+ * second press of Save publishes a version identical to the one it just made.
+ *
+ * It reproduces only on a project that had **no** schema, and the mechanism is
+ * why: `useActiveSchema` answers 404 there and therefore holds no data, so the
+ * invalidation the save triggers puts that query back into `pending` rather than
+ * leaving it in `error` (TanStack's `fetchState` resets the status whenever
+ * `data === undefined`), `SchemaSection` swaps the editor for a `LoadingState`,
+ * and the `mutate()`-level `onSuccess` that re-bases the draft is dropped with
+ * the unmounted observer. On a project that already had a version the query has
+ * data, nothing unmounts, and the re-base fires — which is the whole of why this
+ * needs its own fixture rather than a line in the block above.
+ *
+ * **The `delay` on that stub is load-bearing, and it is the honest model rather
+ * than a contrivance.** A stubbed `fetch` that resolves in the same microtask
+ * never lets React commit the pending render, so the editor never unmounts and
+ * the defect vanishes — measured: with an instant stub this test passed against
+ * the unfixed code. Every real request takes longer than zero.
+ */
+describe("saving twice with nothing edited in between", () => {
+  it("issues one request on a project that had no schema", async () => {
+    let published: { project_id: string; version: number; classes: unknown[] } | null = null;
+    let posts = 0;
+    // Stateful on purpose: the 404 is the *state* this defect needs, and a frozen
+    // stub would answer 404 forever and never let the save land.
+    handlers.push((request) => {
+      const path = new URL(request.url).pathname;
+      if (request.method === "GET" && /\/schema$/.test(path)) {
+        schemaReads += 1;
+        return published === null
+          ? { status: 404, body: { code: "SCHEMA_NOT_FOUND", message: "no schema yet" } }
+          : { status: 200, body: published, delay: 5 };
+      }
+      if (request.method === "POST" && /\/schema\/versions$/.test(path)) {
+        posts += 1;
+        published = {
+          project_id: PROJECT,
+          version: (published?.version ?? 0) + 1,
+          classes: [PEDESTRIAN],
+        };
+        return { status: 201, body: published };
+      }
+      return undefined;
+    });
+
+    render(
+      mount(
+        <>
+          <ProjectScreen projectId={PROJECT} tab="schema" />
+          <Toaster />
+        </>,
+      ),
+    );
+    await screen.findByTestId("schema-editor");
+    // Class zero, not two: a project with no schema seeds an empty draft, so
+    // `draftAClass` above — which counts from the fixture's two — cannot be used.
+    await userEvent.click(screen.getByTestId("add-class"));
+    await userEvent.type(screen.getByTestId("class-name-0"), "pedestrian");
+
+    await userEvent.click(screen.getByTestId("save-schema"));
+    await waitFor(() =>
+      expect(screen.getByTestId("schema-status").textContent).toContain("Version 1 active"),
+    );
+
+    await userEvent.click(screen.getByTestId("save-schema"));
+
+    // `DESIGN.md`: pressing Save with nothing to save answers, and issues no
+    // request. The count is the claim; the toast is what the person sees.
+    expect(posts).toBe(1);
+    expect(screen.getByTestId("schema-status").textContent).not.toContain("unsaved");
+    expect(await screen.findByText("No changes to save")).toBeDefined();
+  });
+
+  /**
+   * The same defect through the comparison rather than through the baseline.
+   *
+   * A class added here is a literal in `SchemaEditor`'s own key order and a
+   * *new attribute* has no `options` key at all, where the wire sends every
+   * optional field `AttributeBody` declares. `JSON.stringify` over those two
+   * objects is unequal for one identical contract — so the draft reads as dirty
+   * against the version it just published, and presses Save again. That is why
+   * the comparison is a projection and not a stringify, and this is the test
+   * that notices if it goes back.
+   */
+  it("compares a hand-built attribute with the wire's own spelling of it", async () => {
+    let published: { project_id: string; version: number; classes: unknown[] } | null = null;
+    let posts = 0;
+    handlers.push((request) => {
+      const path = new URL(request.url).pathname;
+      if (request.method === "GET" && /\/schema$/.test(path)) {
+        schemaReads += 1;
+        return published === null
+          ? { status: 404, body: { code: "SCHEMA_NOT_FOUND", message: "no schema yet" } }
+          : { status: 200, body: published, delay: 5 };
+      }
+      if (request.method === "POST" && /\/schema\/versions$/.test(path)) {
+        posts += 1;
+        published = {
+          project_id: PROJECT,
+          version: (published?.version ?? 0) + 1,
+          // What the server actually sends back: `AttributeBody` in full, with
+          // the `options` the editor's own literal never carries.
+          classes: [
+            {
+              ...PEDESTRIAN,
+              attributes: [
+                { name: "occluded", kind: "string", required: false, options: null, default: null },
+              ],
+            },
+          ],
+        };
+        return { status: 201, body: published };
+      }
+      return undefined;
+    });
+
+    render(mount(<ProjectScreen projectId={PROJECT} tab="schema" />));
+    await screen.findByTestId("schema-editor");
+    await userEvent.click(screen.getByTestId("add-class"));
+    await userEvent.type(screen.getByTestId("class-name-0"), "pedestrian");
+    await userEvent.click(screen.getByTestId("add-attribute-0"));
+    await userEvent.type(screen.getByTestId("attr-name-0-0"), "occluded");
+
+    await userEvent.click(screen.getByTestId("save-schema"));
+    await waitFor(() =>
+      expect(screen.getByTestId("schema-status").textContent).toContain("Version 1 active"),
+    );
+
+    await userEvent.click(screen.getByTestId("save-schema"));
+
+    expect(posts).toBe(1);
+  });
+});
+
 const PEDESTRIAN = { name: "pedestrian", geometry: "bbox", color: null, attributes: [] };
 const TRAFFIC_LIGHT = { name: "traffic light", geometry: "bbox", color: null, attributes: [] };

openapi.json

@@ -10147,7 +10147,7 @@
         ]
       },
       "post": {
-        "description": "Append the next version of the project's schema.\n\nThe body is the whole proposed version; versions are never edited in place.\n\n`description` is this version's commit message \u2014 written once, here, and\nnever afterwards, because a version is immutable and there is no route that\nedits one. Blank is legal and comes back as null. `created_at` is stamped by\nthe server, so it is a response field and not a request one.\n\n`provenance` says which kind of work is publishing: `curated` for a version\nauthored in a schema editor, `annotation` for one that fell out of adding a\nclass while labeling. It is stored exactly as sent and never inferred, so a\nclient with no opinion omits it and the version records null \u2014 which readers\ngroup with `curated`. It gates nothing and changes no behaviour; it exists so\na version history can separate the milestones from the runs.\n\nRemoving a class or an attribute answers 409 `DESTRUCTIVE_SCHEMA_CHANGE`\nuntil `allow_destructive=true` says so deliberately. If annotations already\nexist under an affected class it answers 409 `SCHEMA_CHANGE_WOULD_ORPHAN`\ninstead, and **no flag overrides that one** \u2014 which is why a client branches\non `code` and not on the status.",
+        "description": "Append the next version of the project's schema.\n\nThe body is the whole proposed version; versions are never edited in place.\n\n**Sending the classes that are already in force writes nothing.** The answer\nis the version that was already active, and it is not an error: the version\na client holds afterwards is the one in force either way, which is the only\nthing it asked for. Identical means the classes match exactly \u2014 names,\ngeometries, colours, attributes and order \u2014 so a colour change is a change\nand does publish a version.\n\n`description` is this version's commit message \u2014 written once, here, and\nnever afterwards, because a version is immutable and there is no route that\nedits one. Blank is legal and comes back as null. `created_at` is stamped by\nthe server, so it is a response field and not a request one.\n\n`provenance` says which kind of work is publishing: `curated` for a version\nauthored in a schema editor, `annotation` for one that fell out of adding a\nclass while labeling. It is stored exactly as sent and never inferred, so a\nclient with no opinion omits it and the version records null \u2014 which readers\ngroup with `curated`. It gates nothing and changes no behaviour; it exists so\na version history can separate the milestones from the runs.\n\nRemoving a class or an attribute answers 409 `DESTRUCTIVE_SCHEMA_CHANGE`\nuntil `allow_destructive=true` says so deliberately. If annotations already\nexist under an affected class it answers 409 `SCHEMA_CHANGE_WOULD_ORPHAN`\ninstead, and **no flag overrides that one** \u2014 which is why a client branches\non `code` and not on the status.",
         "operationId": "create_schema_version",
         "parameters": [
           {

src/visionset/cli/schemas.py

@@ -96,9 +96,10 @@ def schema_apply(
 ) -> None:
     """Add the next schema version from a JSON file.
 
-    Versions are 1..N and none of them ever changes, so this always *adds* one —
-    there is no edit and no rollback, and applying an unchanged document still
-    makes a new version.
+    Versions are 1..N and none of them ever changes, so this *adds* one — there
+    is no edit and no rollback. Applying the document already in force adds
+    nothing and prints the version that was already there, so re-running this in
+    a script is free.
 
     A change that removes a class or an attribute, or narrows one, is refused
     until `--allow-destructive`. A change that would orphan annotations already

src/visionset/kernel/services/schema_service.py

@@ -176,6 +176,26 @@ def create_version(
         1..N with no gaps and no reuse. Nothing is edited: this always inserts,
         which is what makes an old version safe to read forever.
 
+        **Publishing the contract that is already in force writes nothing** and
+        returns the active version unchanged. A history whose entries include
+        "and then nothing changed" is a history somebody has to read past, and a
+        schema editor that answers "saved" is impossible to tell from one that
+        answered "already saved" — so the two are made the same thing.
+
+        Identical means the classes compare equal, which — the models being
+        frozen — covers names, geometries, colours, attributes and order. That is
+        deliberately *not* the same question as an empty ``diff_classes``:
+        ``domain/schema_diff.py`` classifies whether existing annotations survive
+        and ignores ``color`` on purpose, so gating this on the diff would answer
+        "saved" to somebody who changed a swatch and then discard the swatch.
+        Equality implies an empty diff, never the other way round, so the diff
+        remains the one definition of *changed in a way that matters* and no
+        second one is written here.
+
+        Only the **active** version is compared. Re-publishing an older
+        version's contract is a real change — it is what a revert is — and
+        answering it with that old version would leave the newer one in force.
+
         ``description`` is the version's **commit message**: written here and
         never afterwards, because there is no ``update`` on this service and the
         model is frozen. Blank is legal and stored as ``None`` — an empty commit
@@ -216,6 +236,9 @@ def create_version(
                 _require_coherent(proposed)
 
                 active = self.active(uow, project_id)
+                if active is not None and proposed == active.classes:
+                    return active
+
                 diff = diff_classes(() if active is None else active.classes, proposed)
                 if diff.is_destructive:
                     self._refuse_narrowing(uow, project_id, diff, allow_destructive)