Skip to content

Review 5632

Cindy Zhang edited this page Aug 28, 2026 · 11 revisions

Review-5632

PR

#5632 — fix(ci): trace affected PR check dependencies by @cixzhang (bucket: the maintainer)

HEAD REVIEWED

a0c034b7c6fccdf6ff5adea3dd7ed949bd6ac7d0 — direct child of approved #5630 head d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc, stacked on approved #5608 head 37d70566fd5203763877d8f93f8e8469255f9005.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.3 (the official page's Current line at gate 1; its history already contains a 1.14.4 row)

LANE

LANE: full WHY: this changes cross-cutting CI scope and trust boundaries, adds a shared classifier, changes workflow gates, and can silently suppress browser/a11y evidence.

PRIOR REVIEW AND CURRENT MAIN

There was no prior Review-5632 record and no human GitHub feedback at this head. The only PR comments were generated Vercel and PR Analysis reports.

The approved parent records establish that #5608 keeps one baseline-owned membership source and current deletion/failure verdicts, while #5630 keeps PR acceptance/promotion non-pruning, Probe current-only, and removal-bearing plans inside the 25/41 review budget. This top commit changes none of those approved evidence, acceptance, comparison, or promotion files.

Current main is 936256386b74d67f88a41adee235b91699977b2f, the stack base. No current-main commit touches the twelve top-commit files.

PROBLEM

WHY 1: component-only pull requests need bounded visual, RTL, and accessibility checks, while shared rendering inputs cannot be reduced to one trustworthy component list. WHY 2: broad PR sweeps can produce hundreds of shots a reviewer cannot judge, but silently skipping a real dependency leaves a green-looking PR with no evidence. WHY 3: a maintainer needs every rendering/audit input to reach exactly one of two outcomes: a bounded exact PR check or an explicit protected-main deferral. USER-FACING PROBLEM: a maintainer reviewing a component or rendering-infrastructure change needs a truthful scope decision; otherwise the check can either overwhelm review or report no work for a change that affects output. PROBLEM SEVERITY: broken task — an unclassified input can make all three PR browser/a11y jobs skip with no deferral. NEW FEATURE CASE: not a product feature — this repairs existing CI coverage routing. EARLY STOP: clear — the PR body, approved parent contracts, exact-head workflow log, and production classifier probes establish the intended two-outcome model.

VERDICT: clear

SOLUTION

A shared classifier reads changed paths and assigns component source to an exact component set. Shared or uncertain rendering inputs instead mark the visual and accessibility lanes for protected-main coverage. The workflow gates use that result, the report explains it, and lab readiness accepts both the old pathspec syntax and the new affected-root marker.

SOLUTION (3 decisions · 6 runtime/workflow files, +420/−146)

  1. Add one changed-path classifier for exact components and global/uncertain dependencies.
  2. Route CI and report generation through exact scope versus explicit protected-main deferral.
  3. Preserve lab-readiness compatibility by parsing both the legacy pathspec and a new root marker.

BURDEN: medium — one new module, three consumers, one workflow gate, one parser marker, two new output booleans, and 145 focused tests across nine files. BURDEN MATCH: proportionate in shape, but incomplete in coverage — the central classifier is the right-sized owner, yet known inputs still fall through to “no scope.”

VERDICT: BLOCKS — the new two-outcome model has a third silent outcome for real component, theme, browser, and dependency inputs

ARCHITECTURE

OWNER: .github/scripts/lib/affected-scope.js owns changed-path dependency classification. TIER 1: git changed-file list, component package roots, Storybook config, theme/build infrastructure, setup/browser inputs, package manifests, and the lockfile. TIER 2: none. SEAMS: component root versus nested runtime files, shared infrastructure, stable theme packages, Storybook preview/config/static assets, browser setup/version, dependency resolution, mixed component+global changes, missing merge base, analysis/comment output, and legacy/current lab-readiness syntax. BEHAVIOR UNIT: pure utility — the classifier is deterministic and directly probeable from a path list. COMPLEXITY BUDGET: one classifier, one authoritative component-root source, every known input classified exact-or-deferred, one bounded O(changed paths) pass, and zero independent hand-maintained copies. ACTUAL BURDEN: 12 files, +709/−173 total; 6 runtime/workflow files +420/−146; 6 test files +289/−27; one new classifier module; one new comment marker that independently repeats the component roots; no dependency or lockfile change. BURDEN TREND: first reviewed head; relative to approved #5630, the top commit adds one classifier/consumer layer without changing the parent evidence or mutation authorities. RESET TRIGGER: not triggered — this is the first model-level finding, and the contraction is local: complete the classifier and make readiness consume the same root source.

domain fact one authoritative writable source generated / immutable projections other writable copies
affected dependency class affected-scope.js analysis JSON and workflow outputs none, but the source list is incomplete
component package roots COMPONENT_PACKAGES in affected-scope.js exact component records ci.yml affected-root comment, parsed independently by lab readiness — can drift
stable theme eligibility package manifests trusted snapshots and stableThemes none
protected-main coverage scheduled/manual Release Gate visual/a11y verdict and report none
seam driven result
packages/core/src/Button/Button.tsx exact Core/Button scope; no deferral
packages/core/src/Calendar/hooks/index.ts no component and no deferral, although Calendar, DateInput, and DateTimeInput import this runtime barrel
packages/core/src/theme/tokens.ts shared-theme deferral
packages/core/src/utils/parseStyleKey.ts no component and no deferral, although theme rule generation imports it and its own contract says it is shared by CSS generation and component class rendering
apps/storybook/.storybook/preview.tsx Storybook-config deferral
apps/storybook/public/demo.woff2 Storybook-static-asset deferral; the current rendering roots contain no checked-in static/font asset, so this path class is synthetic/future-facing
.github/actions/setup/action.yml browser-version deferral
.nvmrc no component and no deferral, although the setup action names it as the single Node-version source
pnpm-lock.yaml / root or package manifest uncertain dependency deferral
pnpm-workspace.yaml no component and no deferral, although it owns the Vite catalog, overrides, linker, and allowed install scripts
component + lockfile component is discovered internally, but workflow outputs suppress the exact component jobs and defer the whole bundle
no merge base explicit visual/a11y deferral; no broad PR run
legacy pathspec / current marker both parse to Core + Lab; the marker is not behaviorally tied to COMPONENT_PACKAGES and can false-pass after classifier drift

The catch-all metadata exclusion at affected-scope.js lines 17–36 drops nested runtime barrels as well as component-root metadata. The global/uncertain list at lines 39–87 omits actual setup and theme inputs. The workflow then turns any fallthrough into false/no-defer outputs at visual-scope.mjs lines 154–173.

VERDICT: BLOCKS — known inputs can bypass both exact PR scope and explicit deferral, and component roots have two writable representations

IMPACT

A pull request changing a nested component barrel, the Node source of truth, the workspace dependency catalog, or the shared theme selector parser receives has_components=false, has_stable_visual=false, a11y_deferred=false, and stable_visual_deferred=false. Its visual, RTL, and a11y jobs therefore skip as if nothing relevant changed. A mixed component+global change also loses its exact component run instead of preserving the bounded part. NEW FEATURE IMPACT: not a product feature.

VERDICT: BLOCKS — maintainers can receive a clean-looking no-scope result for a real rendering or audit dependency

API

No published package API changes. Internal CI surfaces add analysis.affectedScope, stable_visual_deferred, a11y_deferred, and the affected-root marker.

change public? class doc'd? verdict
+ affectedScope analysis object internal artifact existing PR analysis source/tests finding — incomplete input coverage
+ deferred scope outputs/reasons internal workflow contract existing component/visual outputs source/tests finding — false/no-defer outcome remains
+ affected-component-roots marker internal parser contract legacy pathspec fallback source/tests finding — independent writable root copy

OSSIFICATION: no consumer package surface. The internal contract is repairable before landing; its exact-or-deferred invariant should be pinned now rather than learned from a skipped future PR.

VERDICT: BLOCKS — the new internal scope contract does not represent every dependency it claims to own

THEMING

No token, target, selector, CSS, or rendered theme changes. The review did trace theme dependencies: core theme files, reset/tailwind CSS, themeProps, theming-target discovery, build files, and stable theme packages defer correctly; parseStyleKey.ts, which feeds generateThemeRules, currently falls through.

VERDICT: BLOCKS — one real shared theme-generation input is outside the new dependency model

BREAKING

BEHAVIOR: yes inside CI — exact component jobs now skip on broad/uncertain input and broad work defers to the protected scheduled lane. API: no public package API change. VISUAL: no product pixels change in this top commit. THEME: no theme contract changes; only scope classification changes.

VERDICT: BLOCKS — intended routing works for listed paths, but omitted paths silently retain the old false-negative behavior

PERFORMANCE & RESOURCES

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 render path. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or lockfile delta. Classification is one bounded pass over changed paths plus set deduplication/sort; no recursion, browser, network request, or unbounded capture is added.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the twelve top-commit files are CI scripts/tests, one workflow, and lab-readiness tooling. No component implementation, story, style, theme, asset, or rendered app file changes. Storybook was not started.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No product role, accessible state, focus path, string, locale, or direction behavior changes. The finding is about whether a11y/RTL checks run: exact component source is scoped correctly in the positive case, but the omitted component/global inputs above can skip both PR coverage and explicit deferral.

VERDICT: BLOCKS — the changed gate can silently omit required a11y/RTL evidence

JUDGEMENT

slot verdict
PROBLEM clear
SOLUTION BLOCKS — silent third outcome remains
ARCHITECTURE BLOCKS — incomplete dependency model and duplicated roots
IMPACT BLOCKS — real inputs can look irrelevant
API BLOCKS — internal scope contract is incomplete
THEMING BLOCKS — shared parser omitted
BREAKING BLOCKS — false-negative routing remains
PERFORMANCE clear
VISUAL clear
A11Y & I18N BLOCKS — required evidence can skip

GOAL: partly met — six requested positive classes route as designed, but four actual repository inputs and nested runtime barrels produce neither exact scope nor deferral; mixed changes suppress the exact component lane. DISPOSITION: incomplete dependency classification and duplicated root ownership block now. Acceptance criteria: (1) every input in the declared component/theme/Storybook/asset/browser/dependency classes resolves to exact scope or explicit deferral; (2) nested runtime barrels, .nvmrc, pnpm-workspace.yaml, and parseStyleKey.ts are pinned; (3) mixed component+global behavior preserves the exact bounded component lane or explicitly proves why whole-PR deferral is required; (4) current lab readiness derives roots from the executable classifier while legacy pathspec parsing remains covered; (5) a non-exempt path inside the declared dependency roots cannot silently mean “no scope.” ADVICE: bounded outcome criteria — keep the new classifier as the owner; do not add another independent path registry. AUTHOR CAN PROCEED: yes — the defect and acceptance criteria are concrete; implementation choice remains with the author. WORST OUTCOME: “maintainers can receive a clean-looking no-scope result for a real rendering or audit dependency” → request changes.

JUDGEMENT NEEDED: none — correctness defect in the existing CI contract.

request changes

  1. Known rendering/audit inputs fall through the classifier, and the readiness root marker can drift from executable scope. → a maintainer can merge a change after visual, RTL, and a11y jobs all skip without an explicit protected-main deferral · .github/scripts/lib/affected-scope.js:17-87

REVIEW

Semantic verdict: request changes

Thanks — centralizing scope is the right shape, but this head can still return “no scope” for real inputs. ROOT_METADATA drops every nested index.ts, including the Calendar hook barrel used by Calendar and both date inputs. The dependency list also misses .nvmrc, pnpm-workspace.yaml, and the shared theme parseStyleKey.ts; each produces no component and no deferral. Mixed component+global changes suppress the exact component lane too. Lab readiness then trusts a comment marker that can drift from the executable map.

Please keep the executable classifier as the one root source, and ensure every input in these declared dependency classes resolves to exact scope or explicit protected-main deferral, with the negative and mixed cases pinned.

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None.

EVIDENCE I DID NOT SPEND

  • Exact-head focused Vitest passed 9 files / 145 tests; changed-script syntax, actionlint for the three interacting workflows, and check:repo passed.
  • Exact-head CI run 33164175791 passed every executed code job. Its scope log correctly classified the parent-stack workflow/script paths as browser-version-input; pr-a11y, pr-rtl, and Stable visual regression skipped by design.
  • Security review found a fork head with changed workflows/scripts, but no package, lockfile, workspace, Node-version, lifecycle-script, permission, env/credential read, or network-call delta in the top commit; parent execution surfaces were already approved at their exact heads.

TIME

TIME total 10m setup 3m exact heads, fresh code/wiki checkouts, current authorities, trust screening, dependency seed reading 3m parent records, semantic delta, dependency consumers, workflow logs measuring 2m 145 focused tests, syntax, actionlint, repository checks, classifier/parser probes writing 2m presentation, two critic passes, exact-head GitHub/wiki delivery waste 1m first broad patch render was truncated and replaced with semantic-diff and direct-file reads

WHAT I COULD NOT VERIFY

  • The changed trusted workflow_run comment path cannot execute from PR-head code; the current generated comment therefore does not exercise this head's deferral wording.
  • The protected scheduled Release Gate was not dispatched. No Storybook/browser run was required for this non-rendering CI change.

What changed before posting

Posted as drafted in the exact-head GitHub COMMENT review; GitHub recorded COMMENTED at a0c034b7c6fccdf6ff5adea3dd7ed949bd6ac7d0.


Round 2 — exact-head delta re-review

PR

#5632 — fix(ci): trace affected PR check dependencies by @cixzhang (bucket: the maintainer)

HEAD REVIEWED

d8d553774f79b80672035c0fbd8c2cf8a082d732 — direct child of approved #5630 head d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc, stacked on approved #5608.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4

LANE

LANE: full WHY: this re-review changes cross-cutting visual/a11y scope, trusted workflow routing, and the source of lab-readiness evidence after a prior architecture request-changes round.

PRIOR REVIEW AND CURRENT MAIN

Our Round 1 review at a0c034b7c6fccdf6ff5adea3dd7ed949bd6ac7d0 asked for nested runtime barrels, .nvmrc, pnpm-workspace.yaml, and parseStyleKey.ts to resolve exact-or-deferred; mixed exact+broad input to retain both lanes; unknown/non-exempt inputs not to become no-scope; one executable component-root source; and legacy/current readiness syntax coverage. The replacement head shares parent d7681c97 with the reviewed head; the semantic delta is 10 files, +169/−43.

The exact PR head remained d8d553774f79b80672035c0fbd8c2cf8a082d732, clean and mergeable, throughout the run. The approved parent contracts from #5608 and #5630 were re-read; this delta does not change their baseline ownership, removal authority, Probe policy, or promotion mutation paths.

PROBLEM

WHY 1: component-only pull requests need bounded visual, RTL, and accessibility checks, while shared or unknown rendering inputs need explicit protected-main deferral. WHY 2: a third outcome — neither exact work nor deferral — makes a green-looking skip indistinguishable from irrelevant files; a split exact+broad result can also route into a workflow path that refuses its own scope. WHY 3: the maintainer needs one truthful, executable routing decision for every non-exempt changed path before trusting the review evidence. USER-FACING PROBLEM: a maintainer changing a nested barrel, shared parser/hook, or the shared RTL audit harness can still receive missing evidence or a trusted-plan failure even though the classifier reports a relevant change. PROBLEM SEVERITY: broken task — the named parseStyleKey.ts case deterministically enters exact capture and is then refused as broad-only, while other non-exempt paths still emit no visual or a11y scope. NEW FEATURE CASE: not a product feature — this repairs existing CI coverage routing. EARLY STOP: clear — the prior acceptance criteria, current source, GitHub-output probes, workflow predicates, and production CLI failure establish the intended and actual behavior.

VERDICT: BLOCKS — the exact-or-deferred contract still has rejecting and silent third outcomes

SOLUTION

The delta correctly adds the four named inputs to classification, keeps exact component data when a known global input also defers, and exports a component-root command for readiness. It then introduces a second exact-visual predicate over Core files, while the helper retains its own component/root predicates and readiness detects an invocation string before loading the helper through a fallback. These independent interpretations disagree on the named parser, nested barrels, unknown shared inputs, and malformed current syntax.

SOLUTION (3 decisions · 4 runtime/workflow files, +68/−24; 10 files, +169/−43 including tests)

  1. Expand known global/uncertain inputs and nested-barrel component discovery — fixes the named positive cases in the helper.
  2. Separate exact visual presence from broad deferral so mixed inputs can set both — fixes the output booleans but not every downstream route.
  3. Replace the readiness comment marker with a helper command — points at the right owner, but the command is unused by CI classification and the parser fallback can accept a missing/mistyped helper.

BURDEN: medium — one classifier, a second visual runtime predicate, two routing booleans, workflow predicates, a root CLI, a readiness loader/fallback, and six focused test surfaces. BURDEN MATCH: disproportionate at the contract boundary — the repair adds another representation of exact scope and roots instead of making one classifier result authoritative.

VERDICT: BLOCKS — multiple predicates still own the same scope facts

ARCHITECTURE

OWNER: .github/scripts/lib/affected-scope.js should own component identity, component roots, global/uncertain classification, and exact-or-deferred outcomes; workflows and readiness should consume that result. TIER 1: changed-file list, Core/Lab component roots, Storybook inputs, theme/build infrastructure, browser/setup inputs, package/dependency inputs, and protected-main coverage. TIER 2: none. SEAMS: nested barrels, component metadata exemptions, shared Core hooks/utilities, known/unknown audit inputs, named global inputs, exact+broad mixtures, trusted-plan routing, current helper syntax, malformed helper syntax, and legacy pathspec syntax. BEHAVIOR UNIT: one pure classifier plus one executable serialization of its result; callers should not re-derive component/root/scope semantics. COMPLEXITY BUDGET: one classifier, one component-root source, one exact-or-deferred result per non-exempt path, one mixed union, zero shadow predicates or fallback data sources. ACTUAL BURDEN: COMPONENT_ROOTS, componentSource()'s separate core|lab regex, visual-scope.mjs's separate isRuntimeCoreFile() and component extraction, an unused workflow COMPONENT_ROOTS variable, and a readiness parser that recognizes the command text then can load another checkout through a catch-all fallback; 10-file delta, +169/−43. BURDEN TREND: first reviewed head had one incomplete classifier plus a copied marker; current head adds the missing named entries but also adds another root representation and exact-visual routing predicate. The number of owners for component/root/scope facts grew rather than contracted. RESET TRIGGER: triggered — the prior one-source finding was repaired by adding another representation/guard for the same facts, and the named parseStyleKey.ts path now fails between those representations. Redraw around one classifier result before adding cases.

domain fact one authoritative writable source generated / immutable projections other writable copies
component roots intended COMPONENT_ROOTS helper CLI output componentSource() regex; isRuntimeCoreFile() root regex; readiness legacy parser
exact component identity componentSource() affected.components visual-scope.mjs independently derives stableComponents from stableCoreFiles
global/uncertain deferral inputKind() reason lists and workflow outputs visual-scope.mjs independently broadens all Core runtime files
mixed exact+broad routing classifier result CI outputs / trusted scope workflow chooses exact capture or broad defer as mutually exclusive paths
readiness roots helper should be sole current source current CLI output; legacy pathspec parse catch-all fallback can source roots outside the inspected fixture/repoRoot
seam driven result
.nvmrc fixed: visual+a11y explicit browser-version-input deferral
pnpm-workspace.yaml fixed: visual+a11y explicit lockfile-ambiguity deferral
nested Calendar/hooks/index.ts a11y exact Calendar; visual outputs has_stable_visual=false and stable_visual_deferred=false
parseStyleKey.ts reports exact=true + broad=true, no exact component/story; exact workflow calls trusted-plan, which exits 1: broad stable scope must be deferred instead of captured
component + .nvmrc exact and broad booleans both true; exact a11y components retained, but visual broad-defer step is suppressed when exact is true and report generation does not carry its deferred reasons
unknown apps/storybook/rtl-audit/rtl-audit.mjs all exact/deferred visual+a11y outputs false — the real RTL harness is silent no-scope
shared packages/core/src/hooks/useLayer.tsx exact visual=true + broad visual=true with no component; no a11y component or deferral; trusted-plan rejection shape
helper-root mutation advertising Core only still classifies Lab/Drawer exact — the list does not own classification
valid current helper CLI prints Core + Lab roots
malformed component-roots-broken CLI exits 0 with no output; readiness parser still returns Core + Lab when the fixture helper is absent because the regex/fallback false-passes
legacy pathspec still parses Core + Lab in the focused regression test
exempt test/doc files remain intentionally no-scope

