Make promotion observable (F5, F17, F18) by JArmandoAnaya · Pull Request #308 · Robomous/VisionSet

JArmandoAnaya · 2026-08-04T19:00:57Z

Task 5 of the 2026-08 checkpoint audit remediation. Closes F5, F17, F18. First task in the run with a wire change.

The finding

Promoting a completed batch succeeds. It always did. What a person could observe afterwards was the word "Promoted" on the button they had just pressed and nothing else — and nothing else was structurally possible:

promotion is not a transition, so the batch stays completed;
no read model recorded that anything had entered the trunk;
the response — an AssetPage of the assets this press actually promoted — was discarded unread;
the invalidations refetched data that was byte-identical for that screen.

So three outcomes were indistinguishable: promoted 3 of 48 (the real reported shape, 3 annotated and 45 skipped), promoted nothing because it was already done (promotion is a union, so a second press legitimately moves zero), and the press did nothing at all. A user seeing no change concludes the third, which is the only one that was never true.

Wire (F17) — `promoted_asset_count` on `BatchOut`

How many of this batch's assets are in the trunk right now. Current membership rather than a promotion log: a curator removing an asset takes it back out, and "how much of this batch is in the dataset" is the question anybody looking at a batch is actually asking. Derived per call, no migration — Release.asset_count is the frozen counterpart and belongs to the release.

The cost model is why it is a parameter. promoted is passed into the projection rather than read inside it, so a listing of twenty batches is one extra query rather than twenty: batch.asset_ids is already in memory (it is what asset_count counts) and the rest is a set intersection. Keyword-only with no default — a default would report zero promoted for a batch nobody checked, which is a number that looks like an answer and is not one.

New kernel read DatasetService.member_asset_ids is the cheap half of assets: that one resolves every member to an Asset because its callers render them; this one answers is this in the trunk, which needs the id alone.

Published on all four surfaces in the same change — REST, visionset.wire, MCP, CLI --json.

UI (F5) — two numbers, because neither alone is the answer

The response says what this press did and cannot be recovered afterwards. promoted_asset_count says what is in the trunk now and is the only half that survives a reload. A screen with only the first forgets; a screen with only the second cannot tell a fresh promotion from an old one.

promotionSummary is the pure part, and the decision in it is not obvious: zero promoted is not a failure, and it has two different causes. Zero moved with assets in the trunk is "Already in the dataset"; zero moved with nothing in the trunk is a batch of skipped frames. The skipped count gets its own clause because "Promoted 3" over a batch of 48 reads as a failure unless somebody says where the other 45 went.

The button no longer carries the outcome in its label. A flip is not a report — and it made a second press look forbidden when it is merely a no-op.

F18 — the gallery can promote now

PromoteButton is shared, not copied. The gallery is the screen somebody is actually on when they finish a batch, and it had no promote control in any state and no link to the dataset — so a person could settle forty-eight frames there and have nowhere to put them. Both screens render the same control, capability-gated on promote, and a successful promotion offers the way onward to where the work landed (the information-architecture rule that the dataset is one click from anywhere it is relevant).

Decisions taken autonomously

promoted_asset_count over a last_promoted_at timestamp. The audit offered either. A count answers "did my work reach the dataset" directly; a timestamp needs a DatasetChange query the persistence port cannot express (Repository.list takes one parent_id) and would have meant a new port method or a migration.
Current membership, not a promotion log. A curator's removal takes an asset back out. The alternative — "was ever promoted" — would keep saying yes about work somebody deliberately removed.
Passed in, not read per batch. One query per request instead of one per batch; stated in the model's comment so the next person does not "simplify" it.
The label stops reporting. Freed so a second press reads as available, which it is.

Found, not fixed

accepted is still unreachable in the UI — T3 gated Accept correctly on review_pending, and nothing produces review_pending yet (F24). T6, next.
The dataset screen still has no "remove asset" control although the route exists; pre-existing scope fact recorded in that module's docstring.

Test plan

All four suites, since scripts/check.sh runs none of the browser ones:

bash scripts/check.sh — green (2109 python tests, ui-core vitest, linters, import-linter, openapi + generated-client drift, mcp drift, version sync).
CI=1 npx playwright test — 170 green.
CI=1 npx playwright test -c playwright.cycle.config.ts — green.

New kernel tests (6) for member_asset_ids, including that it agrees with assets, that a removal leaves the set, and that promoting twice does not double it. New server tests (6) for the count: zero before, 3 after, 2 when one frame was skipped, unchanged on a repeat press, present in the listing, and reduced by a curator's removal. New promote.test.tsx (11) covering all three outcomes and the refusal. Gallery gains capability-gated promote coverage.

