Skip to content

fix(schema): make inherited type composition scope- and topology-exact [roadmap:deterministic-substrate] - #495

Merged
tcballard merged 4 commits into
mainfrom
claude/federated-spec-composition
Sep 22, 2026
Merged

tcballard merged 4 commits into
mainfrom
claude/federated-spec-composition

Conversation

@tcballard

@tcballard tcballard commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This is PR E in the post-merge review series. It fixes the ADR-150 composition-correctness findings:

  1. An override rationale depended on the command's scope (medium). A type override's Decision was looked up only among the files the command captured. With a valid override in place, decided validate decisions/runbooks, stats/doctor on a subdirectory, validate --top-level, and v1 subdirectory scopes all failed with corpus-federation-invalid-override … does not resolve to a local artifact. Now a root-owned rationale that is not in scope is resolved over a walk of the root's working tree, with every materialised parent excluded. Inherited sources are already captured whole, so they are unchanged.
  2. The memo key ignored topology (medium). The composed-registry memo key framed only (source, config bytes). Parent edges live in the manifests, so a long-running process that saw the graph change under byte-identical configs kept serving the old registry and skipped findings a fresh process raises. The key now frames the root, plus each source's layer and canonical parent list.
  3. Key order alone produced a type conflict (low–medium). ArtifactSpec equality compared order-preserving Vecs, contradicting the ADR-150 amendment ("key order and whitespace cannot manufacture a conflict"). Element identity now compares map-like fields (metadata, descriptions, guidance, synonyms, starter_bodies) as maps. List order stays significant because it is rendered order.
  4. Rationale IDs were case-sensitive (low). They now casefold, as ADR-137 artifact-override rationales do (composition.rs validate_override).
  5. The decided new / migrate collision set in v2 was narrower than released (low). Since fix(new): compose the collision scope of a version-2 federation from its corpus directory [roadmap:deterministic-substrate] #489 a v2 repository counted only the composed closure of one top-level directory. It now also unions a plain repository walk, so identifiers in sibling top-level directories and in replaced parent artifacts stay issued.
  6. A placeholder source label (low). A local bundle in a config without corpus.source was reported in artifact_spec_bundles as "source": "local", the same literal as the prefer: local keyword. It is now null. The field is still Unreleased, so this is not a contract break.

