Files
notarius/docs/roadmap/implementation.md

29 KiB

D&D Extraction Module Audit Implementation Plan

Status: Ready for implementation

This document turns the completed findings in D&D Extraction Module Refactoring Audit 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:

    // 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:<lane-id>:dnd/spells:mapping_policy; and
    • extract:<lane-id>: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:

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:

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:

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:

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:

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:

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:

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:

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:

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:

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:

    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:

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:

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.