The cycle suite caught a real one: it asserted the button's label flips to "Promoted" — the exact non-feedback F5 removes. Rewritten to assert the sentence, plus a new step that reloads the page and asserts the trunk count survives, which is the half the response cannot provide.

Thirteen mock files gained promoted_asset_count — a new required wire field breaks hand-written mocks at runtime, which is the gotcha #304 recorded.

`DatasetService.assets` resolves every member to an `Asset` because its callers render them. The question the wire needed is *is this in the trunk*, which needs the id alone — so `member_asset_ids` skips one lookup per member, and over a dataset of fifty thousand that is the whole cost of the call. A **set**, and returned rather than answered per id, because the caller that wanted this asks about a batch's worth of assets at once: how much of a completed batch has reached the trunk is a question about an intersection, and asking it one id at a time is the shape that turns one read into N. Current membership, never a promotion log. A curator removing an asset takes it out of the answer, which is right for the question anybody asks it — "is my work in the dataset" is about now. `member_asset_ids_of` sits beside `assets_of` for the reason that one sits beside this service: a second walk of `dataset_member` written elsewhere is a second chance to disagree with this one. Unlike `assets_of` it cannot raise `WorkspaceCorrupt` — a member naming an asset that is gone is still a member, and answering "which ids are in" does not require the rows behind them. No migration: derived per call, like every other count that describes now.

Promotion is not a transition — the batch stays `completed` — and no read model recorded that anything had entered the trunk. So a client could not tell "promoted 3 of 48" from "promoted nothing because it was already done" from "the press did nothing at all", and the third is what a user concludes (audit F17). `promoted_asset_count` on `BatchOut` is how many of this batch's assets are in the trunk **right now**. Current membership rather than a promotion log: a curator removing an asset takes it back out, and "how much of this batch is in the dataset" is the question anybody looking at a batch is actually asking. Derived per call and never stored — `Release.asset_count` is the frozen counterpart and belongs to the release. **The cost model is the reason it is a parameter.** `promoted` is passed into the projection rather than read inside it, so a listing of twenty batches is one extra query rather than twenty: `batch.asset_ids` is already in memory (it is what `asset_count` counts) and the rest is a set intersection. Keyword-only with no default, because a default would report zero promoted for a batch nobody checked — a number that looks like an answer and is not one. Published on all four surfaces in the same change: REST, the `visionset.wire` projection, MCP and the CLI's `--json`. The contract test's pair now uses a `promoted` set that actually intersects — a count of zero would have agreed with itself even if the intersection were wrong.

Audit findings F5 and F18. The call always worked. What a person could observe afterwards was the word "Promoted" on the button they had just pressed and **nothing else** — and nothing else was structurally possible: promotion is not a transition, so the batch stays `completed`, and the response carrying *the assets this press actually promoted* was discarded unread. Three outcomes were indistinguishable: 1. promoted 3 of 48 — the real shape reported, 3 annotated and 45 skipped; 2. promoted nothing because it was already there (promotion is a **union**, so a second press legitimately moves zero); 3. the press did nothing at all. A user seeing no change concludes (3), which is the only one that was never true. `promotionSummary` is the pure part of telling them apart, and the decision in it is not obvious: **zero promoted is not a failure**, and it has two different causes. Zero moved with assets in the trunk is "already there"; zero moved with nothing in the trunk is a batch of skipped frames, which `PROMOTABLE_PROGRESS` excludes on purpose. The skipped count gets a sentence because "Promoted 3" over a batch of 48 reads as a failure unless somebody says where the other 45 went. **Two numbers, because neither alone is the answer.** The response says what this press did and cannot be recovered afterwards; `promoted_asset_count` says what is in the trunk now and is the only half that survives a reload. The button no longer carries the outcome in its label. A flip is not a report, and it also made a second press look forbidden when it is merely a no-op. `PromoteButton` is shared rather than copied, and that is what closes F18: the gallery — the screen somebody is actually on when they finish a batch — had no promote control in any state and no link to the dataset, so a person could settle forty-eight frames and have nowhere to put them. Both screens now render the same control, and a successful promotion offers the way onward to where the work landed.

