Add the D&D harmonization roadmap and implementation plan
This commit is contained in:
186
docs/roadmap/dnd.md
Normal file
186
docs/roadmap/dnd.md
Normal file
@@ -0,0 +1,186 @@
|
||||
# D&D Module Harmonization And Prompt Reuse
|
||||
|
||||
This roadmap records proposed improvements to the spell, NPC, and combat-turn
|
||||
pipeline lanes. Current implemented behavior remains documented in the
|
||||
[module internals](../internal/modules.md) and
|
||||
[LLM runtime internals](../internal/llm.md). The work below is not yet an
|
||||
implemented contract.
|
||||
|
||||
The three lanes already share the same overall decomposition: a typed
|
||||
extractor, durable codec, merger, normalizer, shape and provenance validators,
|
||||
domain-specific validation, embedded prompt assets, and central production
|
||||
registration. Spell catalog resolution, NPC identity and registry support, and
|
||||
combat normalization invariants are intentional domain differences. The goal
|
||||
is to remove mechanical drift without hiding those differences behind a broad
|
||||
generic abstraction.
|
||||
|
||||
## Target State
|
||||
|
||||
### Prompt Reuse Is A Rendered-Request Invariant
|
||||
|
||||
Provider prompt caching depends on the request prefix being byte-for-byte
|
||||
identical. Similar prose, duplicated files, or equivalent structured values are
|
||||
not sufficient. Treat message role, content bytes, ordering, cache-control
|
||||
metadata, input rendering, and any provider-visible separators as part of the
|
||||
cache identity.
|
||||
|
||||
The target prompt layout has the longest valid common prefix before any
|
||||
module-specific message:
|
||||
|
||||
- Put shared static system, extraction-evidence, and in-world-identity policy
|
||||
before dynamic transcript and campaign-reference messages so reuse does not
|
||||
depend on identical transcript content.
|
||||
- Keep the shared transcript and campaign-reference messages in the same roles
|
||||
and order in every D&D extractor.
|
||||
- Keep cache-control declarations identical on corresponding messages.
|
||||
- Define one canonical ordering and serialization for shared named inputs.
|
||||
Continue resolving the deprecated `roster` input into the canonical `party`
|
||||
input before prompt rendering.
|
||||
- Keep genuinely universal extraction rules in shared message assets. These
|
||||
rules include using only transcript units as evidence, treating references as
|
||||
disambiguation, returning schema-conforming JSON only, using integer
|
||||
`start_unit_id` and `end_unit_id` values, omitting `source_id` for the mapper
|
||||
to assign, citing all factual claims, and preferring narrow ranges over broad
|
||||
ranges that bridge unrelated conversation.
|
||||
- Use one shared in-world identity message for lanes whose artifacts name
|
||||
actors or participants. It contains only rules that spells, NPCs, and combat
|
||||
turns can follow verbatim; artifact-specific identity, alias, target, and
|
||||
relationship rules remain local.
|
||||
- Keep rules shared by only a subset in subset-specific shared assets. The
|
||||
immediate-declaration-and-resolution boundary and NPC-registry grounding are
|
||||
shared by spells and combat. The NPC extractor does not consume a prior NPC
|
||||
registry.
|
||||
- Place subset-specific and module-specific messages only after the longest
|
||||
useful all-lane prefix. Place schema-specific task instructions last.
|
||||
- Avoid inserting a nominally shared message when its rendered input or
|
||||
wording differs by lane. Factor the common bytes into one message and leave
|
||||
the differences in later messages instead.
|
||||
|
||||
Tests inspect the fully rendered Scriptorium request boundary. Source-level
|
||||
message identity alone is not treated as proof of cache identity.
|
||||
|
||||
### Exact Prompt And Input Identity Is Protected
|
||||
|
||||
Focused tests at the narrowest stable boundary expose the fully prepared
|
||||
provider-neutral request.
|
||||
|
||||
- Render each extractor prompt with the same transcript and campaign
|
||||
references and assert that the intended common message prefix has identical
|
||||
roles, content bytes, ordering, and cache-control metadata.
|
||||
- Assert that spells and combat turns render identical NPC-registry messages
|
||||
for the same bound or unbound registry.
|
||||
- Verify common input material identity, including name, media type, content,
|
||||
digest, origin URI, size, empty-value representation, reference ordering,
|
||||
and `roster` fallback behavior.
|
||||
- Test both identical and intentionally different chunks and reference sets so
|
||||
the test proves the cache boundary rather than merely snapshotting one
|
||||
request.
|
||||
- Add an explicit assertion for the length of the common prefix. A new
|
||||
module-specific message inserted inside that prefix should require deliberate
|
||||
review.
|
||||
- Test that every prompt fingerprint includes exactly the assets actually
|
||||
rendered by that prompt. The current grouped reference hash helper should be
|
||||
replaced or refined so a module does not fingerprint an unused shared asset,
|
||||
while no used asset is omitted.
|
||||
- Prefer semantic assertions over complete prompt snapshots, except for the
|
||||
common rendered prefix whose exact bytes are the behavior under protection.
|
||||
|
||||
### Stable Extraction Preparation Is Centralized
|
||||
|
||||
Move the three identical chunk-input preparation implementations into a D&D
|
||||
shared helper. The helper should clone the supplied material, fall back to the
|
||||
chunk content, verify byte equality, and fill the canonical name, media type,
|
||||
and size without retaining mutable request data.
|
||||
|
||||
Nil context, cancellation, source, chunk, and empty-unit checks remain local so
|
||||
typed result handling and module error context stay explicit. Do not wrap the
|
||||
complete LLM call, response DTO mapping, catalog preparation, registry
|
||||
preparation, or manifest metadata in a generic extractor framework.
|
||||
|
||||
Use the shared helper as the single source of the `transcript` prompt material
|
||||
so identical input requests cannot drift between lanes.
|
||||
|
||||
### Validator Checkpoint And Composition Policy Is Aligned
|
||||
|
||||
- Add explicit versioned policy checkpoint fingerprints to the spell shape,
|
||||
source-reference, and source-relatedness validators. Bump a policy version
|
||||
whenever acceptance, rejection, warning, or diagnostic-selection behavior
|
||||
changes.
|
||||
- Shape validators own malformed artifact shape; later validators defer when
|
||||
shape is invalid.
|
||||
- Make all source-reference validators collect bounded diagnostics through the
|
||||
shared D&D diagnostics package rather than mixing first-error and aggregate
|
||||
behavior.
|
||||
- Use artifact-qualified reason codes consistently unless a reason code is
|
||||
intentionally a stable cross-artifact contract.
|
||||
- Add a compact cross-lane validator contract test covering deterministic
|
||||
execution class, strict empty options, registration, policy fingerprinting,
|
||||
prerequisite behavior, and diagnostic bounds.
|
||||
|
||||
### Source-Evidence Traversal Is Shared
|
||||
|
||||
Extract a D&D helper that validates source ranges and returns cited units or
|
||||
text once, in source-document order, without repeating units covered by
|
||||
overlapping ranges. Use it from spell, NPC, and combat relatedness validators.
|
||||
|
||||
Keep matching policy artifact-specific:
|
||||
|
||||
- spell matching may use canonical catalog names and aliases;
|
||||
- NPC matching may use NPC comparison keys and aliases; and
|
||||
- combat matching may apply separate actor and declaration heuristics.
|
||||
|
||||
Review Unicode normalization, apostrophe handling, word boundaries, short-name
|
||||
false positives, multiword identities, and overlapping ranges with shared
|
||||
table-driven fixtures. Relatedness remains a warning heuristic and should not
|
||||
be presented as proof that every semantic claim is supported.
|
||||
|
||||
## Structural Cleanup
|
||||
|
||||
### Lane Registration
|
||||
|
||||
Reduce repetition in the central D&D registrar with focused registration
|
||||
helpers grouped by modules, validators, prompt assets, and default chains.
|
||||
Retain artifact-specific append and deep-clone behavior in the registrar,
|
||||
central ownership of validator ordering, and explicit typed registration.
|
||||
Avoid reflection and heterogeneous erased lane descriptors.
|
||||
|
||||
### Naming And Package Conventions
|
||||
|
||||
Adopt consistent conventions for package aliases, module keys, artifact kinds,
|
||||
prompt IDs, schema IDs, reason codes, policy IDs, metadata fields, and
|
||||
checkpoint fingerprint names. Compatibility-sensitive identifiers should
|
||||
change only through an explicit migration; internal aliases can be harmonized
|
||||
independently.
|
||||
|
||||
### Schemas And Assets
|
||||
|
||||
- Keep private LLM response schemas distinct from durable artifact codec
|
||||
schemas. They represent different trust and compatibility boundaries.
|
||||
- Keep source-reference schema definitions package-owned in this work; do not
|
||||
add schema composition or generation machinery solely to deduplicate them.
|
||||
- Prefer one embedded shared asset over synchronized copies whenever content
|
||||
must be identical for prompt caching.
|
||||
- Make prompt fingerprints derive from an explicit prompt asset manifest, or
|
||||
from the prepared prompt definition, so message composition and provenance
|
||||
cannot drift independently.
|
||||
|
||||
Prompt factoring must not be accepted solely because cache reuse improves.
|
||||
Retain or restore module-specific wording when evaluation shows a meaningful
|
||||
quality regression. Record cache observations using non-secret request and
|
||||
usage metadata rather than prompt or transcript payloads.
|
||||
|
||||
## Completion Criteria
|
||||
|
||||
This roadmap is complete when:
|
||||
|
||||
- the three lanes expose the same stable structural conventions while keeping
|
||||
documented domain differences local;
|
||||
- shared provider-visible messages and inputs are produced from one source and
|
||||
verified byte-for-byte at the rendered-request boundary;
|
||||
- the common prompt prefix is deliberate, tested, and as long as extraction
|
||||
quality permits;
|
||||
- every validator policy that affects reusable results participates in
|
||||
checkpoint identity;
|
||||
- source-reference traversal and diagnostics no longer drift between lanes;
|
||||
- registration remains explicit and type-safe; and
|
||||
- focused D&D tests plus repository-wide tests, vetting, and the CLI build pass.
|
||||
592
docs/roadmap/implementation.md
Normal file
592
docs/roadmap/implementation.md
Normal file
@@ -0,0 +1,592 @@
|
||||
# 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. Tests must
|
||||
use `scriptorium.PreparedRun.Messages`; comparing source Markdown is
|
||||
insufficient.
|
||||
- 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 ./...`.
|
||||
- Avoid broad snapshots. Exact byte assertions are warranted only for the
|
||||
rendered common message prefixes because byte identity is the feature.
|
||||
- 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: Characterize The Rendered Cache Boundary
|
||||
|
||||
### Goal
|
||||
|
||||
Protect the current three-message common prefix and establish reusable test
|
||||
support before changing prompt composition.
|
||||
|
||||
### Implementation
|
||||
|
||||
1. Add a cross-lane prompt test under
|
||||
`internal/modules/dnd/shared`, using the external test package
|
||||
`shared_test` so it can import all three extractor packages without an
|
||||
import cycle.
|
||||
2. In one `llm.AssetRegistry`, register spell, NPC, and combat prompt assets.
|
||||
Build one offline Scriptorium engine with one test profile and use
|
||||
`Engine.Prepare` for every prompt. Supply identical transcript, players,
|
||||
party, and glossary inputs. Supply the same unbound NPC JSON to spell and
|
||||
combat, plus the spell catalog required only by spells.
|
||||
3. Add a small test helper that compares an expected prefix of
|
||||
`[]scriptorium.RenderedMessage` field by field: role, exact content string,
|
||||
and a canonical JSON representation of cache control. Do not compare prompt
|
||||
IDs, rendered prompt hashes, output contracts, or schemas across modules;
|
||||
those are intentionally different.
|
||||
4. Assert that the first three messages are identical across all lanes and that
|
||||
spell/combat NPC grounding is identical at their current corresponding
|
||||
position. Assert the existing message counts separately so Stage 3 must
|
||||
update them deliberately.
|
||||
5. 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.
|
||||
6. Do not add a second full HTTP/provider test. The Scriptorium engine's
|
||||
prepared messages are the stable boundary owned by this repository;
|
||||
Scriptorium owns provider request serialization. Existing LLM adapter tests
|
||||
continue to prove that prepared message order and content reach the engine
|
||||
request.
|
||||
|
||||
### 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
|
||||
|
||||
- The new cross-lane test fails if any role, content byte, cache-control value,
|
||||
or order within the intended common prefix differs.
|
||||
- Tests distinguish intended prompt identity from intentionally different
|
||||
prompt IDs, schemas, and module-specific suffixes.
|
||||
- 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 exact target prompt layout defined above and maximize the
|
||||
byte-identical all-lane and spell/combat prefixes.
|
||||
|
||||
### 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 for new message counts and positions. Replace
|
||||
content-substring assertions for moved common policy with assertions in the
|
||||
cross-lane rendered-prefix test.
|
||||
8. Update the cross-lane test to assert:
|
||||
- indices 0 through 4 are identical for all three lanes when common inputs
|
||||
are identical;
|
||||
- indices 0 through 2 remain identical when transcript and references differ;
|
||||
- spell and combat indices 5 and 6 are identical for the same NPC registry;
|
||||
- the shared prefixes end at the exact indices in the table; and
|
||||
- module-specific suffixes are not accidentally identical or reordered.
|
||||
9. 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
|
||||
|
||||
- Prepared messages match the target table exactly.
|
||||
- Shared rules exist in one embedded file and have no local paraphrased copy.
|
||||
- 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.
|
||||
- The rendered prompt identity 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;
|
||||
- one exact rendered-prefix suite;
|
||||
- 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.
|
||||
Reference in New Issue
Block a user