# D&D Extraction Module Audit Implementation Plan Status: Ready for implementation This document turns the completed findings in [D&D Extraction Module Refactoring Audit](audit.md#audit-results) into an ordered implementation plan. Each stage is one bounded prompt for an LLM coding agent. Execute the stages in order and complete each stage's gate before starting the next. The target is a corrected and better-harmonized five-extractor family, not a generic extractor framework. Preserve the intentional differences and rejected abstractions recorded in the audit. ## Global Instructions Every stage must: 1. Read this document, the completed audit, `AGENTS.md`, `docs/development.md`, and the task-specific documents identified there. Follow all policies under `docs/policy/`. 2. Inspect the current working tree before editing. Preserve unrelated user changes and do not assume a clean checkout. 3. Prefer the codebase knowledge graph for code discovery and call tracing. Use direct text and file searches for documentation, configuration, prompt assets, schemas, fixtures, and exact string comparison. 4. Implement only the current stage. Do not opportunistically begin a later stage. 5. Keep artifact semantics, private response DTOs and schemas, lane-specific prompts, mapping, diagnostics, and typed test support in their owning packages unless this plan explicitly moves one responsibility. 6. Keep the new shared behavior under `internal/modules/dnd/shared`; do not move D&D concepts into the generic framework. 7. Apply the testing policy. Protect observable behavior and semantic checkpoint identity, not private helper usage, exact prompt text, hashes, message lengths, source-file layout, or test counts. 8. Use focused tests while iterating. Run `gofmt` on changed Go files and `git diff --check` before completing every stage. 9. Update current-behavior documentation only in the documentation stage, after the corresponding behavior exists. 10. Do not alter durable artifact schemas, prompt prose, configuration contracts, reference-slot policy, model-facing response DTO fields, or extraction policy except where a stage explicitly requires it. The following audit decisions remain out of scope: - removal or continued support of the deprecated `roster` reference alias; - renaming the spell private response-schema key or ID; - shared prompt prose or response schemas; - generic manifest, fingerprint, mapper, structured-call, error-prefix, or registration builders; - shared schema-test fixtures or provider fakes; and - framework-level preflight or source-reference APIs. ## Stage 1: Protect Spell And NPC Mapping Identity ### Objective Close the stale-checkpoint gap before changing any mapping or ordering behavior. ### Scope Change only the spell and NPC extractor packages and the narrow integration or checkpoint tests needed to prove their prepared fingerprints affect reuse. ### Implementation 1. Add an unexported mapping-policy constant to each extractor: ```go // internal/modules/dnd/extract/spells mappingPolicy = "dnd.spells.extract_mapping.v1" // internal/modules/dnd/extract/npcs mappingPolicy = "dnd.npcs.extract_mapping.v1" ``` 2. In both packages: - add `"mapping_policy": mappingPolicy` to `ManifestMetadata`; - add `{Name: "mapping_policy", Value: mappingPolicy}` to `CheckpointFingerprints`; - preserve all existing prompt, response-schema, catalog, registry, identity, and digest metadata and fingerprints; and - preserve nil-receiver behavior as it exists at the start of this stage. 3. Do not change mapping, source-reference ordering, prompt or schema content, or artifact output in this stage. ### Tests - Extend each package's metadata/fingerprint test to prove that the mapping policy is present with the declared semantic value and that no existing fingerprint disappears. - In the existing prepared-pipeline integration coverage for spells and NPCs, assert that the scoped prepared fingerprint list contains: - `extract::dnd/spells:mapping_policy`; and - `extract::dnd/npcs:mapping_policy`; using the actual lane IDs from the fixtures. - Add or extend one checkpoint-resume test at the prepared pipeline boundary to prove that a differing or missing scoped mapping-policy fingerprint prevents reuse with the normal identity-mismatch reason. Do not duplicate the checkpoint loader's complete mismatch matrix in both lane suites. ### Validation Run: ```sh go test ./internal/modules/dnd/extract/spells go test ./internal/modules/dnd/extract/npcs go test ./internal/modules/integration go test ./internal/framework/checkpoint ./internal/framework/pipeline git diff --check ``` ### Completion Gate Spell and NPC mapping policies participate in manifest provenance, prepared checkpoint identity, and restore decisions, while extraction output remains unchanged. ## Stage 2: Introduce Document-Aware Source-Reference Ordering ### Objective Create the one shared D&D source-reference primitive accepted by the audit and establish it with the already document-aware NPC-interaction consumers. ### Scope Change `internal/modules/dnd/shared`, `internal/modules/dnd/npcinteractions`, and the NPC-interaction normalizer and invariant validator that currently call the model helper. Do not migrate the other extractors or normalizers yet. ### Shared API Add a document-position index with this public surface: ```go type SourceRefOrder struct { // private immutable snapshot } func NewSourceRefOrder(doc *source.SourceDocument) SourceRefOrder func (o SourceRefOrder) Less(left, right source.SourceRef) bool func (o SourceRefOrder) EarliestValid(refs []source.SourceRef) (position int, ok bool) func (o SourceRefOrder) Canonicalize(refs []source.SourceRef) []source.SourceRef ``` The constructor must snapshot only the source ID and unit-ID-to-position map; the returned value must not retain or mutate the document. ### Required Semantics `Less` must preserve the established NPC-interaction ordering: 1. compare differing source IDs lexically; 2. after equal source IDs, order resolvable start endpoints by indexed document position and before unresolvable start endpoints; 3. use the literal start unit ID as the deterministic tie or invalid fallback; 4. apply the same document-position, resolvability, and literal fallback rules to end endpoints; and 5. return false for exactly equal references. `EarliestValid` must: - consider only references accepted by `source.ValidateRef` for the indexed document; - return the smallest document position of a valid start endpoint; - ignore invalid candidates without mutating or deleting them; and - return `(0, false)` for a nil document, an empty set, or no valid reference. `Canonicalize` must: - return `nil` for nil input and an independently owned non-nil empty slice for non-nil empty input; - clone the input; - stable-sort it with `Less`; - remove only exactly equal `source.SourceRef` values after sorting; and - never rewrite, repair, or discard a distinct invalid candidate. A zero-value `SourceRefOrder` must be safe and deterministic, using literal fallback ordering and treating every reference as invalid for `EarliestValid`. ### Migration 1. Replace the implementation of the focused `internal/modules/dnd/npcinteractions` canonicalization/ordering helpers with the shared primitive. 2. Update its normalizer and invariant validator to use `shared.NewSourceRefOrder(doc)` directly where practical. 3. Remove the old exported `SourceRefLess` and `CanonicalizeSourceRefs` functions if no production or test caller remains. Retain NPC-interaction-specific exact artifact identity and list comparison in the focused model package. 4. This migration must preserve NPC-interaction output exactly, so do not bump its normalization, validation, or extraction policy in this stage. ### Tests Shared table tests must cover: - non-monotonic document unit IDs; - valid and invalid endpoints; - references to another source ID; - stable ties; - exact duplicates versus merely similar ranges; - nil and non-nil empty inputs; - input/output alias safety; - mutation of the source document after constructing the index; and - `EarliestValid` ignoring invalid candidates. Retain or adapt focused NPC-interaction tests to prove its artifact ordering and invariant behavior. Do not add tests that merely assert that a package calls the shared helper. ### Validation Run: ```sh go test ./internal/modules/dnd/shared go test ./internal/modules/dnd/npcinteractions go test ./internal/modules/dnd/normalize/npcinteractions go test ./internal/modules/dnd/validate/npcinteractions/... git diff --check ``` ### Completion Gate The shared API has exhaustive behavioral protection, NPC interactions retain their prior output, and no duplicate document-aware comparator remains in the NPC-interaction model. ## Stage 3: Migrate Normalization And Invariant Consumers ### Objective Make downstream spell, NPC, and combat-turn canonicalization use document order before changing extractor output. ### Scope Change only: - `internal/modules/dnd/normalize/spells`; - `internal/modules/dnd/normalize/npcs`; - `internal/modules/dnd/normalize/combatturns`; and - `internal/modules/dnd/validate/combatturns/invariants`. ### Implementation 1. Construct one `shared.SourceRefOrder` from the normalization or validation request's source document and pass it through the relevant local operation. Do not rebuild the index inside artifact or reference loops. 2. Replace numeric source-reference sorting, exact deduplication, and repeated `source.UnitIndex` scans with `Less`, `Canonicalize`, and `EarliestValid` as applicable. 3. Keep these responsibilities local: - spell catalog canonicalization and duplicate spell identity; - NPC identity and grouping; - combat actor identity, turn comparison, warnings, and duplicate policy; - repair/warning counts and message construction; and - artifact-specific tie-breakers. 4. Derive local repair facts without extending the shared API: - compare the original and canonical slices to determine whether order changed; - compute exact duplicates removed from input and output lengths; and - preserve the current warning reason codes and useful diagnostic context. 5. Update checkpoint identity for every changed semantic owner: - add spell normalizer metadata and fingerprint `normalization_policy = "dnd.spells.normalize.v1"` because it currently has no local normalization-policy fingerprint; - change NPC normalization policy to `dnd.npcs.normalize.v2`; - change combat-turn normalization policy to `dnd.combat_turns.normalize.v2`; and - change the combat-turn normalized-invariant validator policy to `dnd.combat_turns.validator.normalized.v2`. Preserve the spell catalog fingerprint as a separate semantic input. 6. Delete superseded local numeric reference comparators and earliest-position scans after all callers in these packages move. ### Tests For each affected artifact family, add or adapt focused tests using a valid document whose unit IDs are deliberately non-monotonic. Prove: - source references follow document order; - artifact order uses the earliest valid document position where applicable; - invalid candidates remain deterministic and available to validators; - exact duplicate and warning/repair behavior is unchanged except for the corrected ordering; and - returned slices do not alias inputs. Update metadata/fingerprint tests for the exact semantic policy values above. Do not duplicate all shared `SourceRefOrder` edge cases in each package. ### Validation Run: ```sh go test ./internal/modules/dnd/normalize/spells go test ./internal/modules/dnd/normalize/npcs go test ./internal/modules/dnd/normalize/combatturns go test ./internal/modules/dnd/validate/combatturns/invariants git diff --check ``` ### Completion Gate All downstream consumers in scope use one document-position index per operation, non-monotonic IDs are handled correctly, local artifact semantics remain local, and every changed semantic policy invalidates prior checkpoints. ## Stage 4: Migrate Spell And NPC Extraction ### Objective Correct source-reference and artifact ordering in the spell and NPC extractors. ### Scope Change only the spell and NPC extractor packages plus their focused tests. ### Implementation 1. Build one `shared.SourceRefOrder` from `req.Source` after common request prerequisites have passed. 2. Use `Canonicalize` for mapped durable source references. Preserve: - model-produced invalid candidates; - attachment of the current transcript source ID; - exact-only deduplication; - package-owned DTO conversion and identity logic; and - independent ownership of returned artifacts and references. 3. For spell artifact ordering, replace numeric `earliestSourceUnit` behavior with `EarliestValid` document positions: - artifacts with valid evidence sort by their earliest valid transcript position; - valid-evidence artifacts sort before artifacts with no valid evidence; - existing artifact-specific deterministic tie-breakers remain in their current order; and - invalid evidence is retained even though it does not select the earliest position. 4. Use document-aware reference comparison in NPC ordering while preserving NPC identity and artifact tie-breakers. 5. Remove the superseded local numeric comparators and scans. 6. Bump the Stage 1 mapping policies: - `dnd.spells.extract_mapping.v2`; and - `dnd.npcs.extract_mapping.v2`. Update both manifest metadata and checkpoint tests through the constants. ### Tests Add focused regression fixtures with non-monotonic unit IDs for: - spell reference ordering and spell artifact ordering; - NPC reference and artifact ordering; - mixed valid and invalid evidence; - exact duplicate references; - stable artifact ties; and - output mutation safety. Retain existing malformed model-output, provider-error, catalog, identity, and reference-grounding coverage. ### Validation Run: ```sh go test ./internal/modules/dnd/extract/spells go test ./internal/modules/dnd/extract/npcs git diff --check ``` ### Completion Gate Spell and NPC extraction follow transcript order for valid evidence, preserve invalid candidates deterministically, and advertise the new mapping semantics through v2 policy fingerprints. ## Stage 5: Migrate Combat-Turn And NPC-Interaction Extraction ### Objective Complete source-reference harmonization across the four citation extractors and remove superseded reference APIs. ### Scope Change the combat-turn and NPC-interaction extractor packages and the D&D shared package only. ### Implementation 1. In both extractors, construct one `shared.SourceRefOrder` from the request source and use `Canonicalize` for mapped durable references. 2. Preserve actor/NPC identity, enum candidates, artifact ordering, transcript-source attachment, warnings, provider behavior, and local DTO mapping. 3. Bump mapping policies: - `dnd.combat_turns.extract_mapping.v2`; and - `dnd.npc_interactions.extract_mapping.v2`. 4. Delete all superseded local numeric source-reference comparators. 5. Delete `shared.SourceRefCandidate` and its isolated test. DTO-to-durable mapping must continue to attach the trusted current document source ID locally and must not trust a model-supplied source identity. 6. Search the complete D&D module tree for remaining numeric comparisons of `SourceRef` endpoints. Retain a local comparator only if it implements a documented artifact-specific policy; otherwise migrate it to `SourceRefOrder`. ### Tests Add focused non-monotonic-ID regression tests for both extractors, including invalid candidates, exact duplicates, stable ties, and mutation safety. Update metadata/fingerprint expectations for the v2 mapping policies. Do not add a test asserting that `SourceRefCandidate` or a local comparator is absent; compilation and behavioral tests are sufficient. ### Validation Run: ```sh go test ./internal/modules/dnd/extract/combatturns go test ./internal/modules/dnd/extract/npcinteractions go test ./internal/modules/dnd/shared go test ./internal/modules/dnd/... git diff --check ``` ### Completion Gate All citation extractors use the shared document-aware mechanics, no unsafe model-source candidate helper remains, and changed mapping behavior has explicit checkpoint identity. ## Stage 6: Consolidate Common Extraction Preflight ### Objective Give the five extractors one owner for their common request prerequisites while keeping lane-specific checks and error context local. ### Scope Change `internal/modules/dnd/shared` and the five extractor packages. ### Shared API Replace `ChunkPromptMaterial` with: ```go func PrepareChunkExtraction( ctx context.Context, req contracts.TypedExtractionRequest, ) (contracts.LLMInputMaterial, error) ``` ### Required Semantics The helper must validate in this order: 1. context is non-nil; 2. the context has no existing error; 3. source is non-nil; 4. chunk is non-nil; 5. the chunk contains at least one materialized unit; and 6. source input is cloned/defaulted and its content exactly matches the chunk content. Preserve the current material defaults for name, media type, and size. The returned `LLMInputMaterial` and its content must not alias the request. The helper returns domain-neutral error details without an extractor name. Each extractor must wrap helper failures through its existing local error function so diagnostics retain lane context. ### Migration For all five extractors: - keep nil receiver and nil LLM-client checks local and before the shared helper; - call the helper once before specialized reference projection, provider invocation, or mapping; - retain catalog/NPC registry and lane-specific checks locally; - preserve the existing provider-error prefixes and result types; and - remove duplicated context/source/chunk/unit/material checks. Delete the exported `ChunkPromptMaterial` function after the fifth caller migrates. Do not retain an alias solely for internal compatibility. ### Tests - Move the full common preflight matrix to table-driven shared tests: nil context, canceled context, nil source, nil chunk, empty units, mismatched content, defaulted material, preserved explicit material, and mutation safety. - Each extractor package retains only tests that add value locally: nil receiver/client, one representative wrapped preflight error, specialized dependency/reference behavior, provider failure, and mapping. - Remove redundant per-package common-branch tests once the shared contract owns them; do not preserve five copies for coverage symmetry. ### Validation Run: ```sh go test ./internal/modules/dnd/shared go test ./internal/modules/dnd/extract/... git diff --check ``` ### Completion Gate All five extractors use `PrepareChunkExtraction`, validation order and error context are preserved, common branches have one test owner, and no exported compatibility alias remains. ## Stage 7: Make Deterministic Validation Own Scene Semantics ### Objective Remove duplicated semantic policy from the scene-description private transport schema. ### Scope Change only the scene-description extractor's private schema and focused schema and extraction tests, plus shape-validator tests if a missing semantic case is discovered. ### Implementation 1. In `dnd_scene_descriptions_llm.v1.json`, retain: - the schema dialect and ID; - the top-level object type; - `additionalProperties: false`; - required `kind`, `title`, and `summary` fields; and - string types for all three fields. 2. Remove: - the `kind` enum; and - `minLength` from `title` and `summary`. 3. Do not change `SchemaVersion`, `ResponseSchemaKey`, `ResponseSchemaID`, `ResponseSchemaName`, the private DTO, prompt text, or durable artifact schema. The schema content digest will change and must naturally invalidate prior extractor checkpoint identity. 4. Keep the scene-description shape validator as the sole owner of supported kinds and non-blank title/summary semantics. ### Tests - Update private-schema tests so unsupported kinds and empty strings are valid transport values, while missing fields, wrong JSON types, unknown fields, collections, and malformed JSON remain rejected. - Ensure shape-validator tests cover unsupported kind, empty and whitespace-only title, and empty and whitespace-only summary. - Add one extractor-level test proving a structurally valid but semantically invalid provider response is decoded and returned for deterministic validation rather than rejected by the private schema boundary. - Assert the schema digest is valid and mutation-safe, but do not freeze its exact hash. ### Validation Run: ```sh go test ./internal/modules/dnd/extract/scenedescriptions go test ./internal/modules/dnd/validate/scenedescriptions/shape git diff --check ``` ### Completion Gate The private schema owns only transport structure, the deterministic validator owns scene semantics, and the changed schema digest invalidates stale checkpoints without a version rename. ## Stage 8: Protect Prompt Order And Cache Boundaries ### Objective Give all four citation extractors the same level of behavioral protection as the scene-description prompt without freezing prompt content. ### Scope Change only the Scriptorium asset tests for spells, NPCs, combat turns, and NPC interactions. Do not edit manifests or prompt assets unless a test exposes an actual mismatch with the already documented current contract. ### Expected Prepared Sequences Using each package's real embedded registry and representative inputs, assert these complete ordered message identities: | Lane | Ordered messages | | --- | --- | | NPC | system, extraction evidence, identity, campaign references, task, instructions, transcript | | Spell | system, extraction evidence, identity, campaign references, NPC registry, spell catalog, task, instructions, transcript | | Combat turn | system, extraction evidence, identity, campaign references, NPC registry, task, instructions, transcript | | NPC interaction | system, extraction evidence, identity, campaign references, names-only NPC registry, task, instructions, transcript | For NPCs, assert ephemeral cache boundaries on identity, campaign references, and instructions. For spells, combat turns, and NPC interactions, assert ephemeral boundaries on identity, campaign references, the NPC registry, and instructions. Assert that every other message, including the final transcript, has no cache control. Use the prepared message role plus rendered input/source identity already available from Scriptorium to distinguish messages. If an asset-only message does not expose a stable identity, assert its position and role without asserting exact prose. ### Tests Consolidate overlapping assertions within each package where that improves clarity. Retain existing asset registration, content-safety, and prompt digest tests when they protect distinct risks. Do not assert: - exact prompt text; - prefix length or total byte count; - an exact prompt hash; - private manifest file layout; or - that a particular shared helper or asset path was used. ### Validation Run: ```sh go test ./internal/modules/dnd/extract/spells go test ./internal/modules/dnd/extract/npcs go test ./internal/modules/dnd/extract/combatturns go test ./internal/modules/dnd/extract/npcinteractions git diff --check ``` ### Completion Gate All four suites protect the complete documented role/input order and cache flags through real prompt preparation, without change-detector assertions. ## Stage 9: Package Hygiene And Extractor Documentation ### Objective Remove the remaining misleading package surface and document the common contract future D&D extractors must follow. ### Scope Change the spell and combat-turn extractor packages and `docs/internal/modules.md`. Include no new production abstraction. ### Code Cleanup 1. Delete the unused singular `ArtifactType` constants from the spell and combat-turn extractor packages. `ArtifactKind` and the durable typed model remain authoritative. 2. Add the same nil guard used by the other extractors to spell `ManifestMetadata`: ```go if e == nil { return nil } ``` 3. Add one focused zero-value spell metadata test. Do not add tests for the absence of deleted constants. ### Documentation Update `docs/internal/modules.md` as the canonical current-behavior owner: 1. Replace the stale `ChunkPromptMaterial` consumer description with `PrepareChunkExtraction` and name all five current extractor consumers. 2. Add a compact `### D&D Extractor Contract` subsection within or immediately after `## Adding An Extension`. 3. State requirements as behaviors and ownership boundaries, not mandatory filenames: - reject unknown options unless an option namespace is intentionally extensible; - use shared common preflight while retaining receiver, dependency, and lane-specific checks locally; - return independently owned results and exposed metadata safe for caller mutation; - keep the private response DTO, structural schema and identity, provider-response mapping, durable conversion, and lane diagnostics package-owned; - include every stable semantic input capable of changing durable output in checkpoint identity, considering prompts, schemas, mapping, canonicalization, prepared reference projections, identity, normalization, and trimming as applicable; and - consider focused behavioral coverage for construction/registration, option rejection, preflight, provider failure, structured decoding, mapping/ownership, prompt role/input/cache order, and checkpoint invalidation. 4. Link rather than duplicate: - prompt ordering, shared-asset, cache, and private-schema rules in `docs/internal/llm.md`; - checkpoint and reference behavior in `docs/internal/pipeline.md`; - architecture ownership rules; and - the testing policy. 5. Explicitly avoid prescribing exact prompt content or length, hashes, test counts, filenames, fixture layouts, or generic implementation builders. ### Validation Run: ```sh go test ./internal/modules/dnd/extract/spells go test ./internal/modules/dnd/extract/combatturns go test ./internal/modules/dnd/shared git diff --check ``` Validate every new documentation link and confirm the text describes behavior implemented by Stages 1 through 8. ### Completion Gate The misleading exports are gone, spell metadata is nil-safe, the shared preflight documentation is current, and future extractors have one concise normative checklist linked to the canonical detailed policies. ## Stage 10: Integrated Regression And Audit Closure ### Objective Verify the complete refactor as one system and remove any residual duplication or stale references introduced or exposed by the migration. ### Scope This is a verification and narrowly scoped correction stage. Do not introduce new abstractions or expand product behavior. ### Required Review 1. Re-read every audit finding and confirm it is addressed: - document-aware reference and spell artifact ordering; - spell/NPC mapping fingerprints; - shared common extraction preflight; - deterministic ownership of scene semantics; - complete prompt-order/cache tests; - package hygiene; and - the documented D&D extractor contract. 2. Search for and resolve only genuine leftovers: - numeric ordering of D&D `SourceRef` endpoints where document order is the intended policy; - production calls to removed helpers; - stale references to `ChunkPromptMaterial`, `SourceRefCandidate`, the deleted NPC-interaction comparators, or deleted `ArtifactType` constants; - missing policy bumps for behavior changed by this plan; - raw schema or prompt bytes in diagnostics; and - documentation that still describes pre-refactor behavior. 3. Confirm the intentional differences and rejected abstractions in the audit remain intact. 4. Confirm each changed fingerprint value is bounded, deterministic, non-secret, and included at the correct component scope. 5. Confirm no test added by this work is a prefix-length, exact-hash, helper-usage, or file-layout change detector. ### Validation Run the full repository checks: ```sh go test -count=1 ./... go vet ./... go build ./cmd/notarius gofmt -l . git diff --check ``` `gofmt -l .` must print no files. If a command fails, diagnose and correct only failures caused by this implementation. Report unrelated pre-existing failures without modifying their owners. ### Completion Gate All audit findings are implemented, every repository-wide check passes except any clearly reported pre-existing failure, current-behavior documentation is accurate, and no superseded helper or policy copy remains. ## Open Questions None. The audit findings, API ownership, semantic policy values, migration order, test boundaries, documentation owner, and intentionally deferred product decisions are specified above.