Skip to content

fix(validate): requiredSubComponents ids resolve to components or sub-components#25

Merged
ryandmonk merged 1 commit into
mainfrom
fix/required-subcomponents-resolution
Jul 22, 2026
Merged

fix(validate): requiredSubComponents ids resolve to components or sub-components#25
ryandmonk merged 1 commit into
mainfrom
fix/required-subcomponents-resolution

Conversation

@ryandmonk

Copy link
Copy Markdown
Contributor

Problem

validate has been red on main since #22. The studio governance sync added two required-composition rules (rule.review-offers-alternatives, rule.option-states-attributes) whose requiredSubComponents reference selectable-card and metadata-list — deliberately declared as top-level components in the Astryx contract. The static governance-consistency check required those ids to resolve strictly in the sub-component vocabulary, so examples/astryx.dspack.json failed.

The static validator was the outlier: dspack-gen's runtime linter matches requiredSubComponents against descendant nodes by component id (either kind), the spec's §5 wording says "matching descendants", and the shipped governance and goldens already rely on that interpretation.

Change (narrow)

  • scripts/validate.mjsrequiredSubComponents[].id now resolves to either a declared top-level component or a declared composition sub-component. Resolution guards vocabulary only; satisfaction stays a runtime (S3) concern — declaration in the contract never satisfies the rule. on references remain sub-component-only per spec.
  • fixtures/negative/rule-required-subcomponent-unknown-ref.dspack.json — new negative fixture: an id absent from both vocabularies is rejected with the resolution error.
  • spec/dspack-v0.3.md §5 — one clarifying paragraph stating the identifier-resolution rule; the existing "matching descendants" semantics are untouched.

Regression coverage

Case Covered by
Top-level component as required descendant examples/astryx.dspack.json (the two rules + ex.plan-comparison) — positive suite
Declared sub-component as required descendant examples/shadcn-ui.dspack.json (rule.alertdialog-requires-cancel, rule.table-carries-structure, rule.table-carries-caption) — positive suite, unchanged
Id exists globally but absent beneath governed ancestor Runtime concern, unchanged: dspack-gen src/core/lint/lint.test.ts ("Required sub-component 'alert-dialog-cancel' (min 1) not found among descendants") + violating goldens F3/F4/F9. The static harness never evaluates satisfaction (S1/S2 only, per its header), so no conflation is possible here.
Id absent from both vocabularies New negative fixture (this PR)

Verification

  • npm run validate: schema-compile PASS (5 schemas); astryx ✔, shadcn ✔ — examples PASS (2 documents).
  • npm run validate -- --fixtures negative: 18/18 rejected, each for its intended reason; all pre-existing fixtures' rejection messages unchanged, new fixture rejects with requiredSubComponents reference 'ghost-panel' does not resolve in the contract.
  • Diff is limited to the validator, one new fixture, and the spec paragraph — no examples or goldens change.

No RFC/DX work included. Once this merges, #24 (documentation-only RFC) should go green on rebase/rerun with no changes.

🤖 Generated with Claude Code

…-components

The static governance-consistency check required requiredSubComponents
ids to resolve strictly in the sub-component vocabulary, while the
runtime linter (dspack-gen) and the spec's own wording match descendant
nodes by component id — either kind. The shipped astryx governance
(rule.review-offers-alternatives, rule.option-states-attributes) relies
on the runtime interpretation, so the validator has failed main since
the studio governance sync (#22). Align the static check with runtime
semantics: the id must be declared as a top-level component or a
composition sub-component; declaration alone never satisfies the rule
at lint time. `on` references remain sub-component-only per spec.

Adds a negative fixture for an id absent from both vocabularies and a
spec §5 sentence stating the identifier-resolution rule. Runtime
satisfaction (matching descendants beneath each governed node) is
unchanged and covered by dspack-gen's lint tests and F3/F4/F9 goldens.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 22, 2026 17:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the static governance-consistency validation so requiredSubComponents[].id may resolve to either a top-level component id or a composition sub-component id (matching runtime lint behavior and the spec’s “matching descendants” semantics), while keeping on references sub-component-only.

Changes:

  • Update scripts/validate.mjs resolution rules: requiredSubComponents ids now resolve against both component and sub-component vocabularies; on remains sub-component-only.
  • Add a negative fixture ensuring requiredSubComponents ids that resolve to neither vocabulary are rejected with the expected resolution error.
  • Clarify spec v0.3 §5 with an explicit identifier-resolution rule for requiredSubComponents (without changing satisfaction semantics).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
spec/dspack-v0.3.md Clarifies that requiredSubComponents[].id must be declared as either a component or sub-component id; matching remains by descendant node component id.
scripts/validate.mjs Adjusts static resolution checks so requiredSubComponents ids may resolve to either vocabulary while keeping on sub-component-only.
fixtures/negative/rule-required-subcomponent-unknown-ref.dspack.json Adds a negative fixture covering unresolved requiredSubComponents ids.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ryandmonk
ryandmonk merged commit 849b533 into main Jul 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants