Finish D&D entity contract cleanup
This commit is contained in:
@@ -1,384 +0,0 @@
|
||||
# Canonical D&D Entity Contracts Remediation Plan
|
||||
|
||||
## Objective
|
||||
|
||||
Address the remaining correctness and maintainability findings discovered after
|
||||
implementing [Canonical D&D Entity Registries And Occurrences](entity-registries.md).
|
||||
The work must preserve the completed six-lane registry/occurrence architecture
|
||||
while fixing deterministic item identity consolidation, hardening currency
|
||||
reconciliation, making item-occurrence grounding fail closed, removing an
|
||||
unnecessary NPC-identity dependency from item behavior, and aligning NPC
|
||||
registry eligibility with the downstream `mentioned` occurrence contract.
|
||||
|
||||
Complete Stages 15 through 18 in numerical order. Stages 1 through 14 are
|
||||
already complete and are summarized below rather than retained as historical
|
||||
implementation instructions. Each remaining stage is intentionally bounded for
|
||||
one gpt-5.6-terra implementation prompt. Do not combine stages unless a later
|
||||
review explicitly revises this plan.
|
||||
|
||||
## Completed Work Summary: Stages 1 Through 14
|
||||
|
||||
The completed stages established the target state in the feature roadmap:
|
||||
|
||||
- NPCs, locations, and items now each have a canonical registry lane and a
|
||||
registry-backed occurrence lane under consistent `dnd/<entity>-registry` and
|
||||
`dnd/<entity>-occurrences` module keys.
|
||||
- Durable artifact types, kinds, `v1` schemas, codecs, evidence projections,
|
||||
mergers, validators, prompts, assets, capabilities, and package namespaces
|
||||
use registry/occurrence vocabulary without compatibility aliases.
|
||||
- Registry records contain deterministic IDs, canonical names, and transcript
|
||||
evidence. Occurrence records carry exact registry ID/name pairs and their own
|
||||
current-transcript evidence.
|
||||
- The complete D&D example produces all three registries in an earlier step and
|
||||
supplies them through explicit generated references to later occurrence and
|
||||
action lanes. Enemy events consume the NPC registry and combat-opponent NPC
|
||||
occurrences in a later derived step.
|
||||
- Production registration, default validator chains, integration contracts,
|
||||
current-behavior documentation, maintained examples, and representative
|
||||
ordered-pipeline tests were migrated to the new contracts.
|
||||
- A final repository audit confirmed that the retired pre-release module,
|
||||
artifact, schema, prompt, reference-slot, and package names no longer remain
|
||||
as production compatibility paths.
|
||||
|
||||
These completed contracts are the implementation baseline. The remaining
|
||||
stages are corrective work, not a second namespace or schema migration.
|
||||
|
||||
## Baseline Contract Decisions
|
||||
|
||||
The implementation agent must preserve these decisions throughout the four
|
||||
remaining stages:
|
||||
|
||||
- Item identity is name-based: `item:sha256:` plus the lowercase SHA-256 of
|
||||
compact JSON `["dnd.item_registry.identity.v1", comparison_name]`.
|
||||
Consequently, two item-registry records with the same comparison name cannot
|
||||
remain separate merely because their evidence differs.
|
||||
- NPC identity is likewise name-based; location identity additionally includes
|
||||
its earliest evidence anchor. Do not change any durable ID algorithm in this
|
||||
work set.
|
||||
- Exact comparison-name duplicates are resolved deterministically before any
|
||||
semantic LLM reconciliation. The model is used only for supported alias or
|
||||
semantic-equivalence proposals that deterministic identity cannot decide.
|
||||
- Currency denominations are separate item identities. Singular, plural, and
|
||||
standard abbreviations for the same denomination may be reconciled, but a
|
||||
currency name may never be consolidated with another denomination or with a
|
||||
non-currency item.
|
||||
- Every occurrence must contain an exact registry ID/name pair. An extractor
|
||||
response with an unknown ID or mismatched name is invalid output and must
|
||||
cause the extraction attempt to fail; silently dropping a record is not an
|
||||
allowed repair.
|
||||
- A named or stably designated NPC may enter the NPC registry when established
|
||||
only through a supported third-party mention. Hypothetical, speculative,
|
||||
generic, or unestablished identities remain ineligible.
|
||||
- Registry references provide source-free identity grounding and never become
|
||||
occurrence evidence. No stage may weaken this boundary.
|
||||
|
||||
## Rules For Every Remaining Stage
|
||||
|
||||
- Read and follow `docs/policy/architecture.md`,
|
||||
`docs/policy/documentation.md`, and `docs/policy/testing.md` before editing.
|
||||
- Preserve the fixed pipeline, typed artifact, generated-reference, scheduler,
|
||||
validation, checkpoint, output, and whole-run failure boundaries.
|
||||
- Keep D&D policy in `internal/modules/dnd` and `assets/dnd`. Shared text policy
|
||||
belongs in a D&D shared helper, not a generic framework package.
|
||||
- Keep `assets/assets.go` as the root assets package's only Go file and keep the
|
||||
root package free of business logic and PromptKit dependencies.
|
||||
- Preserve strict private structured-response schemas: reject unknown fields
|
||||
and list every declared property in `required`.
|
||||
- Keep prompt-cache ordering and the byte-identical shared D&D prefix intact.
|
||||
- Update the canonical current-behavior integration or internal documentation
|
||||
in the same stage as any implemented contract or failure-semantics change.
|
||||
Link to canonical owners instead of duplicating volatile facts.
|
||||
- Keep tests deterministic, offline, and credential-free. Add regression tests
|
||||
at the narrowest stable behavior boundary. Do not add live-LLM tests, exact
|
||||
prompt snapshots, prompt-prefix length detectors, private asset inventories,
|
||||
or tests that merely freeze helper placement.
|
||||
- Preserve unrelated user changes. Run `gofmt` on changed Go files and
|
||||
`git diff --check` in every stage. Do not retire this plan or the feature
|
||||
roadmap during implementation.
|
||||
|
||||
## Stage 15: Consolidate Deterministically Identical Item Registry Records
|
||||
|
||||
### Goal
|
||||
|
||||
Make item-registry normalization produce exactly one record for every item
|
||||
comparison name, regardless of whether duplicate candidates cite identical or
|
||||
different transcript ranges.
|
||||
|
||||
### Required changes
|
||||
|
||||
1. Revise deterministic preprocessing in
|
||||
`internal/modules/dnd/normalize/itemregistry` so grouping is keyed by the
|
||||
item identity comparison name alone. Remove source-reference equality as a
|
||||
condition for grouping records whose deterministic IDs are necessarily
|
||||
identical.
|
||||
2. For each comparison-name group:
|
||||
- retain the earliest input record as the deterministic display-name and
|
||||
ordering owner;
|
||||
- union source references from every member;
|
||||
- canonicalize and deduplicate the union with the existing
|
||||
`shared.SourceRefOrder` behavior;
|
||||
- retain the sorted unique set of original input indexes for diagnostics;
|
||||
and
|
||||
- derive the final ID from the retained canonical display name through the
|
||||
existing item identity package.
|
||||
3. Keep this consolidation entirely deterministic. Do not send equal-key
|
||||
candidates to the LLM reconciliation prompt, and do not change semantic
|
||||
alias reconciliation for records with different comparison names.
|
||||
4. Preserve stable output order, warning bounds, content-safe diagnostics,
|
||||
nil/empty collection conventions, and input non-mutation. Remove imports or
|
||||
helpers made obsolete by dropping source-reference equality.
|
||||
5. Replace the existing test expectation that equal-name items with different
|
||||
evidence survive as two records. Add focused behavior coverage proving that
|
||||
such records become one valid item with unioned canonical evidence, one
|
||||
deterministic ID, earliest-display-name retention, stable placement, and no
|
||||
input mutation.
|
||||
6. Include a normalizer/default-chain regression case showing that normalized
|
||||
output with cross-chunk equal-name candidates passes item-registry identity
|
||||
validation rather than producing duplicate canonical-name or duplicate-ID
|
||||
rejection.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- Item normalization cannot emit two records with the same item comparison
|
||||
name or deterministic ID solely because their evidence differs.
|
||||
- The retained record contains the canonical union of every duplicate member's
|
||||
evidence and remains stable across repeated normalization.
|
||||
- Different comparison names remain eligible for the existing bounded semantic
|
||||
reconciliation path.
|
||||
- No durable schema, item ID algorithm, module key, or artifact kind changes.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/normalize/itemregistry ./internal/modules/dnd/items/identity ./internal/modules/dnd/validate/itemregistry/identity ./internal/modules/dnd/register
|
||||
go test ./internal/modules/dnd/...
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Stage 16: Make Currency Reconciliation Fail Closed
|
||||
|
||||
### Goal
|
||||
|
||||
Prevent an LLM reconciliation proposal from combining currency with a
|
||||
non-currency item or combining distinct currency denominations.
|
||||
|
||||
### Required changes
|
||||
|
||||
1. Refactor the item-registry semantic-consolidation guard in
|
||||
`internal/modules/dnd/normalize/itemregistry/reconciliation.go` to classify
|
||||
every proposed member before applying a group. Preserve the existing five
|
||||
supported D&D denominations and their singular, plural, and standard
|
||||
abbreviation aliases.
|
||||
2. Apply this exact decision table:
|
||||
|
||||
| Proposed group contents | Result |
|
||||
| --- | --- |
|
||||
| no recognized currency members | allow the existing item reconciliation policy to decide/apply the already validated proposal |
|
||||
| only recognized currency members of one denomination | allow consolidation |
|
||||
| recognized currency members of different denominations | reject the proposal group |
|
||||
| one or more recognized currency members plus any non-currency member | reject the proposal group |
|
||||
|
||||
An unrecognized name must not be treated as the same denomination merely
|
||||
because another member is recognized currency.
|
||||
3. On rejection, preserve every deterministic input record exactly as the safe
|
||||
fallback, keep its canonicalized evidence, count the proposal as rejected
|
||||
for the existing retry/fallback behavior, and emit one bounded content-safe
|
||||
warning for the group. Do not partially consolidate a rejected group.
|
||||
4. Generalize the current warning text so it accurately covers both mixed
|
||||
currency/non-currency groups and conflicting denominations without exposing
|
||||
transcript content unnecessarily.
|
||||
5. Add table-driven regression coverage for same-denomination aliases,
|
||||
different denominations, currency plus an ordinary item, multiple ordinary
|
||||
items, and a mixed group whose canonical member is the non-currency item.
|
||||
Test observable normalized results and warnings rather than the private
|
||||
helper's implementation.
|
||||
6. Preserve proposal validation, retry counts, safe fallback, warning limits,
|
||||
deterministic ordering, and non-mutation outside this additional safety
|
||||
rule.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- `Gold Pieces` cannot be consolidated with `Longsword`, regardless of which
|
||||
member the model selects as canonical.
|
||||
- Currency aliases such as `GP`, `Gold Piece`, and `Gold Pieces` may still
|
||||
resolve to one gold-denomination item when proposed safely.
|
||||
- Different denominations always remain distinct.
|
||||
- Invalid groups preserve the deterministic candidates and follow the current
|
||||
retry/fallback contract without a partial merge.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/normalize/itemregistry
|
||||
go test ./internal/modules/dnd/validate/itemregistry/... ./internal/modules/dnd/register
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Stage 17: Enforce Item Occurrence Grounding And Own Shared Text Comparison
|
||||
|
||||
### Goal
|
||||
|
||||
Make item-occurrence extraction reject invalid registry pairs instead of
|
||||
silently losing events, and remove its accidental semantic dependency on the
|
||||
NPC identity package while preserving comparison behavior exactly.
|
||||
|
||||
### Required changes
|
||||
|
||||
1. Change response mapping in
|
||||
`internal/modules/dnd/extract/itemoccurrences/canonicalize.go` so an unknown
|
||||
`item_id` or a `name` that does not exactly match that ID's canonical
|
||||
registry name returns a bounded, content-safe extraction error identifying
|
||||
the occurrence index and violated field. Do not `continue`, omit the bad
|
||||
record, repair it by name, or return a partial artifact.
|
||||
2. Ensure the mapping error propagates through the existing extractor boundary
|
||||
as a failed extraction attempt so the configured pipeline retry policy may
|
||||
obtain a corrected structured response. Do not add a module-local retry
|
||||
loop or convert this failure into a validator rejection after accepting
|
||||
partial output.
|
||||
3. Add extraction regression cases for an unknown ID, a mismatched name, and a
|
||||
response containing a valid occurrence followed by an invalid one. All must
|
||||
return an error and no accepted partial result. Mirror the established NPC
|
||||
occurrence extractor behavior where practical without introducing a generic
|
||||
framework abstraction.
|
||||
4. Introduce one D&D-owned shared text comparison primitive under
|
||||
`internal/modules/dnd/shared` implementing the current byte-for-byte
|
||||
semantics: Unicode NFKC normalization, curly/modifier apostrophe mapping,
|
||||
whitespace collapse, and Unicode case folding. Give the semantic policy a
|
||||
stable explicit identity such as `dnd.text_comparison.v1`.
|
||||
5. Remove the `shared` package's existing import of `npcs/identity` by making
|
||||
shared token matching use the new primitive. Make NPC, location, and item
|
||||
identity `ComparisonKey` functions delegate to the shared primitive while
|
||||
retaining their public package functions and all existing entity-specific
|
||||
ID policy constants and digest algorithms.
|
||||
6. Change `internal/modules/dnd/itemoccurrences` to use the shared D&D
|
||||
comparison primitive rather than `npcs/identity`. Do not change display
|
||||
trimming, party-holder rules, exact-duplicate identity, or ordering.
|
||||
7. Add the shared comparison policy identity to item-occurrence extractor and
|
||||
normalizer manifest/checkpoint semantics wherever comparison behavior can
|
||||
affect mapped or normalized output. Keep values content-safe. Document in
|
||||
the nearest D&D internal owner that a semantic change to shared comparison
|
||||
requires an explicit policy-version review for every affected identity,
|
||||
mapping, normalization, and validator policy; do not calculate a brittle
|
||||
fingerprint from private helper source.
|
||||
8. Consolidate duplicated comparison test cases into focused shared-helper
|
||||
behavior coverage, while retaining entity identity tests that protect each
|
||||
durable ID contract. Prove existing representative NPC, location, item, and
|
||||
item-occurrence outputs are unchanged by the ownership refactor.
|
||||
9. Update `docs/integrations/dnd-item-occurrence-artifacts.md` if it currently
|
||||
says extraction omits unknown or mismatched pairs. The canonical contract
|
||||
must say the response is rejected as invalid model output and may be retried;
|
||||
normalization and validation remain defense-in-depth for artifacts entering
|
||||
through other boundaries.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- Item-occurrence extraction never succeeds with a silently shortened artifact
|
||||
after receiving an invalid registry pair.
|
||||
- The framework's existing retry behavior receives the mapping error without a
|
||||
new hidden retry mechanism.
|
||||
- No item-occurrence or shared D&D package imports NPC identity solely for text
|
||||
comparison.
|
||||
- All existing comparison keys and durable entity IDs remain unchanged for the
|
||||
same inputs.
|
||||
- Item-occurrence checkpoint reuse is invalidated when its declared comparison
|
||||
policy identity changes.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/shared ./internal/modules/dnd/npcs/identity ./internal/modules/dnd/locations/identity ./internal/modules/dnd/items/identity
|
||||
go test ./internal/modules/dnd/itemoccurrences ./internal/modules/dnd/extract/itemoccurrences ./internal/modules/dnd/normalize/itemoccurrences ./internal/modules/dnd/validate/itemoccurrences/...
|
||||
go test ./internal/modules/dnd/...
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Stage 18: Align NPC Mention Eligibility And Finish Vocabulary Cleanup
|
||||
|
||||
### Goal
|
||||
|
||||
Allow the NPC registry to establish named NPCs that appear only in supported
|
||||
third-party mentions, and remove the remaining occurrence/interactions naming
|
||||
residue without broadening the registry to speculative or generic identities.
|
||||
|
||||
### Required changes
|
||||
|
||||
1. Revise `assets/dnd/npc-registry/extract/prompts/instructions.md` so a proper
|
||||
name or stable unique NPC designation is eligible when the transcript
|
||||
establishes it through a factual third-party mention, even if the NPC is not
|
||||
physically present and does not speak or enter combat in that chunk.
|
||||
2. Keep the exclusion boundary explicit: omit hypothetical or speculative
|
||||
people, names used only in an imagined example, generic roles, anonymous
|
||||
groups, player characters, and labels invented by the model. Do not require
|
||||
dialogue, physical presence, or direct action as evidence of identity.
|
||||
3. Preserve separation of concerns. The registry prompt must extract only NPC
|
||||
identity and identity evidence; it must not classify an occurrence as
|
||||
`mentioned` or copy occurrence semantics into the registry artifact. The
|
||||
later NPC occurrence lane remains responsible for emitting the `mentioned`
|
||||
fact with independently cited current-transcript evidence.
|
||||
4. Review the shared D&D identity/evidence instructions and remove or revise
|
||||
any contradictory wording using the canonical asset owner. Do not duplicate
|
||||
the new rule across module-specific and shared assets merely for emphasis,
|
||||
and do not disturb shared prompt ordering or cacheable prefix bytes unless a
|
||||
genuinely shared contradiction must be corrected.
|
||||
5. Update the canonical NPC registry and NPC occurrence integration documents,
|
||||
and `docs/internal/dnd.md` only where needed, so they consistently distinguish
|
||||
a transcript-established mentioned-only identity from an occurrence. Link
|
||||
between canonical owners instead of copying category lists or schemas.
|
||||
6. Correct the duplicated phrase `NPC occurrence occurrences` in the NPC
|
||||
occurrence prompt. Rename stale local aliases such as `interactionmodel` in
|
||||
NPC occurrence code and stale `interactionContent` test variables to
|
||||
occurrence vocabulary. Correct similarly scoped registration labels that
|
||||
inaccurately say `locations` when they describe another entity. Do not make
|
||||
unrelated wording changes outside the entity modules.
|
||||
7. Run existing prompt preparation and module behavior tests. Add a focused
|
||||
deterministic regression test only where there is a stable behavioral seam;
|
||||
do not snapshot the prose, assert an exact prompt fragment, or add a prompt
|
||||
change detector. Use optional human prompt evaluation to assess model
|
||||
interpretation, not as an automated acceptance gate.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- The NPC registry prompt permits a factual named third-party mention to
|
||||
establish an NPC while continuing to reject hypothetical and generic names.
|
||||
- The registry output remains the minimal `{id, name, source_refs}` contract;
|
||||
mention classification remains in NPC occurrences.
|
||||
- Current integration and internal documentation describe the same boundary
|
||||
without duplicating volatile contract detail.
|
||||
- NPC occurrence production code, prompt prose, and touched tests contain no
|
||||
stale interaction-era local vocabulary.
|
||||
- Prompt manifests still prepare successfully with the established shared
|
||||
prefix and final module-instruction ordering.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/dnd/extract/npcregistry ./internal/modules/dnd/extract/npcoccurrences ./internal/modules/dnd/normalize/npcoccurrences ./internal/modules/dnd/validate/npcoccurrences/... ./internal/modules/dnd/register
|
||||
go test ./internal/modules/dnd/... ./internal/modules/integration/...
|
||||
git diff --check
|
||||
```
|
||||
|
||||
## Final Verification
|
||||
|
||||
After Stage 18, run the repository-wide checks and validate both maintained D&D
|
||||
configurations:
|
||||
|
||||
```sh
|
||||
go test -count=1 ./...
|
||||
go vet ./...
|
||||
go build ./cmd/notarius
|
||||
go run ./cmd/notarius config validate --config examples/dnd-minimal.config.yml --pipeline dnd-session
|
||||
OPENROUTER_API_KEY=validation-placeholder go run ./cmd/notarius config validate --config examples/dnd-complete.config.yml --pipeline dnd-session
|
||||
git diff --check
|
||||
```
|
||||
|
||||
The implementation is complete only when all commands pass and a final review
|
||||
confirms that the feature roadmap's durable schemas, module names, evidence
|
||||
boundaries, and ordered registry handoffs remain unchanged except for the
|
||||
explicitly corrected failure and eligibility behavior above.
|
||||
|
||||
## Open Questions
|
||||
|
||||
None. The stages above adopt the long-term-maintainable choices from the audit:
|
||||
deterministic identity consolidation, fail-closed semantic safeguards, strict
|
||||
extractor grounding, a D&D-owned shared comparison policy, and support for
|
||||
factual mentioned-only NPC identities.
|
||||
Reference in New Issue
Block a user