The representative split is visual-scope.mjs lines 154–171: exact visual scope is re-derived independently from affected components/deferral, then trusted workflow routing treats exact and broad as exclusive actions. The rejecting guard is visual-acceptance.mjs lines 767–775, and the root split begins at affected-scope.js lines 12–38.

VERDICT: BLOCKS — component identity, roots, and routing still have multiple owners and disagree on real inputs

IMPACT

A maintainer changing parseStyleKey.ts reaches a trusted publisher failure rather than protected-main deferral. A nested barrel still receives no visual scope, the real RTL audit harness can receive no scope anywhere, and a shared hook can lack a11y/RTL deferral. Mixed known input retains its exact checks, but the visual evidence path does not publish the broad half of the routing decision. NEW FEATURE IMPACT: not a product feature.

VERDICT: BLOCKS — required review evidence can still be absent, incomplete, or deterministically red

API

No published package API change. Internal CI surfaces add exactStableVisual, componentRoots(), the component-roots CLI mode, new scope booleans, and parser syntax.

change public? class doc'd? verdict
+ componentRoots(): string[] + CLI output internal CI/parser contract existing component package map tests/source BLOCKS — does not own classifier roots
+ exactStableVisual: boolean internal trusted workflow contract existing stable component/story scope tests/source BLOCKS — separate predicate conflicts with deferral
~ mixed exact+broad booleans internal workflow output existing exact/deferred lanes tests/source note — flags are preserved, but visual downstream union is incomplete
~ readiness current syntax + legacy fallback internal audit parser existing legacy pathspec test/source BLOCKS — malformed/missing helper can false-pass

OSSIFICATION: no consumer package surface. These internal fields gate required evidence, so locking in mutually inconsistent meanings creates silent skips and trusted-job failures.

VERDICT: BLOCKS — the internal scope contract is not single-source or total

THEMING

No CSS, theme target, token, variable, or rendered theme output changes. parseStyleKey.ts is correctly identified as shared theme infrastructure by the helper, but the visual caller also marks it exact and sends it to a trusted-plan path that rejects it.

VERDICT: BLOCKS — the specifically requested shared theme parser still cannot complete its routing path

BREAKING

BEHAVIOR: yes inside CI — the four named classifications and mixed flags change, but some now route to failure or remain silent. API: no published package API. VISUAL: no product pixels change in this delta. THEME: no theme contract change; only theme-change evidence routing changes.

VERDICT: BLOCKS — intended scope correction remains incomplete end to end

PERFORMANCE & RESOURCES

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 render path. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or lockfile delta. Classification remains O(changed paths) plus set/sort work; the defect is ownership/routing rather than cost.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the delta changes CI scripts/tests, workflows, and lab-readiness parsing only. No component implementation, story, style, asset, theme package, or rendered app path changes. No Storybook server or browser was started.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No product role, accessible state, focus path, string, locale, or direction behavior changes. The affected gate is itself an a11y/RTL evidence boundary: nested barrels now reach exact a11y, but unknown shared inputs can still produce neither component scope nor protected-main a11y deferral.

VERDICT: BLOCKS — required a11y/RTL evidence can still silently disappear

JUDGEMENT

slot verdict
PROBLEM BLOCKS — exact/deferred routing still has silent and rejecting outcomes
SOLUTION BLOCKS — added predicates do not share one owner
ARCHITECTURE BLOCKS — root/component/scope facts have multiple writable representations
IMPACT BLOCKS — required evidence can be absent or red
API BLOCKS — internal contract meanings disagree
THEMING BLOCKS — named parser path still fails
BREAKING BLOCKS — CI behavior remains incomplete
PERFORMANCE clear
VISUAL clear
A11Y & I18N BLOCKS — unknown shared inputs still skip deferral

GOAL: partly met — .nvmrc, pnpm-workspace.yaml, helper-level nested barrels, parseStyleKey.ts classification, and mixed booleans move as requested; the production parseStyleKey.ts route still fails, nested visual scope and unknown/global totality remain incomplete, and roots still have shadow parsers. DISPOSITION: the prior one-source/exact-or-deferred finding still blocks now. Acceptance criteria: one executable classifier/root source drives exact components and broad deferral; every non-exempt path gets at least one truthful outcome; mixed input preserves and publishes both outcomes; parseStyleKey.ts completes by deferral rather than exact-plan refusal; valid current and legacy syntax pass while missing/malformed current syntax fails closed. ADVICE: bounded contraction criteria — remove shadow derivation and consume one classifier result; no implementation is prescribed. AUTHOR CAN PROCEED: yes — the failing paths, invariants, and positive/negative/mixed regression matrix are deterministic. WORST OUTCOME: “A maintainer can receive no required evidence or a trusted-plan failure for a real rendering/audit dependency.” → request changes.

JUDGEMENT NEEDED: none — correctness defect in an existing CI contract.

request changes

  1. Scope and root facts still have multiple owners, producing silent or rejecting third outcomes. → a maintainer changing a nested barrel, shared parser/hook, or the real RTL audit harness can lose visual/a11y evidence or receive a deterministic trusted-plan failure · visual-scope.mjs:154 → confirmed independently by production CLI output/failure plus root/parser mutations that leave the test suite green while breaking the claimed contract.

REVIEW

Semantic verdict: request changes

Thanks — the four named inputs now classify, and mixed changes preserve the exact booleans, but the exact/deferred contract is still split.

At this head, parseStyleKey.ts becomes both exact and broad while yielding no exact component/story; the exact workflow path then calls trusted-plan, which refuses it as broad-only. A nested barrel still has no visual outcome, and the real RTL audit harness has no outcome anywhere. The root command is also not the owner: changing its roots leaves component classification unchanged, while readiness accepts a missing or mistyped helper through its fallback.

Please contract this to one executable classifier consumed by workflow and readiness, with every non-exempt path producing exact scope or explicit deferral. Pin the positive, negative, ambiguous, mixed, and legacy/current syntax cases.

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — the defect is one cross-file ownership/routing model.

EVIDENCE I DID NOT SPEND

  • .nvmrc and pnpm-workspace.yaml now produce broad visual+a11y deferral with the intended reasons; component + .nvmrc retains exact component and broad flags.
  • Exact-head GitHub reports 17 successful and 5 path-skipped checks, 0 pending/failed; the check-components log classifies this stack’s known workflow/scripts as browser-version-input and defers them.
  • Security screening found no top-delta dependency, lockfile, lifecycle-script, permission, credential, or network-call addition; parent execution surfaces were already approved at exact heads.

TIME

TIME total 12m setup 4m exact heads, trust screening, fresh isolated code/wiki worktrees, dependency clone reading 3m current kit/rubric, all Review-5608/5630/5632 rounds, source and workflow consumers measuring 3m focused tests/checks, old/new matrix, production CLI failure, root/parser mutations, exact CI writing 2m presentation, template gate, two critic passes, GitHub/wiki delivery waste 1m first verification command inherited the parent directory and was rerun from the exact-head worktree

WHAT I COULD NOT VERIFY

  • The changed trusted workflow_run publisher cannot execute PR-head workflow code; exact-head Actions therefore prove repository code checks, not this modified default-branch publisher path.
  • The protected scheduled Release Gate was not dispatched, as requested. No Storybook/browser run was needed for this non-rendering CI change.

What changed before posting

Posted as drafted in the exact-head GitHub COMMENT review; GitHub recorded COMMENTED at d8d553774f79b80672035c0fbd8c2cf8a082d732.


Round 3 — exact-head delta re-review

PR

#5632 — fix(ci): trace affected PR check dependencies by @cixzhang (bucket: the maintainer)

HEAD REVIEWED

fbf0e9b1dddfee059fca6a3d5e305901cf0b74f9 — direct child of approved #5630 head d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc, stacked on approved #5608.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4

LANE

LANE: full WHY: this re-review must settle a standing architecture block across changed-path classification, trusted visual routing, exact a11y/RTL filters, and lab-readiness contract parsing.

PRIOR REVIEW AND CURRENT MAIN

Our Round 2 review at d8d553774f79b80672035c0fbd8c2cf8a082d732 asked for one executable classifier/root source consumed by workflows and readiness, every non-exempt path to produce exact scope or explicit deferral, mixed scope to preserve both outcomes, parseStyleKey.ts to defer rather than enter rejected planning, and current syntax to fail closed while legacy syntax remains compatible.

This replacement commit has the same approved parent. Ignoring formatting-only rewrites, the prior-head delta changes three files by +38/−8: the helper becomes directly executable, visual scope consumes its component parser, and CI invokes the helper before the visual wrapper. Current main remains 936256386b74d67f88a41adee235b91699977b2f, the stack base; no current-main commit invalidates these paths.

PROBLEM

WHY 1: one component path must produce the same exact a11y and RTL filter everywhere it is used. WHY 2: if a workflow rebuilds that filter independently, changing the shared classifier can leave the gate green while the browser job receives no component. WHY 3: the maintainer then cannot trust a green routing contract to mean the named component was actually audited. USER-FACING PROBLEM: a maintainer extending or correcting component scope can update the advertised owner while a11y/RTL keep using a shadow parser; the tests still report green and the required exact check can silently disappear or broaden. PROBLEM SEVERITY: broken task — the PR exists to eliminate scope drift, but an executable mutation proves that drift remains undetected at the workflow boundary. NEW FEATURE CASE: not a product feature — this repairs existing CI evidence routing. EARLY STOP: clear — prior acceptance criteria, exact-head source, workflow output tracing, direct syntax probes, and a mutation run establish both the intended and actual contracts.

VERDICT: BLOCKS — exact component identity still has multiple executable owners

SOLUTION

The changed-path helper now correctly owns the named path classifications and the stable visual wrapper consumes its component parser. It also emits exact component names as workflow outputs. The a11y and RTL steps do not consume those outputs: they rebuild their filters from the older analysis component lists, while readiness recognizes command text and can load a helper outside the inspected repository after failure.

