-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5630
#5630 — fix(ci): simplify PR visual scope by @cixzhang (bucket: the maintainer)
1fc6cbe3d65123d3c3e60e6e3c9b7a54f9838cd7 — direct child of approved #5608 head 37d70566fd5203763877d8f93f8e8469255f9005.
LOOP VERSION: 1.9.1
AUDIT RUBRIC: 1.14.1
LANE: full
WHY: this changes a trusted workflow_run publisher, private-theme admission, persisted visual-baseline membership, and the release authority approved in #5608.
No GitHub review, inline comment, or existing Review-5630 record existed. The two existing issue comments are the Vercel deployment report and the PR Analysis Report; neither contains review feedback.
The reviewed commit has exactly one parent, approved #5608 head 37d70566. Current main remains 93625638, already represented in that parent stack. This review rechecked every #5608 wiki revision and both exact-head GitHub COMMENT reviews before evaluating the child delta.
WHY 1: broad component and theme plans can produce hundreds of pictures, too many for a person to inspect during a pull request.
WHY 2: truncating that plan gives the reviewer a partial answer that can look complete.
WHY 3: visual approval is release evidence, so its plan must be both human-sized and explicit when the protected daily gate owns the full answer.
USER-FACING PROBLEM: a maintainer reviewing a visual change needs a small, predictable set of representative pictures, while global changes must defer intact rather than present a sampled decision.
PROBLEM SEVERITY: harmful friction — the protected release gate still exists, but the PR review set can become too large to inspect meaningfully.
NEW FEATURE CASE: not a new feature — this contracts and clarifies an existing internal CI authority.
EARLY STOP: clear — the PR body names the scope, the #5510-shaped case, the two thresholds, and the private Probe constraint.
VERDICT: clear
The trusted publisher derives a complete PR plan before deciding whether a person can review it. Touched Core components use one Theme Sheet under Neutral and Probe; explicitly tagged stories use Neutral. Global scopes defer to the daily gate, while oversized compact plans warn or defer without being sliced.
SOLUTION (3 decisions · 4 runtime scripts + 1 workflow)
- Define compact component and tagged-story plans — serves the stated human-sized review problem.
- Build before budgeting, warn above 24, defer above 40, and defer global theme/shared scopes — serves the no-truncation requirement.
- Admit private Probe pictures into trusted PR evidence and promotion — serves the stated Probe evidence requirement, but conflicts with the release baseline’s private-theme policy.
BURDEN: medium-high — 12 changed files, +1030/−255; 442 runtime/workflow additions, 569 test additions, two docs; no new dependency, module, workflow, or public package API.
BURDEN MATCH: disproportionate at one seam — the compact planner is justified, but carrying private Probe pictures into the release baseline reintroduces the membership ambiguity #5608 removed.
VERDICT: BLOCKS — accepted Probe keys enter a baseline whose release owner permanently excludes them
OWNER: the trusted PR planner owns PR evidence membership; the accepted release baseline owns persisted membership and the protected release lane owns removal reporting.
TIER 1: Storybook index/tags, workspace package manifests, visual-gate config, the #5608 baseline accounting model, and trusted default-branch workflow code.
TIER 2: none.
SEAMS: file-scope classification, trusted plan, PR report, acceptance, post-merge recapture/promotion, release accounting, explicit prune.
BEHAVIOR UNIT: pure planner/classifier functions plus one trusted workflow orchestration path; all required planning states are driven without a browser.
COMPLEXITY BUDGET: one PR-plan owner, one persisted-baseline membership owner, two fixed review thresholds, zero conflicting eligibility policies.
ACTUAL BURDEN: 12 files; 4 runtime scripts + 1 trusted workflow; one new compact-plan helper; one new changed-story projection; one Probe exception in report, recapture, and promotion; 263 focused tests.
BURDEN TREND: approved #5608 ended at 10 files and +963/−222; this child adds +1030/−255 across 12 files without a new module, but reopens the same persisted-membership boundary.
RESET TRIGGER: triggered — promotion now accepts the private Probe policy that release accounting rejects for the same durable key.
| domain fact | one authoritative writable source | generated / immutable projections | other writable copies |
|---|---|---|---|
| changed PR paths | GitHub’s exact-head file list and snapshots | trusted scope JSON | none |
| compact PR plan | buildPrVisualPlan |
capture manifest and immutable PR evidence | none |
| release baseline membership | accepted baseline record under #5608 | release accounting categories | Probe promotion writes keys that package metadata immediately classifies outside release membership |
| review budget | trusted workflow thresholds 24 / 40 | warning or skipped verdict | none |
| required seam | driven exact-head result |
|---|---|
| Theme Sheet | Neutral light/dark + Probe light/dark: 4 exact keys |
stable-visual |
Neutral light/dark: 2 exact keys |
| missing Theme Sheet | one actionable error naming both missing components and Core/<Component>
|
| budget | 24 captures; 25 warns; 40 captures with warning; 41 defers; the full count is retained |
| global scope | shared Core infrastructure and stable-theme source both classify broadStableVisual: true and take the separate defer path |
| #5510-shaped component | 121 indexed stories reduce to the Theme Sheet’s 4 shots |
| Probe persistence | end-to-end promotion writes core-button--theme-sheet__probe-light with stableThemeVisual: false
|
| Probe after story deletion | release accounting remains intentionallyExcluded: 1, with 0 release-manifest keys and 0 removals |
| #5608 invariants | 263/263 visual-gate tests pass, including accounting, failed-zero-removal, removal refusal, exact promotion, publication lock, and recovery identity |
The split is visible at the promotion admission boundary: Probe is allowed despite stableThemeVisual: false. Promotion then writes every accepted key into the baseline at lines 946–950, while release accounting classifies that same private theme as ineligible at plan.mjs lines 224–264.
VERDICT: BLOCKS — persisted Probe membership has two incompatible policies and no complete removal lifecycle
After a maintainer accepts a component PR, its Probe pictures are written into the release baseline. If that Theme Sheet or component later disappears, the daily release lane still classifies those keys as intentionally excluded, PR acceptance cannot remove them, and explicit prune can only consume keys from a canonical release removal verdict. The maintainer therefore accumulates accepted keys that no authority can recapture or present for removal.
NEW FEATURE IMPACT: not a new feature.
VERDICT: BLOCKS — the exact accepted-key lifecycle invariant approved in #5608 regresses for Probe
No published package API change.
| change | public? | class | doc’d? | verdict | |
|---|---|---|---|---|---|
+ |
Storybook tag stable-visual
|
internal CI/story metadata | existing Storybook tags | both READMEs + tests | clear |
+ |
scope field stableStoryFiles: string[]
|
internal immutable workflow data | existing stableComponents / stableThemes
|
source + tests | clear |
+ |
buildPrVisualPlan(...) |
internal pure helper | existing buildPlan
|
JSDoc + tests | clear |
~ |
accepted private theme policy: Probe permitted | internal persisted CI policy | no safe release-baseline sibling | source + tests | BLOCKS |
OSSIFICATION: no consumer surface. The internal baseline format is durable release state, however, so accepting a new key class without a complete lifecycle is still expensive to undo.
VERDICT: BLOCKS — internal Probe admission changes durable release state
No CSS, token, target, variable, selector, or rendered theme behavior changes. Probe remains a private package by manifest; the defect is that CI persists its pictures into the stable release baseline despite that classification.
VERDICT: note — private theme metadata is preserved, but its baseline lifecycle is inconsistent
BEHAVIOR: yes, intentionally inside CI — component plans shrink, tagged stories opt in, broad plans defer, and Probe can enter trusted evidence. The unintended behavior is that Probe can now enter the durable release baseline.
API: no published API, prop, export, or default change.
VISUAL: no component, story, style, or pixel output changes.
THEME: no theme contract changes; package privacy remains unchanged.
VERDICT: BLOCKS — internal persisted behavior regresses the accepted-key ownership contract
EFFECTS: zero.
| Effect + deps | external system | why render/handler cannot do it | measured render cost | lifetime + cleanup | focused test |
|---|---|---|---|---|---|
| none | none | n/a | n/a | n/a | n/a |
RENDER: no UI render path.
LISTENERS/OBSERVERS: none.
LAYOUT: none.
BUNDLE: no dependency or lockfile delta. Planning is bounded and deterministic: one 121-story component yields 4 shots; 41-shot plans defer before browser installation/capture.
VERDICT: clear — the compact plan removes rather than adds capture work; no runtime dependency or process leak
VISUAL CHECK: not applicable
WHY: all changed runtime behavior is CI planning, trusted publication, and documentation. No component, story rendering, style, or package visual output changes. Exact-head Stable visual regression is path-skipped and is not claimed as evidence. No Storybook server or browser was started.
VERDICT: clear
REMEDY SEARCH: not triggered — no proven visual defect
No rendered element, role, accessible state, focus path, translation, locale formatting, or directional layout changes. Exact-head pr-a11y and pr-rtl are path-skipped and are not claimed as coverage.
VERDICT: clear
| slot | verdict |
|---|---|
| PROBLEM | clear |
| SOLUTION | BLOCKS — Probe persistence exceeds the compact-evidence problem |
| ARCHITECTURE | BLOCKS — persisted membership has conflicting policies |
| IMPACT | BLOCKS — accepted Probe keys cannot reach a removal decision |
| API | BLOCKS — internal durable admission is incomplete |
| THEMING | note — package privacy itself remains correct |
| BREAKING | BLOCKS — #5608’s ownership invariant regresses |
| PERFORMANCE | clear |
| VISUAL | clear |
| A11Y & I18N | clear |
GOAL: partly met — all seven requested planning/scope behaviors are proved, but private Probe evidence is promoted into stable release state and then excluded from every release removal decision.
DISPOSITION: one architecture/correctness finding blocks now. Keep Probe evidence outside the release baseline, or give every persisted Probe key one complete, authoritative accounting/capture/removal lifecycle.
ADVICE: bounded outcome criteria — no implementation is prescribed; the invariant is that no accepted key may be simultaneously excluded from release planning and unreachable by removal authority.
AUTHOR CAN PROCEED: yes — the defect, affected lifecycle, and acceptance criterion are all deterministic.
WORST OUTCOME: “The maintainer accumulates accepted Probe keys that no authority can recapture or present for removal.” → request changes.
JUDGEMENT NEEDED: none — this is a regression against the approved #5608 contract, not a new API or design choice.
comment — semantic request changes (self-authored draft)
- Probe promotion creates accepted keys with no complete release lifecycle.
→ after a Theme Sheet/component disappears, the release maintainer cannot recapture or approve removal of its accepted Probe keys ·
visual-acceptance.mjs:795→ confirmed independently by an end-to-end Probe promotion and a production-module story-present/story-deleted accounting probe.
Semantic verdict: request changes
Probe is meant to stay private/test-only, but this head promotes accepted Probe images into the release baseline at the promotion boundary. The release planner then classifies the same keys as intentionally excluded, so it never recaptures or reports their removal; scoped PR acceptance is add/update-only, and explicit pruning only accepts keys from a canonical release removal verdict.
Once a Theme Sheet or component disappears, those Probe keys are stranded in the accepted baseline—the same unowned-key failure #5608 just closed.
Please keep Probe evidence out of the release baseline, or give those keys one complete accounting and removal lifecycle.
[Reviewed by Robohands]
None — the lifecycle crosses promotion and release accounting, so the summary carries one representative anchor.
- Exact-head focused suite: 19 files, 263/263 tests; four changed runtime scripts pass
node --check; actionlint passes both interacting workflows. - Exact-head Actions: CI run 33153224666 passed every executed job; all other required contexts and statuses are green. The three component-only jobs are correctly path-skipped.
- Trust scan: unchanged lockfile/root scripts; no dependency, permission, secret, shell-expansion, public-package, or private-residue delta. The trusted workflow reads PR-head story files as data and never checks out PR code.
TIME total 13m
setup 3m exact heads, fresh isolated checkout, public rules/wiki, trust guard
reading 4m complete child delta, all #5608 review revisions, current source/workflow paths
measuring 4m 263 tests, 2 focused Probe probes, threshold/scope matrix, CI/security checks
writing 2m presentation and two critic passes
waste 1m retried one wiki-history extraction after shell word-splitting
- The edited
workflow_runworkflow cannot execute from PR-head YAML; its behavior is covered by exact-head actionlint, workflow-contract tests, and production-module probes rather than a PR-head workflow run. - The protected daily release gate was not dispatched. No Storybook/browser run was needed for these non-rendering planning semantics.
Posted as drafted in the exact-head GitHub COMMENT review. An accidental leading blank line was removed immediately so Semantic verdict: request changes is literally the first line; no review words changed.
#5630 — fix(ci): simplify PR visual scope by @cixzhang (bucket: the maintainer)
c51b1b93fa6267ab5470f2af0fc2fb58564619d3 — direct child of approved #5608 head 37d70566fd5203763877d8f93f8e8469255f9005.
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.1
LANE: full WHY: this re-review changes trusted visual evidence, persisted baseline mutation, and the release authority; it must resolve the prior lifecycle block and preserve every affected path.
The prior Robohands review at 1fc6cbe3d65123d3c3e60e6e3c9b7a54f9838cd7 concluded that accepted Probe images entered the release baseline and then became uncapturable and unprunable. The c51b1b9 delta fixes that finding: post-merge verification still recaptures Neutral and Probe, but promotion persists only Neutral and records Probe as reviewed current-only evidence. This round extends rather than contradicts that finding with a separate scope-lifecycle defect.
Current main remains 936256386b74d67f88a41adee235b91699977b2f, already represented in the approved parent stack, and no later main commit touches the reviewed files.
WHY 1: full component/theme visual plans are too large for useful pull-request review, while truncating them makes partial evidence look complete. WHY 2: maintainers need a compact exact-head plan and a protected full release decision without letting private Probe evidence become stable release state. WHY 3: the scope contract must remain usable for the whole file lifecycle, including removing an opted-in story.
USER-FACING PROBLEM: a maintainer deleting a stable-visual story file receives a failed trusted publisher instead of a clean PR scope handoff to the release authority.
PROBLEM SEVERITY: broken task — the deletion is classified from the default-branch file after the head lookup returns 404, but the exact-head Storybook contains no matching story, so trusted-plan refuses the empty plan.
NEW FEATURE CASE: not a product feature — this changes internal CI scope and evidence policy.
EARLY STOP: clear — source tracing and a production-CLI reproducer establish the deletion failure independently.
VERDICT: BLOCKS — the new opt-in scope cannot remove its own story file
The trusted publisher builds a complete compact plan before budgeting it. Changed Core components use their Theme Sheet in Neutral and Probe; tagged stories use Neutral; broad or over-budget plans defer without truncation. Probe is verified again after merge but filtered before the stable baseline write. The same head-snapshot path does not represent deletion, so classification substitutes the default-branch file and asks the exact-head planner for a story that no longer exists.
SOLUTION (4 decisions · 5 runtime/workflow files, +479/−170; 12 files, +1268/−261 total)
- Component scope uses one required Theme Sheet in Neutral and Probe, light and dark.
- Changed
stable-visualstories add Neutral light/dark coverage. - Plans warn above 24 and defer intact above 40; theme/global scope defers directly.
- Probe remains current-only through recapture and is excluded from baseline mutation.
BURDEN: medium-high — one trusted publisher, one compact-plan helper, one changed-story projection, one current-only policy, fixed budget thresholds, and 266 focused tests. BURDEN MATCH: proportionate except for the missing deleted-file state in the new story-scope projection.
VERDICT: BLOCKS — exact-head absence is not represented in the scope snapshot
OWNER: GitHub’s exact PR head owns changed-file content and absence; the built exact-head Storybook owns current stories; the accepted baseline owns stable persisted keys.
TIER 1: trusted workflow_run publisher, Storybook index/tags, visual-gate config, accepted baseline, and protected release gate.
TIER 2: none.
SEAMS: file-scope classification, compact planning, trusted capture/report, acceptance, post-merge recapture/promotion, next-PR planning, release accounting, explicit prune, and capture failure.
BEHAVIOR UNIT: pure scope/planner/accounting/comparison functions plus the existing trusted workflow and acceptance CLI.
COMPLEXITY BUDGET: one exact-head scope source, one PR-plan owner, one stable-baseline owner, one removal authority, two fixed thresholds, zero substituted stale content.
ACTUAL BURDEN: 12 files; 5 runtime/workflow files +479/−170; 5 test files +770/−78; 2 docs +19/−13; no dependency, module, workflow, registry, or public API added; 19 focused files / 266 tests.
BURDEN TREND: prior reviewed head 1fc6cbe3 → current adds 38/−7 runtime lines and 201 tests to make Probe current-only; the prior baseline-membership defect contracts, but the new story-scope path remains incomplete for deletion.
RESET TRIGGER: not triggered — the prior model-level baseline finding is fixed; this is one bounded missing state in the PR scope source.
| domain fact | one authoritative writable source | generated / immutable projections | other writable copies |
|---|---|---|---|
| changed story content or absence | GitHub exact PR head | fileSnapshots |
default-branch file incorrectly substitutes for an absent head file |
| compact PR plan | exact-head Storybook index + trusted scope | capture manifest and immutable evidence | none |
| Probe current-only policy | trusted plan/config | accepted recapture plan and reviewedCurrentOnly decision entry |
none |
| stable baseline membership | gh-pages baseline manifest | release accounting categories | none |
| removal authority | canonical protected release verdict | explicit named prune | none |
| seam | driven result |
|---|---|
| Theme Sheet | 4 exact keys: Neutral light/dark + Probe light/dark |
stable-visual |
2 exact keys: Neutral light/dark |
| missing Theme Sheet | one actionable error naming the missing component |
| 121-story component | 4 shots; no truncation |
| budget | 24 captures; 25 warns; 40 captures with warning; 41 defers with all 41 counted |
| global scope | shared Core and stable-theme changes both defer |
| Neutral + Probe acceptance | both are recaptured; only Neutral is written to manifest/PNG baseline |
| next PR | Probe light/dark are regenerated with current-only policy despite no Probe baseline |
| removed Probe evidence | scoped acceptance rejects it before mutation; no Probe key exists for release accounting or prune |
| capture failure | failed verdict, zero removals |
| upsert-only | promotion decision has promoted: [Neutral], reviewedCurrentOnly: [Probe], pruned: []
|
| live baseline + exact-head CI index | 974 = 882 current stable + 92 intentionally excluded + 0 legacy + 0 unclassified; zero Probe keys/files |
deleted stable-visual file |
404 leaves no head snapshot; classifier falls back to base file, returns stable scope; exact-head plan is empty and production CLI exits trusted visual plan has invalid size 0
|
The blocking path begins at pr-comment.yml lines 228–238: a listed story file that is absent at the PR head reaches the literal catch if (error.status !== 404) throw error;, leaving no authoritative snapshot. visual-scope.mjs lines 44–47 then reads the default-branch copy.
VERDICT: BLOCKS — exact-head absence is replaced by stale default-branch content, so deletion cannot complete
The prior Probe lifecycle is fixed with no stable Probe baseline or orphan. Separately, a maintainer deleting a file that contains an opted-in stable story cannot get the trusted publisher green: scope says the deleted story still exists, while the exact-head Storybook correctly says it does not.
NEW FEATURE IMPACT: not a product feature; the affected person is the maintainer changing visual evidence policy, and the observable failure is a red trusted plan before capture.
VERDICT: BLOCKS — deleting opt-in coverage is not a usable builder path
No published package API change.
| change | public? | class | doc’d? | verdict | |
|---|---|---|---|---|---|
+ |
Storybook tag stable-visual
|
internal CI/story metadata | existing Storybook tags | both READMEs + tests | BLOCKS on deleted-file lifecycle |
+ |
scope field stableStoryFiles: string[]
|
internal immutable workflow data | existing component/theme scope | source + tests | BLOCKS because absence is not encoded |
+ |
baselinePolicy: 'current-only' for Probe plan shots |
internal immutable evidence metadata | private Probe contract | source + tests | clear |
~ |
promotion filters current-only/Probe before baseline mutation | internal persisted CI policy | existing upsert-only promotion | source + tests | clear |
OSSIFICATION: no consumer surface. The internal scope field is still workflow contract; treating missing head content as base content freezes an incorrect deletion behavior into that contract.
VERDICT: BLOCKS — the changed-story scope has no correct deletion representation
No CSS, token, target, variable, selector, or rendered theme behavior changes. Probe remains private; Neutral remains the only PR evidence theme eligible for stable promotion.
VERDICT: clear
BEHAVIOR: yes inside CI — compact scope, opt-in story coverage, defer thresholds, and current-only Probe promotion; deleting an opted-in story newly fails the trusted publisher. API: no published package API. VISUAL: no component/story/style pixels change. THEME: no theme contract changes.
VERDICT: BLOCKS — the new scope behavior makes a valid story deletion fail
EFFECTS: zero.
| Effect + deps | external system | why render/handler cannot do it | measured render cost | lifetime + cleanup | focused test |
|---|---|---|---|---|---|
| none | none | n/a | n/a | n/a | n/a |
RENDER: no UI render path. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or lockfile delta. A 121-story component reduces to 4 shots; plans above 40 defer before browser installation and keep the full count.
VERDICT: clear
VISUAL CHECK: not applicable
WHY: all changed behavior is CI planning, trusted evidence, workflow orchestration, and documentation. No component, story implementation, style, or rendered output changes. Exact-head Stable visual regression is path-skipped and is not claimed as evidence; no Storybook server/browser was started.
VERDICT: clear
REMEDY SEARCH: not triggered — no proven visual defect
No rendered element, role, accessible state, focus path, translation, locale formatting, or directional layout changes. Exact-head pr-a11y and pr-rtl are path-skipped and are not claimed as coverage.
VERDICT: clear
| slot | verdict |
|---|---|
| PROBLEM | BLOCKS — deleting opted-in story coverage fails trusted planning |
| SOLUTION | BLOCKS — exact-head absence is not represented |
| ARCHITECTURE | BLOCKS — default-branch content substitutes for the head source |
| IMPACT | BLOCKS — a valid builder deletion cannot complete |
| API | BLOCKS — new internal scope lacks a deletion state |
| THEMING | clear |
| BREAKING | BLOCKS — new CI regression |
| PERFORMANCE | clear |
| VISUAL | clear |
| A11Y & I18N | clear |
GOAL: partly met — the prior Probe lifecycle blocker is fixed and all requested compact-plan/budget/accounting behaviors pass, but deleting a stable-visual story produces false scope and a zero-plan failure.
DISPOSITION: one new scope finding blocks now. A listed story file absent at the exact head must not be classified from default-branch content; its PR path must remain non-removal-authoritative and let the protected release gate own any baseline deletion.
ADVICE: bounded outcome criteria — represent exact-head absence authoritatively; no implementation is prescribed.
AUTHOR CAN PROCEED: yes — the failure, affected state, and required outcome are deterministic.
WORST OUTCOME: “A maintainer cannot delete an opted-in visual story because trusted scope resurrects the base file and then asks exact-head Storybook to plan a story that no longer exists.” → request changes.
JUDGEMENT NEEDED: none.
comment — semantic request changes (self-authored draft)
- [BLOCKS] Exact-head story absence is replaced with default-branch content.
→ a maintainer deleting a
stable-visualstory gets a zero-plan publisher failure instead of a release-owned removal ·pr-comment.yml:228→ confirmed independently by source tracing and an executable production-CLI reproduction. This is new in Round 2 but changes the PR’s scope contract, so re-blocking is warranted.
Semantic verdict: request changes
Thanks — the Probe lifecycle from my prior review is fixed: promotion recaptures both themes but persists only Neutral, the next PR regenerates Probe evidence, and neither PR acceptance nor release accounting can prune or strand a Probe key.
One scope case still blocks undrafting. Deleting a stable-visual story makes the head-content request return 404; the snapshot loop leaves that absence unrecorded, so classification falls back to the default-branch file while exact-head Storybook has no matching story. trusted-plan fails with invalid size 0, and the builder cannot remove opt-in coverage.
Please make head-file absence authoritative while keeping the protected release gate as the only removal authority.
[Reviewed by Robohands]
None — the defect crosses trusted head retrieval, scope classification, and exact-head planning.
- Exact-head local focused suite: 19 files, 266/266 tests; six runtime scripts pass
node --check; actionlint passes all three interacting workflows. - Exact-head checks: 14 check-run successes + 5 documented skips;
review-required, Vercel, andvisual-acceptancestatuses are success. CI run 33156738791 is green atc51b1b9. - Independent lifecycle probe used production modules/CLI with capture-shaped Probe metadata (no persisted policy field): Neutral alone entered the baseline; Probe remained current-only, reappeared in the next plan, could not authorize removal, and release capture failure reported zero removals.
TIME total 15m setup 3m rules, exact heads, trust guard, isolated code/wiki worktrees, clone/reconcile install reading 4m prior #5608/#5630 rounds, delta, workflow and lifecycle source measuring 5m 266 tests, actionlint, syntax, live baseline/index accounting, lifecycle and deletion probes writing 3m presentation, two critic passes, exact-head GitHub/wiki delivery waste 1m corrected one artifact-path/accounting probe setup error
- The protected scheduled Release Gate was not dispatched, as requested.
- No Storybook server/browser was run; this is a non-rendering CI change, and the exact-head CI-built index was read only as data.
Posted as drafted in the exact-head GitHub COMMENT review; GitHub recorded COMMENTED at c51b1b93fa6267ab5470f2af0fc2fb58564619d3.
#5630 — fix(ci): simplify PR visual scope by @cixzhang (bucket: the maintainer)
640b1d4b57737a2a411c86f13e42835e2e5c0119 — direct child of approved #5608 head 37d70566fd5203763877d8f93f8e8469255f9005.
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4
LANE: full WHY: this exact-head re-review changes trusted visual evidence, scope budgeting, and the accepted-baseline lifecycle after two prior request-changes rounds.
Our Round 1 review found that Probe entered the stable baseline and then had no release lifecycle; Round 2 verified that fix. Our Round 2 review found that deleting a stable-visual story fell back to base content and then failed with an empty exact-head plan. The new commit fixes that planning/reporting defect: the deleted story is absent from head capture, zero head shots are valid, and the baseline image becomes separate before-only expected-removal evidence.
Current main remains 936256386b74d67f88a41adee235b91699977b2f, already represented in the approved parent stack, and no later main commit touches the reviewed files.
WHY 1: full component/theme visual plans can be too large for useful pull-request review, while truncation makes partial evidence look complete.
WHY 2: maintainers need compact exact-head captures, before-only evidence for deleted opt-in stories, and a protected full release decision.
WHY 3: every emitted evidence bundle must have one bounded review path and a terminal status; otherwise valid story deletion cannot land.
USER-FACING PROBLEM: a maintainer deleting an opted-in stable story now gets the correct before-only evidence, but the PR stays pending forever because the only acceptance command refuses removal evidence; mixed bundles are stranded too.
PROBLEM SEVERITY: broken task — visual-acceptance requires /accept-visual and the endpoint deterministically refuses the bundle.
NEW FEATURE CASE: not a product feature — this changes internal CI scope and evidence policy.
EARLY STOP: clear — exact-head production CLI probes establish the stuck removal-only and mixed states, plus a separate 24/40-budget bypass.
VERDICT: BLOCKS — correct removal evidence has no terminal PR state
The trusted planner now separates current head captures from baseline-backed expected removals. The capture and report paths support zero current shots, preserve before-only pixels, and avoid inferring unrelated removals. Two downstream consumers still implement the old shots-only contract: acceptance cannot close any bundle containing a removal, and workflow budgeting reads array .length from a conditional object plan.
SOLUTION (4 decisions · 6 runtime/workflow files, +449/−68 since the last reviewed head)
- Derive deleted stable-story keys from the baseline and exact-head Storybook absence.
- Carry expected removals beside, never inside, the head capture plan.
- Publish before-only removal evidence while preserving unrelated baseline keys.
- Keep Probe current-only, the release lane removal-authoritative, and PR baseline mutation upsert-only.
BURDEN: medium-high — one trusted publisher, a conditional array/object plan format, capture context, report derivation, acceptance state, fixed budgets, and 270 focused tests. BURDEN MATCH: disproportionate at the downstream contract — the extra plan representation is not consumed consistently, so valid removal evidence cannot finish and its budget is not enforced.
VERDICT: BLOCKS — the new removal representation stops before terminal state and budget enforcement
OWNER: the exact trusted PR plan owns both current shots and expected removals; the PR status machine owns review completion; the protected release gate remains the only baseline-removal authority.
TIER 1: exact-head Storybook index, trusted baseline, trusted default-branch publisher, visual acceptance state, and protected release gate.
TIER 2: none.
SEAMS: file-scope classification, trusted plan, zero-shot capture, report publication, removal-only state, mixed state, acceptance, warning/defer, post-merge promotion, Probe filtering, and release accounting.
BEHAVIOR UNIT: pure planner/comparator functions plus the existing trusted workflow and acceptance CLI.
COMPLEXITY BUDGET: one exact plan schema, one count covering current plus before-only evidence, one terminal PR state that never mutates removal keys, one protected removal authority.
ACTUAL BURDEN: 13 files, +1716/−328 over approved #5608; latest delta 6 files, +449/−68; the exact plan now has two wire shapes (array or {shots, expectedRemovals}), while the shell budget, deferral reader, and acceptance state retain shots-only assumptions; 270 focused tests.
BURDEN TREND: prior reviewed head c51b1b93 → current grew by 449/−68 across six files to add the missing removal state; planning/reporting improved, but downstream contract burden grew because not every consumer moved with the new representation.
RESET TRIGGER: triggered — the missing deletion state was repaired by adding a second plan representation, and old consumers now disagree about that same plan fact. Contract the plan to one normalized shape/count rather than adding per-consumer guards.
| domain fact | one authoritative writable source | generated / immutable projections | other writable copies |
|---|---|---|---|
| current shots + expected removals | trusted plan | capture manifest, evidence verdict, budget count | array-only assumptions in workflow/deferral |
| PR evidence decision | immutable evidence + explicit review acknowledgement | commit status |
state requires acceptance that accept forbids |
| stable baseline removal | protected release verdict | explicit enumerated prune | none; PR remains non-mutating |
| Probe policy | current-only plan metadata | recapture and reviewedCurrentOnly decision |
none |
| seam | driven result |
|---|---|
| removal-only plan | 0 head shots + expected removals; capture exits 0 with Captured 0/0
|
| removal-only report | changed verdict with before-only delta and no after image |
| mixed current + deleted | current shots and expected removal are reported; unrelated baseline key is absent |
| removal-only state |
pending / decision — “Stable visual changes await /accept-visual.” |
| removal-only acceptance | refused: scoped PR acceptance cannot authorize baseline removals
|
| mixed acceptance | same refusal; valid changed/added current images cannot be accepted either |
| object-plan budget | workflow .length is undefined; 24/40 comparisons emit integer errors and capture proceeds |
| object-plan deferral |
trusted-defer refuses invalid size unknown
|
| Probe lifecycle | recaptured as current-only, omitted from stable baseline, removed Probe evidence cannot prune |
| live release accounting | 974 = 882 current stable + 92 intentionally excluded + 0 legacy + 0 unclassified |
| capture failure | inherited exact-release comparison returns failed evidence with zero removals |
| #5510-shaped plan | 121 stories → 4 Theme Sheet shots; both Probe shots carry current-only
|
| global scope | trusted deferral succeeds with all 974 baseline shots represented |
The stuck state is at visual-acceptance.mjs lines 365–370 and 510–520. The plan-shape mismatch is at pr-comment.yml lines 338–359 and visual-acceptance.mjs lines 643–652.
VERDICT: BLOCKS — the plan/evidence model has no consistent downstream completion contract
A maintainer deleting one opted-in story sees correct evidence but can neither accept it nor reach a successful required status. A mixed bundle also strands valid current-image changes. At 25/41 total review items, the promised warning/defer boundary silently disappears, so the same removal lane can exceed the human-review budget it was introduced to protect. NEW FEATURE IMPACT: not a product feature.
VERDICT: BLOCKS — valid story deletion cannot complete, and large deletion sets bypass review bounds
No published package API change.
| change | public? | class | doc’d? | verdict | |
|---|---|---|---|---|---|
~ |
exact plan: array or {shots, expectedRemovals}
|
internal trusted workflow | existing exact-plan contract | source/tests | BLOCKS — consumers disagree on shape/count |
+ |
expected-removal capture context/evidence | internal immutable evidence | existing delta model | source/tests | BLOCKS — no terminal status path |
~ |
Probe current-only promotion | internal persisted CI policy | existing upsert-only promotion | source/tests | clear |
OSSIFICATION: no consumer API. The internal wire contract is still durable across workflow steps; a conditional representation without one parser creates the observed split.
VERDICT: BLOCKS — internal plan and evidence consumers do not share one contract
No CSS, token, target, variable, selector, or rendered theme behavior changes. Probe remains private/current-only, and live baseline accounting contains zero Probe keys.
VERDICT: clear
BEHAVIOR: yes inside CI — deleted stable stories now emit before-only evidence, but that evidence cannot reach success and mixed bundles cannot be accepted. API: no published API. VISUAL: no component/story/style pixel output changes. THEME: no theme contract changes.
VERDICT: BLOCKS — the new valid deletion path deadlocks its required status
EFFECTS: zero.
| Effect + deps | external system | why render/handler cannot do it | measured render cost | lifetime + cleanup | focused test |
|---|---|---|---|---|---|
| none | none | n/a | n/a | n/a | n/a |
RENDER: no UI render path.
LISTENERS/OBSERVERS: none.
LAYOUT: none.
BUNDLE: no dependency or lockfile delta. Array plans still warn above 24 and defer above 40; any plan containing an expected removal bypasses both thresholds because its count is undefined.
VERDICT: BLOCKS — removal plans can run beyond the explicit human-review capture budget
VISUAL CHECK: not applicable
WHY: all changed behavior is CI planning, evidence publication, workflow orchestration, and documentation. No component, story implementation, style, or rendered product output changed. Exact-head Stable visual regression, pr-a11y, and pr-rtl are path-skipped and are not claimed as evidence. No Storybook server/browser was started.
VERDICT: clear
REMEDY SEARCH: not triggered — no proven visual defect
No rendered element, role, accessible state, focus path, translation, locale formatting, or directional layout changes.
VERDICT: clear
| slot | verdict |
|---|---|
| PROBLEM | BLOCKS — correct removal evidence has no terminal state |
| SOLUTION | BLOCKS — downstream consumers retain shots-only assumptions |
| ARCHITECTURE | BLOCKS — one plan fact has two incompatible representations |
| IMPACT | BLOCKS — valid deletion cannot land; large sets bypass bounds |
| API | BLOCKS — internal plan/evidence contract is incomplete |
| THEMING | clear |
| BREAKING | BLOCKS — required status deadlock |
| PERFORMANCE | BLOCKS — 24/40 budget bypass |
| VISUAL | clear |
| A11Y & I18N | clear |
GOAL: partly met — prior Probe and empty-plan blockers are fixed through capture/reporting, but removal-only and mixed evidence cannot reach success, and any plan containing removals bypasses the warning/defer budget. DISPOSITION: one plan/evidence lifecycle finding blocks now. Acceptance criteria: (1) removal-only and mixed evidence can reach a terminal successful PR state without PR-side baseline removal; (2) one normalized plan count includes shots plus expected removals; (3) 25 warns and 41 defers before Playwright for array and removal-bearing plans; (4) protected release remains the only removal mutation authority; (5) unrelated keys, Probe policy, capture-failure zero-removal, accounting, and promotion invariants remain. ADVICE: bounded contraction criteria — use one plan schema/count and one non-mutating completion contract; implementation details remain the author’s. AUTHOR CAN PROCEED: yes — the two failing states and required invariants are deterministic. WORST OUTCOME: “A maintainer receives valid before-only evidence, but the required status can only ask for an acceptance the endpoint refuses.” → request changes.
JUDGEMENT NEEDED: none.
comment — semantic request changes (self-authored draft)
- Expected-removal evidence is produced but cannot complete the PR, and its plan bypasses review budgets.
→ a maintainer deleting a stable story remains permanently pending; mixed bundles are also unacceptable, while 25/41-item plans skip warning/deferral ·
visual-acceptance.mjs:365
Semantic verdict: request changes
Thanks — the prior deletion blocker is fixed through report generation: deleted stable stories stay out of head capture, zero-shot capture succeeds, and removal-only/mixed evidence preserves unrelated keys.
The new removal lane still has no terminal PR state: state requires /accept-visual, while accept rejects every bundle containing a removal. Mixed bundles are stranded too. Also, the 24/40-shot budget reads array .length; removal plans are objects, so neither warning nor deferral runs and trusted-defer rejects that shape.
Please give removal evidence a non-mutating terminal state and make one normalized plan count drive capture, warning, and deferral.
[Reviewed by Robohands]
None — the defect crosses planning, workflow budgeting, evidence state, and acceptance.
- Full exact-head visual-gate suite: 19 files, 270/270 tests; selected changed tests: 6 files, 118/118. Five runtime files pass
node --check; actionlint passes five interacting workflows;check:repopasses. - Exact-head remote checks: 17 pass, 5 documented path skips, 0 pending/failed; CI run 33159863395 is green. The changed
workflow_runexecutes default-branch code, so local contract/probe evidence is authoritative for the new workflow path. - Direct probes: 0/0 removal capture succeeded; removal-only and mixed states were pending then refused by acceptance; object-plan 24/40 checks produced integer errors and proceeded; object
trusted-deferfailedinvalid size unknown; global defer covered all 974 baseline keys; #5510 shape produced four shots.
TIME total 10m setup 3m exact heads, fresh code/wiki checkouts, rules/rubric, untrusted workflow guard reading 2m all #5608/#5630 rounds, old→new delta, plan/capture/report/state/promotion paths measuring 4m 270 tests, syntax, actionlint, check:repo, exact CI, live accounting, lifecycle/budget probes writing 1m presentation, template gate, two critic passes, delivery preparation waste 1m one quoted-glob test invocation selected only the explicit file and was rerun correctly
- The protected scheduled Release Gate was not dispatched, as requested.
- No Storybook server/browser was run; this is a non-rendering CI change and production modules plus the exact-head CI-built index were used as data.
Posted as drafted in the exact-head GitHub COMMENT review; GitHub recorded COMMENTED at 640b1d4b57737a2a411c86f13e42835e2e5c0119.
#5630 — fix(ci): simplify PR visual scope by @cixzhang (bucket: the maintainer)
d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc — direct child of approved #5608 head 37d70566fd5203763877d8f93f8e8469255f9005.
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4
LANE: full WHY: this exact-head re-review changes trusted visual evidence, acceptance state, post-merge promotion, and human-review budgeting after three prior request-changes rounds.
Our Round 3 review at 640b1d4b57737a2a411c86f13e42835e2e5c0119 concluded that valid removal-only and mixed evidence could not finish because /accept-visual rejected removals, while removal-bearing object plans bypassed the 24/40 review budget and trusted-defer rejected their shape. This replacement commit keeps the approved #5608 parent and fixes each prior ask; the tree delta from the reviewed head is six files, +184/−52.
Current main remains the PR base 936256386b74d67f88a41adee235b91699977b2f, already represented in the approved parent stack. No newer main commit invalidates the reviewed paths.
WHY 1: deleted opt-in stories correctly produced before-only evidence but the required status could only request an acceptance that rejected the bundle. WHY 2: mixed bundles stranded valid current changes, and removal-bearing plans escaped the human review budget. WHY 3: a trusted evidence lane must finish every valid bundle without gaining authority to remove stable baseline entries. USER-FACING PROBLEM: a maintainer deleting an opted-in story could inspect correct evidence yet never complete the PR, while large removal sets could bypass the review boundary. PROBLEM SEVERITY: broken task — the prior production paths deterministically remained pending/refused or skipped budget enforcement. NEW FEATURE CASE: not a product feature — this repairs an existing internal CI contract. EARLY STOP: clear — prior evidence, the exact delta, focused tests, and independent production-CLI fixtures establish the failure and repaired outcome.
VERDICT: clear
Removal evidence is now an acceptable, immutable review decision with a baseline preimage and no after image. Post-merge planning and promotion ignore removed entries, so the PR can finish without pruning; changed/added entries still recapture and upsert normally. Shot-only arrays and removal-bearing objects normalize to current shots plus expected removals, and every budget consumer counts both.
SOLUTION (3 decisions · 3 runtime scripts + 1 workflow)
- Accept removal-only and mixed evidence as a terminal reviewed bundle.
- Filter removed entries from post-merge recapture/promotion so PR promotion remains add/update-only.
- Standardize the removal field as
expectedRemovedand count current plus removed entries at the 24/40 boundaries, with array compatibility.
BURDEN: medium — the six-file delta adds +46/−28 runtime/workflow lines and +138/−24 test lines; no dependency, module, workflow, persisted schema version, or public package API is added. BURDEN MATCH: proportionate — the changes move existing consumers onto the already-introduced removal representation and add negative controls at the mutation boundary.
VERDICT: clear
OWNER: the trusted PR plan owns current shots plus expected removals; the immutable acceptance record owns the human decision; the protected release gate remains the only baseline-removal authority.
TIER 1: exact-head Storybook index, trusted baseline, default-branch publisher, acceptance state, post-merge recapture/promotion, and protected release gate.
TIER 2: none.
SEAMS: removal-only evidence, mixed evidence, preimage validation, terminal status, array/object budgets, trusted deferral, Probe current-only recapture, #5510-shaped component scope, post-merge promotion, and unrelated baseline entries.
BEHAVIOR UNIT: pure plan normalization plus existing acceptance/promotion CLI paths; every state is executable without rendering product UI.
COMPLEXITY BUDGET: one logical plan contract, one human count, one immutable acceptance record, one non-pruning PR promotion path, and one protected removal authority.
ACTUAL BURDEN: 13 total PR files and +1860/−340 over approved #5608; the re-review delta is 6 files, +184/−52; no new owner/module/workflow; one normalized removal field, one array compatibility path, and 99 focused tests plus an independent six-assertion lifecycle probe.
BURDEN TREND: prior reviewed head 640b1d4b → current grows +184/−52 to close the two downstream contract gaps; ownership and durable representations stay flat, and no new guard or authority is introduced.
RESET TRIGGER: not triggered — the prior reset criterion is satisfied by making every consumer interpret the same current-plus-removed contract rather than adding another plan representation or mutation lane.
| domain fact | one authoritative writable source | generated / immutable projections | other writable copies |
|---|---|---|---|
| current shots + expected removals | trusted exact plan | capture context, report deltas, budget total | none |
| reviewed bundle | immutable acceptance record | required status and post-merge recapture plan | none |
| PR baseline mutation | existing promotion path | decision log with pruned: []
|
none; removed entries are filtered |
| stable baseline removal | protected release verdict | explicit enumerated prune | none in PR acceptance/promotion |
| Probe policy | trusted plan metadata | current-only recapture and decision log | none |
| seam | driven result |
|---|---|
| removal-only acceptance/state | accepts; status becomes success / accepted
|
| removed evidence shape | baseline preimage required; after hash, after file, and shot are absent |
| mixed acceptance | changed + removed accept together; post-merge plan contains only the changed key |
| promotion | changed key upserts; removed and unrelated keys remain; decision records pruned: []
|
| negative controls | removed evidence with an after image or without the reviewed preimage is refused |
| warning boundary | array 25 and object 24+1 both count 25 and warn |
| defer boundary | array 41 and object 40+1 both count 41; trusted-defer emits skipped evidence with total 41 |
| Probe lifecycle | remains current-only and absent from stable baseline mutation |
| #5510-shaped scope | 121 indexed stories contract to four Theme Sheet shots, including two current-only Probe shots |
VERDICT: clear
A maintainer can now accept deletion-only or mixed evidence and receive a successful required status. That decision cannot remove a stable baseline key; only the protected release gate can later present and apply the deletion. The same person sees the same warning at 25 total review items and deferral at 41 regardless of plan wire shape. NEW FEATURE IMPACT: not a product feature.
VERDICT: clear
No published package API change.
| change | public? | class | doc'd? | verdict | |
|---|---|---|---|---|---|
~ |
exact plan {shots, expectedRemoved} with shot-only array compatibility |
internal trusted workflow | existing exact-plan contract | source/tests | clear |
~ |
acceptance entries may be removed with preimage and no after/shot |
internal immutable evidence | existing delta kinds | source/tests | clear |
~ |
post-merge plan/promotion filter removed entries | internal CI behavior | existing upsert-only promotion | source/tests | clear |
OSSIFICATION: no consumer surface. The internal record keeps the existing delta kind and baseline authority; no schema version or second removal mechanism is added.
VERDICT: clear
No CSS, token, target, variable, selector, or rendered theme behavior changes. Probe remains private and current-only; its two Theme Sheet shots are recaptured but never promoted into the stable baseline.
VERDICT: clear
BEHAVIOR: yes inside CI — valid removal evidence can now finish acceptance, while promotion ignores removed entries and budgets count them. API: no published API, prop, export, or default change. VISUAL: no component, story, style, or pixel output change. THEME: no theme contract change; Probe’s private/current-only policy is preserved.
VERDICT: note — intentional internal completion of the removal-evidence lifecycle
EFFECTS: zero.
| Effect + deps | external system | why render/handler cannot do it | measured render cost | lifetime + cleanup | focused test |
|---|---|---|---|---|---|
| none | none | n/a | n/a | n/a | n/a |
RENDER: no product UI render path. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or lockfile delta. Object and array plans both defer at 41 before Playwright installation/capture; the compact #5510-shaped plan remains four shots.
VERDICT: clear
VISUAL CHECK: not applicable WHY: all changed behavior is CI planning, evidence validation, workflow budgeting, acceptance, promotion, tests, and documentation. No component, story implementation, style, or rendered product output changed. Exact-head Stable visual regression is path-skipped and is not claimed as evidence; no Storybook server/browser was started.
VERDICT: clear
REMEDY SEARCH: not triggered — no proven visual defect
No rendered element, role, accessible state, focus path, translation, locale formatting, or directional layout changes. Exact-head pr-a11y and pr-rtl are path-skipped and are not claimed as coverage.
VERDICT: clear
| slot | verdict |
|---|---|
| PROBLEM | clear |
| SOLUTION | clear |
| ARCHITECTURE | clear |
| IMPACT | clear |
| API | clear |
| THEMING | clear |
| BREAKING | note — intended internal behavior completion |
| PERFORMANCE | clear |
| VISUAL | clear |
| A11Y & I18N | clear |
GOAL: met — independent production-CLI fixtures move removal-only and mixed bundles to success / accepted, enforce removal preimage/no-after, preserve removed and unrelated baseline keys with pruned: [], count array/object plans identically at 25/41, and retain four-shot Probe/#5510 behavior.
DISPOSITION: the Round 3 lifecycle/model finding is fixed; no negative current-head code finding remains.
ADVICE: omitted — the requested outcomes are met without a new authority or speculative follow-up.
AUTHOR CAN PROCEED: yes — semantic code review is complete; landing remains separately gated on visual-acceptance infrastructure validation.
WORST OUTCOME: none found — PR acceptance can acknowledge removals but cannot apply them.
JUDGEMENT NEEDED: none — defect correction inside the existing CI authority.
comment — semantic approve (self-authored draft)
No findings.
Semantic verdict: approve
Thanks — removal-only and mixed evidence now finish successfully, both plan shapes share the 25/41 budget, and promotion remains upsert-only with Probe current-only.
[Reviewed by Robohands]
None.
- Exact-head focused Vitest: 5 files, 99/99 tests; five changed runtime scripts pass
node --check; actionlint passes the three interacting workflows;check:repopasses. - Independent production-CLI fixtures cover removal-only, mixed, invalid-after, missing-preimage, 25/41 array/object budgets,
trusted-defer, Probe, #5510 scope, and unrelated-key preservation. - Exact-head required code checks are green in CI run 33164172672; the PR is draft and mergeable.
TIME total 8m setup 2m exact heads, fresh code/wiki checkouts, current rules/rubric, changed-workflow guard reading 2m all #5608/#5630 rounds and the 640b1d4b→d7681c97 delta measuring 2m focused tests, syntax, actionlint, check:repo, CI, and independent lifecycle/budget probes writing 2m presentation, literal template gate, two critic passes, and delivery waste 1m one full PR diff render was broader than the requested delta and was replaced with the six-file comparison
- The changed
workflow_runpath cannot execute from PR-head YAML. GitHub currently reportsvisual-acceptance: success — No stable visual scope, but that default-branch result does not exercise this PR’s changed visual infrastructure; the separate landing gate remains pending as directed. - The protected scheduled Release Gate was not dispatched. No Storybook/browser run was needed for this non-rendering CI change.
Posted as drafted in the exact-head GitHub COMMENT review; GitHub recorded COMMENTED at d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc.
#5630 — fix(ci): simplify PR visual scope by @cixzhang (bucket: the maintainer)
04246c0051029d017e6aa76b3b649bc55bb622dc — the sole commit above current main a83c559a51fb86260f8309f695b68acd612b2eef, the landed #5608.
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4
LANE: full WHY: this is an exact-head restack revalidation of trusted visual evidence, acceptance state, post-merge promotion, and human-review budgeting after four prior rounds.
Our Round 4 review approved d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc: removal-only and mixed evidence finished, both plan shapes shared 25/41 budgets, promotion remained no-prune, and Probe remained current-only.
The restack changes only commit identity. The old parent 37d70566 and landed #5608 parent a83c559a have the same tree 3d673168; the old and new #5630 heads have the same tree d696514f; both commits have stable patch id 8aa2e884; and their tree diff is empty. The new head is a direct child of current main, with exactly one commit in origin/main..HEAD.
WHY 1: full component/theme visual plans can be too large for useful pull-request review, while truncation makes partial evidence look complete. WHY 2: maintainers need compact exact-head captures, before-only evidence for deleted opt-in stories, and a protected full release decision. WHY 3: every valid evidence bundle needs a bounded terminal path without giving PR acceptance baseline-removal authority. USER-FACING PROBLEM: a maintainer needs deletion-only and mixed visual evidence to finish safely, with the same review budget as current-shot evidence and no PR-side pruning. PROBLEM SEVERITY: broken task — before the approved head, removal evidence could not finish and removal-bearing plans bypassed the budget. NEW FEATURE CASE: not a product feature — this repairs an internal CI contract. EARLY STOP: clear — the prior failure and approved remedy are established, and the restack is byte-equivalent.
VERDICT: clear
The trusted planner carries current pictures and before-only removals as one logical review bundle. Acceptance records the immutable decision, while post-merge promotion recaptures and upserts only current entries; protected release remains the sole removal authority. One normalized total drives warning and deferral for both supported plan representations.
SOLUTION (3 decisions · unchanged from approved head)
- Accept removal-only and mixed evidence as terminal reviewed bundles.
- Filter removed entries from post-merge recapture and promotion so PR promotion cannot prune.
- Count current plus removed entries identically at warning 25 and deferral 41, retaining shot-array compatibility.
BURDEN: medium — 13 files and +1860/−340 over landed #5608; no new dependency, owner, module, workflow, persisted schema version, or public package API. BURDEN MATCH: proportionate — every consumer uses the existing current-plus-removed contract, and this restack adds no code or representation.
VERDICT: clear
OWNER: the trusted PR plan owns current shots plus expected removals; the immutable acceptance record owns the human decision; the protected release gate remains the only baseline-removal authority.
TIER 1: exact-head Storybook index, trusted baseline, default-branch publisher, acceptance state, post-merge recapture/promotion, and protected release gate.
TIER 2: none.
SEAMS: removal-only evidence, mixed evidence, preimage validation, terminal status, array/object budgets, trusted deferral, Probe current-only recapture, #5510-shaped component scope, post-merge promotion, and unrelated baseline entries.
BEHAVIOR UNIT: pure plan normalization plus existing acceptance/promotion CLI paths; every requested state is executable without rendering product UI.
COMPLEXITY BUDGET: one logical plan contract, one human count, one immutable acceptance record, one non-pruning PR promotion path, and one protected removal authority.
ACTUAL BURDEN: byte-identical to approved Round 4 — 13 files and +1860/−340; one normalized removal field, one array compatibility path, no new owner/module/workflow, 99 focused tests plus an independent lifecycle fixture.
BURDEN TREND: approved d7681c97 → restacked 04246c00 is flat: identical base trees, identical head trees, identical stable patch id, and zero tree diff.
RESET TRIGGER: not triggered — no implementation, owner, representation, guard, or policy changed during restack.
| domain fact | one authoritative writable source | generated / immutable projections | other writable copies |
|---|---|---|---|
| current shots + expected removals | trusted exact plan | capture context, report deltas, budget total | none |
| reviewed bundle | immutable acceptance record | required status and post-merge recapture plan | none |
| PR baseline mutation | existing promotion path | decision log with pruned: []
|
none; removed entries are filtered |
| stable baseline removal | protected release verdict | explicit enumerated prune | none in PR acceptance/promotion |
| Probe policy | trusted plan metadata | current-only recapture and decision log | none |
| seam | driven result |
|---|---|
| removal-only evidence | before-only delta; preimage required; after hash, after file, and shot absent |
| mixed acceptance | changed + removed accepted together; state is success / accepted
|
| post-merge plan | contains only the changed key |
| promotion | changed key upserts; removed and unrelated keys remain; pruned: []
|
| negative controls | missing removal preimage and forbidden removal after image both reject |
| warning boundary | array 25 and object 24+1 both count 25 |
| defer boundary | array 41 and object 40+1 both count 41; object trusted-defer records total 41 |
| Probe lifecycle | Theme Sheet Probe shots remain current-only, recaptured, and absent from stable mutation |
| #5510-shaped scope | 121 indexed stories contract to four Theme Sheet shots |
VERDICT: clear
The maintainer sees the same terminal state and the same 25/41 review boundary for deletion-only, mixed, and current-only plans. Restacking on landed #5608 changes no runtime behavior. NEW FEATURE IMPACT: not a new feature.
VERDICT: clear
No published package API change.
| change | public? | class | doc'd? | verdict | |
|---|---|---|---|---|---|
~ |
exact plan {shots, expectedRemoved} with shot-array compatibility |
internal trusted workflow | existing exact-plan contract | source/tests | clear |
~ |
acceptance entries may be removed with preimage and no after/shot |
internal immutable evidence | existing delta kinds | source/tests | clear |
~ |
post-merge plan/promotion filter removed entries | internal CI behavior | existing upsert-only promotion | source/tests | clear |
OSSIFICATION: no consumer surface; the restack adds no contract change.
VERDICT: clear
No CSS, token, target, variable, selector, or rendered theme behavior changes. Probe remains private and current-only.
VERDICT: clear
BEHAVIOR: no restack delta; the approved internal CI behavior is byte-identical. API: no published API, prop, export, or default change. VISUAL: no component, story, style, or pixel output change. THEME: no theme contract change.
VERDICT: clear
EFFECTS: zero.
| Effect + deps | external system | why render/handler cannot do it | measured render cost | lifetime + cleanup | focused test |
|---|---|---|---|---|---|
| none | none | n/a | n/a | n/a | n/a |
RENDER: no product UI render path. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or lockfile delta; array/object plans both defer at 41 before browser installation/capture.
VERDICT: clear
VISUAL CHECK: not applicable
WHY: the exact diff changes CI planning, evidence validation, workflow budgeting, acceptance, promotion, tests, and docs only. Local scope classification is hasStableVisual: false; exact-head Stable visual regression, pr-a11y, and pr-rtl are correctly path-skipped, while visual-acceptance is success with No stable visual scope. No Storybook or browser was started.
VERDICT: clear
REMEDY SEARCH: not triggered — no proven visual defect
No rendered element, role, accessible state, focus path, translation, locale formatting, or directional layout changes.
VERDICT: clear
| slot | verdict |
|---|---|
| PROBLEM | clear |
| SOLUTION | clear |
| ARCHITECTURE | clear |
| IMPACT | clear |
| API | clear |
| THEMING | clear |
| BREAKING | clear |
| PERFORMANCE | clear |
| VISUAL | clear |
| A11Y & I18N | clear |
GOAL: met — the restacked commit is byte-equivalent to the approved head, and exact-head fixtures re-prove terminal mixed acceptance, removal preimage/no-after, no-prune promotion, 25/41 parity, trusted object deferral, Probe current-only semantics, and 121→4 scope reduction. DISPOSITION: no negative current-head finding remains; semantic approval is unchanged. ADVICE: omitted — the approved implementation and invariants are unchanged. AUTHOR CAN PROCEED: yes — restack revalidation is complete; no code or design decision remains. WORST OUTCOME: none found — the new commit changes only parent/identity, not source or behavior.
JUDGEMENT NEEDED: none — defect correction inside the existing CI authority.
comment — semantic approve (self-authored draft)
No findings.
Semantic verdict: approve
Restack revalidated at 04246c0: identical patch/tree to the approved head, one commit on current main; lifecycle, 25/41 budgets, Probe, and no-prune promotion remain green.
[Reviewed by Robohands]
None.
- Restack proof: old/new base trees
3d673168, old/new head treesd696514f, stable patch id8aa2e884, empty old→new tree diff, and exactly one commit above current main. - Exact-head focused Vitest: 5 files, 99/99; selected invariant rerun 8/8; five runtime scripts pass
node --check; three interacting workflows pass actionlint with the unchanged custom-runner label ignored;check:repopasses. - Exact-head remote state: 17 passing checks/statuses, 5 documented path skips, 0 pending/failed; CI run 33183589053 is green;
visual-acceptanceis success with no stable scope.
TIME total 14m setup 4m exact heads, fresh code/wiki checkouts, public rules/rubric, untrusted workflow guard reading 3m all four prior rounds, restack topology, and current acceptance/promotion/budget paths measuring 4m 99 tests, selected invariant rerun, independent lifecycle fixture, syntax/actionlint/check:repo, exact CI writing 3m presentation, literal template gate, two critic passes, and delivery waste 2m corrected the standalone negative fixture and excluded a pre-existing custom-runner lint diagnostic
- The changed
workflow_runpublisher cannot execute from PR-head YAML; exact-head contract tests and production-CLI fixtures cover it instead. - The protected scheduled Release Gate remained disabled and was not dispatched. No Storybook/browser run was needed for this non-rendering CI change.
Posted as drafted in the exact-head GitHub COMMENT review; GitHub recorded COMMENTED at 04246c0051029d017e6aa76b3b649bc55bb622dc.