# D&D Module Refactoring Implementation Plan

## Objective

Address the five remaining D&D module refactoring findings without changing
artifact schemas, prompt content, module keys, validator policy, warning or
rejection semantics, durable media types, or pipeline configuration.

The completed implementation must:

- make every D&D append-order merger return independently owned nested data and
  preserve absent versus present-empty list representation consistently;
- use one source-document index per operation wherever D&D code repeatedly
  validates or resolves source references;
- centralize the strict candidate JSON encoding and decoding mechanics shared
  by all five D&D artifact codecs while retaining artifact ownership in each
  codec package;
- remove unused source-reference canonicalization results from the NPC and
  combat-turn normalizers; and
- remove the unused internal NPC identity and registry aliases.

This plan implements internal refactoring only. It does not add extractors,
fields, classifications, prompts, validators, compatibility shims, or user
configuration.

## Governing Constraints

Follow:

- [Architecture](../policy/architecture.md), especially explicit stage
  ownership, domain-first dependencies, and the rule against abstractions that
  serve only hypothetical reuse;
- [Testing Policy](../policy/testing.md), especially behavior-level tests,
  focused regression coverage for data integrity, and avoidance of tests that
  merely detect private implementation changes;
- [Documentation Policy](../policy/documentation.md), keeping current
  implementation detail in [Module Internals](../internal/modules.md); and