SOLUTION (3 decisions · one new module plus four runtime/workflow consumers)

  1. Make the shared helper executable and serialize exact/deferred scope — correct and useful.
  2. Route stable visual component identity through the helper — fixes the prior rejected exact-plan path.
  3. Keep a11y/RTL selection on the older analysis parser and readiness on a catch-all loader fallback — leaves the one-source contract incomplete.

BURDEN: medium — 14 top-commit files, +916/−195 against the approved parent; one classifier module, two workflow classifier invocations, two component-list representations, one readiness command marker/fallback, and 158 focused tests. BURDEN MATCH: disproportionate at the remaining seam — the new executable output is written but discarded before the two browser jobs that need it.

VERDICT: BLOCKS — the repair adds an output without removing the consumers’ shadow parser

ARCHITECTURE

OWNER: .github/scripts/lib/affected-scope.js should own component identity, component roots, and exact/deferred changed-path scope; visual, a11y, RTL, reporting, and readiness should consume source-identified projections. TIER 1: git changed paths, Core/Lab roots, Storybook/theme/browser/dependency inputs, trusted visual publisher, a11y/RTL browser jobs, and lab-readiness derivation. TIER 2: none. SEAMS: nested barrels, shared theme parsers, Node/workspace globals, mixed exact+broad unions, exact a11y/RTL filters, current helper syntax, malformed/missing helper syntax, and legacy pathspec compatibility. BEHAVIOR UNIT: one pure classifier plus one executable serialization; workflows should pass its component list rather than reconstruct it. COMPLEXITY BUDGET: one classifier/root source, one exact component set, one exact-or-deferred decision per path, one mixed union, and zero shadow parsers or fallback sources. ACTUAL BURDEN: one helper module; visual scope consumes it; analysis both imports it and separately rebuilds component identity; CI writes affected_components/affected_core_components but exports neither and both browser jobs read newComponents + modifiedComponents; readiness conditionally requires the helper but catches failure by loading from the process working directory. BURDEN TREND: Round 1 had one incomplete classifier plus a copied root marker; Round 2 added the named cases but also shadow visual/root predicates; Round 3 contracts visual component identity into the helper, but exact a11y/RTL identity and readiness failure behavior remain independently writable. Owner count shrinks by one and remains above the stated budget. RESET TRIGGER: remains triggered — the prior architecture reset required one executable owner, while this round leaves the same disputed component fact in the helper and the analysis parser and adds an unused workflow serialization.

domain fact one authoritative writable source generated / immutable projections other writable copies
changed-path dependency class affected-scope.js visual scope and analysis metadata none for the named inputs
exact component identity intended helper components affected_components / affected_core_components analyze-pr.js independently derives newComponents / modifiedComponents, and a11y/RTL consume those instead
current component roots helper COMPONENT_ROOTS component-roots CLI output readiness first parses CI command text, then can load a helper from another working directory
exact/broad visual routing helper plus stable story/theme metadata trusted scope JSON no rejected exact plan for the named broad-only cases
seam driven result
nested Calendar/hooks/index.ts fixed: exact Core/Calendar component and exact visual scope
parseStyleKey.ts fixed: exact visual false, broad visual/a11y true; protected-main deferral precedes exact planning
.nvmrc / pnpm-workspace.yaml fixed: explicit broad visual/a11y deferral
Core/Button + .nvmrc exact Button plus broad visual/a11y flags are both retained
Lab/Drawer helper reports exact @astryxdesign/lab:Drawer
workflow use of exact components helper writes the names, but the check-components job does not expose them; a11y and RTL read the independent analysis arrays
shadow-parser mutation changing only analysis’s Lab directory makes its exact list empty while the helper still reports Lab/Drawer; all 80 focused contract tests pass
current helper syntax real repository returns Core + Lab roots
legacy pathspec syntax explicit compatibility parser returns Core + Lab roots
missing current helper readiness still returns Core + Lab by loading the process-working-directory helper
mistyped component-roots-broken syntax readiness still returns Core + Lab for the same reason
exact-head Actions parent-stack workflow/script paths defer broadly; a11y, RTL, and visual are skipped, so the run does not exercise an exact component filter

The representative workflow boundary is ci.yml lines 478–484: a11y builds its component filter from analysis.json; the RTL step repeats the same expression at lines 722–740. The helper’s emitted component outputs are only defined and tested, not consumed. Readiness’s cross-checkout fallback is at automated.mjs lines 100–112.

VERDICT: BLOCKS — component identity and current-syntax failure still have multiple executable sources

IMPACT

A maintainer changing the shared classifier’s roots or component policy can get a green helper contract while the exact a11y and RTL jobs receive a different filter. The executable Lab mutation demonstrates the full failure: the advertised owner still reports Drawer, the browser-job source becomes empty, and every focused contract test remains green. A missing or mistyped current helper can also be certified by unrelated local code rather than failing the inspected repository. NEW FEATURE IMPACT: not a product feature.

VERDICT: BLOCKS — required evidence can silently use a different component set from the advertised scope owner

API

No published package API changes. Internal workflow surface adds executable JSON/GitHub outputs for affected components and deferred reasons.

change public? class doc'd? verdict
+ affected_components / affected_core_components internal workflow output existing component filter source/tests BLOCKS — written but not consumed by a11y/RTL
~ stable visual exact scope from helper component identity internal trusted scope existing visual scope source/tests clear
~ readiness current helper syntax + legacy pathspec internal audit parser existing compatibility contract source/tests BLOCKS — missing/malformed current syntax false-passes

OSSIFICATION: no consumer package surface. These internal values gate whether required evidence runs, so independent parsers create the exact silent-drift class the PR is intended to remove.

VERDICT: BLOCKS — the internal exact-component contract is serialized but not authoritative

THEMING

No CSS, token, target, variable, selector, or rendered theme output changes. The named shared parser now correctly broad-defers before exact planning.

VERDICT: clear

BREAKING

BEHAVIOR: yes inside CI — nested barrels and named global inputs now route correctly, but the exact a11y/RTL component filter still comes from the prior parser. API: no published package API. VISUAL: no product pixel change. THEME: no theme contract change; only evidence routing changes.

VERDICT: BLOCKS — the intended one-source routing behavior is incomplete at two browser-job consumers

PERFORMANCE & RESOURCES

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 render path. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or lockfile delta. Classification remains one bounded pass over changed paths; the defect is ownership and workflow consumption, not cost.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the top commit changes CI scripts/tests, workflows, and lab-readiness tooling only. No component implementation, story, style, asset, theme package, or rendered app path changes. No Storybook server or browser was started.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No product role, accessible state, focus path, string, locale, or direction behavior changes. The affected boundary decides whether exact a11y/RTL evidence runs: both jobs still derive their component list from analysis.json rather than the helper output, and the mutation test proves the two can diverge without a failing contract test.

VERDICT: BLOCKS — exact a11y/RTL selection does not consume the shared component source

JUDGEMENT

slot verdict
PROBLEM BLOCKS — one-source evidence routing remains unproved
SOLUTION BLOCKS — executable output is discarded before a11y/RTL
ARCHITECTURE BLOCKS — exact component identity still has shadow parsers
IMPACT BLOCKS — browser jobs can receive a different component set
API BLOCKS — internal output is not authoritative
THEMING clear
BREAKING BLOCKS — intended routing is incomplete
PERFORMANCE clear
VISUAL clear
A11Y & I18N BLOCKS — exact filters remain independently derived

GOAL: partly met — nested barrels, parseStyleKey.ts, .nvmrc, pnpm-workspace.yaml, and mixed exact+broad classification now produce the required values; the exact component outputs are not consumed by a11y/RTL, and missing/malformed current readiness syntax still false-passes. DISPOSITION: the prior one-source architecture finding still blocks now. Acceptance criteria: (1) the executable classifier’s exact component set is the filter input for a11y and RTL, not merely another artifact field; (2) a drift mutation in any former parser fails a contract test; (3) current helper syntax loads only the inspected repository and missing/malformed current syntax fails closed; (4) legacy pathspec compatibility remains explicit; (5) the passing named path and mixed-union cases remain. ADVICE: bounded contraction criteria — consume the existing executable output and remove or demote shadow derivation; no new owner is needed. AUTHOR CAN PROCEED: yes — the ownership defect, failed mutation guard, and exact acceptance criteria are deterministic. WORST OUTCOME: “the shared classifier says a component is in scope while both browser-job filters receive no component and every contract test stays green” → request changes.

JUDGEMENT NEEDED: none — correctness and architecture defect inside an existing CI contract.

request changes

  1. Exact component identity is emitted by one module but consumed from another parser. → a maintainer changing scope can silently lose the exact a11y/RTL component filter while the advertised classifier and all contract tests stay green · ci.yml:478 → confirmed independently by source/output tracing and a Lab-parser mutation that produced divergent filters while 80 focused tests passed.

REVIEW

Semantic verdict: request changes

We asked for one executable scope owner; the named path cases now route correctly, but the contract still stops at classification.

At the browser-job filter boundary, a11y—and the matching RTL step—rebuild their component filter from analysis.json’s independent newComponents + modifiedComponents parser instead of consuming the component list emitted by affected-scope.js. Mutating only that shadow parser to drop Lab leaves the shared classifier reporting Lab/Drawer while both jobs receive no component; all 80 focused contract tests still pass. Lab readiness likewise accepts missing or mistyped current helper syntax by loading another checkout through its fallback.

Please make the executable classifier’s component set the a11y/RTL workflow input and make current-syntax readiness fail closed, while keeping legacy syntax as an explicit compatibility branch.

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — one cross-file ownership defect carries the verdict.

EVIDENCE I DID NOT SPEND

  • Exact-head focused Vitest passed 9 files / 158 tests; the drift mutation passed 5 files / 80 tests while producing the helper-versus-analysis mismatch.
  • Actionlint passed the four interacting workflows; six changed runtime files passed node --check; git diff --check and check:repo passed.
  • Exact-head CI run 33171909063 is green for every executed job; exact a11y, RTL, and visual jobs are path-skipped because this stack’s workflow/script files broad-defer.

TIME

TIME total 12m setup 3m exact heads, fresh code/wiki checkouts, live rules/rubric, trust screening, dependency seed reading 3m every parent/current round, prior-head semantic delta, workflow and readiness consumers measuring 4m 158 tests, path/syntax matrix, shadow-parser mutation, actionlint, syntax, repository and CI checks writing 2m presentation, two critic passes, exact-head GitHub/wiki delivery waste 1m one broad source search produced irrelevant component-path references and was replaced with targeted consumer reads

