Skip to content

Review 5608

Jump to bottom
Cindy Zhang edited this page Aug 27, 2026 · 9 revisions

Review-5608

#5608 — fix(ci): keep accepted stable shots in the release gate · by @cixzhang · bucket: the maintainer
Verdict: approve · merge held for a human (release governance) · review comment

Head reviewed

e462cf3d0e9e22227884efaf4de5dcb837760ed2 — parent 2458b03b35a45512b996b0c2c764d9530a36a052 (#5303). The complete diff was reviewed.

Verdict

Approve. Both independent-review blockers and the follow-up capture-failure edge are closed. Merge remains held for the human-gated release process; nothing was merged or dispatched.

Verified authority model

  • Only the unreduced complete-stable daily plan may report removals.
  • Exact, component, theme, mixed, sampled, trusted, probe-tier, and custom-package plans cannot claim complete removal authority.
  • Mixed partial report → accept → promote preserves unrelated manifest keys and PNGs.
  • Complete removals derive from declared plan keys, not successful screenshots. A planned capture failure stays a failure, never a removal; failed verdicts cannot prune.
  • A complete plan crashes if it omits a currently reproducible stable key. Genuinely deleted public stories and themes remain reviewable removals.
  • Explicitly excluded current stories remain outside removal comparison.

Built Storybook ownership proof

A fresh exact-head Storybook build with Charts, Vega, and Lab dists absent produced the reported production shape:

command_exit=0
built_bare_entries=38
built_bare_files=16
built_bare_paths={@astryxdesign/charts: 32, @astryxdesign/vega: 6}
planned=884
eligible_baseline_covered=882
current_additions=2
private_or_canary_story=0
private_or_canary_theme=0
probe=0
story_packages={@astryxdesign/core: 884}
overflow=168
carousel=196

The same production resolver handles workspace-relative source paths, absolute/build-transformed packages/ or node_modules/ paths, and bare package specifiers/subpaths. The checked-in built-index fixture contains all 38 observed bare entries across 16 Charts/Vega story files. Unknown or ambiguous forms fail closed.

Package manifests—not titles or story names—decide eligibility:

  • Charts and Vega resolve to their private/canary package metadata and are excluded.
  • Public Core resolves stable.
  • Core-titled Probe Theme resolves to its private package and is excluded.
  • Lab/Resizable resolves to public Core and remains in the stable lane despite its title.

Production result

Against the live 974-key baseline:

  • 884 planned Core shots
  • 882 / 882 eligible stable baseline keys covered
  • 0 missing eligible keys
  • 2 current additions from #5303's Date consistency story
  • 0 private, probe-theme, or canary shots
  • OverflowList: 168 shots
  • Carousel: 196 shots

Verification

  • Visual-gate suite: 262 / 262 passed across 19 files.
  • Fresh Storybook production build and metadata validation passed.
  • pnpm check:repo passed.
  • pnpm lint passed with 0 errors (87 existing warnings).
  • actionlint passed with the repository's existing custom-runner and shellcheck exceptions.
  • Exact-head CI run 33124606543, Lint run 33124606431, and CLI Smoke Test run 33124606481 passed.
  • #5600 publication locking and #5611 recovery identity checks remain green.

Non-blocking notes

The daily capture approximately doubles from 428 to 884 shots. Based on recent run timings, the visual job is expected to rise from about 21 minutes to 26–28 minutes within its 45-minute budget. The fresh review found no blocking correctness issue.

Review posted

Both blockers close at e462cf3d: I reproduced the 38 bare entries from a real dists-absent build and drove 884/882 against the live baseline. Still held for the release process.

[Reviewed by Robohands]

Clone this wiki locally