-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5632
#5632 — fix(ci): trace affected PR check dependencies by @cixzhang (bucket: the maintainer)
a0c034b7c6fccdf6ff5adea3dd7ed949bd6ac7d0 — direct child of approved #5630 head d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc, stacked on approved #5608 head 37d70566fd5203763877d8f93f8e8469255f9005.
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: 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.
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.
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
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)
- Add one changed-path classifier for exact components and global/uncertain dependencies.
- Route CI and report generation through exact scope versus explicit protected-main deferral.
- 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
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
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
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
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
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
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 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: not triggered — no proven visual defect
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
| 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
- 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
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]
None.
- Exact-head focused Vitest passed 9 files / 145 tests; changed-script syntax, actionlint for the three interacting workflows, and
check:repopassed. - 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 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
- The changed trusted
workflow_runcomment 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.
Posted as drafted in the exact-head GitHub COMMENT review; GitHub recorded COMMENTED at a0c034b7c6fccdf6ff5adea3dd7ed949bd6ac7d0.
#5632 — fix(ci): trace affected PR check dependencies by @cixzhang (bucket: the maintainer)
d8d553774f79b80672035c0fbd8c2cf8a082d732 — direct child of approved #5630 head d7681c97aa7c1c2567c7f36ac91860a9bb7a18dc, stacked on approved #5608.
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.14.4
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.
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.
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 newly introduced workflow input 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
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)
- Expand known global/uncertain inputs and nested-barrel component discovery — fixes the named positive cases in the helper.
- Separate exact visual presence from broad deferral so mixed inputs can set both — fixes the output booleans but not every downstream route.
- 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
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 workflows, 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 .github/workflows/new-visual-job.yml
|
all exact/deferred visual+a11y outputs false — 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
A maintainer changing parseStyleKey.ts reaches a trusted publisher failure rather than protected-main deferral. A nested barrel still receives no visual scope, a new workflow path 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
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
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
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
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 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: not triggered — no proven visual defect
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
| 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
- Scope and root facts still have multiple owners, producing silent or rejecting third outcomes.
→ a maintainer changing a nested barrel, shared parser/hook, or new workflow input 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.
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]
None — the defect is one cross-file ownership/routing model.
-
.nvmrcandpnpm-workspace.yamlnow produce broad visual+a11y deferral with the intended reasons; component +.nvmrcretains 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-inputand 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 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
- The changed trusted
workflow_runpublisher 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.
Posted as drafted in the exact-head GitHub COMMENT review; GitHub recorded COMMENTED at d8d553774f79b80672035c0fbd8c2cf8a082d732.