feat(equipment): Asset.alternate_identifiers (PIDINST instance-tier identity)#20
Merged
Merged
Conversation
…dentity)
Closes the instance-tier identity gap per the locked Stage 1
memo project_asset_alternate_identifiers_design.md. Purely
additive: Asset gains alternate_identifiers: frozenset[
AlternateIdentifier]; AssetRegistered payload gains optional
alternate_identifiers key; 2 NEW targeted-mutation slices
(add_asset_alternate_identifier + remove_asset_alternate_
identifier) mirror the add_asset_port / remove_asset_port
precedent exactly; 1 forward-only Atlas migration adds JSONB
column + partial GIN index.
AlternateIdentifier is a flat (kind, value) VO. kind is a
closed StrEnum (SerialNumber | InventoryNumber | Other)
verbatim from PIDINST v1.0 property 13 alternateIdentifierType
controlled vocabulary. value is a trimmed 1-200 char string;
operators may enter serial numbers like "PCO-EDGE-5.5-001234"
or inventory tags like "INV-2026-001"; CORA stays opaque on
the value semantics.
Frozenset enforces no-duplicate (kind, value) pairs on the
same Asset at the aggregate level. NO cross-Asset uniqueness
check in v1 (operator-curation discipline; revisit if pilot
ops demand a global SerialNumber registry).
New error classes (3) in aggregates/asset/state.py:
- InvalidAlternateIdentifierValueError (422)
- AssetAlternateIdentifierAlreadyPresentError (409)
- AssetAlternateIdentifierNotPresentError (409)
Wired into routes.py cannot_transition_cls + validation
tuples. Both new slices ship full REST + MCP surface with
contract + integration tests; 2 deciders ship paired
Hypothesis PBTs.
Slice modifications:
- register_asset/{command,decider,route,tool}.py: optional
alternate_identifiers parameter passed through to
AssetRegistered event
- features/add_asset_alternate_identifier/{...}.py: NEW
slice, targeted-mutation, strict-not-idempotent (mirrors
add_asset_port precedent)
- features/remove_asset_alternate_identifier/{...}.py: NEW
slice, mirror of add slice
- aggregates/asset/{state,events,evolver,__init__}.py:
field + 2 new events + VO + closed enum + 3 error classes
- projections/asset.py: 3 new event handlers write JSONB
column with canonical-sorted ordering
- routes.py + wire.py + tools.py: 3 errors + 2 routers + 2
handlers + 2 MCP tools
Atlas migration 20260603100000_add_asset_summary_alternate_
identifiers.sql: ALTER TABLE ADD COLUMN JSONB NOT NULL
DEFAULT '[]'::jsonb + partial GIN index WHERE
jsonb_array_length(alternate_identifiers) > 0 for future
find-by-serial queries. Forward-only; no event upcasting.
Cross-BC binding: NONE. The slice is intra-Asset; no
load_model / load_family preflight. Simpler than the prior
Asset.model_id slice which needed cross-BC family_lookup.
PIDINST instance-tier identity is now complete:
- catalog tier (manufacturer + part_number): Model
- instance binding: Asset.model_id (PR #18)
- instance identifier (SerialNumber + InventoryNumber):
Asset.alternate_identifiers (this PR)
This unblocks the PIDINST DOI mint serializer slice (next).
One trivial post-implementation fix: the projection
integration test for the default-empty-array case asserted
the JSON string "[]" but asyncpg deserializes JSONB to a
Python list; assertion updated to accept both shapes (some
asyncpg codec paths return a string).
Tests: 3142 Asset + Model + Alternate Identifier +
architecture tests pass; 0 failures. ruff clean, pyright
0/0/0, tach clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Stage 3 gate review (7 axes) flagged 4 P0 + 4 P1; this commit
lands all of them via Path A (restore the Decommissioned guard
per memo Lock E) rather than Path B (amend memo to relaxed
semantics).
P0-1 + P0-2 + P0-3: lifecycle guard missing. Both new deciders
omitted the Decommissioned guard the design memo's Lock E
mandated (mirror add_asset_port / remove_asset_port precedent),
leaving AssetCannotAddAlternateIdentifierError fully wired but
never raised, and leaving REST 409 + MCP tool descriptions
advertising a guard the server would never enforce.
Fix: restore the lifecycle guard at the top of both decide()
bodies after the state-is-None check. Both deciders now raise
AssetCannotAddAlternateIdentifierError(asset_id, state.lifecycle)
when state.lifecycle is AssetLifecycle.DECOMMISSIONED, mirroring
the add_asset_port / remove_asset_port shape verbatim. The shared
Cannot* error class is now raised from both add and remove sides
per its own docstring.
Flipped the permissive parametrize entry to a reject entry in
both decider unit tests; extended the paired Hypothesis PBTs to
cover the rejection; added 409 contract tests + MCP isError tests
+ PG integration tests for the Decommissioned case.
P0-4: 403 description leak. Both new routes carried "Authorize
port denied the command." copy-pasted from add_asset_port. Fix:
"Authorize policy denied the command." on both routes.
P1-1: shared AlternateIdentifierBody helper bypassed. The helper
at equipment/_alternate_identifier_body.py was built for these
slices and its own docstring promised reuse, but both routes +
both MCP tools redeclared the inline (kind, value) Pydantic
fields. Fix: wire the helper through identifier: Alternate
IdentifierBody in both routes + both tools; dispatch via
body.identifier.to_domain().
P1-2: remove-route 400 description cited InvalidAlternate
IdentifierError; actual class is InvalidAlternateIdentifier
ValueError. Fix: append the Value suffix.
P1-3: remove-decider docstring named AlreadyPresent as a
rejection cause (copy-paste from add-side). Fix: remove the
AlreadyPresent reference; remove side raises only NotPresent +
the new lifecycle guard.
P1-4: design memo stale. AssetAlternateIdentifierAlreadyExists
Error was renamed to AssetAlternateIdentifierAlreadyPresentError
(matches ModelFamilyAlreadyPresent precedent); REST shape
shipped as POST add/remove (matches add_asset_port precedent),
not POST + DELETE. Memo updated to match shipped code; Lock E
lifecycle-guard wording preserved (Path A restored it).
NEW FITNESS GATE (prevent recurrence of P0-2): tests/
architecture/test_equipment_error_classes_have_producers.py
asserts every <X>Cannot<Verb>Error declared in cora.equipment.
aggregates.{asset,model,family,frame,mount}.state has at least
one raise site in cora.equipment.features.*. Would have caught
the dead AssetCannotAddAlternateIdentifierError class in the
prior commit.
Tests: 1505 alternate-identifier + architecture tests pass; 0
failures. ruff clean; pyright 0/0/0; tach clean.
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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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 PIDINST instance-tier identity gap per the locked Stage 1 memo
project_asset_alternate_identifiers_design.md. Together with PR #17 (Model aggregate) and PR #18 (Asset.model_id binding), this completes the catalog tier + instance binding + instance identifier chain that the future PIDINST DOI mint slice depends on.Assetgains additivealternate_identifiers: frozenset[AlternateIdentifier](empty default).AlternateIdentifieris a flat(kind, value)VO.AlternateIdentifierKindis a closed StrEnum with values verbatim from PIDINST v1.0 property 13 controlled vocabulary:SerialNumber | InventoryNumber | Other. Operators may enter serial numbers likePCO-EDGE-5.5-001234or inventory tags likeINV-2026-001; CORA stays opaque on the value semantics.Two NEW targeted-mutation slices
add_asset_alternate_identifier+remove_asset_alternate_identifiermirroradd_asset_port/remove_asset_portprecedent exactly: strict-not-idempotent, Decommissioned-lifecycle guard, bare Handler (no idempotency), single-stream-write, no cross-BC IO.Commits (2)
3fb2601c8Implementation: aggregate-core extensions + 2 new slices + register_asset modification + 3 new error classes + 1 forward-only Atlas migration + projection updates + wiring. ~5300 LOC + 50 files.e19a0123cGate-review P0/P1 fixes: Path A on Lock E (restore Decommissioned guard in both deciders); use AlternateIdentifierBody helper across routes + tools; fix 4 doc-drift leaks; add NEWtest_equipment_error_classes_have_producers.pyfitness gate to prevent recurrence of consumer-without-producer drift.Verification
test_equipment_error_classes_have_producers.pywould have caught the dead-error-class P0 in the first commit had it existed; now it exists for all future Equipment BC slices.Design-memo conformance
All Locks A through I honored (with Lock E restored via Path A). All Anti-hooks respected.
WHERE jsonb_array_length > 0Memo refinements landed alongside fixes
AssetAlternateIdentifierAlreadyExistsErrorrenamed toAssetAlternateIdentifierAlreadyPresentError(matchesModelFamilyAlreadyPresentprecedent).POST .../add-alternate-identifier+POST .../remove-alternate-identifier(matchesadd_asset_port/remove_asset_portprecedent), notPOST + DELETE.Deferred (tracked as Watch items)
Note on main CI
Main has been CI-red for several commits due to a pre-existing
conduct_procedureOperation BC test bug (unrelated to this PR; admin-merged on PR #17 + PR #18 with the same scope-isolation reasoning). This PR will inherit that failure; admin-merge expected.Co-Authored-By: Claude Opus 4.7 noreply@anthropic.com
🤖 Generated with Claude Code