fix(design-system): let the visual baseline binding accept a refresh, not only the first adoption - #1742
Conversation
… not only the first adoption `validateCandidateSourceBinding` required `candidateSourceHead` to name a commit whose `AWAITING_BASELINE` still declared the canonical six, and the current tree to declare none. That models the FIRST adoption and nothing else. After #1729 landed, no commit on main declares the six again, so no future capture head could ever satisfy it -- the binding was satisfiable exactly once, and that once has passed. The consequence is only latent today, and this commit does not pretend otherwise: `validateLinuxVisualBaselineSet` short-circuits on `declaredPaths.length === 0` and no surface yet declares `baseline.status: "committed"`, so the whole contract is dormant and refreshing a golden is currently ungated. The defect bites the moment those surfaces are declared (#242) -- at which point the first intentional design change would leave the goldens red with no supported way to re-adopt them, which is precisely backwards for a gate meant to make design changes provable. So the binding now accepts either shape: FIRST ADOPTION candidate head declares the six, current declares none. REFRESH both ends empty, suite byte-identical. A refresh is the ordinary case once baselines exist: a surface is deliberately re-shot and its goldens replaced while the suite itself does not change at all. Normalisation already blanks the AWAITING values on both sides, so the "suite changed beyond the AWAITING declaration" check stays exact for both. Nothing is loosened beyond that. A candidate head with five ids, seven ids, a dynamic set, a spread or a duplicate still fails, as do a non-empty current list, a non-ancestor head, a hash mismatch, a wrong platform and an unreviewed set. The one previously-untested shape -- a refresh -- now has its own case, because the gap was invisible precisely because nothing exercised it. Verified: tests/design-system-adoption.test.ts 51 passed (51), including the new refresh case; check:design-system-adoption and check:design-system-contract exit 0; format:check clean. Refs #242, #118
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe visual baseline adoption script now accepts canonical six-ID states for first adoption and empty states for refreshes. Tests cover empty-to-empty refreshes and updated invalid-state messages. ChangesVisual baseline adoption
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Summary
validateCandidateSourceBindingrequiredcandidateSourceHeadto name a commit whoseAWAITING_BASELINEstill declared the canonical six, and the current tree to declare none. That models the first adoption and nothing else. After test(visual): adopt the six linux visual baselines and satisfy their provenance contract #1729 landed, no commit on main declares the six again, so no future capture head can satisfy it — the binding was satisfiable exactly once, and that once has passed.Stated plainly: the consequence is latent today, not live.
validateLinuxVisualBaselineSetshort-circuits ondeclaredPaths.length === 0and no surface declaresbaseline.status: "committed", so the contract is dormant and refreshing a golden is currently ungated. This is insurance, not a rescue. The defect bites the moment those surfaces are declared (#242) — at which point the first intentional design change would leave the goldens red with no supported way to re-adopt them, which is precisely backwards for a gate meant to make design changes provable.A refresh is the ordinary case once baselines exist: a surface is deliberately re-shot and its goldens replaced while the suite itself does not change. Normalisation already blanks the
AWAITING_BASELINEvalues on both sides, so the "suite changed beyond the AWAITING declaration" check stays exact for both shapes.Nothing is loosened beyond that. A candidate head with five ids, seven ids, a dynamic set, a spread or a duplicate still fails, as do a non-empty current list, a non-ancestor head, a SHA-256 mismatch, a wrong platform and an unreviewed set. The one previously-untested shape — a refresh — now has its own case, which is why the gap was invisible: nothing exercised it.
Verification
tests/design-system-adoption.test.ts—Tests 51 passed (51), including the new refresh case.npm run check:design-system-adoption— exit 0,design-system adoption checked: 53 components, 55 roots.npm run check:design-system-contract— exit 0.npm run format:check—All matched files use Prettier code style!Risk and rollout
git revert.Refs #242, #118.
Summary by CodeRabbit
Bug Fixes
Tests