837 lines
29 KiB
Markdown
837 lines
29 KiB
Markdown
# 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:<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:
|
|
|
|
```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.
|