WHAT I COULD NOT VERIFY

  • The changed trusted workflow_run publisher cannot execute PR-head workflow code; current Actions prove repository checks and broad deferral, not an exact component consumer path.
  • The protected scheduled Release Gate was not dispatched. No Storybook/browser run was needed for this non-rendering CI change.

What changed before posting

Posted as drafted in the exact-head GitHub COMMENT review; GitHub recorded COMMENTED at fbf0e9b1dddfee059fca6a3d5e305901cf0b74f9.


Round 4 — one-time manual final delta re-review

PR

#5632 — fix(ci): trace affected PR check dependencies by @cixzhang (bucket: the maintainer)

HEAD REVIEWED

a5676f81cab6fe9ecaa1352ff0b73aa2e8338e82 — direct child of approved #5630 head d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc, stacked on approved #5608.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4

LANE

LANE: full WHY: this fourth exact-head round must settle a standing one-source architecture block across every PR visual/a11y/RTL consumer and fail-closed lab-readiness parsing.

PRIOR REVIEW AND CURRENT MAIN

Our Round 3 review at fbf0e9b1dddfee059fca6a3d5e305901cf0b74f9 asked for the executable classifier’s exact component set to feed a11y/RTL, every former parser’s drift mutation to fail, and current helper syntax to fail closed while legacy syntax stays explicit. The current five-file delta fixes the a11y/RTL filters and rejects missing, malformed-export, throwing, and cross-checkout fallback helpers, but it leaves another PR visual parser and accepts malformed invocation syntax.

The exact head remained a5676f81cab6fe9ecaa1352ff0b73aa2e8338e82, clean and mergeable, throughout the review. Its parent is the approved #5630 head; current main remains base 936256386b74d67f88a41adee235b91699977b2f, so no later-main change invalidates this delta.

PROBLEM

WHY 1: every PR browser/audit consumer must receive the same exact component and global-deferral decision. WHY 2: a handwritten inventory can omit a real workflow while all listed-consumer tests stay green, and a prefix parser can certify a command CI would not execute. WHY 3: maintainers need scope/readiness evidence to fail on drift rather than silently widen, skip, or certify a broken gate. USER-FACING PROBLEM: a maintainer changing component or story scope can still receive visual evidence derived from the legacy component parser, while a malformed current readiness command is reported as covering Core and Lab. PROBLEM SEVERITY: broken task — one executable drift mutation survives the complete consumer-contract test and two malformed current commands false-pass. NEW FEATURE CASE: not a new feature — this repairs existing CI evidence routing. EARLY STOP: clear — the prior acceptance criteria, exact-head source, consumer inventory, direct path probes, and executable mutations establish the remaining failures.

VERDICT: BLOCKS — the one-source/fail-closed goal remains incomplete

SOLUTION

The delta adds versioned scope metadata and a declared consumer list, routes the exact a11y and RTL filters through the shared analysis projection, and makes readiness import and execute the inspected helper. Those changes fix the Round 3 Lab drift and cross-checkout fallback. The consumer list and its readiness copy are still handwritten and omit the in-workflow Stable visual job, while readiness recognizes a command prefix and then executes a different canonical command.

SOLUTION (3 decisions · 3 runtime/workflow files, +63/−13; 5 files, +174/−13 including tests)

  1. Publish schema, roots, and a declared consumer inventory from the classifier.
  2. Read a11y/RTL component filters from analysis.affectedScope.components.
  3. Validate, import, and execute the inspected scope helper for readiness.

BURDEN: medium — one classifier, three serialized projections, three handwritten consumer inventories/checks, two workflow component-filter paths, and one readiness syntax/loader boundary. BURDEN MATCH: disproportionate at the remaining boundary — metadata names five consumers without enforcing the actual set, so the same one-source defect survives behind another list.

VERDICT: BLOCKS — the repair does not cover every consumer or the actual current command syntax

ARCHITECTURE

OWNER: .github/scripts/lib/affected-scope.js should own component identity, roots, and exact/deferred scope; every visual/a11y/RTL/readiness consumer should execute it or consume a source-identified projection. TIER 1: changed paths, Core/Lab roots, exact component filters, stable-story scope, broad deferral, PR visual jobs, and lab-readiness derivation. TIER 2: none. SEAMS: nested barrels, shared theme parser, Node/workspace inputs, exact+broad mixtures, Lab a11y/RTL filters, stable-story-only visual scope, malformed current helper invocations, explicit legacy pathspec syntax, and the real RTL harness input. BEHAVIOR UNIT: one pure classifier plus one executable serialization; consumer membership and invocation syntax need executable validation rather than parallel lists/prefixes. COMPLEXITY BUDGET: one classifier/root source, one complete consumer inventory derived from actual workflow use, one exact-or-deferred result per non-exempt path, and zero shadow parsers or fallback commands. ACTUAL BURDEN: 14 top-commit files, +1080/−198 against approved #5630; the final delta is 5 files, +174/−13. The helper CONSUMERS, readiness EXPECTED_SCOPE_CONSUMERS, and workflow test’s hand-selected jobs are three inventories; ci.pr-visual is in none. Readiness matches a command prefix but always executes canonical component-roots itself. BURDEN TREND: Round 3 was 14 files, +916/−195; current grows by +174/−13. A11y/RTL and cross-checkout loading contract correctly, but consumer/invocation representations grow while the same one-source finding remains. RESET TRIGGER: triggered — this is the fourth round of the same model-level finding, and the repair adds another consumer representation without covering the actual consumer set.

domain fact one authoritative writable source generated / immutable projections other writable copies
exact component identity affected-scope.js analysis.affectedScope, visual-scope result ci.pr-visual still reads newComponents + modifiedComponents
scope-consumer membership none none helper CONSUMERS, readiness expected list, workflow-test slices; all omit ci.pr-visual
current readiness invocation ci.yml command helper module/CLI result prefix regex accepts malformed command then runs a different canonical command
broad/global deferral affected-scope.js visual/a11y reasons the RTL harness path still has no exact or deferred outcome
seam driven result
nested Calendar/hooks/index.ts exact Core/Calendar; no deferral
parseStyleKey.ts no exact component; visual+a11y defer with shared-theme-token-infrastructure
.nvmrc visual+a11y defer with browser-version-input
pnpm-workspace.yaml visual+a11y defer with lockfile-ambiguity
Core/Button + .nvmrc exact Button plus both broad-defer flags
Lab/Drawer exact Lab component; a11y/RTL consume affectedScope; Lab-root mutation is rejected by 2 failing tests
a11y+RTL old-parser mutation rejected by the workflow contract test
cross-checkout readiness fallback mutation rejected by the missing/malformed/throwing helper test
missing / malformed exports / throwing helper all return no roots at exact head
Stable visual legacy-parser mutation all 13 workflow consumer-contract tests still pass
stable-story-only change visual-scope.mjs reports exact story scope, while ci.pr-visual sees no legacy component and enters the broad full-surface arm
component-roots-broken / component-roots --unexpected both return Core + Lab instead of failing closed
apps/storybook/rtl-audit/rtl-audit.mjs no exact component and no protected-main deferral

VERDICT: BLOCKS — actual consumers and current-syntax validity are not represented by one executable contract

IMPACT

The a11y and RTL component filters now follow the shared source and reject a Lab-parser drift. The remaining Stable visual job can still widen or diverge under a legacy parser while its inventory test stays green, and lab readiness can certify Core/Lab coverage for a command CI would not execute as intended. NEW FEATURE IMPACT: not a new feature.

VERDICT: BLOCKS — maintainers can receive green contract tests for a drifted visual consumer or malformed readiness wiring

API

No published package API change. Internal CI surfaces add schema/version metadata, a consumer list, component roots, and changed workflow/readiness parsing.

change public? class doc'd? verdict
+ scope schema/version, roots, consumer inventory internal CI metadata existing affected-scope contract source/tests BLOCKS — inventory is incomplete and duplicated
~ a11y/RTL filters from affectedScope.components internal workflow contract existing exact component filter source/tests clear
~ readiness helper validation/execution internal audit contract existing current + legacy syntax source/tests BLOCKS — malformed current invocation false-passes

OSSIFICATION: no consumer package surface. These internal fields gate review evidence, so a partial inventory or permissive parser directly recreates the drift this PR is meant to remove.

VERDICT: BLOCKS — the internal contract is not complete or fail-closed

THEMING

No CSS, token, target, variable, selector, or rendered theme output changes. parseStyleKey.ts still broad-defers correctly.

VERDICT: clear

BREAKING

BEHAVIOR: yes inside CI — a11y/RTL now use the canonical component projection and readiness removes the cross-checkout fallback; another visual parser and malformed syntax remain. API: no published package API. VISUAL: no product pixel change. THEME: no theme contract change; only evidence routing changes.

VERDICT: BLOCKS — intended one-source routing remains incomplete at a visual consumer and readiness parser

PERFORMANCE & RESOURCES

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 render path. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or lockfile delta; classification remains bounded by changed paths.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the final delta changes CI scripts/tests, one workflow, and lab-readiness tooling only. No component implementation, story, style, asset, theme package, or rendered app path changes. Exact-head Stable visual regression, pr-a11y, and pr-rtl are path-skipped and are not claimed as evidence.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No product role, accessible state, focus path, string, locale, or direction behavior changes. The affected boundary determines whether exact a11y/RTL evidence runs: both exact filters now consume the canonical analysis projection, and their legacy-parser mutant fails. The actual RTL harness input itself still produces no exact or deferred scope.

VERDICT: BLOCKS — the exact filters are fixed, but the shared RTL audit input remains outside the total path contract

JUDGEMENT

slot verdict
PROBLEM BLOCKS — one-source/fail-closed goal remains incomplete
SOLUTION BLOCKS — declared inventory does not enforce the actual consumers
ARCHITECTURE BLOCKS — one visual parser and malformed invocation remain independent
IMPACT BLOCKS — green tests can certify drifted scope/readiness
API BLOCKS — internal contract is incomplete
THEMING clear
BREAKING BLOCKS — intended CI behavior is incomplete
PERFORMANCE clear
VISUAL clear
A11Y & I18N BLOCKS — RTL harness input still has no outcome