… is repeatable (#314) (#322) * test(app): the cycle suite's project name is run-scoped, so --repeat-each works `scripts/cycle_server.sh` rebuilds the workspace once per *server start* and `--repeat-each` reuses that one server, so a project created from a fixed literal made the flag useless: repeat 2 onward died on `POST /projects -> 409`, a wall standing in front of everything the suite is about. Repetition meant N whole invocations at about ninety seconds of rebuild each. Unique names rather than teardown, because teardown that has to survive a mid-run crash is a second bug surface, and there is nothing to tear down when nothing collides. The project is the only name that had to move, and the spec now says so, so the next collision is looked for rather than assumed: a release tag is unique per dataset, a batch name is not unique at all, and a source's idempotency key (project, kind, path, fps) leads with the project. Verified rather than reasoned about: --repeat-each=3 gives three `POST /projects -> 201`, zero 409 anywhere in the run, and the only 4xx at all is the expected schema-less-project 404, once per repetition. Refs #314 * chore(scripts): check.sh runs the browser suites, with --fast and timings check.sh ran no browser suite while calling itself the canonical "before you say it works" invocation. Two of the three suites were invisible to it, and the real-server cycle run was three separate times the *only* one to catch a regression during the 2026-08 remediation run (#306, #308, #309) -- one of which shipped on a green run of this script and went red in CI. The full run is the default and --fast is the exception. That inversion is the point: the previous default was silently the fast one. - `browser` is addressable as a group and is a complete run on its own, because each Playwright config's webServer.command builds the engine and the design system first. - CI=1 is set by the script, for the Playwright steps, from frontend/app, in a subshell so the cd cannot leak into a later group. It is load-bearing: reuseExistingServer: !CI means a stale vite server on :5273 answers instead of the build under test. The lesson lived in a skill and in memories; it lives in the tooling now. - Skipping prints a banner, *after* the verdict -- it exists to qualify "All checks passed", so it has to be the thing still on screen once that line has scrolled off. It prints whenever the browser suites did not run, not only under --fast: a partial run is the same lie however it was asked for. - Per-step timings with a total, so the cost is visible and a step that quietly doubles gets noticed. - --fast together with an explicit `browser` group is refused rather than resolved. Either answer would guess at which half of a contradictory command line was meant. The timing table is length-checked before expanding, and the reason is written down: macOS still ships bash 3.2, where "${arr[@]}" on an empty array is an unbound variable under set -u and kills the script. The neighbouring `failed` block had always been written that way and never said why. No CI or ruleset change. The two jobs already run these suites and stay separate -- each is a required check with its own cache and report artifact -- and no job name moved. Refs #314

* feat(kernel): the trunk can be asked who is in it, cheaply `DatasetService.assets` resolves every member to an `Asset` because its callers render them. The question the wire needed is *is this in the trunk*, which needs the id alone — so `member_asset_ids` skips one lookup per member, and over a dataset of fifty thousand that is the whole cost of the call. A **set**, and returned rather than answered per id, because the caller that wanted this asks about a batch's worth of assets at once: how much of a completed batch has reached the trunk is a question about an intersection, and asking it one id at a time is the shape that turns one read into N. Current membership, never a promotion log. A curator removing an asset takes it out of the answer, which is right for the question anybody asks it — "is my work in the dataset" is about now. `member_asset_ids_of` sits beside `assets_of` for the reason that one sits beside this service: a second walk of `dataset_member` written elsewhere is a second chance to disagree with this one. Unlike `assets_of` it cannot raise `WorkspaceCorrupt` — a member naming an asset that is gone is still a member, and answering "which ids are in" does not require the rows behind them. No migration: derived per call, like every other count that describes now. * feat(wire): a batch says how much of it is in the dataset Promotion is not a transition — the batch stays `completed` — and no read model recorded that anything had entered the trunk. So a client could not tell "promoted 3 of 48" from "promoted nothing because it was already done" from "the press did nothing at all", and the third is what a user concludes (audit F17). `promoted_asset_count` on `BatchOut` is how many of this batch's assets are in the trunk **right now**. Current membership rather than a promotion log: a curator removing an asset takes it back out, and "how much of this batch is in the dataset" is the question anybody looking at a batch is actually asking. Derived per call and never stored — `Release.asset_count` is the frozen counterpart and belongs to the release. **The cost model is the reason it is a parameter.** `promoted` is passed into the projection rather than read inside it, so a listing of twenty batches is one extra query rather than twenty: `batch.asset_ids` is already in memory (it is what `asset_count` counts) and the rest is a set intersection. Keyword-only with no default, because a default would report zero promoted for a batch nobody checked — a number that looks like an answer and is not one. Published on all four surfaces in the same change: REST, the `visionset.wire` projection, MCP and the CLI's `--json`. The contract test's pair now uses a `promoted` set that actually intersects — a count of zero would have agreed with itself even if the intersection were wrong. * feat(ui-core): promotion says what it did, and the gallery can do it Audit findings F5 and F18. The call always worked. What a person could observe afterwards was the word "Promoted" on the button they had just pressed and **nothing else** — and nothing else was structurally possible: promotion is not a transition, so the batch stays `completed`, and the response carrying *the assets this press actually promoted* was discarded unread. Three outcomes were indistinguishable: 1. promoted 3 of 48 — the real shape reported, 3 annotated and 45 skipped; 2. promoted nothing because it was already there (promotion is a **union**, so a second press legitimately moves zero); 3. the press did nothing at all. A user seeing no change concludes (3), which is the only one that was never true. `promotionSummary` is the pure part of telling them apart, and the decision in it is not obvious: **zero promoted is not a failure**, and it has two different causes. Zero moved with assets in the trunk is "already there"; zero moved with nothing in the trunk is a batch of skipped frames, which `PROMOTABLE_PROGRESS` excludes on purpose. The skipped count gets a sentence because "Promoted 3" over a batch of 48 reads as a failure unless somebody says where the other 45 went. **Two numbers, because neither alone is the answer.** The response says what this press did and cannot be recovered afterwards; `promoted_asset_count` says what is in the trunk now and is the only half that survives a reload. The button no longer carries the outcome in its label. A flip is not a report, and it also made a second press look forbidden when it is merely a no-op. `PromoteButton` is shared rather than copied, and that is what closes F18: the gallery — the screen somebody is actually on when they finish a batch — had no promote control in any state and no link to the dataset, so a person could settle forty-eight frames and have nowhere to put them. Both screens now render the same control, and a successful promotion offers the way onward to where the work landed.

… is repeatable (#314) (#322) * test(app): the cycle suite's project name is run-scoped, so --repeat-each works `scripts/cycle_server.sh` rebuilds the workspace once per *server start* and `--repeat-each` reuses that one server, so a project created from a fixed literal made the flag useless: repeat 2 onward died on `POST /projects -> 409`, a wall standing in front of everything the suite is about. Repetition meant N whole invocations at about ninety seconds of rebuild each. Unique names rather than teardown, because teardown that has to survive a mid-run crash is a second bug surface, and there is nothing to tear down when nothing collides. The project is the only name that had to move, and the spec now says so, so the next collision is looked for rather than assumed: a release tag is unique per dataset, a batch name is not unique at all, and a source's idempotency key (project, kind, path, fps) leads with the project. Verified rather than reasoned about: --repeat-each=3 gives three `POST /projects -> 201`, zero 409 anywhere in the run, and the only 4xx at all is the expected schema-less-project 404, once per repetition. Refs #314 * chore(scripts): check.sh runs the browser suites, with --fast and timings check.sh ran no browser suite while calling itself the canonical "before you say it works" invocation. Two of the three suites were invisible to it, and the real-server cycle run was three separate times the *only* one to catch a regression during the 2026-08 remediation run (#306, #308, #309) -- one of which shipped on a green run of this script and went red in CI. The full run is the default and --fast is the exception. That inversion is the point: the previous default was silently the fast one. - `browser` is addressable as a group and is a complete run on its own, because each Playwright config's webServer.command builds the engine and the design system first. - CI=1 is set by the script, for the Playwright steps, from frontend/app, in a subshell so the cd cannot leak into a later group. It is load-bearing: reuseExistingServer: !CI means a stale vite server on :5273 answers instead of the build under test. The lesson lived in a skill and in memories; it lives in the tooling now. - Skipping prints a banner, *after* the verdict -- it exists to qualify "All checks passed", so it has to be the thing still on screen once that line has scrolled off. It prints whenever the browser suites did not run, not only under --fast: a partial run is the same lie however it was asked for. - Per-step timings with a total, so the cost is visible and a step that quietly doubles gets noticed. - --fast together with an explicit `browser` group is refused rather than resolved. Either answer would guess at which half of a contradictory command line was meant. The timing table is length-checked before expanding, and the reason is written down: macOS still ships bash 3.2, where "${arr[@]}" on an empty array is an unbound variable under set -u and kills the script. The neighbouring `failed` block had always been written that way and never said why. No CI or ruleset change. The two jobs already run these suites and stay separate -- each is a required check with its own cache and report artifact -- and no job name moved. Refs #314

JArmandoAnaya added 3 commits August 4, 2026 12:00

JArmandoAnaya enabled auto-merge (squash) August 4, 2026 19:00

JArmandoAnaya merged commit 3497e3b into main Aug 4, 2026
14 checks passed

JArmandoAnaya deleted the feat/t5-observable-promotion branch August 4, 2026 23:52

Navigation Menu

Uh oh!

Make promotion observable (F5, F17, F18) - #308

Make promotion observable (F5, F17, F18)#308
JArmandoAnaya merged 3 commits into
mainfrom
feat/t5-observable-promotion

JArmandoAnaya commented Aug 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JArmandoAnaya commented Aug 4, 2026

The finding

Wire (F17) — promoted_asset_count on BatchOut

UI (F5) — two numbers, because neither alone is the answer

F18 — the gallery can promote now

Decisions taken autonomously

Found, not fixed

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Wire (F17) — `promoted_asset_count` on `BatchOut`