Repository navigation
Implement canonical persistent typed IDs with deterministic non-reuse (REQ-CORE-001) #46
Description
Activity
AUTHOR claim
- Role: AUTHOR
- Scope:
src/domaincanonical persistent typed ID primitives (REQ-CORE-001) — the 13 ID kinds/prefixes from06 - Handoff/01 — CORE_SCHEMA_AND_LIFECYCLES.mdsection 2, a deterministic run-scoped allocator with permanent non-reuse of retired IDs, focused tests, minimalsrc/index.tsexports if appropriate, a concise identity API/decision note underdocs/specordocs/adr, anddocs/spec/IMPLEMENTATION_STATUS.md. - Branch:
claude/issue-46-canonical-typed-ids - Known blocker: none yet.
AUTHOR handoff
- Branch:
claude/issue-46-canonical-typed-ids - Tested revision:
77d5770 - Pull request: Implement canonical persistent typed IDs (REQ-CORE-001) #48 (
Closes #46) - Checks:
npm ci/typecheck/test/buildpassed;dotnet build/test --configuration Releasepassed (45/45, unchanged);python3 -m unittest discover -s scripts/testspassed (40/40);python3 scripts/policy_guard.py --base masterpassed (6 files, no product/policy mix, no secrets). - Decisions: recorded in
docs/adr/0003-canonical-identity-and-allocation.md— branded-string kind separation, one allocator per run (no module singleton), sequence-based (not content-hash) IDs matching the spec's own dynamic-State precedent, duplicate creation keys fail explicitly rather than idempotently replaying, and a documented lexicographic-creation-key stable-order contract (allocateInCreationKeyOrder) for pre-allocation batches. docs/spec/IMPLEMENTATION_STATUS.md: addedREQ-CORE-001asIN_PROGRESSreferencing this PR; only promotes toIMPLEMENTEDonce merged, per the runbook.- What remains: none of this unit's own acceptance criteria are outstanding. Two things are explicitly out of scope and noted in the PR as "Not checked"/"Unknown": no integration-level guard yet enforces sorted-creation-key call order on future WorldState/genesis callers (there is no such caller code yet to guard), and the real canonical pre-allocation ordering key for scenario/world-genesis entities is left to whichever future unit implements
REQ-CONFIG-001/REQ-CORE-003.
Setting
status:needs-review. Not merging — that is the ACCEPTOR's role.- Branch:
AUTHOR claim
Role: AUTHOR. Addressing
abogun-product's CHANGES_REQUESTED review on PR #48 (at head77d5770), which is item 1 in the runbook's work-selection order (open PR of mine with changes requested).Scope: fix the two acceptance-criterion gaps only, in
src/domain/id.tsandsrc/domain/id.test.ts:allocateInCreationKeyOrderwas not failure-atomic — a batch could partially reserve identities before throwing on an in-batch duplicate or an overlap with a creation key already used by the allocator (including a retired one).allocate/the batch helper performed no runtime boundary validation ofkind/creationKey— an unknown kind, an inheritedObject.prototypeproperty used as a kind, or a non-string key would mutate state or silently produce a malformed id instead of being rejected.
No expansion to registries, economics, workflows, or new policy, per the review's explicit instruction.
Branch:
claude/issue-46-canonical-typed-ids(existing branch for PR #48; no new branch created).Known blocker: none.
AUTHOR claim
Role: AUTHOR. Addressing the metadata-only
CHANGES_REQUESTEDreview fromabogun-producton PR #48 (review at headc584d830ce46d68a398c089b06d72b1ced7b33c0), which is item 1 in the runbook's work-selection order (open PR of mine with changes requested).Scope: the PR body only. The reviewer's own comment confirms the code findings are resolved and independently re-verified at
c584d830; the only remaining gap is that the PR body still describes the superseded head77d5770and 33 tests instead of the actual current head and 40 tests. No source change, no empty commit — the review explicitly asked for a body correction plus a correction handoff, not a code change.Branch:
claude/issue-46-canonical-typed-ids(existing branch for PR #48; no new branch, no new commit).Known blocker: none.
AUTHOR correction handoff
Addresses
abogun-product's metadata-onlyCHANGES_REQUESTEDreview
(#48 (review),
review IDPRR_kwDOEUhO_M8AAAABMNN7Cg) on PR #48.- Head: unchanged,
c584d830ce46d68a398c089b06d72b1ced7b33c0. No source line
changed by this correction — only the PR body. - Metadata fixes applied to the PR body:
- Replaced every reference to the superseded head
77d5770with the
actual current headc584d830ce46d68a398c089b06d72b1ced7b33c0. - Replaced every reference to "33 tests" with the actual current count,
40 tests (9 test files). - Added an explicit summary of the two fixes from the first review round
(batch failure-atomicity via preflight validation andhasCreationKey;
runtimekind/creationKeyboundary validation) and named their
regression tests under "Changed artifacts". - Refreshed the Checks table with freshly measured results at
c584d830:npm ci/typecheck/test(40/40)/buildall passed;
dotnet restore/build --configuration Release/test --configuration Releaseall passed (45/45, unchanged) —dotnetwas available in this
run, unlike the prior reviewer note that localdotnetwas
unavailable to them;scripts/tests(40/40) andpolicy_guard.py
(6 changed files, passed) also re-run and confirmed passing at this
head. - "Not checked" and "Assumptions and unknowns" sections carried forward
unchanged — nothing in them was stale.
- Replaced every reference to the superseded head
Requesting re-review. Returning Issue #46 to
status:needs-review.- Head: unchanged,
ACCEPTED and merged.
Merged revision: PR #48, squash commit
58a0b49156131ec81fcb23d357f7e15b327bde4d(source headc584d830ce46d68a398c089b06d72b1ced7b33c0, branchclaude/issue-46-canonical-typed-ids, now deleted).Independent verification performed this run (checked out
c584d830locally, not taking the PR body's word for it):npm ci— clean install, 0 vulnerabilitiesnpm run typecheck— cleannpm test— 9/9 test files, 40/40 tests passednpm run build—vite buildsucceededdotnet restore/dotnet build --configuration Release --no-restore— 0 warnings, 0 errorsdotnet test --configuration Release --no-build— 45/45TradeCraftSimulation.Testspassed- Required CI checks (
build-and-test,typescript,policy-guard) confirmed green at the exact headc584d830viagh pr checks/gh pr view
Acceptance criteria — all 7 met with evidence observed directly in
src/domain/id.test.ts/id.typecheck.test.tsruns above: 13 kind prefixes + compile-time kind separation, cross-run replay determinism + stable creation-key order, CORE-T16 retirement/non-reuse, duplicate/invalid-input rejection without state corruption (including the failure-atomic batch preflight and runtime kind/key validation added in the fix round), full regression coverage with all pre-existing suites unchanged/green, browser build usable (no Node-only imports), and a complete, accurate handoff record.Scope: confined to
src/domain/id.ts,id.test.ts,id.typecheck.test.ts,src/domain/index.ts,docs/adr/0003-canonical-identity-and-allocation.md,docs/spec/IMPLEMENTATION_STATUS.md— matches the Issue's declared scope. No workflow/policy files touched, no secrets in the diff, no test or invariant weakened.Issue closed by the merge; removing
status:needs-reviewper the runbook now that it carries no active status.
Goal
Implement REQ-CORE-001: browser-capable canonical TypeScript opaque persistent typed IDs and deterministic run-scoped allocation with permanent reservation of retired IDs.
Evidence
The registry marks REQ-CORE-001 READY/P0 and points to docs/spec/mirror/06 - Handoff/01 — CORE_SCHEMA_AND_LIFECYCLES.md, section 2 (Numeric and identity conventions). Section 13 CORE-T16 requires retained references never to bind to a successor instance. Current src/domain only contains scaffolding. M0 migration units merged in #24, #40, #29, #43; visible preview #45 merged as d27a94e. The actual Pages deployment and production UI interaction were verified in #42 (comment). Its existing implementation-status row is stale, not an unfulfilled implementation dependency.
Scope
Canonical identity primitives and focused tests in src/domain, minimal exports through src/index.ts if appropriate, a concise identity API/decision note under docs/spec or docs/adr, and docs/spec/IMPLEMENTATION_STATUS.md. Provide the 13 ID kinds/prefixes named in section 2, type-safe boundaries and a deterministic allocation/reservation lifecycle. Define the caller contract for stable creation keys/order; do not generate identities from array indexes, map enumeration, randomness or wall-clock time. Keep allocator state owned by one run, not a shared module singleton. Reconcile the existing REQ-VISUALIZATION-003 row against merged #45 with the proving test and deployment evidence; record REQ-CORE-001 honestly as pending until its own merge.
Non-goals
No complete WorldState/entity registries, keyed RNG subsystem, config/world generation, tick orchestration, economic mechanisms, C# changes, new UI/hosting, workflow/runbook/permission changes or repair of #33/#44. Do not claim M1 complete: the world-gen preview is a later dependent requirement.
Acceptance criteria
Verification
npm ci, npm run typecheck, npm test, npm run build; dotnet build/test Release; policy-guard and existing scripts/tests. All three required CI checks must be measured passed at the exact reviewed head. Independently reproduce the non-reuse regression/negative control. AUTHOR implements but never merges; a separate ACCEPTOR run reviews and merges only after all gates pass.