GOAL: partly met — a11y/RTL consume affectedScope, Lab and cross-checkout fallback mutants fail, and missing/malformed-export/throwing helpers return no roots; the actual Stable visual consumer is omitted and its drift mutant survives, malformed current commands return roots, and the RTL harness path remains no-scope. DISPOSITION: the standing one-source architecture finding still blocks now. Acceptance criteria: (1) every actual PR visual/a11y/RTL/readiness consumer is inventoried from executable use and consumes the shared result; (2) mutating any former parser makes a contract test fail; (3) current helper syntax is matched/executed exactly and rejects missing, malformed, throwing, mistyped, or extra-argument forms; (4) the RTL harness input resolves to explicit protected-main coverage; (5) the passing nested-barrel, shared-parser, Node/workspace, mixed, Lab, and legacy cases remain. ADVICE: bounded contraction criteria — remove the remaining shadow parser and derive the inventory/syntax check from actual executable wiring; do not add another list. AUTHOR CAN PROCEED: yes — the failing consumer, commands, path, and regression outcomes are deterministic. WORST OUTCOME: “all consumer-contract tests stay green while a real Stable visual job no longer receives the canonical exact scope.” → request changes.

JUDGEMENT NEEDED: none — correctness and architecture defect inside an existing CI contract.

request changes

  1. The declared consumer inventory omits a real Stable visual scope parser. → a maintainer can receive broad or divergent visual evidence while the shared classifier and all consumer-contract tests stay green · ci.yml:633
  2. Readiness recognizes malformed current commands and the RTL harness path has no scope outcome. → a Lab promotion can be certified against wiring CI would not execute, while a change to the audit implementation receives no explicit coverage route · automated.mjs:143

REVIEW

Semantic verdict: request changes

We asked for every scope consumer to use one executable owner. The a11y/RTL filters and helper loading now do, but the consumer inventory still misses pr-visual.

Stable visual regression still builds its component plan from newComponents + modifiedComponents. Mutating only that parser to return no components left all 13 consumer-contract tests green; a stable-story-only change likewise has exact story scope in visual-scope.mjs but falls into this job’s broad full-surface arm.

Lab readiness also accepts component-roots-broken and component-roots --unexpected: its prefix match recognizes either, then executes the canonical command instead. Both returned Core + Lab in the fail-closed probe. The real RTL harness path still yields neither exact scope nor protected-main deferral.

Please include every PR visual consumer in the executable contract, make each drift mutant fail, and reject malformed current helper invocations.

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — both defects cross inventory, workflow, and readiness boundaries.

EVIDENCE I DID NOT SPEND

  • Exact-head path matrix: nested Calendar barrel exact; parseStyleKey.ts, .nvmrc, and workspace config defer; mixed Button + Node keeps exact plus broad; Lab/Drawer reaches both exact a11y/RTL consumers.
  • Exact PR test plan: 9 files / 162 tests passed; an additional compare suite brings the focused total to 10 files / 182 tests. Twelve changed runtime/test scripts pass syntax; exact actionlint, git diff --check, and check:repo pass.
  • Exact-head Actions: 17 checks/statuses pass and 5 are path-skipped, with 0 pending/failed. Stable visual, pr-a11y, and pr-rtl did not exercise an exact component consumer on this head.

TIME

TIME total 12m setup 3m exact heads, claim, fresh code/wiki worktrees, current rules/rubric, trust screening reading 3m every parent/current review round, five-file delta, full consumer and workflow dataflow measuring 4m path/readiness matrix, four mutations, 182 tests, syntax, actionlint, repo checks, exact CI writing 2m presentation, literal template gate, two critic passes, GitHub/wiki delivery waste 1m four mutation probes were first launched with a reserved shell variable and repeated serially

WHAT I COULD NOT VERIFY

  • Modified PR workflow code cannot execute from the PR head; exact-head Actions prove repository checks and broad path-skips, not these exact consumer paths.
  • The protected scheduled Release Gate was not dispatched, and no automation state was changed. No Storybook/browser run was needed for this non-rendering CI delta.

What changed before posting

Posted as drafted in the exact-head self-authored GitHub COMMENT review; GitHub recorded COMMENTED at a5676f81cab6fe9ecaa1352ff0b73aa2e8338e82.


Round 5 — exact green-head final delta re-review

PR

#5632 — fix(ci): trace affected PR check dependencies by @cixzhang (bucket: the maintainer)

HEAD REVIEWED

e0148bb94091ff082ff77dd8d92bdf0e36a1009c — direct child of approved #5630 head d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc, stacked on approved #5608 head 37d70566fd5203763877d8f93f8e8469255f9005.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4

LANE

LANE: full WHY: this final round must clear four standing scope/architecture findings across every PR visual, a11y, RTL, and readiness consumer, and must verify a rewritten functional delta plus an exact green header-only tip.

PRIOR REVIEW AND CURRENT MAIN

Our Round 4 review at a5676f81cab6fe9ecaa1352ff0b73aa2e8338e82 asked for every actual PR visual/a11y/RTL/readiness consumer to use the executable shared result; every former parser drift mutant to fail; malformed current helper invocations to fail closed; the RTL harness to receive explicit protected-main coverage; and the nested-barrel, shared-parser, Node/workspace, mixed, Lab, exact, and legacy cases to remain.

The replacement functional head 8f37fbb650116265d5b5476ffe00e315cb1268b1 shares the approved parent d7681c97 and changes 10 files, +406/−69 from Round 4. Exact head e0148bb9 differs from it by only the required two-line copyright header on the new composite action; failed CI run 33178767409 identifies that single missing header, and current exact-head CI is green. Current main remains 936256386b74d67f88a41adee235b91699977b2f; no touched path moved after Round 4.

PROBLEM

WHY 1: PR visual, a11y, and RTL jobs previously rebuilt component scope through separate parsers, so one consumer could drift while another stayed green. WHY 2: a maintainer could receive skipped, broadened, or misleading review evidence for the same changed path even though the advertised classifier was correct. WHY 3: required evidence is trustworthy only when every workflow consumer enters through one executable scope contract and broad inputs fail visibly into protected-main coverage. USER-FACING PROBLEM: a maintainer reviewing component or rendering-infrastructure work needs every browser/audit job to use the same exact-or-deferred decision; otherwise a green review can omit the path that changed. PROBLEM SEVERITY: broken task — Round 4 proved one real visual consumer and malformed readiness syntax could diverge while the contract tests stayed green. NEW FEATURE CASE: not a new feature — this repairs existing CI evidence routing. EARLY STOP: clear — prior findings, exact-head source, action-use inventory, path probes, mutation failures, and CI establish the need and result.

VERDICT: clear

SOLUTION

One local composite action now runs the shared changed-path classifier in both changed-file and analysis-artifact modes. The check-components, accessibility, stable visual, and RTL jobs consume that action’s outputs instead of rebuilding component identity. Lab readiness validates the action, every current workflow entry, the helper schema, and a representative classification while retaining the explicit legacy pathspec branch.

SOLUTION (3 decisions · 10-file functional delta, +406/−69)

  1. Add one executable workflow entry over the existing shared classifier.
  2. Replace all four PR workflow component parsers with the action’s exact outputs.
  3. Make readiness reject missing action fields, missing consumers, direct/shadow helper commands, malformed current wiring, and helper/schema drift.

BURDEN: medium — one 55-line composite action, one extended classifier result, four workflow invocations, one readiness contract check, and focused source/workflow mutation tests. BURDEN MATCH: proportionate — the standing defect was cross-workflow drift; one shared entry plus fail-closed consumer checks is the smallest model that removes it.

VERDICT: clear

ARCHITECTURE

OWNER: .github/scripts/lib/affected-scope.js owns component identity and exact/broad dependency classification; .github/actions/affected-scope/action.yml is its sole PR workflow entry. TIER 1: changed paths, Core/Lab roots, exact component filters, stable visual supplemental story/theme classification, a11y/RTL jobs, and protected-main deferral. TIER 2: none. SEAMS: direct changed-file input, analysis artifact input, nested barrels, shared theme parser, Node/workspace globals, mixed exact+broad input, Lab components, stable visual Core filtering, RTL harness inputs, malformed/shadow commands, and legacy readiness syntax. BEHAVIOR UNIT: pure utility plus a local composite action — deterministic from a path list and testable without a browser. COMPLEXITY BUDGET: one classifier, one workflow entry, four current PR job consumers, one stable-visual supplement for story/theme metadata, zero workflow component parsers, and one fail-closed readiness contract. ACTUAL BURDEN: 15 top-commit files, +1438/−217 against approved #5630; Round 5 functional delta is 10 files, +406/−69; one new action; four action call sites; 169 focused tests across nine files; four banked mutation classes. BURDEN TREND: Round 4 had one shared classifier plus three inline consumer representations and permissive readiness parsing. Round 5 adds one projection action while deleting every workflow component parser; runtime owners contract to one and every current job mutation is rejected. RESET TRIGGER: cleared by the requested architecture reset — no additional writable classifier or fallback source remains in a PR visual/a11y/RTL workflow.

domain fact one authoritative writable source generated / immutable projections other writable copies
exact component identity affected-scope.js composite action outputs, analysis.affectedScope, visual-scope’s imported result report-only newComponents/modifiedComponents remain, but no workflow gate consumes them
PR workflow scope entry local composite action four ci.yml uses; workflow/readiness contract tests none
stable story/theme visual scope visual-scope.mjs, layered over the shared classifier check-components flags and trusted PR-comment scope none
broad/global and RTL-harness deferral affected-scope.js inputKind() visual/a11y/RTL booleans and reasons none
readiness compatibility current composite-action contract; explicit legacy pathspec branch helper schema/root/sample validation no catch-all or cross-checkout fallback
seam driven result
packages/core/src/Button/Button.tsx exact Core/Button; no broad deferral
packages/lab/src/Drawer/index.ts exact Lab/Drawer; a11y/RTL consume the shared output
packages/core/src/Calendar/hooks/index.ts exact Core/Calendar nested-barrel scope
packages/core/src/utils/parseStyleKey.ts no exact component; visual, a11y, and RTL broad-defer as shared theme infrastructure
.nvmrc all three lanes broad-defer as browser-version input
pnpm-workspace.yaml all three lanes broad-defer as lockfile ambiguity
Button + .nvmrc exact Button and all broad-defer flags coexist
RTL script, targets, weekly workflow, summary, and rtlStyles.ts all classify rtl-harness-input and broad-defer
component-roots-broken / component-roots --unexpected readiness returns no roots; injected shadow command fails the workflow contracts
action input modes stdin changed-files and analysis.json produce byte-equivalent classifier results

