Skip to content

fix(wire): withdraw the BatchAction.DELETE declaration - #356

Merged
JArmandoAnaya merged 1 commit into
mainfrom
fix/331-withdraw-delete-declaration
Aug 5, 2026
Merged

fix(wire): withdraw the BatchAction.DELETE declaration#356
JArmandoAnaya merged 1 commit into
mainfrom
fix/331-withdraw-delete-declaration

Conversation

@JArmandoAnaya

Copy link
Copy Markdown
Contributor

What changed

allowed_actions declared delete on draft, approved and in_annotation, but no route, MCP tool or control could perform it. The declaration is withdrawn: BatchAction loses its DELETE member and BATCH_GATES its DELETABLE_STATES entry.

BatchService.delete and DELETABLE_STATES are kept. The rule survives and the SDK still enforces it — including BatchImmutable on a completed batch. What is withdrawn is the promise, not the capability.

Why option 1 (withdraw) rather than routing it

Recorded on #331 by Armando, 2026-08-05:

withdraw the DELETE declaration — the wire stops promising an action no surface performs; the declaration returns together with the route and its UI, when a caller asks for them. Supersedes any prior direction discussed elsewhere. Rationale: under ui-capabilities, a declared action is a promise a conforming client must try to keep; with 0.1.0 in close-out, withdrawing is the smaller, fully reversible step.

This was a declared-but-unreachable orphan: the capability contract failing from the wire side. The ui-capabilities skill's rule is that the frontend renders what the wire declares, so an action named in allowed_actions obliges every conforming client to offer a control for it. Withdrawing is one enum member and is reversed by adding it back in the same change as the route.

The contract test needed no edit, and that is the point

tests/kernel/test_capabilities.py derives its cases from BatchAction itself, so it can only ever check members that exist — it cannot assert an absence. Two things follow, and both are in this diff rather than assumed:

  • The pre-existing test_every_action_is_decided_by_exactly_one_source asserts set(BATCH_MOVES) | set(BATCH_GATES) == set(BatchAction). The suite therefore structurally forbids "keep the vocabulary, drop the gate" — withdrawing the member and its gate together was the only shape that passes.
  • One test is added, test_no_state_declares_delete_because_nothing_can_perform_it, which names the withdrawn string directly. Proving a member is gone takes spelling it.

Where the rationale lives

In a # comment above BatchAction, not in its docstring: FastAPI copies a docstring verbatim into openapi.json as the schema description. Writing it as a docstring first produced a nine-line RST-flavoured essay in the public contract with double backticks rendering literally. The docstring stays the one short sentence a client should read.

The second roster — caught only by the browser suite

The withdrawal broke the e2e suite, and tsc could not see it. There are two transcriptions of the batch action table:

  • frontend/ui-core/src/testing/wire.fixtures.ts — typed BatchAction, so removing the member made it a compile error.
  • frontend/app/e2e/_wire.ts — typed readonly string[], so it compiled fine and served "delete" to the stub routes. The generated checks.ts then rejected the payload inside a hook, which surfaces as a query that never resolves: every gallery spec timed out with "element(s) not found" rather than an assertion naming the cause.

Both are now correct. This is the multi-roster trap in the refactor protocol's sweep rule — the phrase grep found one copy and the type system found one copy, and neither found the same one.

Found, not fixed

  • frontend/app/e2e/_wire.ts is held true by nothing. Its own docstring claims tests/kernel/test_capabilities.py holds its values true; that test does not read this file. Typing it against the generated BatchAction union would convert this whole failure class from a 20-second timeout into a compile error, but @visionset/ui-core exports only its package root, so it would mean widening the public API — a design change beyond this task. Worth an issue.
  • Stale remote branch fix/317-ingest-failure-name (PR kernel: an ingest failure names the item, not the server's directory (#317) #321, squash-merged 2026-08-05). Pruned separately as cleanup, not by this PR.

batch-lifecycle skill — verified, unchanged

The task called for adding the live-trunk line if missing. It is not missing: #355 added it in decision 8 — "The projection is live: an edit inside an open batch reaches the trunk on save, not on promotion. Releases are unaffected — the manifest is a frozen blob." No skill change.

Test plan

Full scripts/check.sh coverage, run in stages under the harness ceiling. Every exit code recorded:

Stage Result Exit
pytest tests/kernel tests/architecture tests/formats tests/jobs 1457 passed, 2 skipped 0
pytest tests/server tests/cli tests/mcp tests/scripts tests/packaging tests/test_versioning.py 1092 passed, 1 skipped 0
pytest tests/examples 41 passed 0
ruff check . (whole tree, post-rebase) All checks passed 0
ruff format --check . 302 files formatted 0
mypy src/visionset no issues, 132 files 0
lint-imports 3 kept, 0 broken 0
pnpm -r build ok 0
pnpm test 768 annotator + 525 ui-core + node gates 0
pnpm -r lint ok 0
pnpm generate:client:check api.ts and checks.ts match openapi.json 0
pnpm version:check all at 0.0.1-beta.2 0
playwright test (annotator + app e2e) 185 passed 0
playwright test -c playwright.cycle.config.ts (real server) 1 passed 0

2590 python tests. The pytest split was derived from ls tests/ at run time.

Rebased onto 6c23930 (#355). docs/batches.md was the expected conflict site; git merged it without a textual conflict, so the merged section was re-read end to end — #355's promotion-semantics text (lines 180–247) is intact and the withdrawal paragraph sits after the declaration table, forward-referencing the ## Deleting a batch section that documents the retained SDK method.

Closes #331

`allowed_actions` named `delete` on three batch states, but no route, MCP
tool or control could perform it. Under the `ui-capabilities` contract the
frontend renders what the wire declares, so a declared action obliges every
conforming client to offer it — naming one nothing can carry out makes the
wire the source of a control that cannot work.

`BatchService.delete` and `DELETABLE_STATES` are kept: the rule survives and
the SDK still enforces it. What is withdrawn is the promise. The declaration
comes back when the route does, in the same change.

The rationale sits in a comment above `BatchAction` rather than in its
docstring, because FastAPI copies a docstring verbatim into `openapi.json` as
the schema's `description`.
@JArmandoAnaya
JArmandoAnaya enabled auto-merge (squash) August 5, 2026 12:33
@JArmandoAnaya
JArmandoAnaya merged commit 9d026f8 into main Aug 5, 2026
14 checks passed
@JArmandoAnaya
JArmandoAnaya deleted the fix/331-withdraw-delete-declaration branch August 5, 2026 12:41
JArmandoAnaya added a commit that referenced this pull request Aug 5, 2026
…g on it (#361)

A branch, worktree, PR or "unpushed work" a prompt describes is a snapshot,
not a fact. #228 was handed over as a local-only branch with no remote and no
PR; another session had pushed, merged and cleaned it up inside the hour, so
the only work left was a `git worktree prune`. #356's predicted rebase
conflict had already dissolved the same way.

The rule sits with worktree setup, because that is where a stale premise is
cheapest to catch: one `git fetch --prune`, one `gh pr list --state all
--head`, one `gh issue view --json state,closedAt`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kernel/api: BatchAction.DELETE is declared on three states and reachable from none

1 participant