- the implemented [D&D extractor contract](../internal/modules.md#dd-extractor-contract).

Preserve these decisions throughout the work:

- Keep the five artifact-specific merger functions. Do not replace them with a
  callback-driven generic merger.
- Use `source.DocumentIndex` as the canonical index for repeated source
  reference validation and position lookup. Do not introduce another
  independent unit-ID map or validation implementation.
- Keep artifact schemas, approved-value validation, kinds, media types, and
  contextual errors in their artifact codec packages.
- Keep artifact-specific response mapping, chronology sorting, normalization,
  and validators local. The shared work in this plan is limited to demonstrated
  mechanics with identical semantics.
- Preserve existing error classifications, diagnostic scopes, ordering,
  truncation, and approval behavior. Exact non-contractual wording may remain
  unchanged where practical, but tests should assert stable semantics rather
  than duplicate entire error strings.
- Do not add benchmarks or performance-sensitive timing assertions. Verify
  index reuse through design and ordinary behavioral tests, not through
  brittle implementation or latency detectors.

Each stage below is one implementation prompt. Complete and verify a stage
before beginning the next.

## Stage 1: Harmonize D&D Merger Ownership And Presence

Update the D&D append-order mergers in
`internal/modules/dnd/register/merge.go`.

Implementation:

1. Add artifact-specific clone functions for spell casts and NPCs, including a
   fresh copy of each non-nil `SourceRefs` slice. Match the existing
   combat-turn and NPC-interaction ownership behavior.
2. Change `appendSpellLists` and `appendNPCLists` to append cloned records
   rather than shallow-copying records from their inputs.
3. Change `appendSpellLists` to use the same list-presence rule as the other
   four mergers:
   - no inputs, or only inputs with a nil `SpellCasts` field, produce a nil
     `SpellCasts` field;
   - any non-nil input field makes the result field non-nil, including when all
     present fields are empty; and
   - records retain deterministic input and record order.
4. Do not alter scene-description merge behavior: its record contains a scalar
   source reference and requires no nested clone.

Tests:

- Extend registrar merger tests at the package-level merge boundary.
- For spells and NPCs, mutate a merged record's first source reference and
  prove that the corresponding input remains unchanged.
- Cover no inputs, nil-only inputs, present-empty input, and ordered populated
  inputs for spells. Retain the existing equivalent NPC, combat-turn,
  interaction, and scene coverage without duplicating it elsewhere.

Verification:

```sh
go test ./internal/modules/dnd/register
```

## Stage 2: Add Indexed Citation Resolution And Migrate Relatedness Validators

Replace repeated validation and endpoint scans in D&D citation text assembly
with one operation-scoped indexed resolver.

Implementation:

1. In `internal/modules/dnd/shared`, introduce a small citation resolver that:
   - is constructed from one non-nil `*source.SourceDocument`;
   - retains that immutable operation input and one
     `source.NewDocumentIndex(doc)`;
   - validates each requested range through `DocumentIndex.ValidateRef`;
   - resolves endpoints through `DocumentIndex.Position`;
   - includes overlapping source units only once;
   - returns cited unit text in document order, joined exactly as today; and
   - preserves the current contextual failure for an invalid source range.
2. Replace the top-level repeated-scan `CitedText` API with the resolver API.
   Do not retain two production implementations of citation traversal.
3. Construct one resolver per relatedness-validation invocation and reuse it
   for every record in that request.
4. Migrate the spell, NPC, combat-turn, NPC-interaction, and scene-description
   relatedness validators. Preserve all current deferral, warning, token
   matching, warning-cap, and diagnostic behavior.

Tests:

- Adapt shared citation tests to the resolver's package-level behavior.
- Preserve coverage for nil documents, invalid ranges, overlapping ranges,
  disjoint ranges, document ordering, and newline joining where those risks are
  currently covered.
- Run existing relatedness tests unchanged except for construction/API updates
  required by the refactor. Do not add tests that inspect the resolver's map,
  count index construction, or assert performance.

Verification:

```sh
go test ./internal/modules/dnd/shared \
  ./internal/modules/dnd/validate/spells/source_relatedness \
  ./internal/modules/dnd/validate/npcs/source_relatedness \
  ./internal/modules/dnd/validate/combatturns/source_relatedness \
  ./internal/modules/dnd/validate/npcinteractions/source_relatedness \
  ./internal/modules/dnd/validate/scenedescriptions/source_relatedness
```

## Stage 3: Reuse Document Indexes Across D&D Validators

Migrate repeated source-reference validation and position lookup in rejecting
and invariant validators to one `source.DocumentIndex` per validation
invocation.

Implementation:

1. Construct the index only after any existing shape-deferral or request
   precondition logic that should run first.
2. Use `DocumentIndex.ValidateRef` inside source-reference loops in the spell,
   NPC, combat-turn, NPC-interaction, and scene-description source-reference
   validators.
3. Pass an operation-scoped index into invariant helper functions that
   repeatedly validate references or compare source positions. Apply this to
   combat turns, NPC interactions, and scene descriptions where relevant.
4. Replace scene invariant `source.UnitIndex` calls inside sort comparisons
   with `DocumentIndex.Position`.
5. Preserve extract-stage chunk-containment checks, validator deferral,
   aggregate diagnostic order, reason codes, and truncation. The index changes
   lookup mechanics only.
6. Do not extract the artifact-specific validator loops into a generic callback
   helper.

Tests:

- Existing validator suites should continue to express the behavioral
  contract. Add or change tests only if a source-document boundary is not
  already protected.
- Do not add source-code inspection tests or assertions that a particular
  helper was called.

Verification:

```sh
go test ./internal/modules/dnd/validate/...
```

## Stage 4: Reuse Document Indexes In Domain And Normalization Logic

Finish the operation-scoped index migration outside validators and simplify
the NPC and combat-turn canonicalization call sites.

Implementation:

1. In spell and combat-turn normalization, construct one
   `source.DocumentIndex` for the normalization operation and pass it through
   duplicate-identity helpers instead of calling `source.ValidateRef` for each
   record and reference.
2. In scene-description normalization, replace its local unit-position map and
   repeated `source.ValidateRef` calls with one `source.DocumentIndex`. Use the
   same index for validation and chronological ordering.
3. Update `internal/modules/dnd/npcinteractions` canonical helpers so callers
   performing repeated validity checks can supply and reuse an operation-scoped
   index. Propagate the index through interaction normalization and invariant
   validation as needed. Do not keep a second loop that delegates to
   `source.ValidateRef`.
4. Review `internal/modules/dnd/shared/unit_refs.go`. Where one extraction
   mapping operation resolves multiple unit IDs, construct one document index
   at the operation boundary and resolve all unit IDs through it. Preserve the
   existing field-scoped errors and raw candidate mapping behavior.
5. Remove the NPC and combat-turn `canonicalizeSourceRefs` wrappers that return
   change and duplicate-count values their callers discard. Call
   `SourceRefOrder.Canonicalize` directly and perform the single behavioral
   comparison each normalizer actually needs.
6. Keep the spell normalizer's richer `canonicalizeSourceRefs` helper because
   it intentionally distinguishes ordering changes from duplicate removal for
   diagnostics.
7. Do not alter normalization identities, collapse eligibility, ordering,
   warning scopes, or checkpoint fingerprints.

Tests:

- Run the existing extractor, domain-helper, and normalizer tests as behavioral
  regression coverage.
- Add coverage only for an observable boundary found to be missing; do not
  test tuple arity, helper presence, map construction, or private call paths.

Verification:

```sh
go test ./internal/modules/dnd/extract/... \
  ./internal/modules/dnd/npcinteractions \
  ./internal/modules/dnd/normalize/...
```

## Stage 5: Centralize Strict D&D Candidate JSON Mechanics

Extract the identical candidate JSON encoding and strict decoding behavior used
by the five D&D artifact codecs.

Implementation:

1. Add a narrowly named shared package beneath
   `internal/modules/dnd/codec/` for generic typed candidate JSON mechanics.
   It may depend only on the Go standard library and must not import an
   artifact-specific codec or D&D artifact type.
2. Provide generic helpers that:
   - encode a typed candidate with `json.Marshal`;
   - decode exactly one typed JSON value;
   - reject unknown fields with `DisallowUnknownFields`;
   - reject trailing JSON values; and
   - wrap failures with a caller-supplied artifact label so existing codec
     errors retain useful context.
3. Migrate the spell, NPC, combat-turn, NPC-interaction, and
   scene-description codecs to delegate only `EncodeCandidate` and
   `DecodeCandidate` mechanics to the helper.
4. Keep `Codec`, `New`, schema loading, kind, version, media type,
   `Encode`/`Decode` approved-value validation, and artifact-specific
   validation in each owning package.
5. Preserve the exact accepted and rejected JSON language. In particular, do
   not make candidate decoding permissive, add normalization, or merge durable
   schemas.

Tests:

- Give the shared package focused table-driven tests for successful typed
  round-trip, unknown-field rejection, malformed JSON, and trailing-value
  rejection.
- Retain each artifact codec's package-level boundary tests and maintained
  durable fixtures. Remove only tests that become exact redundant copies of
  helper mechanism tests and provide no artifact-boundary confidence.
- Do not consolidate artifact fixtures or schemas.

Verification:

```sh
go test ./internal/modules/dnd/codec/...
```

## Stage 6: Remove Unused NPC Aliases And Close Documentation

Remove the unnecessary internal API surface and update the canonical
implemented-behavior documentation after all preceding refactors are present.

Implementation:

1. Delete `npcs/registry.New`; `Resolve` remains the single registry
   construction entry point.
2. Delete `npcs/identity.Validate`; `ValidateList` remains the explicit
   whole-list validation entry point.
3. Confirm there are no production or test callers before deletion. Do not add
   compatibility aliases or deprecation shims: these are internal,
   pre-release APIs.
4. Update [Module Internals](../internal/modules.md) concisely to reflect:
   - independently owned nested merger output;
   - operation-scoped indexed source-reference and citation traversal; and
   - the shared strict candidate JSON mechanism with artifact-owned durable
     codec policy.
5. Do not add these implementation details to user configuration, integration
   contracts, or ADRs. No user-visible or durable contract changes are
   intended.
6. Run formatting over changed Go files and inspect the final diff for
   accidental schema, prompt, configuration, or fingerprint changes.

Tests and verification:

```sh
go test ./internal/modules/dnd/...
go test ./...
go vet ./...
go build ./cmd/notarius
```

The final implementation is complete when:

- all commands above pass;
- the five artifact mergers preserve order, presence, and independent nested
  ownership;
- repeated D&D source-reference operations use one document index at their
  operation boundary;
- all five codecs share strict candidate JSON mechanics without surrendering
  artifact-specific policy;
- the NPC and combat normalizers compute only the canonicalization results they
  use;
- the two unused aliases are absent; and
- `git diff` shows no prompt, schema, durable artifact, module-key,
  configuration, or checkpoint-policy changes.

## Open Questions

None. The implementation choices required for this refactoring are resolved
above.