Four action uses are present in ci.yml: check-components, pr-a11y, pr-visual, and pr-rtl. Repository-wide workflow inspection found no direct affected-scope.js invocation and no newComponents + modifiedComponents or affectedScope.components component parser in any workflow; visual-scope.mjs remains only for stable story/theme metadata and imports the shared classifier.

VERDICT: clear

IMPACT

Maintainers now get one component set in every PR visual/a11y/RTL job, while shared or ambiguous inputs retain explicit protected-main deferral. Removing an action consumer, restoring the old visual parser, adding a direct malformed helper command, or dropping a declared contract consumer fails focused tests before the drift can ship. NEW FEATURE IMPACT: not a new feature.

VERDICT: clear

API

No published package API changes. Internal CI adds the local action inputs/outputs and an rtl deferral projection; both action input modes are pinned to the same helper output.

change public? class doc'd? verdict
+ composite changed-files / analysis-file inputs internal workflow contract existing changed-path classifier action/test clear
+ exact component/core-component, a11y/RTL deferral, reasons, and JSON outputs internal workflow contract existing affected-scope result action/test clear
~ readiness current action contract + explicit legacy pathspec internal audit contract existing compatibility path source/test clear

OSSIFICATION: no consumer package surface. Workflow consumers now share one executable projection, and the two input forms are parity-tested.

VERDICT: clear

THEMING

No CSS, token, target, variable, selector, or rendered theme output changes. The existing shared-theme path remains broad: parseStyleKey.ts produces shared-theme-token-infrastructure deferral in visual, a11y, and RTL.

VERDICT: clear

BREAKING

BEHAVIOR: yes inside CI — all four PR scope jobs now consume one action, malformed current wiring fails closed, and RTL harness changes explicitly defer. API: no published package API change. VISUAL: no product pixels, DOM, styles, stories, assets, or rendered theme output change. THEME: no theme contract change; only evidence routing for shared theme inputs remains broad.

VERDICT: clear

PERFORMANCE & RESOURCES

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 render path. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or lockfile delta. Scope work remains one bounded pass over changed paths; the action invokes the same helper once per consumer.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: the functional delta changes CI scripts/tests, one local action, one workflow, and lab-readiness tooling only. It changes no component implementation, story, style, asset, theme package, DOM, or rendered app path. Storybook/browser capture was not required.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No product role, accessible state, focus path, string, locale, direction, or rendered behavior changes. The evidence gate itself is repaired: exact a11y and RTL filters consume the action outputs, and the RTL harness script, targets, weekly workflow, summary, and shared RTL utility all broad-defer. Their broad-defer and mixed-exact cases were driven directly.

VERDICT: clear

JUDGEMENT

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 — all four actual PR jobs enter through the composite action; repository-wide workflow search finds no shadow component parser; four independent mutants fail; malformed command probes fail closed; and every prior exact/broad/mixed/RTL path returns its intended outcome. DISPOSITION: every Round 4 finding is satisfied; no new blocking finding. The PR remains a draft, so the self-authored public disposition is a semantic COMMENT approving once undrafted. ADVICE: omitted — no corrective action remains. AUTHOR CAN PROCEED: yes — undraft when the approved parent stack is ready; this review authorizes no merge or auto-merge. WORST OUTCOME: none found → approve once undrafted.

JUDGEMENT NEEDED: none — correctness repair inside the existing CI contract.

approve once undrafted

REVIEW

Semantic verdict: approve once undrafted

Thanks — one action now owns four PR scope jobs. Drift mutants fail, the exact/deferred matrix holds, and the tip is header-only.

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None.

EVIDENCE I DID NOT SPEND

  • Exact-head focused Vitest passed 9 files / 169 tests. Consumer-removal, pr-visual shadow-parser, malformed direct-command, and declared-inventory mutants each failed the intended workflow/readiness contracts; the restored pair passed 40/40.
  • Exact action input parity and 14-case path matrix passed; the nested barrel, parseStyleKey.ts, .nvmrc, workspace, mixed, Core, Lab, and five RTL-harness cases produced the intended exact/broad outcomes. Twelve changed scripts passed node --check; exact PR-plan actionlint, git diff --check, and check:repo passed.
  • Exact-head CI run 33180678847 has 17 passing and 5 intentionally skipped checks, with 0 failed/pending. Its check-components log runs the new action and broad-defers this workflow/script stack; the prior red run and 8f37fbb..e0148bb diff independently prove the final tip is copyright-only.

TIME

TIME total 12m setup 2m exact heads, fresh code/wiki worktrees, current rules/rubric, fork trust screening, dependency seed reading 3m all four prior rounds, PR body, functional/header deltas, every workflow consumer, readiness/action contracts measuring 5m path matrix, four mutations, 169 tests, syntax, actionlint, repository gate, exact CI logs writing 2m presentation, literal template gate, two critic passes, GitHub/wiki delivery waste 1m first actionlint run emitted known repository shell/runner diagnostics before the PR’s exact flags were used

WHAT I COULD NOT VERIFY

  • Exact-head Actions path-skipped Stable visual, pr-a11y, and pr-rtl because this stack changes shared workflow/scripts; their exact component branches were verified locally through clean tests and mutations rather than a live exact-component PR run.
  • The protected scheduled Release Gate, weekly RTL/a11y sweeps, and disabled Automations were not dispatched or changed. No Storybook/browser run was required for this non-rendering CI delta.

What changed before posting

Posted as drafted in the exact-head self-authored GitHub COMMENT review; GitHub recorded COMMENTED at e0148bb94091ff082ff77dd8d92bdf0e36a1009c.


Round 6 — authoritative post-#5608 restack review

PR

#5632 — fix(ci): trace affected PR check dependencies by @cixzhang (bucket: the maintainer)

HEAD REVIEWED

1763ae1bc9ef4b5f8e1c8f802146da31869b0334 — direct child of code-approved #5630 exact head 04246c0051029d017e6aa76b3b649bc55bb622dc, whose parent is landed #5608 merge commit a83c559a51fb86260f8309f695b68acd612b2eef.

AUTHORITATIVE ROUND: Round 5's approval at e0148bb9 is invalidated. #5608 landed one minute after that review, changing the stack parent; this round reviews the restacked exact head fresh.

VERSIONS

LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4

LANE

LANE: full WHY: this is a fresh restack review of a cross-workflow exact/deferred scope authority after the prior approval was invalidated by a parent landing.

PRIOR REVIEW AND CURRENT MAIN

Round 5 approved e0148bb94091ff082ff77dd8d92bdf0e36a1009c on parent d7681c97. That approval cannot carry across #5608's land without restack proof and a current-head contract review.

The restack itself is exact: old and landed #5608 trees are both 3d673168; old/new #5630 trees are both d696514f; old/new #5632 trees are both f98f0264; the old/new #5632 top patches are byte-identical and share stable patch id 7ba1fbfb; git range-diff reports e0148bb = 1763ae1. The current head is a direct child of the separately re-approved #5630 head. No #5608 implementation byte is charged to this top commit.

Fresh review found a standing top-commit contract gap that Round 5 missed: generic shared Core hooks/utilities receive neither exact scope nor protected-main a11y/RTL deferral, while the separate visual wrapper broad-defers them. The helper CLI also accepts malformed command forms with exit 0.

PROBLEM

WHY 1: every changed path that can alter shared visual, accessibility, or RTL behavior needs one truthful exact-or-deferred decision. WHY 2: shared focus and behavior utilities feed many components, so treating them as irrelevant skips the precise jobs most likely to catch regressions. WHY 3: a maintainer must not see visual deferral beside a no-component/no-deferral a11y result for the same file. USER-FACING PROBLEM: a maintainer changing useFocusTrap, useListFocus, useLayer, or a shared runtime utility can receive no exact a11y/RTL run and no explicit protected-main deferral, while the visual lane independently says the change is broad. PROBLEM SEVERITY: broken task — the new one-source contract produces contradictory coverage decisions for reachable shared infrastructure. NEW FEATURE CASE: not a product feature — this repairs CI evidence routing. EARLY STOP: clear — source tracing, direct production-module probes, the clean focused suite, and exact-head CI establish the mismatch.

VERDICT: BLOCKS — shared runtime inputs still have a silent third outcome in a11y/RTL

SOLUTION

The PR correctly centralizes named component and global path cases behind one helper and one composite action. Four CI jobs use the action, current readiness validates those call sites, and named exact/broad/mixed cases work. The helper's global classifier remains enumerative, while visual-scope.mjs keeps a separate catch-all for non-component Core runtime; that split leaves ordinary shared hooks/utilities broad only in visual scope. The executable CLI also defaults unknown commands to empty-stdin classification and ignores extra arguments to component-roots.

SOLUTION (3 decisions · 15-file top delta, +1438/−217)

  1. Add one executable classifier and composite action for exact component/global scope.
  2. Route four PR jobs through the action and validate the wiring from readiness.
  3. Retain a visual-only shared-Core catch-all and permissive CLI dispatch, leaving the one-source/fail-closed model incomplete.

BURDEN: medium — one classifier, one composite entry, four action call sites, one visual supplement, one readiness contract, and 169 focused tests. BURDEN MATCH: disproportionate at the remaining boundary — one visual-only path parser exists because the shared classifier does not own the class.

VERDICT: BLOCKS — the action is not the sole owner of broad shared-runtime classification

ARCHITECTURE

