fix(shadcn): close renderer drift against the emitted A2UI contract, with props-level parity tests - #47
Merged
Merged
Conversation
A2UI's A3 gate validates a message against the catalog, so it passes a
renderer that ignores every prop it is handed, and a screenshot cannot
say which emitted prop produced which pixel. This suite closes that gap
at the prop level, against real emitter output only: every distinct A2UI
instance in packages/contracts/out and in the recorded replay fixtures,
folded last-write-wins, with nothing hand-listed.
Four properties, each grounded in how the stack runs:
- consumption: perturbing an emitted prop must change the markup;
- distinguishability: every legal value of an emitted enum prop must
render distinguishably (a projection may restyle, not collapse);
- default fidelity: A2UI's GenericBinder resolves raw properties and
never runs the zod schema's .default(), so an omitted prop reaches
the renderer as undefined and the renderer's fallback is the only
thing that can honor the catalog default;
- content: slots built exactly once, repeated items and table rows
preserved, blank output failed rather than silently passed.
Landed fail-first, against the pre-fix renderers: 4 of 13 fail, naming
13 ignored-prop instances, 8 collapsed enum vocabularies, and 2 wrong
catalog defaults (AlertDialog dresses the contract's `primary` default
as a destructive confirm). The structural detectors are additionally
turned on deliberately broken visuals so no guard is vacuous.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six renderers were dropping or flattening contract vocabulary, so a schema-valid emitted surface drew wrong under this design system — the one failure mode A3 validation and screenshots both wave through. Measured against every distinct emitted A2UI instance in the repo (172: 31 from the contracts build, 141 from the recorded fixtures). Ignored props — the renderer never read them (11 emitted instances): Table.density (8), Table.dividers (1), Table.isStriped (1) — a compact, striped, grid-divided ticket table drew as a default one; TextField.size (1); SelectableCard.variant (2) — an option emitted `blue` drew identical to `default`. Collapsed vocabulary — read, then flattened onto one treatment: Badge's fourteen variants onto four, so `success` was pixel-identical to `info` in five emitted instances; Card's and SelectableCard's color variants onto the token background. Wrong catalog defaults — A2UI's GenericBinder resolves raw properties and never runs the schema's .default(), so an omitted prop arrives as undefined and the renderer's fallback is the contract's last defense: AlertDialog defaulted `actionVariant` to destructive where the catalog says primary, dressing every ungoverned confirmation as dangerous; Badge defaulted to the filled primary treatment where the catalog says neutral. The projections stay native to shadcn — spacing/border utilities for density and dividers, the input scale for size, and shadcn's documented "badge or card with an explicit color class" idiom for the color vocabulary — and every catalog value keeps a treatment of its own. The emitter, the catalogs and the dspack contracts are untouched: the emitted contract is the source of truth and the renderer moved to it. emitted-prop-parity.test.tsx: 4 failed / 13 before, 13 passed after. Package suite 3 -> 16 tests; repo suite 92 -> 105, all green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
CI note: the This branch does not touch Resyncing it here would mean editing a contract copy, which is the one thing the upstream-first rule reserves for a dspack-side decision — so it is deliberately left alone rather than papered over inside a renderer PR. It needs its own change, either Verified locally in the meantime, at 768d8de: The Playwright suites were not run locally, but nothing in |
There was a problem hiding this comment.
🟡 Changes recommended
There are two confirmed correctness gaps (Table header divider behavior vs dividers, and parity test perturbation using unbound values) that could misrepresent contract semantics or miss drift.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR aligns the shadcn renderer implementations with the emitted A2UI contract vocabulary (fixing previously ignored/flattened props) and adds an emitted-prop parity test suite that uses real emitter output as a guard against future renderer drift.
Changes:
- Add a corpus-driven “emitted prop vs consumed prop” parity test suite plus structural/content guards.
- Project previously ignored props/variants into shadcn renderers (Table density/dividers/striping, TextField size, Card/SelectableCard/Badge variants, AlertDialog actionVariant default).
- Introduce shared surface-variant styling and update Tailwind sources; add React/ReactDOM dev deps for server markup tests.
File summaries
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates lockfile entries for React/ReactDOM typings and packages. |
| packages/shadcn-renderers/package.json | Adds devDependencies needed for SSR-markup testing (react/react-dom + types). |
| packages/shadcn-renderers/src/styles.css | Adds Tailwind @source so class strings in surface-variants.ts are scanned. |
| packages/shadcn-renderers/src/surface-variants.ts | Introduces a shared mapping from catalog surface variants to shadcn/Tailwind classes. |
| packages/shadcn-renderers/src/emitted-corpus.ts | Adds test-only emitted-instance corpus loader backed by contracts output + replay fixtures. |
| packages/shadcn-renderers/src/emitted-prop-parity.test.tsx | Adds corpus-driven parity and structural/content guard tests. |
| packages/shadcn-renderers/src/components/TableRender.tsx | Implements density/dividers/striping handling in Table rendering. |
| packages/shadcn-renderers/src/components/TextFieldRender.tsx | Implements size projection onto shadcn input scale. |
| packages/shadcn-renderers/src/components/SelectableCardRender.tsx | Applies shared surface variant styling to SelectableCard. |
| packages/shadcn-renderers/src/components/CardRender.tsx | Applies shared surface variant styling to Card variants. |
| packages/shadcn-renderers/src/components/BadgeRender.tsx | Expands badge variant treatments to preserve full catalog distinction. |
| packages/shadcn-renderers/src/components/AlertDialogRender.tsx | Fixes default/fallback for actionVariant to match catalog default. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 11/12 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
80
to
+85
| return ( | ||
| <div className="w-full overflow-auto rounded-md border"> | ||
| <table className="w-full caption-bottom text-sm"> | ||
| {headers.length > 0 && ( | ||
| <thead className="border-b"> | ||
| <tr className={TR}> | ||
| <tr className="transition-colors"> |
Comment on lines
+117
to
+121
| for (const [prop, value] of Object.entries(props)) { | ||
| if (prop === "action") continue; // dispatch behavior, not markup | ||
| const other = perturb(name, prop, instance.component[prop]); | ||
| if (JSON.stringify(other) === JSON.stringify(instance.component[prop])) continue; | ||
| if (render(name, { ...props, [prop]: other }) === baseline) { |
Comment on lines
+22
to
+27
| export const CONTRACTS_OUT = fileURLToPath(new URL("../../contracts/out/", import.meta.url)); | ||
| export const FIXTURES_DIR = fileURLToPath(new URL("../../replay/fixtures/", import.meta.url)); | ||
|
|
||
| export const catalog: Record<string, any> = JSON.parse( | ||
| readFileSync(join(CONTRACTS_OUT, "catalog.v0_9_1.json"), "utf8"), | ||
| ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six shadcn renderers were dropping or flattening contract vocabulary. A schema-valid emitted surface that draws wrong is failed representation evidence, and it is exactly the failure A3 and screenshots both wave through: A3 validates the message against the catalog, so it passes a renderer that ignores every prop it is handed, and a screenshot shows pixels without saying which emitted prop produced them. This fixes the drift and adds a props-level guard.
Measured, not assumed
Corpus: every distinct A2UI instance dspack-emit has actually produced in this repo — 172 instances, folded last-write-wins per (file, surfaceId, component id):
packages/contracts/out/*.surface.json(the contracts build'semitSurfaceoutput)packages/replay/fixtures/*.json(recorded runs; the A2UI operations ride inside AG-UITOOL_CALL_RESULTpayloads)Method: render each instance under the shadcn registry, then perturb one emitted prop at a time to another legal catalog value. If the markup does not change, the renderer is provably ignoring that prop. Then render every legal value of each emitted enum prop and look for values that collapse onto identical output, and render each prop omitted and compare against its catalog default.
The briefed figure was "7 of 22"; I measured different numbers and am reporting mine. 22 is close to the 23 distinct
(component, prop-set)shapes in the corpus, of which 6 drift — but at instance granularity it is 11 of 172 instances with an outright ignored prop, rising to 16 of 172 once Badge'ssuccess/infocollapse is counted. Six renderers drift, which does match.Drift inventory
TableRenderdensity(8 instances),dividers(1),isStriped(1)TextFieldRendersize(1)SelectableCardRendervariant(2)bluedrew identical todefaultBadgeRendervariantsuccesspixel-identical toinfo(5 emitted instances), the 9 colors all one outline; and the fallback for an omitted variant was the filled primary treatment where the catalog default isneutralCardRendervariantdefaultAlertDialogRenderactionVariantprimary, dressing every ungoverned confirmation as dangerousThe default-fidelity class is live, not theoretical: A2UI's
GenericBinderresolves raw component properties and never runs the zod schema's.default(), so an omitted prop reaches the renderer asundefinedand the renderer's own fallback is the only thing that can honor the catalog default.Fixes stay native to shadcn — spacing/border utilities for density and dividers, the input scale for size, and shadcn's documented "badge or card with an explicit color class" idiom for the color vocabulary — and every catalog value keeps a treatment of its own. The emitter, the emitted catalogs and the dspack contracts are untouched: the emitted contract is the source of truth and the renderer moved to it. Independent of any dspack-emit branch.
Fail-first evidence
Tests landed first, against the pre-fix renderers (
470b62b), then the fix (768d8de). Reproduce withgit checkout 470b62b && pnpm --filter @dspack-studio/shadcn-renderers test:After the fix, the same 13 pass unchanged.
What the parity tests now guard
packages/shadcn-renderers/src/emitted-prop-parity.test.tsx, driven byemitted-corpus.ts. Nothing is hand-listed — the corpus is read from real emitter output, so growing the scenario shelf grows the guard.actionVariantcase) must not.itemslabel/value and every column header present.The structural detectors are written as pure
(markup, emitted) -> violationsfunctions and are additionally turned on deliberately broken visuals (a visual that ignores its slot, one that duplicates a child, one that drops a row, one that renders nothing), so no guard is decoration.Verification
pnpm test)@dspack-studio/shadcn-rendererspnpm -r typecheckAlso confirmed the projections are real pixels, not just distinct class strings: rebuilt
dist/shadcn.cssand rendered all 14 badge variants, all 13 surface variants, the three densities, four divider styles, striping, the three field sizes and both alert-dialog action variants — every one visibly distinct, the omitted-variant badge grey, and the omitted-actionVariantdialog no longer red. (Screenshots are a sanity check on top of the props-level suite, never a substitute for it.)Left unfixed, deliberately
Column.justify: startandstretchshare a flex treatment. A2UI'sstretchhas nojustify-contentequivalent; the un-drifted Astryx reference maps it tostartidentically. No instance emitsjustify, so the distinguishability check does not reach it. Fixing it would mean inventing a treatment neither design system has.Button.isIconOnlyand the commonaccessibilityprop are ignored by both registries. The Astryx renderer documentsisIconOnlyas waiting on an icon vocabulary in the catalog. Neither is emitted anywhere in the corpus.Textinstances carry neithertextnorchildren(fixture-001#text_3,fixture-002#text_5/6/11,fixture-004#title/intro,fixture-005#title/intro). They render blank under both registries, so this is not renderer drift — it is upstream of the renderer, in generation or in the recorded fixtures. Flagged here rather than papered over with renderer-side placeholder text, which would fabricate content the surface never carried. The blank-content check skips instances that promise no content, so it neither fails on these nor pretends they are fine.No packages published, nothing deployed, nothing merged.
🤖 Generated with Claude Code