accept 0018: migration chain ambiguity#47
Merged
Conversation
Names the canonical configuration-time category ``checkpoint_state_migration_chain_ambiguous`` that §10.12.1 and §10.12.2 had previously described only as "a configuration-time error" without a canonical identifier. Updates both sections to reference the category by name. Rewrites §10.10's mutual-exclusion paragraph to list all four migration-related categories with the new routing precedence (registry well-formedness → version compatibility → chain application → deserialization). Adds conformance fixture 047 covering both ambiguity cases (duplicate (from, to) pair at registration; multiple distinct shortest paths at chain resolution) via a new harness primitive ``expected_chain_ambiguity_error`` that accepts the category surfacing at either build time or during resume — preserves §10.12.2's compile-time-SHOULD / load-time-acceptable carve-out. Pre-1.0 PATCH bump (v0.15.1): no new normative behavior; just a canonical identifier and conformance coverage for behavior already mandated in v0.15.0.
The accept commit landed the proposal under ``proposals/`` but missed the corresponding ``docs/proposals/`` symlink that the mkdocs site uses to render proposals. Both the markdown link validator and the strict mkdocs build flagged the missing target when changelog and capability spec text referenced the new proposal. Add the symlink ``docs/proposals/0018-state-migration-chain-ambiguity.md`` → ``../../proposals/0018-state-migration-chain-ambiguity.md``, matching the existing pattern for proposals 0001–0017.
There was a problem hiding this comment.
Pull request overview
Accepts proposal 0018 to canonically name state-migration chain ambiguity errors and add conformance coverage for duplicate migration pairs and ambiguous shortest migration paths.
Changes:
- Adds
checkpoint_state_migration_chain_ambiguousto pipeline-utilities §10 error semantics. - Adds fixture 047 covering duplicate-pair registration and diamond shortest-path ambiguity.
- Marks proposal 0018 accepted and adds a 0.15.1 changelog entry.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
spec/pipeline-utilities/spec.md |
Adds the new ambiguity category and updates migration routing/chain-resolution wording. |
spec/pipeline-utilities/conformance/047-state-migration-chain-ambiguous.yaml |
Adds conformance cases for duplicate migration pairs and ambiguous shortest paths. |
spec/pipeline-utilities/conformance/047-state-migration-chain-ambiguous.md |
Documents fixture 047 behavior and pass/fail expectations. |
proposals/0018-state-migration-chain-ambiguity.md |
Updates proposal status and accepted date. |
CHANGELOG.md |
Adds the 0.15.1 release notes for proposal 0018. |
Comments suppressed due to low confidence (1)
spec/pipeline-utilities/conformance/047-state-migration-chain-ambiguous.yaml:93
- The new either-timing assertion is nested under
resumehere, while the duplicate-pair case places the same primitive at the case top level. Since this primitive is supposed to accept build-time failures as well as resume-time failures, the schema location should be consistent (or explicitly documented as valid in both places) so adapters know where to look before they compile the graph.
resume:
from_seeded_record: true
expected_chain_ambiguity_error: checkpoint_state_migration_chain_ambiguous
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Landing page fixture count: 118 → 119 (fixture 047 added by this proposal brings pipeline-utilities from 46 to 47). - Proposals index table: add row for 0018, capability pipeline-utilities, status Accepted.
Three review threads resolved together per the review skill's one-commit-at-the-end rule. - **Reclassify v0.15.1 → v0.16.0 (PATCH → MINOR).** Naming a canonical error category that didn't exist before is implementation-visible: implementations that previously raised an arbitrary configuration error (a language-native ``ValueError``, a generic ``Error``) must now surface ``checkpoint_state_migration_chain_ambiguous`` to pass fixture 047. Matches the precedent set by proposal 0014's category additions, which shipped as the v0.12.0 MINOR bump. CHANGELOG section header, Notes rationale, and skip-ahead sentence all reflect the bump. - **Refresh README version metadata.** ``Current spec version`` updated to v0.16.0; pipeline-utilities row's ``Latest | Fixtures`` columns updated to ``0.16.0 | 47``. Other capability rows untouched (0018 only modifies pipeline-utilities). - **Normalize fixture mock names.** Fixture 047 referenced two mock identifiers that don't exist in the current harness (``bumps_field_x``, ``add_new_field_default``). Replaced both with the existing ``should_not_run`` mock — ambiguity detection fires before any migration runs per §10.12.1 / §10.12.2, so the mock is referenced but never invoked. ``should_not_run`` is already in the harness, its name self-documents the expectation, and any invocation would surface a clear failure mode for an implementation that missed the ambiguity check.
The fixture .md described case 1 as "two migrations register the same (v1, v2) pair with different migration functions" — wording left over from before the prior commit replaced the invented mocks with a single ``should_not_run`` reference. Update the description to match the YAML: duplicate-pair detection is independent of function identity per §10.12.1, so both migrations reference the same ``should_not_run`` mock; the ambiguity check fires before either function is invoked. Single-message change keeps the YAML comment and the companion .md telling adapter authors the same story.
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.
Summary
Accepts proposal 0018: names the canonical category
checkpoint_state_migration_chain_ambiguousfor the state-migration ambiguity cases that §10.12.1 and §10.12.2 previously described as "a configuration-time error" without a canonical identifier, and adds fixture 047 exercising both cases.Changes
proposals/0018-state-migration-chain-ambiguity.md— Status: Draft → Accepted; Accepted date filled in.spec/pipeline-utilities/spec.md:chain_ambiguous(build/load) →migration_missing(no chain) →migration_failed(chain raised) →record_invalid(post-migration deserialize fails).spec/pipeline-utilities/conformance/047-state-migration-chain-ambiguous.{yaml,md}— new fixture with two cases:duplicate_pair_at_registration— two migrations with same(v1, v2)pair.ambiguous_shortest_paths_at_resolution— diamondv1→v2→v4+v1→v3→v4from seeded record at v1, current schema at v4.Both cases use the new
expected_chain_ambiguity_error: <category>harness primitive that accepts the named category surfacing at either build time or during resume. The primitive preserves §10.12.2's carve-out for implementations that detect ambiguity at either point.CHANGELOG.md—[0.16.0]section dated 2026-05-15 with the additions, the §10 spec-text changes, and the pre-1.0 MINOR bump rationale.README.md—Current spec versionupdated to v0.16.0; pipeline-utilities row'sLatest | Fixturesupdated to0.16.0 | 47.docs/index.md,docs/proposals.md— landing fixture count 118 → 119; proposals index table gains the row for 0018.docs/proposals/0018-state-migration-chain-ambiguity.md— symlink so the proposal page renders on the docs site, matching the pattern for 0001–0017.Versioning
Pre-1.0 MINOR bump (v0.15.0 → v0.16.0). Although v0.15.0 already mandated "a configuration-time error" for both ambiguity cases, naming a canonical category that didn't exist before is implementation-visible: implementations that previously raised an arbitrary error (a language-native
ValueError, a genericError, etc.) must now surfacecheckpoint_state_migration_chain_ambiguousto pass fixture 047. Matches the precedent set by proposal 0014's category additions, which shipped as the v0.12.0 MINOR bump.Test plan
uv run mkdocs build --strictpasses locally.python scripts/validate_markdown_links.pypasses locally.v0.16.0on the merge commit.