feat(equipment): Fixture PIDINST write path (Phase 2)#45
Merged
Conversation
Closes the write side of the Fixture-tier PIDINST loop. Operators can
now mint and assign a DOI (or Handle) to a Fixture via the dedicated
slice; subsequent GET /fixtures/{fixture_id}/pidinst returns the
assigned PID instead of the urn:uuid: fallback.
WHY: Phase 1 (PR #43) shipped only the read closure with a data-substrate
Fixture.persistent_id field. Without the assign slice, operators had to
manually mutate the field (impossible by design). This slice ships the
mutation end-to-end on the Stub minter so the workflow can be proven
without DataCite credentials; Phase 3 swaps the Stub for the production
adapter.
Set-once domain invariant enforced at the decider per F3.3 (DataCite
Findable immutability): once Fixture.persistent_id is set, no further
assign accepted. Note: Fixture has NO lifecycle gate today (no
Decommissioned state), so no FixturePersistentIdAssignmentForbiddenError
class is needed (asymmetry with Asset slice F).
Server-mint posture per [[project-non-determinism-principle]]: route
accepts (scheme, suffix | None), handler resolves PersistentIdentifier
from the SHARED DoiMinter port (per L5: one port, two callers; no
FixtureDoiMinter). The decider stays pure.
DoiMinter port reused unchanged from Asset slice F:
- Port: cora.equipment.ports.doi_minter.DoiMinter
- Stub adapter: cora.equipment.adapters.stub_doi_minter.StubDoiMinter
- Wired via wire_equipment(deps) at app.state.equipment.doi_minter
- PersistentIdentifierMintError -> 502 already mapped from Asset slice F
New for Fixture:
- FixturePersistentIdAssigned event (alphabetical position in __all__;
union placement matches Asset convention: genesis-first then new)
- FixturePersistentIdAlreadyAssignedError + MalformedFixturePersistentIdentifierError
in aggregates/fixture/state.py per from_stored convention
- assign_fixture_persistent_id slice (command + decider + handler +
route + tool); slice directory carries SUBJECT noun (avoids the slice
F naming-fitness late-rename)
- AssignFixturePersistentIdRequest / AssignFixturePersistentIdResponse
Pydantic wire models
- FixtureSummaryProjection extended: subscribes
FixturePersistentIdAssigned + new persistent_id JSONB column via
inline jsonb_build_object (mirrors alternate-identifier pattern)
- Atlas migration 20260605100000_add_fixture_summary_persistent_id.sql
Scale: 27 files, 3040 insertions / 25 deletions. 32 unit + 22
integration + 1 contract new tests; architecture fitness green
including paired-PBT for the new decider. Adversarial 8-claim 3-vote
verification returned 21/24 confirmed (7/8 survived; one cosmetic flag
on the FixtureEvent union order which actually matches Asset's genesis-
first convention).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…ction metadata assertion The static test_projection_metadata assertion enumerates the full subscribed_event_types frozenset. Phase 2 added FixturePersistentIdAssigned to the projection writer but missed the corresponding test update. CI caught it; targeted local pytest runs did not cover this file. Same regression pattern as Asset slice F (PR #38 fixed by fe9430b); need to add the existing tests/unit/<bc>/test_*_projection.py files to the pre-push targeted run list when touching projection writers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
xmap
added a commit
that referenced
this pull request
Jun 5, 2026
The three Fixture-PIDINST integration test files landed via PRs #43 + #45 after this branch was created, so the install-then-register choreography this branch enforces was not applied to their fixture setups. Their tests register an Asset and then immediately call register_fixture binding that Asset, which trips the new INV-4 orphan guard (FixtureAssetNotInstalledError) added in this branch. Adds a sibling helper `install_existing_asset_into_fresh_mount` in `tests/integration/_equipment_helpers.py` that activates an already-registered Asset, registers a fresh Frame + Mount under uuid4 ids, and installs the Asset. Use this when the test already needed to register the Asset itself (to bind a model_id or seed an owner) and now needs to satisfy INV-4 before calling register_fixture. Companion to the existing seed_installed_asset helper that handles the no-model / no-owner case from scratch. Each of the three PIDINST test files calls the new helper right after its register_asset step. Slot codes use the asset's uuid as a suffix to keep them unique across helper calls within the same test run. All 10 PIDINST tests now pass locally. Pre-this-fix the CI run on this branch was failing 6 of them with FixtureAssetNotInstalledError. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
xmap
added a commit
that referenced
this pull request
Jun 5, 2026
…s for INV-4 Companion to 71bb940: two more Fixture-PIDINST integration test files from PR #45 (Phase 2 write path) need the same install-before- register treatment to satisfy the new INV-4 orphan guard. - `test_get_fixture_pidinst_after_assign.py`: `_seed_asset_with_owner_and_model` - `test_assign_fixture_persistent_id_tool.py`: inline asset setup in `_seed_fixture` Both gain a call to `install_existing_asset_into_fresh_mount` right after their `register_asset` step, slot_code suffixed with the asset's uuid for in-test uniqueness. All 21 PIDINST tests (5 files) now pass locally; previous CI run on this branch failed 10 of the 11 from these two files. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
xmap
added a commit
that referenced
this pull request
Jun 5, 2026
…#44) * feat(equipment): register_fixture rejects Decommissioned bound assets Adds a single cross-aggregate guard: every Asset referenced by `slot_asset_bindings` must NOT be Decommissioned. Mirrors the sibling AssetCannotAttachToFixtureError precondition at attach-time and rejects at register-time so the operator does not register a Fixture that would inevitably fail later at attach_asset_to_fixture (Fixture is single-event-genesis and cannot be amended). - New FixtureAssetNotAttachableError carries the sorted-first offending asset_id and current lifecycle string, mirroring the FixtureAssetNotFoundError deterministic-error precedent in the same decider. - RegisterFixtureContext gains a `lifecycle_by_asset_id` dict populated from the existing per-Asset load_asset gather (no extra round-trip, no new projection). Default empty dict means decider-only unit tests that exercise other invariants leave the guard inactive (mirrors family_ids_by_asset_id's relaxed default). - Decider guard ordering: existence (FixtureAssetNotFoundError) -> NEW lifecycle (FixtureAssetNotAttachableError) -> unknown-slot -> cardinality -> family-mismatch -> param-overrides. Operator sees the most actionable error first. - Route + OpenAPI 409 description list the new cause; routes.py wires the new error into the existing _handle_cannot_transition family alongside its Asset-BC mirror. Zero existing tests break because every existing happy-path test uses Active assets. Two new decider unit tests (guard fires with sorted-first determinism + empty-dict short-circuit), plus one integration test exercising the full handler stack end-to-end against Postgres (register_asset -> decommission_asset -> register_fixture -> FixtureAssetNotAttachableError). Closes INV-5 from the Fixture+Mount+Asset alignment plan; first half of Slice 3 (INV-4 orphan-binding guard deferred to Slice 3b to keep this slice ripple-free). Slice 3a in the Option A sequence. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(equipment): register_fixture rejects orphan bound assets (INV-4) Adds the second cross-aggregate guard from Slice 3 (3a shipped the Decommissioned-lifecycle half). Every Asset referenced by `slot_asset_bindings` must currently be installed in some Mount; operators that try to register a Fixture for un-racked assets are rejected with `FixtureAssetNotInstalledError` instead of silently materializing an unbacked composition. Closes INV-4 from the Fixture+Mount+Asset alignment plan: the contract becomes install_asset -> register_fixture, never the reverse. - New FixtureAssetNotInstalledError carries the sorted-first orphan asset_id (matches sibling FixtureAssetNotFoundError / FixtureAssetNotAttachableError deterministic-error precedent). - RegisterFixtureContext gains a `mount_id_by_asset_id` field shaped as `dict[UUID, UUID | None] | None`. Whole-None disables the guard for the pool-less test path (matches install_asset / decommission_asset short-circuit convention); per-entry None signals an orphan and fires the guard. - Handler kicks off three concurrent I/O streams via asyncio: the existing `assembly_state_task` + per-Asset `load_asset` gather, plus a new `mount_ids_future` returned by `asyncio.gather` over `load_asset_location` calls. `asyncio.gather` already schedules its children concurrently, so storing the future (without await) is the canonical way to start a gather alongside other work. - Decider guard ordering: existence -> NEW Decommissioned (3a) -> NEW orphan (3b) -> unknown-slot -> cardinality -> family-mismatch -> param-overrides. Decommissioned check fires before orphan when both apply (lifecycle is the more fundamental constraint). - Route + OpenAPI 409 description list the new cause; routes.py wires the new error into _handle_cannot_transition. Test ripple (5 integration files updated to install-then-register): factored a shared `seed_installed_asset` helper into `tests/integration/_equipment_helpers.py` that registers Frame + Mount + Asset, activates and installs the Asset, drains projections, and returns the ids. Each rippled test calls the helper before register_fixture, so the outer FixedIdGenerator id pool only budgets for the post-seed work. Asset version assertions bumped +1 to account for the AssetActivated event the helper emits. Three new decider unit tests (orphan fires with sorted-first determinism, pool-None short-circuit, ordering vs Decommissioned) plus one new integration test that exercises the full handler stack end-to-end against Postgres without installing the Asset first and asserts FixtureAssetNotInstalledError fires. Slice 3b in the Option A sequence; gate-reviewed 4/5/5 (DDD-fit should-fix on concurrency folded in here). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(architecture): make_inmemory_kernel may not be called from production code Adds a parametrized AST fitness test that scans every git-tracked .py file for direct calls to `make_inmemory_kernel(...)` and asserts each call site is either the definition module `src/cora/infrastructure/deps.py` or under `tests/`. `make_inmemory_kernel` builds a Kernel with `pool=None`. Three cross-aggregate guards short-circuit on `pool=None` for the pool-less test path: install_asset (since 5g), decommission_asset (slice 1), register_fixture (slice 3a). If a future production module accidentally wires the in-memory primitive (alternate entrypoint, MCP-only deployment, lazy-pool refactor), every one of those guards silently disables with zero test failure. This is the "load-bearing assumption with no fitness" risk the slice 3a / 3b gate-review adversarial flagged, and the rule-of-three trigger the `feedback_pool_none_short_circuit_rule_of_three.md` memo captured. Test shape mirrors `test_kernel_construction_single_site.py`: textual `make_inmemory_kernel(` filter + AST `ast.Call` walk + a `_PRODUCTION_ALLOWLIST` with one entry (deps.py) + a bulk allowance for any path under `tests/` + a drift-catcher that the allowlist file exists. Mutation-checked: dropping a one-liner `make_inmemory_kernel(...)` call site at `src/cora/_pool_none_regression.py` makes the test fail at the exact line; removing it returns the suite to green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * review(equipment): address PR #44 second-pass findings (concurrency + determinism + audit-tag strip + docs) Independent review of the INV-4 (orphan-bound-asset) guard surfaced five should-fix findings across correctness, test coverage, naming conventions, and docs. Four addressed here; the fifth (contract-tier pool=None bypass) is closed by the sibling commit adding a `make_inmemory_kernel` production-call-sites architecture fitness. Concurrency (handler.py): the three I/O streams that load the assembly, the per-Asset state, and per-Asset mount location used a hand-rolled `asyncio.create_task` + bare `asyncio.gather` future with sequential awaits. If any one of them raised, the others were never awaited and continued running in the background, surfacing as 'task exception never retrieved' warnings and holding pool connections after the handler had already returned an error. Rewrapped in `asyncio.TaskGroup` so a failure in any child cancels the siblings deterministically before the handler unwinds. Sorted-first determinism (test_register_fixture_decider*.py): the existing test for the orphan guard used a single asset_id, which made sorting trivially identity-preserving. A regression from `sorted(..., key=str)[0]` to `next(iter(...))` would have passed silently. Added a deterministic two-line test with three concrete UUIDs whose dict insertion order disagrees with their stringified sort order, plus a Hypothesis property test parameterized over frozenset[UUID] of 2-5 orphans that asserts the raised id equals `sorted(orphan_ids, key=str)[0]` regardless of iteration order. Naming hygiene (CLAUDE.md hard rule): stripped 14 occurrences of the audit-plan tags 'INV-4' and 'slice 3b' from source and test files. The durable WHY (cross-aggregate guard requiring referenced Assets to be currently installed in some Mount) was already stated alongside each tag, so the strip is mechanical and the prose survives. Module docs (docs/architecture/modules/equipment/index.md): the RegisterFixture error taxonomy listed only the pre-existing causes; both FixtureAssetNotAttachable (lifecycle guard, pre-existing miss from slice 3a) and FixtureAssetNotInstalled (install-required guard added by this slice) are now first-class entries. The register_fixture invariants prose at the top of the doc also gained the lifecycle + install preconditions that the decider now enforces. Tests after fix: 21/21 register_fixture decider tests pass (incl. 1 new example + 1 new PBT); 1726 equipment unit + integration tests pass; 16911 architecture fitnesses pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(equipment): install bound Assets in PIDINST fixture tests for INV-4 The three Fixture-PIDINST integration test files landed via PRs #43 + #45 after this branch was created, so the install-then-register choreography this branch enforces was not applied to their fixture setups. Their tests register an Asset and then immediately call register_fixture binding that Asset, which trips the new INV-4 orphan guard (FixtureAssetNotInstalledError) added in this branch. Adds a sibling helper `install_existing_asset_into_fresh_mount` in `tests/integration/_equipment_helpers.py` that activates an already-registered Asset, registers a fresh Frame + Mount under uuid4 ids, and installs the Asset. Use this when the test already needed to register the Asset itself (to bind a model_id or seed an owner) and now needs to satisfy INV-4 before calling register_fixture. Companion to the existing seed_installed_asset helper that handles the no-model / no-owner case from scratch. Each of the three PIDINST test files calls the new helper right after its register_asset step. Slot codes use the asset's uuid as a suffix to keep them unique across helper calls within the same test run. All 10 PIDINST tests now pass locally. Pre-this-fix the CI run on this branch was failing 6 of them with FixtureAssetNotInstalledError. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <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
1 participant
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
Closes the write side of the Fixture-tier PIDINST loop. Operators can now mint and assign a DOI (or Handle) to a Fixture via the dedicated slice; subsequent
GET /fixtures/{fixture_id}/pidinstreturns the assigned PID instead of theurn:uuid:fallback.FixturePersistentIdAssignedevent + 2 new error classes (FixturePersistentIdAlreadyAssignedError,MalformedFixturePersistentIdentifierError) inaggregates/fixture/state.pyassign_fixture_persistent_idslice (command + decider + handler + route + tool); slice directory carries SUBJECT nounAssignFixturePersistentIdRequest/AssignFixturePersistentIdResponsePydantic wire modelsFixtureSummaryProjectionextended: subscribesFixturePersistentIdAssigned+ newpersistent_idJSONB column via inlinejsonb_build_object20260605100000_add_fixture_summary_persistent_id.sqlWhy
Phase 1 (PR #43) shipped only the read closure with a data-substrate
Fixture.persistent_idfield. Without the assign slice, operators had no way to set it. This slice ships the mutation end-to-end on the Stub minter so the workflow can be proven without DataCite credentials; Phase 3 swaps the Stub for the production adapter.DoiMinter port REUSED unchanged from Asset slice F (Lock 5)
One port, two callers; no
FixtureDoiMinter. Port atcora.equipment.ports.doi_minter.DoiMinter, Stub adapter atcora.equipment.adapters.stub_doi_minter.StubDoiMinter, wired viawire_equipment(deps)atapp.state.equipment.doi_minter.PersistentIdentifierMintError-> 502 already mapped from Asset slice F.Key locks
Fixture.persistent_idis set, no further assign acceptedFixturePersistentIdAssignmentForbiddenErrorclass (asymmetry with Asset slice F, deliberate per memo)PersistentIdentifierfrom the SHAREDDoiMinterport before invoking the pure deciderassign_fixture_persistent_idcorrect from the startScale + verification
27 files, 3040 insertions / 25 deletions.
RaisingDoiMinterfixture; MCP tool; round-trip get_fixture_pidinst_after_assign)FixtureEventunion order which matches Asset's genesis-first convention).Follow-ups
Generated with Claude Code