A diamond that re-opens a collision is documented, not changed (root → A → G and root → B → G, where A resolves G's runbook). A's resolution governs A's view only. The root reaches G's declaration again through B and must record its own override, which may prefer any source in its inherited view. This matches ADR-147's explicit diamond convergence for artifact overrides: carrying A's choice past B's path would let one parent silently decide for a sibling that never opted in. A unit test locks it in.

Roadmap / ADR Trace

  • Roadmap: decisions/roadmaps/deterministic-substrate.md (Tranche C)
  • ADR-150: inherited spec bundles (element identity, the override rationale, composition)
  • ADR-137 / ADR-147: rationale casefolding; explicit diamond convergence
  • ADR-083: provenance of the local bundle
  • ADR-007: the artifact_spec_bundles change lands before release

Scope

  • rust/rac-engine/src/spec_composition.rs: composition_key, root_tree_items, same_content, and casefolded rationale matching with a lazy root-tree fallback in verify_override_rationales; unit tests.
  • rust/rac-engine/src/federated_corpus.rs, graph_federated_corpus.rs: both call sites pass the root source and the root-tree walk.
  • rust/rac-engine/src/scaffold.rs: the v2 collision set unions a plain repository walk.
  • rust/rac-engine/src/spec.rs, output.rs, doctor.rs: SourceSpecBundle.source is Option<String>.
  • rust/decided/tests/spec_federation.rs, spec_bundle.rs: CLI coverage.
  • docs/validation.md, docs/cli.md (collision scan), decisions/designs/third-party-artifact-extensibility.md, CHANGELOG.md.

Product / Architecture Decisions

  • A type override is config-level policy, so its validity cannot depend on the directory a command names. For the root, the "declaring corpus" is its working tree minus materialised parents, the same boundary the released collision walk uses.
  • The in-scope lookup stays the fast path. The whole-tree walk runs at most once per composition, and only when a root-owned rationale is missing from scope, so the behaviour and cost of every currently passing case are unchanged.
  • Diamonds re-collide (see above), rather than carrying a middle node's resolution.

User-Facing Contract

  • Commands scoped below a corpus, or with --top-level, no longer fail a valid type override.
  • Lowercased rationale IDs resolve.
  • Bundles that differ only in JSON key order compose silently.
  • artifact_spec_bundles[].source is null for an unnamed local bundle.
  • No change to any output where none of these applies: the CLI golden battery (135 cases) and MCP golden battery are byte-identical to main on the same tree.

Verification

  • cargo fmt --check, cargo clippy --workspace --release --no-deps -- -D warnings (the CI invocation), cargo test --workspace --release: pass.
  • Mutation checks. Each new test fails when its fix is reverted:
    • key_order_alone_is_not_a_conflict_but_list_order_is fails under plain ==.
    • a_topology_change_under_identical_configs_recomposes fails under the old config-only key.
    • a_type_override_rationale_resolves_across_the_whole_local_corpus fails without the fallback and without casefolding.
    • a_version_one_type_override_rationale_resolves_outside_the_command_scope fails without the fallback.
  • Conformance certification 11/11; live-corpus invariants PASS.
  • Corpus gates: validate, relationships --validate, review (no P1/P2), export --agent-rules --check, watchkeeper --base origin/main: all exit 0.

Review Path

  1. spec_composition.rs: verify_override_rationales and root_tree_items, then composition_key, then same_content.
  2. The two call sites in federated_corpus.rs / graph_federated_corpus.rs.
  3. scaffold.rs issued_ids.
  4. The tests, then the docs and design wording.

Notes For Reviewer

  • Known limitation. On the decided-mcp cache path the root-tree fallback reads the live working tree, which the generation key does not cover. A rationale Decision outside the served root that is retired while the server runs is noticed only on the next recomposition. The served root normally holds the rationale (in scope, fully keyed), so the fallback is a CLI-scope concern in practice.
  • Not addressed. Composed registries remain &'static and are leaked once per distinct composition key. That is bounded by the distinct configurations and topologies a process sees, at a few KB each. Making them freeable is an Arc refactor across every &'static ArtifactSpec holder, out of scope here.
  • validate decisions/policies on the spec-federation fixture still fails with corpus-federation-invalid-node, because data-retention.md links to an ADR outside that scope. That is a pre-existing v2 relationship-scope property unrelated to overrides, so the new test scopes to a relationship-free decisions/runbooks.
  • The review's other low items (stats by_type order, synonyms in validation, doctor orphan advice) are not in this PR. The corpus/doc corrections are docs(schema): reconcile ADR-150, ADR-083, the design, and docs with the engine [roadmap:deterministic-substrate] #496.

Implementation Process

Implemented with AI assistance under the roadmap contract; final scope, review, and acceptance decisions were made by the maintainer.

…t [roadmap:deterministic-substrate]

Implements decisions/roadmaps/deterministic-substrate.md (Tranche C).

- A type override's rationale resolves in the declaring corpus: a
  root-owned rationale outside the command's directory or --top-level
  scope is looked up over the root's working tree, materialised parents
  excluded, instead of failing as unresolved.
- Rationale identifiers casefold, as ADR-137 artifact overrides do.
- Element identity compares map-like fields as maps, so JSON key order
  no longer manufactures corpus-federation-artifact-type-conflict; list
  order stays significant.
- The composed-registry memo key frames the root, each source's layer,
  and its canonical parent list, so a changed graph under byte-identical
  configs recomposes and raises what a fresh process would.
- decided new / migrate in a version-2 repository union the composed
  closure with a plain repository walk, so identifiers in sibling
  top-level directories and replaced parent artifacts stay issued.
- artifact_spec_bundles reports a null source for a local bundle whose
  config declares no corpus.source, not the `prefer: local` keyword.
… [roadmap:deterministic-substrate]

Implements decisions/roadmaps/deterministic-substrate.md (Tranche C).

Covers version-1 and version-2 rationale resolution outside the command
scope with a lowercased identifier, a parent Decision refused as a local
rationale, decided new in a sibling top-level directory, and the null
source label for an unnamed local bundle. Unit tests cover key-order
identity, topology-aware memoisation, and diamond re-collision.
…lision [roadmap:deterministic-substrate]

Implements decisions/roadmaps/deterministic-substrate.md (Tranche C).

States that type-override rationales resolve case-insensitively across
the declaring corpus, that element identity ignores JSON key order, that
a diamond re-opens a collision a middle node resolved, that the memo key
covers topology, and that an unnamed local bundle reports a null source.
…e [roadmap:deterministic-substrate]

Implements decisions/roadmaps/deterministic-substrate.md (Tranche C).

decided new in a version-2 repository counts every identifier in the
repository as issued, not only the composed closure of the target's
top-level directory.
@tcballard
tcballard force-pushed the claude/federated-spec-composition branch from c593472 to f24fd3f Compare September 22, 2026 21:43
@tcballard
tcballard merged commit 554b7a8 into main Sep 22, 2026
13 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.

1 participant