fix(a2ui): close the casualty-consumption bypass; pin the shadcn contract - #27
Merged
Conversation
The plan-lookup gate refuses a casualty emitted as its own node, but a consuming strategy (subText, subButtonText, subTable, subFlatten) walks the subtree directly — so an author could fold a declared casualty's text into a parent prop and the authored refusal would disappear behind a warning. Reproduced against the production shadcn catalog: giving `card` a subText entry for the declared-casualty `label` emitted the casualty's text into the card title with one warning and no refusal. Consumption is how compounds carry their parts. It is not an escape hatch around "this cannot be represented". The new gate walks any subtree a consuming strategy is about to absorb and refuses, fail-closed and loud, with the authored reason — exactly like the direct path. Scoped precisely: only components the profile declares casualties AND does not map are refused; ordinary sub-component consumption is unchanged (pinned by a second test). Found while measuring the representation gap of the first production catalog (dspack#35), where 17 of 32 components are declared casualties — the value of an authored refusal rises with the size of the catalog, and so does the cost of a silent bypass. Fail-first: both tests were written first; the consumption test fails against the pre-change engine (no EmitSurfaceError thrown) and passes after. 88/88 green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR closes a correctness gap in the A2UI surface emitter where profile-declared “casualty” components could previously be consumed into a parent via subtree-consuming strategies (e.g., subText, subButtonText, subTable, subFlatten) without triggering the authored refusal.
Changes:
- Add a pre-consumption subtree walk in
emitNodeto refuse if an unmapped, declared-casualty component would be consumed. - Introduce
refuseConsumedCasualtyto enforce “fail-closed” behavior during consumption. - Add tests proving (1) consumed casualties refuse with the authored reason and (2) ordinary sub-component consumption still works.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/targets/a2ui/surface.ts | Adds a consumed-subtree casualty refusal gate and the helper walker used to detect declared casualties before consumption. |
| src/profile-load.test.ts | Adds regression tests covering casualty refusal during consumption and ensuring non-casualty consumption remains functional. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…path Review follow-ups on the casualty-consumption gate: - restore the applySubContent doc block, which the new refusal helper had displaced, leaving applySubContent undocumented; - thread each node's own path through the refusal walk so the error locates the casualty rather than the parent that would have consumed it — matching the precision of the direct emission path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dspack#35 merged the 32-component production contract, so check:sync — which follows dspack@main — went red on every branch without a line of code changing. Following main immediately would not produce a bigger catalog but a broken one: measured on a bare local sync, 12 of 14 worked examples refuse to emit, 24 of 32 components fall through unclassified, and 106 sub-components want coverage against 39 today. It would also merge the approved foundation milestone into the T1-T5 representation work that is explicitly paused. So pin, deliberately, at the exact upstream commit this package's profile, renderers and parity invariants were designed against (805732c, v2.3.0). The local copy is byte-identical to it — nothing is forked, copied or re-authored to make the check pass. A pin here is a stronger claim than tracking, not a weaker one. It: - verifies the pinned artifact's sha256 on every run, because a pinned ref must be immutable — a change means force-push or CDN mismatch, never a routine update, and fails the build; - still fails on local drift, exactly as before; - always reports how far behind main it sits, so the 8-component contract can never quietly read as current shadcn coverage; - carries an explicit removal condition and a tracking issue (#28). Declaring 24 new casualties to make the sync green was considered and rejected: those components are "not mapped yet", not "cannot represent", and overloading the casualty vocabulary weakens the fail-closed gate it powers. Refs #28 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Found while measuring the representation gap of the first production catalog (dspack#35 — 32 components, 17 of them declared casualties).
Two changes: the fail-closed fix this PR was opened for, and a contract pin that became necessary mid-flight when dspack#35 merged.
1. The casualty-consumption hole
emitNode's plan-lookup gate refuses a casualty emitted as its own node. But a consuming strategy —subText,subButtonText,subTable,subFlatten— walks the subtree directly and never reaches that gate. So a profile author can fold a declared casualty's text into a parent prop and the authored refusal silently becomes a warning.Reproduced against the production catalog: giving
cardasubTextentry for the declared-casualtylabelemitted the casualty's text into the card title, with one warning and no refusal.That matters more as catalogs grow. In this catalog 53% of components are authored refusals — each one a decision that A2UI cannot represent the concept honestly. A bypass turns those decisions into silent approximations, which is the one thing the emitter is supposed to make impossible.
The fix
Any subtree a consuming strategy is about to absorb is walked first; a component the profile declares a casualty and does not map refuses fail-closed with the authored reason, exactly like the direct path:
Scoped precisely — ordinary sub-component consumption is untouched, pinned by a second test. Review follow-ups: the refusal now throws with the casualty's own path rather than the consuming parent's, and the displaced
applySubContentdoc block is restored.2. The shadcn contract pin
dspack#35 merged the 32-component production contract, so
check:sync— which followeddspack@main— went red on every branch without a line of code changing.Following
mainimmediately would not produce a bigger catalog but a broken one. Measured on a bare local sync:subCoveragechecksresolving to{}So the contract is pinned at the exact upstream commit this package's profile, renderers and parity invariants were designed against —
805732c, v2.3.0. The local copy is byte-identical to it; nothing is forked or re-authored to make the check pass.A pin here is a stronger claim than tracking, not a weaker one. It verifies the pinned artifact's sha256 every run (a pinned ref must be immutable), still fails on local drift, and always reports how far behind
mainit sits so the 8-component contract can never quietly read as current coverage. Record and removal condition:docs/CONTRACT-PIN.md, tracked in #28.Declaring 24 new casualties to make the sync green was considered and rejected: those components are "not mapped yet", not "cannot represent", and overloading the casualty vocabulary weakens the fail-closed gate it powers.
Fail-first
Both tests were written first.
10d330d) — noEmitSurfaceErroris thrown at all.2b90f0a) withexpected '$.root' to be '$.root.children[0]'.TAMPERED … the PINNED artifact itself changed; local drift still exits 1; clean state exits 0.96/96 green. (An earlier revision of this description said 88 and called
src/ingestion.test.tsa pre-existing 0-test failure — that was wrong. It holds 8 tests and failed only because this git worktree lackeddemo/node_modules; with the demo deps installed the whole suite passes.)Consumption is how compounds carry their parts. It is not an escape hatch.
🤖 Generated with Claude Code