Emitter review fixes: audited syntheses, precise subButtonText, npm ci for demo#6
Merged
Merged
Conversation
… reproducible CI Review fixes (Copilot on #4, landing as a follow-up since #4 merged mid-response): - subButtonText now takes the text of the first LABEL-BEARING component under the tracked sub-component (one whose surface plan projects text via textChildProp, e.g. the trigger's button) — never incidental descendant text; docs updated to match, test covers explanatory-text-before-button. - The two remaining silent syntheses now warn: surface-synthesized-text (node text -> synthesized Text child) and surface-synthesized-wrap (multi-child -> synthesized Column) — the "nothing silent" contract is test-enforced for all four synthesis paths. - CI uses npm ci --prefix demo (lockfile-deterministic). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR is a follow-up refinement to the A2UI surface emitter, tightening the semantics around subButtonText, ensuring structural/content syntheses are always audited via warnings, and making the demo install step deterministic in CI.
Changes:
- Clarifies and enforces
subButtonTextsemantics to only capture text from “label-bearing” components (those projecting text viatextChildProp). - Adds explicit warnings for synthesized text primitives and synthesized wrapper components (e.g., Column wrapping for multi-child single-slot).
- Switches the CI demo install step from
npm installtonpm cifor reproducibility.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/transform/profiles.ts |
Documentation update clarifying subButtonText meaning (label-bearing-only). |
src/targets/a2ui/surface.ts |
Implements label-bearing-only subButtonText capture; adds audited warnings for synthesized text and wrap. |
src/surface-emit.test.ts |
Adds coverage for corrected subButtonText behavior and verifies new synthesis warning codes. |
.github/workflows/test.yml |
Uses npm ci --prefix demo to ensure deterministic demo dependency installation in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Follow-up to #4 (merged while the review response was in flight) addressing all four Copilot comments — see the thread replies on #4 for the per-comment mapping.
npm test(31, incl. new coverage for the correctedsubButtonTextsemantics and the two new warning codes) +npm run test:packgreen.🤖 Generated with Claude Code