Files
notarius/docs/roadmap/implementation.md

574 lines
26 KiB
Markdown

# D&D Module Harmonization Implementation Plan
This document is the executable implementation plan for the target state in
[D&D Module Harmonization And Prompt Reuse](dnd.md). It is written for a coding
agent and must be followed in stage order. Each stage must leave the repository
building and its focused tests passing before the next stage begins.
Current behavior is documented in the
[module internals](../internal/modules.md) and
[LLM runtime internals](../internal/llm.md). The policies in
[Architecture](../policy/architecture.md),
[Testing](../policy/testing.md), and
[Documentation](../policy/documentation.md) govern all stages.
## Fixed Decisions And Constraints
The following decisions are complete and are not implementation-time choices:
- Keep spell, NPC, and combat-turn response DTOs, schemas, canonicalization,
codecs, normalizers, and domain validators in their current domain packages.
- Put D&D-only reuse in `internal/modules/dnd/shared`; do not move D&D concepts
into `internal/framework`.
- Keep private LLM response schemas separate from durable codec schemas. Do not
introduce shared JSON Schema fragments or a schema-generation step in this
work.
- Preserve all public module keys, artifact kinds, prompt IDs, schema IDs,
schema versions, media types, reference slot names, durable JSON fields, and
existing reason-code strings. In particular, retain the spell validator's
existing `invalid_source_refs` reason code as a compatibility exception.
- Preserve central ownership and ordering of default validator chains in the
D&D registrar.
- Do not create a generic extractor framework, use reflection for lane
registration, or erase typed artifact relationships outside existing
framework boundaries.
- Exact cache identity means equal ordered message roles, content bytes, and
cache-control values after Scriptorium has rendered the prompt. Enforce this
primarily through one canonical shared asset per shared message and document
the expected order and cache-control policy.
- Scriptorium and provider-specific types remain confined to the LLM runtime,
prompt-asset wiring, and their tests. Production extractors continue to use
only Notarius contracts.
- Default tests remain offline, deterministic, and credential-free. Live model
evaluation is an explicit manual acceptance activity, not part of
`go test ./...`.
- Do not add message-count, common-prefix-length, or complete rendered-prompt
snapshots. They are change detectors rather than durable behavioral tests.
- Update current-behavior documentation only in the stage that changes that
behavior. Do not describe a partially implemented later stage as complete.
## Target Prompt Layout
Stage 3 must produce the following ordered messages. All listed shared entries
must refer to one shared embedded file rather than package-local copies.
| Index | Spell | NPC | Combat turn | Role | Cache control |
| --- | --- | --- | --- | --- | --- |
| 0 | common system | common system | common system | system | none |
| 1 | common extraction evidence | common extraction evidence | common extraction evidence | user | none |
| 2 | common in-world identity | common in-world identity | common in-world identity | user | `ephemeral` |
| 3 | common transcript | common transcript | common transcript | user | `ephemeral` |
| 4 | common campaign references | common campaign references | common campaign references | user | `ephemeral` |
| 5 | common immediate resolution | NPC task | common immediate resolution | user | none |
| 6 | common NPC registry | NPC instructions | common NPC registry | user | `ephemeral` for spell/combat |
| 7 | spell catalog | — | combat task | user | none |
| 8 | spell task | — | combat instructions | user | none |
| 9 | spell instructions | — | — | user | none |
The common extraction-evidence asset must state, in artifact-neutral language,
that:
- transcript units are the only event evidence;
- campaign and registry references may disambiguate but are not evidence;
- every reported factual claim is supported by cited transcript units;
- references use integer `start_unit_id` and `end_unit_id` values;
- `source_id` is omitted because Notarius assigns the current source identity;
- non-contiguous evidence uses multiple narrow ranges rather than a broad
bridge over unrelated conversation; and
- output contains only the configured JSON object and schema-defined fields.
The common in-world-identity asset must require the most specific supported
in-world character or creature identity instead of a human player, transcript
speaker, or the GM as an out-of-world person. It may permit campaign references
to disambiguate an identity, but must not establish participation from a
reference alone. NPC-only exclusion rules and relationship rules remain in the
NPC task or instructions.
The common immediate-resolution asset, used only by spells and combat turns,
must limit an artifact to a declaration/action and its immediate observed
resolution. It must exclude consequences on later turns or elsewhere in the
scene. Spell-only persistent-effect language and combat-only classification
rules remain local.
Remove equivalent prose from package-local task and instruction files after it
has moved to a shared asset. Do not retain paraphrased copies. Read each final
prompt as a whole to remove contradictions and preserve all artifact-specific
requirements.
The cache-control choices above create three all-lane breakpoints and one
spell/combat breakpoint. Use no more than these four markers so the request
remains portable across the configured backends; do not add module-specific
markers in this work.
## Stage 1: Protect Shared Input Behavior And Baseline Prompt Wiring
### Goal
Protect deterministic shared input rendering and confirm the existing prompt
assets prepare successfully before changing prompt composition.
### Implementation
1. Retain one offline prompt preparation test in each extractor package. Each
test should prove registration succeeds, the prompt selects its package-owned
response schema, required dynamic inputs render, and no provider call or
credentials are required. Do not assert total message count, shared-prefix
length, or the complete rendered prompt.
2. Extend `internal/modules/dnd/shared/prompt_inputs_test.go` with one
table-driven identity test covering:
- identical source material and references produce deeply equal common input
materials;
- reference item insertion order does not change rendered reference bytes;
- `roster` fallback produces the canonical `party` material;
- an explicit non-empty `party` wins over `roster`; and
- missing optional slots render the existing single-space placeholder.
3. Do not add a full HTTP/provider test for cross-module prompt equality.
Scriptorium owns provider request serialization, and shared asset ownership
removes the duplicated content that would otherwise need equality testing.
### Verification
Run:
```sh
go test ./internal/modules/dnd/shared ./internal/modules/dnd/extract/spells ./internal/modules/dnd/extract/npcs ./internal/modules/dnd/extract/combatturns
```
### Completion Criteria
- Shared input tests protect deterministic serialization and canonical slot
handling.
- Each extractor's existing behavior-level asset test prepares successfully
without pinning message count or prefix boundaries.
- No production behavior changes in this stage.
## Stage 2: Make Prompt Asset Manifests Exact
### Goal
Use one explicit asset manifest for prompt mounting and prompt fingerprinting,
and stop fingerprinting shared assets a prompt does not render.
### Implementation
1. Replace the broad `sharedPromptFiles`, `CommonHashParts`, and
`ReferenceHashParts` grouping in `internal/modules/dnd/shared/assets.go`
with an explicit manifest abstraction:
```go
type PromptAssetManifest struct {
ModuleDir string
ModuleFiles []promptfs.ModulePromptFile
SharedFiles []string
}
```
Exact formatting may follow `gofmt`, but retain these fields and meanings.
2. Give the manifest two operations:
- `PromptFS(moduleFS fs.FS) (fs.FS, error)`, which resolves only the named
shared files and delegates composition to `promptfs.ModulePromptFS`; and
- `Hash(moduleFS fs.FS) (string, error)`, which hashes the same module and
shared files in manifest order through `llm.HashAssets`.
3. Keep the shared-name-to-embedded-path mapping private to the shared package.
Reject unknown, duplicate, empty, or path-containing shared names. Return
fresh slices so callers cannot mutate package state.
4. In each `scriptorium_assets.go`, declare one package-local manifest that
includes its YAML definition and every Markdown file referenced by that
definition. Keep ordering stable within the module and shared lists. Use
that manifest for both `RegisterPromptAssets` and
`scriptoriumPromptMetadata`.
5. Migrate the scene chunker to the same API because it uses the shared prompt
helper. Its behavior and prompt ordering remain unchanged in this stage.
6. Mount only shared assets actually referenced by each prompt. Before Stage 3:
- NPC and scene prompts must not include or hash `common-dnd-npcs.md`;
- spell and combat prompts must include and hash it; and
- all four prompts must include and hash the shared system, transcript, and
campaign-reference files they render.
7. Retain the response-schema fingerprint as its existing independent
fingerprint. Do not include response schema bytes in the prompt manifest.
### Tests
- Replace broad shared asset tests with table-driven manifest tests for valid
composition, exact mounted files, unknown names, duplicate names, invalid
names, missing module files, missing shared files, and defensive copying.
- Independently construct the expected `llm.AssetHashPart` list in manifest
tests and assert that `Hash` equals `llm.HashAssets` over exactly that list.
This proves unused shared assets are excluded and listed assets participate
without adding mutation hooks for the embedded filesystem.
- Keep one package-level registration test per prompt; remove redundant
per-file mounting assertions when the shared manifest tests already own that
behavior.
### Verification
Run:
```sh
go test ./internal/framework/promptfs ./internal/modules/dnd/shared ./internal/modules/dnd/chunk/scenes ./internal/modules/dnd/extract/spells ./internal/modules/dnd/extract/npcs ./internal/modules/dnd/extract/combatturns
```
### Completion Criteria
- Mounting and hashing are driven by the same ordered manifest.
- No prompt fingerprints an unused shared asset or omits a rendered asset.
- Prepared prompt messages are unchanged from Stage 1.
## Stage 3: Factor And Reorder Shared Prompt Messages
### Goal
Implement the target prompt layout defined above using canonical shared assets
and maximize the reusable all-lane and spell/combat content.
### Implementation
1. Add these embedded assets under
`internal/modules/dnd/shared/assets/prompts`:
- `common-dnd-extraction-evidence.md`;
- `common-dnd-identity.md`; and
- `common-dnd-immediate-resolution.md`.
2. Write their content according to the fixed semantic boundaries in
`Target Prompt Layout`. Use no template inputs in the identity or immediate
assets. The evidence asset also needs no new input; it refers generically to
the transcript and references already presented later.
3. Update the three extractor YAML files to use the exact order, roles, and
cache-control values in the target table. Do not change prompt IDs, versions,
default profiles, input declarations, output schema paths, or repair counts.
4. Update each package's prompt manifest from Stage 2 so it lists exactly the
new shared dependencies in rendered order.
5. Delete duplicated policy prose from local `task.md` and `instructions.md`
files while retaining every module-specific rule. Preserve one and only one
`Return exactly one JSON object` rule through the common evidence message.
6. Keep the spell catalog input and NPC registry input byte generation
unchanged. Keep the NPC extractor free of an `npcs` input.
7. Update package asset tests only for behavioral wiring: prompt preparation,
response schema selection, and required dynamic input rendering. Do not add
assertions for total message count, exact shared-prefix length, or the end
index of a shared section. The shared manifest tests own canonical asset
selection.
8. Update `docs/internal/modules.md` and `docs/internal/llm.md` in the same
change to describe the implemented common-prefix composition, exact prompt
manifest fingerprinting, and cache-control placement. Describe current
behavior, not this staged plan.
### Verification
Run:
```sh
go test ./internal/modules/dnd/shared ./internal/modules/dnd/extract/spells ./internal/modules/dnd/extract/npcs ./internal/modules/dnd/extract/combatturns ./internal/modules/dnd/register
go test ./internal/framework/llm
```
### Completion Criteria
- Shared rules exist in one embedded file and have no local paraphrased copy.
- Every applicable prompt references those shared assets in the documented
order and with the documented cache-control policy.
- Prompt fingerprints change for the intentional prompt contract change and
include every newly rendered asset.
- No artifact schema, durable representation, module selection, or reference
contract changes.
## Stage 4: Centralize Chunk Prompt Material
### Goal
Remove the three identical `chunkSourceInput` implementations and make common
transcript input preparation a single D&D-owned behavior.
### Implementation
1. Add `internal/modules/dnd/shared/extraction_inputs.go` with:
```go
func ChunkPromptMaterial(req contracts.TypedExtractionRequest) (contracts.LLMInputMaterial, error)
```
2. Preserve the existing behavior exactly: clone `req.SourceInput`; fall back
to chunk content and media type when content is empty; require content bytes
to equal `req.Chunk.Content`; default the material name to `source`; default
media type from the chunk; and populate `SizeBytes` when zero.
3. The helper may assume the caller has already checked `req.Chunk != nil`.
Return a D&D-shared error without an extractor name. Each extractor wraps it
with its existing `extractorErrorf`, retaining module context.
4. Replace all three local helpers and remove now-unused `bytes` imports.
5. Do not centralize the remaining request checks. Their typed result handling
and module-specific errors make the small duplication clearer than a
callback- or generic-heavy abstraction.
### Tests
- Add one table-driven shared helper test covering fallback, clone isolation,
mismatch, default fields, and preservation of explicit metadata.
- Remove duplicate extractor tests only when the shared test fully owns the
behavior. Retain one extractor-level test per lane proving helper errors are
wrapped with that module's context.
- Keep existing tests proving all three extractors pass equal common prompt
inputs to the LLM contract.
### Verification
Run:
```sh
go test ./internal/modules/dnd/shared ./internal/modules/dnd/extract/spells ./internal/modules/dnd/extract/npcs ./internal/modules/dnd/extract/combatturns
```
### Completion Criteria
- Only one production implementation prepares chunk prompt material.
- Extractor behavior and public errors retain useful module context.
- Prompt preparation and shared input behavior tests still pass.
## Stage 5: Share Cited Source Traversal
### Goal
Give all relatedness validators one deterministic implementation for resolving,
ordering, and deduplicating cited source units while keeping matching semantics
artifact-specific.
### Implementation
1. Add `internal/modules/dnd/shared/citations.go` with:
```go
func CitedText(doc *source.SourceDocument, refs []source.SourceRef) (string, error)
```
2. Validate every range with `source.ValidateRef`. Resolve ranges against
document order, include each covered source unit once even when ranges
overlap, and join included unit text with a single newline. Return an error
for a nil document or any invalid range. Return `"", nil` for an empty ref
slice with a non-nil document. Do not mutate the document or refs.
3. Add table-driven tests for nil documents, empty refs, invalid source IDs,
unknown/reversed unit IDs, disjoint ranges supplied out of order, adjacent
ranges, and overlapping/duplicate ranges. Output must always follow document
order.
4. Replace spell, NPC, and combat relatedness validators' local cited-text
traversal with `shared.CitedText`.
5. When shape is invalid or `CitedText` returns an error, relatedness validators
approve without relatedness warnings so shape/source-reference validators
remain the sole owners of those defects.
6. Keep matching local:
- spells compare the canonical spell name case-insensitively against combined
cited text;
- NPCs use `identity.ComparisonKey` for names and aliases; and
- combat uses its existing comparison-key actor logic and declaration token
heuristic.
7. Strengthen matching tests with Unicode/apostrophe variants, multiword names,
overlapping ranges, and a short-name substring false-positive case. For
names and actors, require token/word-boundary-aware matching rather than raw
substring matching. For multiword values, match the consecutive normalized
token sequence. Keep the combat declaration rule of at least one normalized
token of four or more runes.
8. Put shared normalized tokenization and consecutive-token matching in
`internal/modules/dnd/shared` only if at least two validators use it after
the change. Otherwise leave the matching helper local; do not create a
single configurable matching engine.
### Verification
Run:
```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
```
### Completion Criteria
- Cited range validation, ordering, overlap handling, and text assembly have
one production owner.
- Relatedness remains warning-only and ignores invalid prerequisite data.
- Artifact-specific matching policies remain understandable in their validator
packages.
## Stage 6: Align Validator Policy And Diagnostics
### Goal
Make validator checkpoint identity, prerequisite handling, and diagnostic
bounding consistent without changing validator-chain order or durable reason
codes.
### Implementation
1. Add these local policy constants and `CheckpointFingerprintProvider`
implementations:
- spell shape: `dnd.spells.validator.shape.v1`;
- spell source refs: `dnd.spells.validator.source_refs.v1`; and
- spell source relatedness: `dnd.spells.validator.source_relatedness.v1`.
2. Do not add a separate policy fingerprint to the spell catalog validator; its
effective catalog digest remains its existing semantic checkpoint identity.
If its non-catalog validation policy changes during this work, add a second
`policy` fingerprint rather than replacing `effective_catalog`.
3. Change spell source-reference validation to approve when spell shape is
invalid, matching NPC and combat prerequisite behavior.
4. Change spell source-reference validation to collect all reference issues,
truncate individual errors with `shared/diagnostics.Truncate`, and return a
bounded aggregate with `shared/diagnostics.Aggregate`. Preserve
`invalid_source_refs`.
5. Confirm NPC and combat source-reference validators follow the same
prerequisite and bounded-aggregate policy; refactor only enough to share
obvious local structure. Do not introduce a generic typed validator builder.
6. Keep shape validators responsible for malformed artifact fields and source
reference validators responsible for document/range validity.
7. Add or consolidate package-level tests for policy fingerprints, invalid-shape
deferral, aggregation of multiple invalid refs, bounded diagnostics, strict
options, registration, and deterministic execution class. Prefer a small
table of behavioral expectations in each typed package over a reflection-
based cross-package harness.
8. Update `docs/internal/modules.md` to describe the aligned prerequisite and
checkpoint policy after it is implemented.
### Verification
Run:
```sh
go test ./internal/modules/dnd/validate/...
go test ./internal/modules/dnd/register
```
### Completion Criteria
- Every deterministic validator policy affecting checkpoint reuse has an
explicit semantic fingerprint.
- Later validators do not duplicate shape rejection.
- All source-reference diagnostic output is bounded.
- Existing reason codes and validator-chain order are unchanged.
## Stage 7: Clarify D&D Registration And Naming
### Goal
Make production composition easier to audit without introducing heterogeneous
generic descriptors or changing registration behavior.
### Implementation
1. Keep package `internal/modules/dnd/register`, but split its current concerns
into focused files:
- `register.go`: public `Register`, registry validation, and ordered execution
of named registration functions;
- `modules.go`: codecs, chunker, extractors, mergers, normalizers, no-op
normalizers, and prompt asset registrations;
- `validators.go`: production and generic test validator registrations;
- `chains.go`: the six default extract/normalize chain mappings; and
- `merge.go`: typed append functions and deep-clone helpers.
2. Use small private functions such as `registerModules`,
`registerValidators`, `registerPromptAssets`, and
`registerDefaultChains`. Keep the existing ordered `registration{name,
register}` error-context pattern within each group.
3. Do not create one slice containing generic lane descriptors; Go cannot retain
the heterogeneous typed codec and module relationships there without
erasure or callbacks that obscure more than they clarify.
4. Keep append and clone behavior in the `register` package for this change.
Moving it would require a new lane-ownership package with no independent
domain responsibility.
5. Normalize import aliases in the registrar to the pattern
`spellextract`, `npcextract`, `combatextract`, `spellnormalize`,
`npcnormalize`, and `combatnormalize`, with corresponding validator aliases.
This is internal naming only.
6. Preserve registration order, error prefixes, default chain contents and
order, reference-slot/spec behavior, and prompt asset collection.
7. Update `register_test.go` only as required by file movement. Tests should
continue asserting observable registry contents and chain policy, not the
new private helper call graph.
### Verification
Run:
```sh
go test ./internal/modules/dnd/register
go test ./internal/modules/dnd/...
```
### Completion Criteria
- Production composition is grouped by responsibility and remains explicit.
- The refactor produces no registry, chain, capability, or error behavior
change.
- Tests do not couple to private registration helpers.
## Stage 8: Final Integration, Documentation, And Evaluation
### Goal
Verify the complete target state, update canonical current-behavior documents,
and gather quality/cache evidence without making live services part of the
default test suite.
### Implementation And Review
1. Re-read `docs/roadmap/dnd.md` and verify every completion criterion against
production code and tests. Do not mark an item complete based only on this
implementation plan.
2. Review naming across the three lanes. Harmonize internal aliases and private
policy constant names, but do not rename compatibility-sensitive identifiers
listed in `Fixed Decisions And Constraints`.
3. Review prompt and durable schemas for accidental duplication. Make no schema
refactor unless composition already exists and the change is behavior-free;
the fixed decision for this plan is to leave them package-owned and separate.
4. Consolidate redundant tests created by intermediate stages. Retain:
- one shared manifest test suite;
- behavior-level prompt preparation tests without prefix-length snapshots;
- focused package tests for artifact-specific prompts and validators; and
- existing production registration contract coverage.
5. Update `docs/internal/modules.md` and `docs/internal/llm.md` so they are the
canonical description of the final implemented behavior. Remove superseded
implementation details rather than appending a second description.
6. Update `docs/roadmap/dnd.md` to record implementation status. Remove completed
future-work details that are fully owned by current internal documentation,
leaving only genuinely deferred outcomes. Do not turn the feature roadmap
into a second current-behavior reference.
7. If credentials and the maintained human-reviewed transcript set are
available, run an explicitly opt-in comparison using identical profiles and
inputs before and after the prompt change. Record only aggregate extraction
review results, prompt token counts, cached-token/cache-write metrics, and
non-secret prompt hashes. Never commit transcript content, rendered prompts,
credentials, endpoints, or private reference material.
8. Live evaluation is not a merge gate when credentials or reviewed fixtures
are unavailable. In that case, record the missing external prerequisite in
the remaining roadmap item; do not add a fake cache-hit claim and do not
weaken offline identity tests.
### Repository Verification
Run all required checks:
```sh
go test ./...
go vet ./...
go build ./cmd/notarius
git diff --check
```
Also inspect the final diff for:
- unintended public identifier or durable schema changes;
- prompt rules duplicated between shared and local assets;
- prompt assets rendered but absent from fingerprints, or fingerprinted but
not rendered;
- provider-specific types outside allowed boundaries;
- tests containing real transcript/reference material or credentials; and
- unrelated changes in a pre-existing dirty worktree.
### Completion Criteria
- Every completion criterion in the feature roadmap is either implemented and
documented in its canonical current-behavior owner or explicitly retained as
deferred roadmap work.
- All focused and repository-wide checks pass.
- The final test suite protects behavioral contracts without retaining
redundant implementation snapshots.
## Open Questions
None. The plan fixes all choices required for implementation. Live provider
evaluation may depend on credentials and reviewed fixtures, but that is an
external acceptance prerequisite rather than an unresolved design decision.