OWNER: .github/scripts/lib/affected-scope.js should own exact component identity and broad/global classification; the composite action should project that result to every PR consumer. TIER 1: Core/Lab component roots, shared Core runtime, Storybook/theme/browser/RTL inputs, exact component filters, and protected-main deferral. TIER 2: none. SEAMS: component source, nested barrels, named theme infrastructure, ordinary shared hooks/utilities, Node/workspace inputs, mixed exact+broad input, RTL harness input, action input parity, current/legacy readiness syntax, and malformed helper CLI arguments. BEHAVIOR UNIT: one pure classifier plus one strict executable projection. COMPLEXITY BUDGET: one classifier, one action entry, four CI job consumers, one source-identified story/theme supplement, zero independent broad-runtime parsers, and fail-closed command dispatch. ACTUAL BURDEN: 15 files, +1438/−217; one helper/action; four action uses; visual-scope.mjs still owns isRuntimeCoreFile() and broadens non-component Core runtime independently; the helper's inputKind() returns null for generic hooks/ and utils/ runtime files; unknown or extra CLI command arguments exit 0. BURDEN TREND: Round 5 → current restack is byte-flat, but fresh class verification finds the prior one-source reset incomplete. No restack byte added the gap; it remains in #5632's unchanged functional patch. RESET TRIGGER: triggered — the approved model still has a second executable owner for broad shared-runtime scope, the same architecture class prior rounds required this PR to remove.

domain fact one authoritative writable source generated / immutable projections other writable copies
exact component identity affected-scope.js action outputs, analysis metadata, visual scope none found
named global/uncertain deferral affected-scope.js action and report outputs none found
generic shared Core runtime deferral none visual-only broad flag visual-scope.mjs:isRuntimeCoreFile(); absent from helper a11y/RTL result
action consumers readiness workflow contract four ci.yml action uses helper CONSUMERS names pr-comment.visual but not ci.pr-visual
helper CLI grammar implicit branch on argv[2] roots or default JSON unknown commands fall into default mode; extra root args are ignored
seam driven result
Core Button.tsx exact Core/Button; no deferral
Lab Drawer/index.ts exact Lab/Drawer for a11y/RTL; no stable visual scope
nested Calendar/hooks/index.ts exact Core/Calendar
parseStyleKey.ts visual/a11y/RTL broad-defer as shared theme infrastructure
.nvmrc all three broad-defer as browser-version input
pnpm-workspace.yaml all three broad-defer as lockfile ambiguity
Button + .nvmrc exact Button plus all broad flags
RTL harness all three broad-defer as rtl-harness-input
useLayer.tsx, useFocusTrap.ts, useListFocus.ts, sharedResizeObserver.ts zero components; visual/a11y/RTL helper deferral all false; visual wrapper independently broad true
missing/malformed-export/throwing readiness helper returns no roots in the explicit fixture test
legacy pathspec / current action syntax both return Core + Lab roots
removed action consumer / shadow parser / direct malformed command / missing declared consumer each mutation reds the two current readiness contract tests; restored head is clean
component-roots-broken exits 0 and emits empty-scope JSON
component-roots --unexpected exits 0 and emits Core + Lab roots

The split is between inputKind(), which returns null for generic shared runtime, and isRuntimeCoreFile(), which broadens the same files only for visual scope. Permissive CLI dispatch is at affected-scope.js lines 197–214.

VERDICT: BLOCKS — one shared-runtime fact still has two outcomes and malformed commands do not fail closed

IMPACT

A maintainer changing a shared focus/navigation hook can see Stable visual defer while has_components=false, a11y_deferred=false, and rtl_deferred=false. Exact a11y/RTL jobs skip, and the generated accessibility section can report no violations instead of naming protected-main coverage. A malformed helper command can also succeed with an unrelated result rather than expose broken wiring. NEW FEATURE IMPACT: not a new feature.

VERDICT: BLOCKS — required evidence can silently disappear for shared runtime changes

API

No published package API changes. Internal CI adds action inputs/outputs, helper result/schema, consumer metadata, and the component-roots CLI mode.

change public? class doc'd? verdict
+ exact component and visual/a11y/RTL deferral result internal CI contract existing PR checks source/tests BLOCKS — shared runtime is not total
+ composite action inputs/outputs internal workflow contract existing classifier action/tests clear for the four current call sites
+ component-roots command internal readiness contract current/legacy root discovery source/tests BLOCKS — unknown/extra arguments succeed

OSSIFICATION: no consumer package surface. This internal contract gates whether required evidence exists, so silent success is a correctness failure rather than a documentation nit.

VERDICT: BLOCKS — the internal executable scope contract is incomplete and permissive

THEMING

No CSS, token, target, variable, selector, or rendered theme output changes. The named parseStyleKey.ts and theme infrastructure cases correctly broad-defer in all three lanes.

VERDICT: clear

BREAKING

BEHAVIOR: yes inside CI — named component/global paths are centralized, but generic shared runtime still gets inconsistent lane behavior. API: no published package API change. VISUAL: no product pixels, DOM, styles, stories, assets, or theme output change. THEME: no theme contract change; only evidence routing for shared infrastructure changes.

VERDICT: BLOCKS — intended one-source CI behavior remains incomplete

PERFORMANCE & RESOURCES

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 render path. LISTENERS/OBSERVERS: none added. LAYOUT: none. BUNDLE: no dependency or lockfile delta. Classification remains bounded by changed paths.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: all top-commit files are CI scripts/tests, workflows, one local action, docs, and lab-readiness tooling. No component implementation, story, style, asset, theme package, DOM, or rendered app path changes. No Storybook/browser capture was required.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No product role, accessible state, focus path, string, locale, direction, or rendered behavior changes. The changed boundary determines whether a11y/RTL evidence runs. Named exact and RTL-harness cases work, but shared focus/navigation hooks receive neither exact scope nor explicit deferral.

VERDICT: BLOCKS — the new gate can silently omit the exact evidence class it protects

JUDGEMENT

slot verdict
PROBLEM BLOCKS — shared runtime has a silent third outcome
SOLUTION BLOCKS — visual retains an independent broad parser
ARCHITECTURE BLOCKS — the one-source reset is incomplete
IMPACT BLOCKS — a11y/RTL evidence can disappear
API BLOCKS — internal command/scope contract is permissive
THEMING clear
BREAKING BLOCKS — intended routing remains incomplete
PERFORMANCE clear
VISUAL clear
A11Y & I18N BLOCKS — shared hooks skip exact and deferred coverage

GOAL: partly met — the four action consumers, requested exact/broad/mixed path matrix, readiness failure fixtures, and four drift mutations pass; generic shared Core runtime still splits visual from a11y/RTL, and malformed helper commands succeed. DISPOSITION: the prior one-source architecture finding blocks again. Acceptance criteria: (1) the executable classifier owns generic shared runtime classification for every lane; (2) visual supplements consume that source rather than independently broadening the class; (3) unknown commands and extra arguments exit nonzero; (4) shared focus/utility and malformed-command regression cases are pinned; (5) all currently passing named cases and mutants remain. ADVICE: bounded outcome criteria — contract the remaining broad parser into the executable owner and fail closed at its CLI boundary. AUTHOR CAN PROCEED: yes — failing files, commands, and expected outcomes are deterministic. WORST OUTCOME: “a shared focus-hook change receives no exact a11y/RTL run and no protected-main deferral while the PR report looks clean” → request changes.

JUDGEMENT NEEDED: none — correctness and architecture defect inside the new CI contract.

request changes

  1. Generic shared Core runtime is broad only in the visual wrapper, not the executable classifier. → maintainers changing shared focus/navigation behavior can lose a11y and RTL evidence while the report looks irrelevant/clean · affected-scope.js:49
  2. The helper accepts unknown commands and extra component-roots arguments with exit 0. → malformed wiring can return an unrelated valid-looking result instead of failing closed · affected-scope.js:197

REVIEW

Semantic verdict: request changes

Thanks — the restack itself is byte-identical, but the one-source scope contract still has a silent shared-infrastructure lane.

inputKind() defers only named utilities. Changes to shared runtime such as useFocusTrap.ts, useListFocus.ts, or sharedResizeObserver.ts return no component and all three deferred flags false; visual-scope.mjs independently broad-defers those same paths, so visual coverage defers while a11y/RTL skip and the PR report can say no violations. The helper also exits 0 for component-roots-broken and component-roots --unexpected.

Please make the executable classifier total for shared runtime inputs, reject unknown/extra CLI arguments, and pin both classes.

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None — both failures cross the executable owner and consumer boundary.

EVIDENCE I DID NOT SPEND

  • Exact restack proof: #5608 old/landed trees 3d673168; #5630 old/new trees d696514f; #5632 old/new trees f98f0264; stable patch id 7ba1fbfb; byte-identical top patches; range-diff equality; direct parent 04246c00.
  • Exact-head focused Vitest passed 9 files / 169 tests. Consumer-removal, visual shadow-parser, malformed direct-command, and declared-inventory mutants each failed the intended readiness contract; restored worktree is clean. Missing/malformed/throwing helper, current/legacy syntax, and action contract fixtures passed 3/3.
  • Action stdin/analysis-file results were byte-identical. Named path matrix passed for Core, Lab, nested barrel, parseStyleKey, .nvmrc, workspace, mixed exact+broad, and RTL harness. The added shared-hook/shared-utility control exposed the contradictory lane result.
  • Twelve changed JS/MJS files pass node --check; the three interacting workflows pass exact actionlint; git diff --check and check:repo pass.
  • Exact-head CI run 33183590319 has 17 passing checks/statuses, 5 intentional skips, and 0 pending/failed. Its log proves the new action executed on the top stack and broad-deferred the listed workflow/visual-gate files.

TIME

TIME total 13m setup 3m exact heads, fresh code/wiki worktrees, current kit/rubric, trust screening, dependency seed reading 3m all Review-5632 rounds, parent authoritative records, restack topology, action/workflow/readiness dataflow measuring 5m 169 tests, path/CLI matrix, four mutations, fail-closed fixtures, syntax/actionlint/repo gates, exact CI writing 2m presentation, two critic passes, exact-head GitHub/wiki delivery waste 1m two shell probes used a reserved scalar/argument form and were immediately rerun correctly

WHAT I COULD NOT VERIFY

  • PR-head workflow code cannot fully exercise the trusted default-branch publisher path; local source-identified contracts and mutations cover it instead.
  • The protected Release Gate, weekly RTL/a11y sweeps, and disabled Automations were not dispatched or changed. No Storybook/browser run was needed for this non-rendering CI delta.

What changed before posting

Posted as drafted in the exact-head self-authored GitHub COMMENT review; GitHub recorded COMMENTED at 1763ae1bc9ef4b5f8e1c8f802146da31869b0334. This Round 6 record supersedes the invalidated Round 5 approval. No merge or auto-merge was enabled.

Clone this wiki locally