385 lines
20 KiB
Markdown
385 lines
20 KiB
Markdown
# 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.
|