The client reads capabilities instead of mirroring the kernel (F1, F6) - #305
Merged
Conversation
… the kernel `batchState.ts` carried `canSkip`/`canRestore`, and its own docstring said what they were: "a mirror of two rows of the kernel's `ASSET_PROGRESS_TRANSITIONS`". The mirror reproduced the progress dimension and dropped the batch-state one — `JobService.mark` runs `require_open_batch` first, deliberately, before it even reaches the no-op check — so on an `approved` or `completed` batch the gallery's bulk bar drew both buttons enabled over frames the kernel refuses without looking at their progress at all (audit finding F1). `allowed_actions` landed on the wire with #304, derived in `kernel/domain/capabilities.py` from those same tables. So this deletes the mirrors rather than fixing them: `canSkip`, `canRestore` and `isApprovable` are gone, and `data/capabilities.ts` is the one seam a screen asks. Three call sites moved: - The gallery's approve button, `isApprovable(state)` -> `declares(batch, approve)`. - The gallery's bulk bar, per-frame `allowed_actions` for the counts. On a batch that is closed to writing every list is empty by construction, so instead of two zeroed buttons the bar states the batch-level reason once and disables them with it. The *selection* stays: choosing a set of frames is the first half of making a correction batch out of them. - `BatchesScreen`'s `Lifecycle` chain, which was a fourth hand-mirror — correct today only because those four rows happen to be one-in one-out, and unable to express `promote`, which is not a transition at all. `hasJobs` survives, re-documented and renamed at its call site to `showsProgress`. It answers whether a draft's documented-zero counts are data, which is a display question; it used to double as the permission gate, which is how it came to be true for two states that refuse every write. Action names are constants (`BATCH_ACTION`, `JOB_ACTION`, `ASSET_ACTION`), so the wire's vocabulary has one spelling in the client and a free-string literal cannot be scattered past a rename.
The gating tests the old mirror could not have passed. Each fixture puts frames in a progress state the transition table says is skippable or restorable, and varies only the *batch* — so a client that reproduces one dimension and drops the other fails on the second scenario and not the first. `assetActions` in both fixture modules now takes the batch state, because that is what the server does: `asset_actions` returns `[]` for every frame of a batch that is not `in_annotation`, whatever the frame's own progress is. A mock that declared actions a real server would withhold is a mock that lies about the thing under test. In chromium as well as vitest, on the `refactor-protocol` rule: the claim is about what a person can press, and a `disabled` attribute jsdom reports is not quite the statement that a browser will not activate a control — and the sentence beside it has to be visible. Refusal rendering, for the mutation this touches: a 409 mid-gesture (the batch moving under the press, which is the only way a refusal still reaches here) is asserted to render prose and to keep the kernel's identifier away from the user, to say one sentence for a rule that refused forty frames, to fall through to the server's own message for a code the vocabulary has never heard of, and to still report the frames that did move.
JArmandoAnaya
enabled auto-merge (squash)
August 4, 2026 16:44
JArmandoAnaya
added a commit
that referenced
this pull request
Aug 4, 2026
* feat(ui-core): a finished batch can be corrected Audit gap G6, and the end of the forward-only story. A `completed` batch is immutable as a workflow unit — the kernel gives it no exit and none is coming. The product's answer to "this frame is wrong" had been a dead end dressed three different ways: an annotator that opened fully editable and refused every save (#306 made it a viewer), a bulk bar whose buttons were live and whose every request 409'd (#305 disabled them with a reason), and a sentence naming a correction batch that nothing could create. Each of those named the route onward deliberately, on the grounds that it beats a friendlier lie. This is what they were waiting for. **Scope is a choice and the default is the whole batch.** "Correct this batch" is the ordinary ask, so `all` sends no `asset_ids` at all — the server's own default is the parent's whole membership, and re-listing forty-eight ids to say so would be the client telling the API something it already knows. `selection` is the other ordinary ask, and it is offered only when there *is* one: a scope choice whose second option covers nothing is a choice between doing something and doing nothing. There is deliberately no "filtered set" option. The gallery's segments are a *view*, and a correction cut from whatever happens to be filtered at the moment of pressing is a batch nobody can describe afterwards — where a selection is a thing somebody chose. `defaultCorrectionName` is pure and exported because it is the part with a decision in it: a dialog that opened blank would make the common case cost a sentence of typing, and numbering keeps a chain readable in a listing. The count comes from the caller, which is already holding the project's batch listing. `CorrectionOf` renders lineage as one hop — what this batch corrects, not how deep the chain goes, which is the honest reading of the one field there is. It renders nothing without a parent: "not a correction of anything" is most batches, and a badge saying so on every one would be noise on the many to inform the few. * feat(ui-core): every sentence that named a correction batch now reaches one Four surfaces, and the point is that they agree. **Gallery header** — the screen somebody is on when they find the frame that is wrong. It takes the current selection, so "these three are bad" is one press rather than a second pass inside the new batch. **Batches row** — beside Promote rather than in an overflow menu, because a completed batch has exactly two things left to do and hiding the second is hiding the one somebody is hunting for. The row also renders what it corrects, so a chain stays readable when the table is sorted by something else. **The annotator's read-only banner** links to the gallery rather than opening a dialog, and that is a product call: creating a batch is a curation act, curation lives on the batch view, and a second place batches are made is a second place the rules can drift. The annotator says which way is forward and hands the person to the screen that owns it. **The bulk bar's withheld sentence** gains "Create one", and the selection it is already holding becomes the correction's scope. The gallery holds the dialog's open state rather than `CorrectionButton` holding its own, because that screen has two ways in and two independent dialogs would be two states that can both be true. `CorrectionButton` is controlled when a caller supplies `open`, uncontrolled otherwise — which keeps the Batches row a one-liner. Lineage on both sides is derived from the project's batch listing rather than fetched: how many corrections point at this batch and what this batch points at are a filter and a lookup over one array the screen's siblings already request. ## Tests Twelve vitest cases for the control and its lineage — the two scopes and exactly what each sends, the selection scope absent when nothing is selected, navigation to the child, a refusal rendered as prose with the dialog staying open (one a dialog closes over is one nobody reads), and the suggested name. The round-trip runs in `cycle.spec.ts` against a **real server**, because the two claims worth making are about the kernel rather than about a stub: that the parent is genuinely untouched after a correction is cut, and that the child is a draft carrying the parent's name in its lineage. It re-checks the parent's trunk count afterwards, so "untouched" means what it says rather than only "still completed". Both fixture modules gain `create_correction` on `completed`, because that is what the server declares — a mock withholding it would describe a batch the API cannot produce. `capabilities.test.ts`'s "names every action the wire has" guard caught the missing constant, which is the job it was written for. One recorder fix: the body-capturing stub in `promote.test.tsx` called `JSON.parse` on every non-GET, and `promote` sends no body — so an empty string threw and failed every test in the file for a reason none of them was about.
JArmandoAnaya
added a commit
that referenced
this pull request
Aug 21, 2026
#305) * refactor(ui-core): the client reads capabilities instead of mirroring the kernel `batchState.ts` carried `canSkip`/`canRestore`, and its own docstring said what they were: "a mirror of two rows of the kernel's `ASSET_PROGRESS_TRANSITIONS`". The mirror reproduced the progress dimension and dropped the batch-state one — `JobService.mark` runs `require_open_batch` first, deliberately, before it even reaches the no-op check — so on an `approved` or `completed` batch the gallery's bulk bar drew both buttons enabled over frames the kernel refuses without looking at their progress at all (audit finding F1). `allowed_actions` landed on the wire with #304, derived in `kernel/domain/capabilities.py` from those same tables. So this deletes the mirrors rather than fixing them: `canSkip`, `canRestore` and `isApprovable` are gone, and `data/capabilities.ts` is the one seam a screen asks. Three call sites moved: - The gallery's approve button, `isApprovable(state)` -> `declares(batch, approve)`. - The gallery's bulk bar, per-frame `allowed_actions` for the counts. On a batch that is closed to writing every list is empty by construction, so instead of two zeroed buttons the bar states the batch-level reason once and disables them with it. The *selection* stays: choosing a set of frames is the first half of making a correction batch out of them. - `BatchesScreen`'s `Lifecycle` chain, which was a fourth hand-mirror — correct today only because those four rows happen to be one-in one-out, and unable to express `promote`, which is not a transition at all. `hasJobs` survives, re-documented and renamed at its call site to `showsProgress`. It answers whether a draft's documented-zero counts are data, which is a display question; it used to double as the permission gate, which is how it came to be true for two states that refuse every write. Action names are constants (`BATCH_ACTION`, `JOB_ACTION`, `ASSET_ACTION`), so the wire's vocabulary has one spelling in the client and a free-string literal cannot be scattered past a rename. * test(ui-core): the batch-state dimension, asserted across the matrix The gating tests the old mirror could not have passed. Each fixture puts frames in a progress state the transition table says is skippable or restorable, and varies only the *batch* — so a client that reproduces one dimension and drops the other fails on the second scenario and not the first. `assetActions` in both fixture modules now takes the batch state, because that is what the server does: `asset_actions` returns `[]` for every frame of a batch that is not `in_annotation`, whatever the frame's own progress is. A mock that declared actions a real server would withhold is a mock that lies about the thing under test. In chromium as well as vitest, on the `refactor-protocol` rule: the claim is about what a person can press, and a `disabled` attribute jsdom reports is not quite the statement that a browser will not activate a control — and the sentence beside it has to be visible. Refusal rendering, for the mutation this touches: a 409 mid-gesture (the batch moving under the press, which is the only way a refusal still reaches here) is asserted to render prose and to keep the kernel's identifier away from the user, to say one sentence for a rule that refused forty frames, to fall through to the server's own message for a code the vocabulary has never heard of, and to still report the frames that did move.
JArmandoAnaya
added a commit
that referenced
this pull request
Aug 21, 2026
* feat(ui-core): a finished batch can be corrected Audit gap G6, and the end of the forward-only story. A `completed` batch is immutable as a workflow unit — the kernel gives it no exit and none is coming. The product's answer to "this frame is wrong" had been a dead end dressed three different ways: an annotator that opened fully editable and refused every save (#306 made it a viewer), a bulk bar whose buttons were live and whose every request 409'd (#305 disabled them with a reason), and a sentence naming a correction batch that nothing could create. Each of those named the route onward deliberately, on the grounds that it beats a friendlier lie. This is what they were waiting for. **Scope is a choice and the default is the whole batch.** "Correct this batch" is the ordinary ask, so `all` sends no `asset_ids` at all — the server's own default is the parent's whole membership, and re-listing forty-eight ids to say so would be the client telling the API something it already knows. `selection` is the other ordinary ask, and it is offered only when there *is* one: a scope choice whose second option covers nothing is a choice between doing something and doing nothing. There is deliberately no "filtered set" option. The gallery's segments are a *view*, and a correction cut from whatever happens to be filtered at the moment of pressing is a batch nobody can describe afterwards — where a selection is a thing somebody chose. `defaultCorrectionName` is pure and exported because it is the part with a decision in it: a dialog that opened blank would make the common case cost a sentence of typing, and numbering keeps a chain readable in a listing. The count comes from the caller, which is already holding the project's batch listing. `CorrectionOf` renders lineage as one hop — what this batch corrects, not how deep the chain goes, which is the honest reading of the one field there is. It renders nothing without a parent: "not a correction of anything" is most batches, and a badge saying so on every one would be noise on the many to inform the few. * feat(ui-core): every sentence that named a correction batch now reaches one Four surfaces, and the point is that they agree. **Gallery header** — the screen somebody is on when they find the frame that is wrong. It takes the current selection, so "these three are bad" is one press rather than a second pass inside the new batch. **Batches row** — beside Promote rather than in an overflow menu, because a completed batch has exactly two things left to do and hiding the second is hiding the one somebody is hunting for. The row also renders what it corrects, so a chain stays readable when the table is sorted by something else. **The annotator's read-only banner** links to the gallery rather than opening a dialog, and that is a product call: creating a batch is a curation act, curation lives on the batch view, and a second place batches are made is a second place the rules can drift. The annotator says which way is forward and hands the person to the screen that owns it. **The bulk bar's withheld sentence** gains "Create one", and the selection it is already holding becomes the correction's scope. The gallery holds the dialog's open state rather than `CorrectionButton` holding its own, because that screen has two ways in and two independent dialogs would be two states that can both be true. `CorrectionButton` is controlled when a caller supplies `open`, uncontrolled otherwise — which keeps the Batches row a one-liner. Lineage on both sides is derived from the project's batch listing rather than fetched: how many corrections point at this batch and what this batch points at are a filter and a lookup over one array the screen's siblings already request. ## Tests Twelve vitest cases for the control and its lineage — the two scopes and exactly what each sends, the selection scope absent when nothing is selected, navigation to the child, a refusal rendered as prose with the dialog staying open (one a dialog closes over is one nobody reads), and the suggested name. The round-trip runs in `cycle.spec.ts` against a **real server**, because the two claims worth making are about the kernel rather than about a stub: that the parent is genuinely untouched after a correction is cut, and that the child is a draft carrying the parent's name in its lineage. It re-checks the parent's trunk count afterwards, so "untouched" means what it says rather than only "still completed". Both fixture modules gain `create_correction` on `completed`, because that is what the server declares — a mock withholding it would describe a batch the API cannot produce. `capabilities.test.ts`'s "names every action the wire has" guard caught the missing constant, which is the job it was written for. One recorder fix: the body-capturing stub in `promote.test.tsx` called `JSON.parse` on every non-GET, and `promote` sends no body — so an empty string threw and failed every test in the file for a reason none of them was about.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task 2 of the 2026-08 checkpoint audit remediation. Closes the consumption half of F6 and all of F1; #304 shipped the wire half.
The finding
batchState.tscarriedcanSkip/canRestore, and its own docstring said what they were: "a mirror of two rows of the kernel'sASSET_PROGRESS_TRANSITIONS". The mirror reproduced the progress dimension and dropped the batch-state one.JobService.markrunsrequire_open_batchfirst, deliberately, before it even reaches the no-op short-circuit — so on anapprovedorcompletedbatch the gallery's bulk bar drew both buttons enabled over frames the kernel refuses without looking at their progress at all, sent one request per frame, took N 409s, and told the user "0 moved, N refused" with every reason destroyed by an emptycatch {}.A drifted mirror is not a bug that gets fixed once. It is a bug that returns every time the kernel grows a precondition the client did not hear about. So the mirrors are deleted rather than corrected.
What changed
New
data/capabilities.ts— the one seam a screen asks.declares(resource, action)/declaring(resources, action)readallowed_actions;withheldBecause(state)is the prose for why an action is unavailable. Action names are constants (BATCH_ACTION,JOB_ACTION,ASSET_ACTION) so the wire's vocabulary has one spelling in the client and a free-string literal cannot be scattered past a rename.Deleted:
canSkip,canRestore,isApprovable. Kept and re-documented:hasJobs— it answers whether a draft's documented-zero counts are data, which is a display question. It used to double as the permission gate, which is how it came to be true for two states that refuse every write; its call site is renamedshowsProgressto stop that happening again.Three call sites moved off the mirrors: the gallery's approve button, the gallery's bulk bar, and
BatchesScreen'sLifecyclechain (a fourth hand-mirror — correct today only because those four rows happen to be one-in one-out, and structurally unable to expresspromote, which is not a transition at all).useBulkSetProgresskeeps the refusals.BulkProgressResult.failed: numberbecamerefusals: Refusal[]. The bar reports moved and why the rest were refused, grouped by code — forty frames refused by one rule is one sentence.New
data/refusals.ts— one code→prose vocabulary, product-wide. This is nominally T4's deliverable; creating it here rather than a local map the next task deletes. T4 still owns folding inFINISH_REFUSALS+ the approve-dialog map and migrating the remaining bare-code sites. Two migrated already because this PR touched them:BatchesScreen's promote and start errors.The behaviour on a closed batch
Selection stays. Choosing a set of frames is the first half of making a correction batch out of them, so the screen does not refuse to let anything be picked; the bar states the batch-level reason once and disables its moves with it —
disabled-with-reason over hidden, per theui-capabilitiesskill. Thecompletedsentence names the forward-only correction model, which is the difference between a dead end and a next step. T10 wires that sentence to a real action.Decisions taken autonomously
refusals.tscreated here rather than in T4 — writing a throwaway local prose map and deleting it one task later is worse than landing the shared one early. Scope noted above.approved/completed— the audit's own note ("selection may remain for future correction-batch flows") and the lower-risk of the two readings: hiding it would have to be un-hidden by T10.hasJobskept, not deleted — it is not a legality claim, and deleting it would mean re-deriving "is a draft's zero a number" at three call sites.NoInfer<A>on both helpers, so the generic binds to the resource and not to the action literal. Reason is written in the module.Found, not fixed
completedbatch and has no read-only mode (F2) — T3.unhandledrejectionhandler (F7) — T4.CompleteBatchButtonstill withholds the press client-side while work is outstanding. That is a UX refinement over a declared action, not a legality claim —batch_actionsdeclarescompletefrom the table alone and documents why (the jobs are a read it cannot do). Left as is; the refusal renders.Test plan
bash scripts/check.sh— green (python 2094, ui-core vitest, all linters, import-linter, openapi + generated-client drift, mcp drift, version sync).pnpm --filter @visionset/app e2e— 162 chromium scenarios green, including three new ones. Run locally becausecheck.shdoes not run Playwright.src/data/capabilities.test.ts(24 cases) — the reading, the constants, the withheld sentences, and the grouping.gallery.test.tsx: the batch-state matrix (approved/completedoffer nothing and send nothing;in_annotationoffers both; the sentence names the batch and the way forward; selection survives) and four refusal-rendering cases.gallery.spec.ts(chromium): the same gating for both closed states, plus selection surviving oncompleted.testing/wire.fixtures.ts,e2e/_wire.ts) now thread the batch state intoassetActions, because that is what the server does — a mock declaring actions a real server would withhold is a mock that lies about the thing under test.