Add feature roadmap and implementation plan for D&D spell extraction and validation upgrades
This commit is contained in:
@@ -1,488 +1,457 @@
|
||||
# Test Suite Policy Review Implementation
|
||||
# D&D Spell Extraction Quality Implementation Plan
|
||||
|
||||
## Status
|
||||
|
||||
Completed on 2026-07-18 by the commit series ending with `4f96abf`. This
|
||||
document is retained as the execution record for the completed test-suite
|
||||
policy review; its stages are no longer active implementation instructions.
|
||||
Ready for implementation. Follow the stages in order. This plan covers the
|
||||
accepted baseline in [D&D Spell Extraction Quality](dnd-spell-extraction.md),
|
||||
not its deferred repair-aware retry or semantic LLM-validation work.
|
||||
|
||||
## Purpose
|
||||
This active plan replaces the completed test-suite implementation record that
|
||||
previously occupied this filename. That review remains documented in
|
||||
[Test Suite Policy Review](tests.md) and repository history; its stages are not
|
||||
instructions for this feature.
|
||||
|
||||
This document records the decision-complete execution plan used to implement
|
||||
the target revisions in [Test Suite Policy Review](tests.md). The stages were
|
||||
followed in order. The feature roadmap owns the review evidence, desired test
|
||||
ownership, required dispositions, and target state; this document records the
|
||||
sequencing, file-level work, validation, and stop conditions used during
|
||||
implementation.
|
||||
## Objective
|
||||
|
||||
This is test-suite harmonization, not a production feature change. Do not
|
||||
modify production behavior. If a new durable test exposes incorrect production
|
||||
behavior, preserve the failing evidence, stop the affected stage, and report
|
||||
the defect separately for explicit scoping.
|
||||
Implement an overlay-capable effective spell catalog, ground every
|
||||
`dnd/spells` extraction request with its canonical names, reject extracted
|
||||
names that do not resolve through that catalog, and establish the existing
|
||||
same-request behavior with `retries: 2` as the measured baseline.
|
||||
|
||||
## Governing Policies
|
||||
The completed feature must fail malformed catalog configuration before source
|
||||
parsing or any LLM call, keep aliases out of the prompt, preserve the current
|
||||
private LLM response schema, and use one effective catalog policy for both the
|
||||
extractor and validator.
|
||||
|
||||
Before implementation, read and follow:
|
||||
## Governing Documents
|
||||
|
||||
- [Testing Policy](../policy/testing.md), especially behavioral ownership,
|
||||
semantic duplication, test doubles, exact diagnostics, and deletion criteria;
|
||||
- [Architecture](../policy/architecture.md), especially dependency direction,
|
||||
typed artifact boundaries, validation ownership, checkpoint safety, and
|
||||
source/reference separation; and
|
||||
- [Documentation Policy](../policy/documentation.md), especially canonical
|
||||
ownership, current-versus-future behavior, and maintenance of test-routing
|
||||
links.
|
||||
Before changing code, read and follow:
|
||||
|
||||
The following constraints apply to every stage:
|
||||
- [D&D Spell Extraction Quality](dnd-spell-extraction.md) for accepted product
|
||||
intent, target state, and deferred work;
|
||||
- [Architecture](../policy/architecture.md) for dependency direction,
|
||||
preparation, domain ownership, validation, reference provenance, and LLM
|
||||
boundaries;
|
||||
- [Documentation Policy](../policy/documentation.md) for canonical document
|
||||
ownership and current-versus-future behavior;
|
||||
- [Testing Policy](../policy/testing.md) for behavioral test ownership and
|
||||
offline/default-suite requirements;
|
||||
- [Pipeline Internals](../internal/pipeline.md), [Module Internals](../internal/modules.md),
|
||||
and [LLM Runtime](../internal/llm.md) for the implementation boundaries being
|
||||
changed; and
|
||||
- [Configuration](../config.md) and the existing D&D integration contracts for
|
||||
current user-visible contracts.
|
||||
|
||||
- preserve unrelated user changes in a dirty worktree;
|
||||
- make no production-code changes unless separately authorized after reporting
|
||||
a confirmed production defect;
|
||||
- add no coverage-percentage target and do not use line coverage as a success
|
||||
metric;
|
||||
- do not mechanically convert tests to tables, consolidate leaf registration
|
||||
tests, or introduce new test frameworks;
|
||||
- keep the default suite deterministic, offline, credential-free, and safe for
|
||||
repeated and parallel execution;
|
||||
- use real filesystem collaborators with `t.TempDir()` for checkpoint behavior;
|
||||
- assert typed/structured outcomes or stable category fragments rather than
|
||||
complete incidental error wording; and
|
||||
- do not update fixtures automatically or add a golden-update path.
|
||||
Preserve unrelated worktree changes. Do not implement anything listed under
|
||||
the feature roadmap's deferred retry and validation section.
|
||||
|
||||
## Stage 0 - Establish the implementation baseline
|
||||
## Fixed Design Decisions
|
||||
|
||||
1. Read [Test Suite Policy Review](tests.md) completely, including the retained
|
||||
ownership map and every required finding.
|
||||
2. Inspect the worktree and preserve unrelated changes. Limit planned edits to
|
||||
tests and the internal documentation routing identified below.
|
||||
3. Run:
|
||||
### Configuration and overlay transport
|
||||
|
||||
Add one optional extract-reference slot named `spell_catalog` to `dnd/spells`.
|
||||
It accepts one UTF-8 `application/json` file with a maximum size of 1 MiB. Use
|
||||
the normal reference binding and materialization path, so configuration-relative
|
||||
paths, CLI overrides, redaction, provenance, and checkpoint reference digests
|
||||
continue to work without a D&D-specific filesystem reader.
|
||||
|
||||
The slot binds an overlay bundle rather than one catalog. One file can therefore
|
||||
carry multiple sourcebook, campaign, or homebrew catalogs without adding list
|
||||
values to the general configuration reference schema. The canonical lane-local
|
||||
form is:
|
||||
|
||||
```yaml
|
||||
artifacts:
|
||||
spells:
|
||||
extract:
|
||||
module: dnd/spells
|
||||
retries: 2
|
||||
references:
|
||||
spell_catalog: ./campaign-spells.json
|
||||
```
|
||||
|
||||
The existing pipeline-level reference-default behavior may also bind the slot.
|
||||
Do not add module options, environment variables, or a new CLI flag for catalog
|
||||
overlays.
|
||||
|
||||
### Overlay bundle contract
|
||||
|
||||
Define and document this strict JSON shape as version 1:
|
||||
|
||||
```json
|
||||
{
|
||||
"schema_version": "notarius.dnd.spell-catalog-overlay.v1",
|
||||
"catalogs": [
|
||||
{
|
||||
"id": "campaign.example",
|
||||
"ruleset": "dnd-5e-2014",
|
||||
"source": {
|
||||
"title": "Example campaign spells",
|
||||
"version": "1",
|
||||
"url": "",
|
||||
"license": ""
|
||||
},
|
||||
"spells": [
|
||||
{
|
||||
"name": "Aegis of Emberfall",
|
||||
"aliases": ["Emberfall Aegis"]
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
Use `encoding/json` with unknown-field rejection and reject trailing JSON
|
||||
values. Require `schema_version` and `catalogs` at the bundle level;
|
||||
`id`, `ruleset`, `source`, and `spells` on each catalog; `title` on each source;
|
||||
and `name` on each spell. Require the exact schema version, at least one
|
||||
catalog, unique trimmed nonempty catalog IDs, the `dnd-5e-2014` ruleset, a
|
||||
trimmed nonempty source title, at least one spell per catalog, and trimmed
|
||||
nonempty canonical names. `source.version`, `source.url`, and `source.license`
|
||||
are optional strings and may be empty so private homebrew material is not
|
||||
forced to claim a publication or license. `aliases` may be omitted or supplied
|
||||
as an array, but it must not be JSON `null`, and every supplied alias must be
|
||||
trimmed and nonempty. The integration contract must state that catalog content
|
||||
is contextual domain data and never source evidence for a cast.
|
||||
|
||||
Overlay spells intentionally carry names and aliases only. Do not invent
|
||||
unknown spell levels or class memberships, and do not weaken the richer
|
||||
embedded SRD catalog contract to accommodate overlay-only recognition data.
|
||||
|
||||
### Effective catalog composition
|
||||
|
||||
Keep the existing embedded SRD catalog and introduce a separate immutable
|
||||
effective recognition-catalog type in `internal/modules/dnd/spells/catalog`.
|
||||
It must expose defensive-copy access to globally sorted canonical names,
|
||||
normalized lookup returning the canonical display name, the ruleset, overlay
|
||||
IDs, and a deterministic semantic digest.
|
||||
|
||||
Compose the effective catalog using these rules:
|
||||
|
||||
1. Start with every canonical SRD name and its aliases.
|
||||
2. Sort overlay catalogs by ID before composition; file order must not choose a
|
||||
winner.
|
||||
3. Normalize lookup keys using the package's existing case, whitespace, and
|
||||
apostrophe rules.
|
||||
4. A new canonical key adds a spell. An overlay canonical key equal to an
|
||||
existing canonical key augments that same spell; retain the established
|
||||
canonical display spelling and merge aliases.
|
||||
5. If equal normalized canonical keys have different display spellings, reject
|
||||
the bundle instead of selecting one by order.
|
||||
6. Repeated aliases mapping to the same canonical spell are idempotent.
|
||||
Canonical-name or alias collisions mapping to different spells are errors,
|
||||
including collisions with the embedded catalog.
|
||||
7. Return canonical names and aliases in deterministic sorted order regardless
|
||||
of input ordering. Do not expose mutable backing slices or maps.
|
||||
|
||||
Compute the effective-catalog digest from a canonical semantic representation
|
||||
that includes the base catalog identity, ruleset, sorted overlay identity and
|
||||
source metadata, and sorted effective canonical-name/alias mappings. Formatting
|
||||
or object-key order alone must not change this semantic digest. Raw reference
|
||||
digests continue to protect checkpoint identity when overlay file bytes change.
|
||||
|
||||
### Construction-time reference availability
|
||||
|
||||
Extend `pipeline.BuildRequest` with a defensively cloned
|
||||
`contracts.ReferenceSet`. `pipeline.Prepare` must provide the materialized
|
||||
references belonging to each constructed stage implementation and to every
|
||||
validator in that stage's chain:
|
||||
|
||||
- chunk references to the chunker and chunk validators;
|
||||
- lane extract references to the extractor and extract validators;
|
||||
- lane merge references to the merger and merge validators; and
|
||||
- lane normalize references to the normalizer and normalize validators.
|
||||
|
||||
Input and output receive an empty set because they cannot declare references.
|
||||
Keep runtime request references unchanged. Construction-time references exist
|
||||
so immutable modules can validate and retain derived data before source parsing;
|
||||
runtime references remain available for run operations and validator context.
|
||||
Do not put D&D catalog types into framework contracts or shared module
|
||||
dependencies.
|
||||
|
||||
Both the spell extractor and catalog validator must construct their immutable
|
||||
effective catalog from the same `spell_catalog` reference bytes. Centralize
|
||||
overlay selection and catalog composition in the D&D catalog package so the two
|
||||
implementations cannot drift. Reject more than one materialized item in the
|
||||
slot even though current configuration produces only one.
|
||||
|
||||
### Prompt grounding
|
||||
|
||||
Add one required structured-completion input named `spell_catalog` with media
|
||||
type `application/json`. Its generated content is exactly a small object whose
|
||||
`spell_names` value is the globally sorted array of effective canonical names.
|
||||
Do not include aliases, levels, classes, source metadata, raw overlay JSON, or
|
||||
spell descriptions.
|
||||
|
||||
Add a package-owned prompt fragment that presents this input as the allowed
|
||||
spell-name catalog. Update task instructions to require canonical catalog names
|
||||
and prohibit returning names absent from the catalog. Keep campaign references
|
||||
separate and retain the rule that references cannot establish that a cast
|
||||
occurred. Do not add an enum to `dnd_spells_llm.v1.json` and do not change the
|
||||
durable `dnd/spell-list` artifact schema.
|
||||
|
||||
Use the digest of the generated prompt-input bytes as its input digest. Extend
|
||||
extractor manifest metadata with the effective catalog semantic digest, base
|
||||
catalog ID, and sorted overlay IDs; do not expose catalog content or local file
|
||||
content in manifests or debug summaries. Existing reference provenance owns
|
||||
overlay origin URI, media type, byte size, and raw digest.
|
||||
|
||||
### Deterministic validation
|
||||
|
||||
Add typed validator key `extract/dnd/spells/catalog` under
|
||||
`internal/modules/dnd/validate/spells/catalog`. Construct it from the effective
|
||||
catalog at preparation time and give it no module options or LLM profile.
|
||||
|
||||
If spell shape is invalid, approve without catalog diagnostics so the shape
|
||||
validator remains the owner of shape rejection. Otherwise, check every
|
||||
nonempty spell name through normalized effective-catalog lookup. Approve
|
||||
canonical names and aliases without mutating the artifact. If any names are
|
||||
unknown, reject the complete extraction result with stable reason code
|
||||
`unknown_spell` and a bounded message identifying all affected spell indices
|
||||
and names. Sort diagnostics by artifact index and cap displayed issues at 20,
|
||||
reporting the number omitted. Do not silently delete casts, convert aliases to
|
||||
canonical names, or emit future structured validation issues in this feature.
|
||||
|
||||
Register the validator immediately after `extract/dnd/spells/shape` in the
|
||||
production default chain. Explicit validator overrides remain authoritative.
|
||||
|
||||
### Retry baseline
|
||||
|
||||
Make no runner, retry, rejection, or validation-result contract changes. The
|
||||
maintained production example continues to set `extract.retries: 2`, meaning
|
||||
one initial extraction plus at most two identical-request attempts around the
|
||||
complete validator chain. Rejections exhausted after three attempts remain
|
||||
nonfatal rejected outputs under current runner semantics.
|
||||
|
||||
Do not add repair prompts, previous candidates, rejection feedback, structured
|
||||
validation issues, error retryability classification, or an LLM validator.
|
||||
|
||||
## Stage 0 — Baseline and scope control
|
||||
|
||||
1. Read the governing documents and inspect the current worktree. Record and
|
||||
preserve changes not owned by this plan.
|
||||
2. Confirm that the embedded catalog loads 319 spells and 779 class
|
||||
memberships and that the maintained production example already contains
|
||||
`extract.retries: 2`.
|
||||
3. Run the baseline commands:
|
||||
|
||||
```sh
|
||||
go test ./...
|
||||
go vet ./...
|
||||
go build ./cmd/notarius
|
||||
go test -race ./internal/framework/pipeline ./internal/framework/llm ./internal/cli ./internal/modules/integration
|
||||
```
|
||||
|
||||
4. If the baseline fails for a reason unrelated to the planned work, stop and
|
||||
report it. Do not rewrite expectations merely to obtain a clean baseline.
|
||||
|
||||
Stage 0 is complete when the starting state and any pre-existing worktree
|
||||
changes are recorded and all baseline commands pass.
|
||||
|
||||
## Stage 1 - Protect filesystem checkpoint compatibility and recovery
|
||||
|
||||
Complete this stage before deleting or consolidating existing tests so the
|
||||
highest-risk protection is strengthened first.
|
||||
|
||||
### 1.1 Checkpoint identity
|
||||
|
||||
Create `internal/framework/checkpoint/identity_test.go`. Test through
|
||||
`NewIdentity` and `Identity.RelativePath`, not private normalization helpers.
|
||||
|
||||
Build one representative `pipeline.ResolvedPipeline` with two artifact lanes,
|
||||
an input module, stable pipeline ID/digest, selected lanes, runtime
|
||||
fingerprints, reference provenance, and provenance fingerprints. Add tests that
|
||||
prove:
|
||||
|
||||
- reordering selected lanes, runtime fingerprints, references, provenance
|
||||
fingerprints, or resolved lanes does not change the resulting identity;
|
||||
- duplicate or blank selected-lane entries do not change the normalized lane
|
||||
set, and blank fingerprint entries do not change the identity;
|
||||
- changing each meaningful identity input independently changes the digest:
|
||||
pipeline ID or digest, input key, raw/source digest, selected lane set,
|
||||
runtime override value, reference digest or target identity, and provenance
|
||||
fingerprint;
|
||||
- an omitted explicit input key falls back to the resolved input module;
|
||||
- missing pipeline ID, pipeline digest, input key, and both source/input digests
|
||||
are rejected with stable category context; and
|
||||
- `RelativePath` is deterministic, slash-separated, relative, confined, and
|
||||
includes the normalized pipeline/input and digest-derived hierarchy without
|
||||
asserting incidental private prefix lengths beyond the documented layout.
|
||||
|
||||
Use independent inputs for each mutation so one case cannot pass because a
|
||||
different field also changed.
|
||||
|
||||
### 1.2 All-stage recorder/loader round trips
|
||||
|
||||
Create `internal/framework/checkpoint/filesystem_test.go`; leave the existing
|
||||
schema-identifier test in `recorder_test.go`.
|
||||
|
||||
Using one real root and one identity, exercise the exported recorder and loader
|
||||
for:
|
||||
|
||||
- source success with a valid document, self-references, metadata, and digest;
|
||||
- extract success with at least one serialized artifact, schema identity,
|
||||
content metadata, chunk provenance, warnings, dependency fingerprints, and
|
||||
one rejected output so `StatusSucceededWithRejections` is round-tripped;
|
||||
- merge success with one serialized artifact, warnings, and dependencies; and
|
||||
- normalize success with one serialized artifact, warnings, and dependencies.
|
||||
|
||||
For each stage, assert the loader returns `Reused`, restores the meaningful
|
||||
values, and preserves serialized bytes and codec identity. Mutate the original
|
||||
inputs after recording and mutate one loaded result before reloading; neither
|
||||
mutation may alter persisted or subsequently loaded state.
|
||||
|
||||
Inspect representative created directories and files to retain the `0700`/
|
||||
`0600` permission contract on platforms where Unix permission bits are
|
||||
meaningful. Do not snapshot the full directory tree or complete JSON documents.
|
||||
|
||||
### 1.3 Invalid and incompatible checkpoint state
|
||||
|
||||
Seed valid state through the recorder, then copy or edit one artifact per
|
||||
subtest. Drive every case through the exported loader method for that stage.
|
||||
Require a non-reused decision and a short category fragment for:
|
||||
|
||||
- missing artifact and malformed JSON;
|
||||
- `WorkspaceSchemaVersionV1` and an unknown workspace schema version;
|
||||
- mismatched checkpoint identity digest;
|
||||
- wrong stage, lane, module, terminal status, or dependency fingerprint;
|
||||
- incomplete serialized artifact kind/schema/schema digest;
|
||||
- malformed base64 and content-digest mismatch;
|
||||
- source document validation or source/output digest mismatch; and
|
||||
- extract, merge, and normalize output-digest mismatch.
|
||||
|
||||
Include one successful `StatusSucceededWithRejections` extract case and prove
|
||||
that non-reusable running, failed, pending, or invalidated statuses remain
|
||||
non-reused. Assert categories, not complete sentences. Missing state should be
|
||||
a normal non-reuse decision; corrupt or incompatible state must never panic or
|
||||
silently reuse.
|
||||
|
||||
### 1.4 Stage validation
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/framework/checkpoint ./internal/framework/pipeline ./internal/cli
|
||||
go test -race ./internal/framework/checkpoint ./internal/framework/pipeline ./internal/cli
|
||||
```
|
||||
|
||||
Stage 1 is complete when identity selection, every persisted stage, mutation
|
||||
ownership, compatibility rejection, and corrupt-state recovery are protected at
|
||||
the real filesystem boundary without production changes.
|
||||
|
||||
## Stage 2 - Remove fake-only, obsolete, and misleading tests
|
||||
|
||||
### 2.1 Framework contracts and helpers
|
||||
|
||||
In `internal/framework/contracts/contracts_test.go`, delete exactly:
|
||||
|
||||
- `TestFakeExtractorReturnsTypedOutput`;
|
||||
- `TestFakeChunkerReturnsSourcePlan`;
|
||||
- `TestFakeChunkerReceivesPerRunContext`;
|
||||
- `TestFakeExtractorReceivesChunkAndAmbientContext`;
|
||||
- `TestFakeMergeNormalizeAndOutputContracts`; and
|
||||
- `TestReferenceSetDataTypes`.
|
||||
|
||||
After removing those tests, delete fake methods/types or imports only when they
|
||||
have no remaining test use. Retain the compile-time interface assertions and
|
||||
the reference/material/artifact clone and JSON non-leakage tests.
|
||||
|
||||
Delete `TestHelpersReturnValidationResults` from
|
||||
`internal/framework/validate/validate_test.go`. Retain `TestApproved` and
|
||||
`TestRejectedTrimsReasonAndMessage`.
|
||||
|
||||
Delete `internal/modules/generic/normalize/noop/typed_test.go`. Do not remove the
|
||||
no-op normalizer's resolver, registration, runner, production-composition, or
|
||||
maintained-example coverage. Retain the direct append-order merger ordering
|
||||
test.
|
||||
|
||||
### 2.2 Obsolete Scriptorium grounding
|
||||
|
||||
Delete `internal/framework/llm/scriptorium_api_test.go` in full. Do not move its
|
||||
unused API inventory elsewhere. Retain and run `scriptorium_client_test.go`,
|
||||
`asset_registry_test.go`, module-local prompt preparation, cancellation,
|
||||
validation, profile, and credential-redaction tests.
|
||||
|
||||
### 2.3 Misleading D&D integration tests
|
||||
|
||||
In `internal/modules/integration/dnd_spells_runner_test.go`, delete exactly:
|
||||
|
||||
- `TestRunnerDoesNotExtractSpellMentionedOnlyInPartyReference`;
|
||||
- `TestRunnerCarriesDNDSpellCastWithInvalidSourceRefToSerializedOutput`; and
|
||||
- `TestRunnerRejectsMalformedDNDSpellsArtifactAtSerializationBoundary`.
|
||||
|
||||
Remove helpers/imports only if unused afterward. Retain
|
||||
`TestRunnerProcessesSeriatimInputWithDNDSpellsExtractor` and
|
||||
`TestRunnerPassesPartyAndGlossaryReferencesToDNDSpellsPrompt` as the two
|
||||
representative cross-family workflows.
|
||||
|
||||
### 2.4 Stage validation
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/framework/contracts ./internal/framework/llm ./internal/framework/validate
|
||||
go test ./internal/modules/generic/normalize/noop ./internal/modules/integration ./internal/modules/dnd/...
|
||||
```
|
||||
|
||||
Stage 2 is complete when all named low-value tests are gone, retained owners
|
||||
still pass, and no production file changed.
|
||||
|
||||
## Stage 3 - Consolidate configuration, CLI state, and resolution ownership
|
||||
|
||||
### 3.1 Version 3 configuration tests
|
||||
|
||||
Before deleting `internal/core/config/v3_test.go`, preserve its only distinct
|
||||
protections:
|
||||
|
||||
- add a version-2 input to the strict file-decoding cases in
|
||||
`file_config_contract_test.go`; require rejection and the migration category,
|
||||
not the complete diagnostic; and
|
||||
- ensure `env_contract_test.go` positively proves that empty configured cache
|
||||
roots resolve to distinct `notarius/chunk-plans` and
|
||||
`notarius/checkpoints` descendants of the supplied per-user cache root.
|
||||
|
||||
Then delete `v3_test.go`. Do not duplicate its defaults, precedence,
|
||||
redaction, invalid-source, or removed-field cases elsewhere.
|
||||
|
||||
### 3.2 CLI state and maintained examples
|
||||
|
||||
Move `emptyLookup` from `internal/cli/state_surfaces_test.go` to
|
||||
`internal/cli/contract_test_helpers_test.go`, then delete
|
||||
`state_surfaces_test.go` in full. Confirm its remaining behavior is still owned
|
||||
by command/run contracts and `state_hardening_test.go`; do not transplant its
|
||||
tests.
|
||||
|
||||
In `internal/cli/production_contract_test.go`:
|
||||
|
||||
- keep `TestDefaultCLICompositionResolvesMaintainedConfigurations`, but rename
|
||||
it to `TestDefaultCLICompositionValidatesRepresentativeConfiguration` and
|
||||
reduce it to one representative `config validate` command using empty/default
|
||||
`Options`; this test owns fallback production composition, not
|
||||
maintained-example enumeration;
|
||||
- remove maintained-example resolution from
|
||||
`TestProductionCatalogCoversMaintainedConfigurations` while retaining
|
||||
required production registry members, typed codec/variant wiring, catalog
|
||||
conversion, and the exact documented spell validator chain; and
|
||||
- remove the maintained-example loop from
|
||||
`TestProductionConfigValidationCoversModuleAndVariantFailures`, retaining one
|
||||
valid baseline and each distinct failure mutation.
|
||||
|
||||
Do not weaken `example_contract_test.go`; it remains the sole owner for loading,
|
||||
resolving, listing, and executing the maintained examples.
|
||||
|
||||
### 3.3 Default and Seriatim resolver duplication
|
||||
|
||||
Delete `internal/framework/pipeline/default_modules_test.go` in full.
|
||||
|
||||
Delete `internal/modules/seriatim/input/transcript/config_test.go` in full.
|
||||
Delete `internal/modules/seriatim/input/transcript/testdata/pipeline.yml` with
|
||||
it; the fixture is owned only by that deleted test file. Retain adapter parsing,
|
||||
leaf registration, the Seriatim runner integration, generic resolver tests,
|
||||
and production examples.
|
||||
|
||||
### 3.4 D&D capability integration
|
||||
|
||||
Rewrite `internal/modules/integration/dnd_spells_config_test.go` to own only two
|
||||
cross-family capability failures:
|
||||
|
||||
1. removing `source.transcript` from the Seriatim input spec must make the D&D
|
||||
spell extractor incompatible; and
|
||||
2. removing `dnd.spell_casts` from the spell extractor spec must make the
|
||||
append-order merger incompatible.
|
||||
|
||||
Use one compact table over a programmatically constructed profile and the
|
||||
smallest catalog capable of resolution. Delete the successful-resolution,
|
||||
stable-digest, and unknown-lane cases. Simplify or remove fixture/catalog
|
||||
helpers that become unnecessary, but preserve helpers used by the retained
|
||||
runner integration in sibling test files.
|
||||
|
||||
### 3.5 Stage validation
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/core/config ./internal/cli ./internal/framework/pipeline
|
||||
go test ./internal/modules/seriatim/... ./internal/modules/integration
|
||||
```
|
||||
|
||||
Stage 3 is complete when each behavior has the owner specified above, the
|
||||
maintained examples are not redundantly enumerated, and all distinct migration,
|
||||
default-composition, and capability protections remain.
|
||||
|
||||
## Stage 4 - Rewrite brittle assertions at durable behavioral boundaries
|
||||
|
||||
### 4.1 Source validation ownership and diagnostics
|
||||
|
||||
In `internal/core/source/source_test.go`:
|
||||
|
||||
- consolidate malformed `SourceRef` categories under the `ValidateRef` tests;
|
||||
- reduce `TestValidateDocumentUnitReferences` to one case proving nested
|
||||
reference failures receive unit/document context and one case proving the
|
||||
document-only self-reference invariant;
|
||||
- retain valid documents, required document/unit fields, duplicate IDs,
|
||||
non-empty units, valid/reversed/missing references, unit lookup, and digest
|
||||
sensitivity/determinism; and
|
||||
- replace exact complete error equality with the minimum stable field/category
|
||||
fragments needed to distinguish each failure.
|
||||
|
||||
Do not add typed production errors during this pass. If stable fragments cannot
|
||||
distinguish meaningful categories without a production change, retain the
|
||||
narrowest current assertion and report that limitation rather than changing
|
||||
production code.
|
||||
|
||||
### 4.2 Private LLM response schemas
|
||||
|
||||
In `internal/modules/dnd/chunk/scenes/schema_test.go`, replace the nested
|
||||
`map[string]any` schema-structure walk with actual JSON Schema validation.
|
||||
Add a small test helper that parses the instance and schema with
|
||||
`jsonschema.UnmarshalJSON`, registers the schema with
|
||||
`jsonschema.NewCompiler().AddResource`, compiles it, and calls
|
||||
`schema.Validate`, matching the existing production validator boundary. Use a
|
||||
representative valid scene response and mutations that reject:
|
||||
|
||||
- obsolete segment-based boundary fields;
|
||||
- non-positive start/end unit IDs;
|
||||
- invalid `primary_mode` and `boundary_confidence` values;
|
||||
- empty boundary caveats; and
|
||||
- unknown properties.
|
||||
|
||||
Retain identity/hash validity, DTO integer decoding, and mutation safety.
|
||||
|
||||
In `internal/modules/dnd/extract/spells/schema_test.go`, validate one legal
|
||||
private LLM response whose references omit `source_id`, and prove that adding a
|
||||
canonical `source_id` is rejected. Retain response identity/hash, mutation
|
||||
safety, and diagnostics non-leakage. Do not conflate this private transport
|
||||
schema with the durable codec schema or alter the maintained durable fixture.
|
||||
|
||||
### 4.3 Framework schema enumeration
|
||||
|
||||
In `internal/framework/llm/schema_registry_test.go`, remove only the assertion
|
||||
that `RegisteredResponseSchemas` has exactly two entries. Continue to assert:
|
||||
|
||||
- returned keys are sorted;
|
||||
- both required framework test schemas are present and valid;
|
||||
- returned bytes are mutation-safe;
|
||||
- diagnostics omit raw schema content; and
|
||||
- D&D schemas are not registered in the domain-neutral framework registry.
|
||||
|
||||
### 4.4 Stage validation
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/core/source ./internal/framework/llm
|
||||
go test ./internal/modules/dnd/chunk/scenes ./internal/modules/dnd/extract/spells ./internal/modules/dnd/codec/spells
|
||||
```
|
||||
|
||||
Stage 4 is complete when failures express behavioral categories, private
|
||||
schemas are validated by accepted/rejected instances, and durable schema
|
||||
compatibility coverage remains unchanged.
|
||||
|
||||
## Stage 5 - Make composition and architecture checks extension-friendly
|
||||
|
||||
### 5.1 Family registrars
|
||||
|
||||
Rewrite the success assertions in:
|
||||
|
||||
- `internal/modules/generic/register/register_test.go`;
|
||||
- `internal/modules/seriatim/register/register_test.go`; and
|
||||
- `internal/modules/dnd/register/register_test.go`.
|
||||
|
||||
Use required-membership helpers rather than exact equality for registered keys
|
||||
and asset names. Prove representative family-owned entries are retrievable or
|
||||
buildable through their registry boundary. Preserve:
|
||||
|
||||
- validation of all required registry/asset dependencies before any mutation;
|
||||
- contextual failure on duplicate family registration;
|
||||
- absence of cross-family composition where that is an explicit architectural
|
||||
ownership rule; and
|
||||
- exact D&D spell default-validator order, because it is documented production
|
||||
policy.
|
||||
|
||||
Do not delete leaf spec/constructor/registration tests and do not move the CLI
|
||||
production catalog into family tests.
|
||||
|
||||
### 5.2 Central import-boundary enforcement
|
||||
|
||||
Move the two rules from
|
||||
`internal/framework/chunkplan/import_boundaries_test.go` into
|
||||
`internal/modules/import_boundaries_test.go`:
|
||||
|
||||
- production files under `internal/core/source` may import neither
|
||||
`internal/framework` nor `internal/modules`; and
|
||||
- production files under `internal/framework/chunkplan` may not import
|
||||
`internal/modules`.
|
||||
|
||||
Extend the centralized checker so its repository walk enforces those rules.
|
||||
Add rule-level synthetic cases for both allowed and forbidden imports, using
|
||||
the existing table/checker style. Retain the existing rejected fixture that
|
||||
proves generic-to-concrete enforcement. Once both unique rules and checker
|
||||
activation are protected centrally, delete
|
||||
`internal/framework/chunkplan/import_boundaries_test.go`.
|
||||
|
||||
### 5.3 Internal documentation routing
|
||||
|
||||
Update only the `Tests To Inspect` routing needed to match the final suite in:
|
||||
|
||||
- `docs/internal/state.md`;
|
||||
- `docs/internal/pipeline.md`;
|
||||
- `docs/internal/modules.md`; and
|
||||
- `docs/internal/llm.md`.
|
||||
|
||||
Replace deleted or nonexistent names such as
|
||||
`internal/cli/state_surfaces_test.go` and `internal/cli/run_test.go` with concise
|
||||
links or paths to the retained command/run contracts, state-hardening,
|
||||
production-composition, maintained-example, checkpoint filesystem, and
|
||||
cross-family integration owners. Do not create an exhaustive test inventory or
|
||||
repeat subsystem contracts owned elsewhere.
|
||||
|
||||
### 5.4 Stage validation
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./internal/modules/... ./internal/framework/chunkplan
|
||||
go test ./internal/cli ./internal/framework/pipeline ./internal/framework/llm
|
||||
```
|
||||
|
||||
Stage 5 is complete when legitimate family additions no longer require closed
|
||||
inventory edits, all architectural rules remain executable in one checker, and
|
||||
internal documentation names only existing retained owners.
|
||||
|
||||
## Stage 6 - Repository acceptance and handoff
|
||||
|
||||
1. Review the final diff against every required finding and retained-owner
|
||||
statement in [Test Suite Policy Review](tests.md). Confirm that no production
|
||||
`.go` file changed.
|
||||
2. Run formatting on changed Go test files, then run:
|
||||
4. If the baseline fails for an unrelated reason, stop and report it. Do not
|
||||
modify unrelated behavior or tests to obtain a clean baseline.
|
||||
|
||||
Stage 0 is complete when the baseline and preserved worktree scope are known.
|
||||
|
||||
## Stage 1 — Inject materialized references during preparation
|
||||
|
||||
1. Add `References contracts.ReferenceSet` to `pipeline.BuildRequest` and clone
|
||||
it in `cloneBuildRequest`.
|
||||
2. Refactor `pipeline.Prepare`, `prepareLane`, and `prepareValidatorChain` so
|
||||
each builder receives the correct materialized reference target described in
|
||||
the fixed decisions. Do not change operation-request delivery.
|
||||
3. Update builder/preparation tests to prove:
|
||||
- construction receives the correct target-specific reference content;
|
||||
- extractor and extract validators receive independent clones of the same
|
||||
logical set;
|
||||
- mutation by one builder cannot affect another builder, the resolved
|
||||
pipeline, or runtime requests; and
|
||||
- empty/unmaterialized reference targets remain valid for callers that use
|
||||
`Prepare` directly.
|
||||
4. Update [Pipeline Internals](../internal/pipeline.md) and
|
||||
[Module Internals](../internal/modules.md) to describe construction-time
|
||||
reference delivery as current behavior once the code lands.
|
||||
5. Run focused framework tests plus all repository-wide validation commands.
|
||||
|
||||
Do not change configuration syntax, reference multiplicity, reference
|
||||
materialization, manifests, or checkpoint formats in this stage.
|
||||
|
||||
Stage 1 is complete when all stage builders receive isolated target references
|
||||
at preparation and existing pipelines behave unchanged.
|
||||
|
||||
## Stage 2 — Implement overlay parsing and effective catalog composition
|
||||
|
||||
1. Add package-owned overlay DTOs, strict decoding, semantic validation, and
|
||||
immutable effective-catalog composition under
|
||||
`internal/modules/dnd/spells/catalog`. Keep overlay DTOs private; expose only
|
||||
the minimal effective-catalog API needed by extraction and validation.
|
||||
2. Preserve `LoadSRD5E2014`, existing SRD metadata, and the existing `Spell`
|
||||
API. Do not represent overlay-only entries as incomplete SRD `Spell` values.
|
||||
3. Add a helper that resolves the optional `spell_catalog` slot from a cloned
|
||||
`ReferenceSet`, enforces zero-or-one JSON item, and returns the base-only or
|
||||
base-plus-overlay effective catalog. Keep all filesystem access in framework
|
||||
reference materialization.
|
||||
4. Add focused behavioral tests for:
|
||||
- base-only canonical names, normalized lookup, and immutability;
|
||||
- a new overlay spell and an alias accepted only by lookup;
|
||||
- augmentation of an existing canonical spell;
|
||||
- deterministic results and semantic digest under reordered JSON catalogs,
|
||||
spells, aliases, and object keys;
|
||||
- duplicate IDs, wrong ruleset/schema version, empty required values,
|
||||
unknown fields, trailing JSON, and empty bundles;
|
||||
- same-key display conflicts and every cross-spell canonical/alias collision
|
||||
category; and
|
||||
- rejection of multiple items or a non-JSON item in the reference slot.
|
||||
5. Add the external overlay format contract under `docs/integrations/` and
|
||||
update the internal catalog inventory only after the behavior exists.
|
||||
6. Run the catalog package tests, import-boundary tests, and repository-wide
|
||||
validation commands.
|
||||
|
||||
Prefer table-driven coverage for meaningful invalid-input categories, but do
|
||||
not duplicate every parser case across higher layers.
|
||||
|
||||
Stage 2 is complete when one immutable effective catalog deterministically
|
||||
represents the embedded base plus a valid overlay bundle and rejects ambiguous
|
||||
composition before execution.
|
||||
|
||||
## Stage 3 — Ground the spell extractor
|
||||
|
||||
1. Declare the optional `spell_catalog` reference slot on both the extractor
|
||||
runtime and `ModuleSpec`, accepting only `application/json`, `Multiple:
|
||||
false`, and `MaxBytes: 1048576`. Keep the existing campaign-reference slots
|
||||
unchanged.
|
||||
2. Update the extractor builder and constructor to resolve and retain the
|
||||
effective catalog at preparation. A malformed overlay must make `Prepare`
|
||||
fail before raw input is read or the LLM is called.
|
||||
3. Generate the canonical-name-only JSON prompt material from the retained
|
||||
catalog and add it to every structured completion request. Extend the
|
||||
Scriptorium prompt declaration and package-owned prompt assets accordingly.
|
||||
4. Update prompt wording to distinguish the allowed spell-name catalog from
|
||||
campaign references and source evidence. Leave both response schemas
|
||||
unchanged.
|
||||
5. Add effective catalog identity to extractor manifest metadata without
|
||||
exposing names, aliases, or raw overlay content. Include the new prompt
|
||||
fragment in prompt hashing.
|
||||
6. Update focused tests to protect:
|
||||
- base-only requests contain all and only sorted canonical names;
|
||||
- aliases and non-name metadata do not appear in prompt input;
|
||||
- overlay canonical names do appear;
|
||||
- prompt/schema preparation succeeds offline with the required input;
|
||||
- prompt and manifest diagnostics omit source, reference, overlay, alias,
|
||||
and catalog-name content; and
|
||||
- malformed overlays fail construction without invoking the fake LLM.
|
||||
7. Update the configuration reference-slot catalog, internal module docs, LLM
|
||||
internals where needed, and the external overlay contract in the same stage.
|
||||
8. Add a safe maintained overlay example file and bind it from the production
|
||||
D&D example. Keep the minimal example base-only. Ensure example tests load
|
||||
and materialize the referenced file, rather than checking YAML syntax alone.
|
||||
9. Run focused extractor, prompt-asset, CLI example, and integration tests plus
|
||||
all repository-wide validation commands.
|
||||
|
||||
Stage 3 is complete when every spell extraction is grounded by the immutable
|
||||
effective catalog and all overlay failures occur before paid work.
|
||||
|
||||
## Stage 4 — Add catalog validation and production policy
|
||||
|
||||
1. Implement and register `extract/dnd/spells/catalog` as specified above,
|
||||
reusing the same catalog resolver as the extractor.
|
||||
2. Insert it immediately after the shape validator in
|
||||
`internal/modules/dnd/register`. Do not reorder or otherwise change the
|
||||
remaining default validators.
|
||||
3. Add package-level tests for canonical names, lookup normalization, aliases,
|
||||
overlay spells, multiple unknown casts, the 20-issue message bound, shape
|
||||
deferral, immutability, strict empty options, execution class, and
|
||||
registration.
|
||||
4. Update production composition tests to protect registration, exact default
|
||||
chain placement, and construction from the same materialized overlay.
|
||||
5. Add one assembled offline retry test using the production D&D registration
|
||||
and a fake structured LLM, with two subcases: one remains unknown through
|
||||
exhaustion, and one becomes catalog-valid on a retry. Configure a test-local
|
||||
retry count and assert calls are bounded by `retries + 1`, exhausted
|
||||
rejection is nonfatal, no rejected attempt advances to merge, and only the
|
||||
accepted attempt contributes warnings and output. Do not repeat generic
|
||||
retry cases already owned by framework tests.
|
||||
6. Update the implemented-validator catalog in Configuration and the D&D
|
||||
validator inventory in Module Internals.
|
||||
7. Run focused validator, registrar, integration, and CLI tests plus all
|
||||
repository-wide validation commands.
|
||||
|
||||
Stage 4 is complete when no artifact containing a name outside the effective
|
||||
catalog can pass the production spell extraction chain.
|
||||
|
||||
## Stage 5 — Verify provenance, identity, and assembled behavior
|
||||
|
||||
1. Add or extend the narrowest existing CLI/checkpoint contract tests to prove
|
||||
that changing overlay file bytes changes checkpoint identity through
|
||||
reference provenance, while semantically reordered overlay content retains
|
||||
the same extractor-reported effective catalog digest. Do not alter
|
||||
checkpoint schemas or path formats.
|
||||
2. Verify that changing the configured overlay binding changes resolved
|
||||
pipeline identity, and that manifest reference provenance records the
|
||||
overlay origin, media type, size, and raw digest without content.
|
||||
3. Exercise a representative production-composed run with an overlay-only
|
||||
canonical spell and confirm prompt grounding, validation acceptance, output,
|
||||
manifest catalog metadata, and reference provenance agree.
|
||||
4. Verify base-only configuration remains valid and produces no overlay
|
||||
provenance or overlay IDs.
|
||||
5. Run:
|
||||
|
||||
```sh
|
||||
go test ./...
|
||||
go vet ./...
|
||||
go build ./cmd/notarius
|
||||
go test -race ./internal/framework/pipeline ./internal/framework/llm ./internal/framework/checkpoint ./internal/cli ./internal/modules/integration
|
||||
go test ./... -shuffle=on -count=5
|
||||
go test -race ./internal/framework/pipeline ./internal/cli ./internal/modules/dnd/extract/spells ./internal/modules/dnd/validate/spells/catalog ./internal/modules/integration
|
||||
```
|
||||
|
||||
3. Generate one coverage report for investigation. Confirm the new checkpoint
|
||||
tests execute source, extract, merge, normalize, manifest-validation, and
|
||||
corruption paths. Do not compare the percentage to the review baseline and
|
||||
do not add tests merely to increase it.
|
||||
4. Check documentation links and `git diff --check`. Confirm no fixtures were
|
||||
updated automatically and no secrets or external-service requirements were
|
||||
introduced.
|
||||
Stage 5 is complete when the assembled offline workflow protects catalog
|
||||
grounding, validation, provenance, retry count, and checkpoint invalidation at
|
||||
their appropriate test boundaries.
|
||||
|
||||
Implementation is complete when:
|
||||
## Stage 6 — Documentation, evaluation handoff, and completion
|
||||
|
||||
- every checkpoint stage has a real filesystem round trip and corrupted or
|
||||
incompatible state is never silently reused;
|
||||
- every deletion or rewrite in the feature roadmap is complete while its named
|
||||
retained owner still passes;
|
||||
- maintained examples have one example-contract owner plus one narrow default
|
||||
production-composition smoke path;
|
||||
- registrar and architecture checks permit legitimate extension without
|
||||
weakening documented ownership or validator order;
|
||||
- the full suite is deterministic, offline, credential-free, race-clean, and
|
||||
diagnostically useful;
|
||||
- internal documentation points to existing test owners; and
|
||||
- all validation commands pass with no production behavior change.
|
||||
1. Review every affected current-behavior owner and remove roadmap-style
|
||||
wording from implemented contracts:
|
||||
- `docs/config.md` owns the slot name, media type, byte limit, binding form,
|
||||
validator key, and default chain;
|
||||
- the new `docs/integrations/` document owns overlay JSON shape and
|
||||
compatibility rules;
|
||||
- `docs/internal/pipeline.md`, `docs/internal/modules.md`, and
|
||||
`docs/internal/overview.md` own construction and component behavior;
|
||||
- maintained copyable files remain under `examples/`; and
|
||||
- operations documentation changes only if runtime handling actually
|
||||
changes.
|
||||
2. Check all links and ensure examples contain no credentials, private paths,
|
||||
transcripts, or copyrighted spell descriptions.
|
||||
3. If an approved LLM profile and human-reviewed transcript corpus are
|
||||
available, run the base-only and overlay-capable baseline with
|
||||
`retries: 2`. Record anonymized aggregate false positives, false negatives,
|
||||
unknown-name rejections, acceptance by attempt, and model-call cost in the
|
||||
feature roadmap without committing sensitive transcripts or raw prompts.
|
||||
4. If those external evaluation inputs are unavailable, mark evaluation as
|
||||
pending external execution and provide the operator with the exact config
|
||||
and command used by the maintained example. Do not fabricate quality
|
||||
results, invoke a paid model without authorization, or make live evaluation
|
||||
part of the default Go test suite.
|
||||
5. Update [D&D Spell Extraction Quality](dnd-spell-extraction.md) status to
|
||||
distinguish completed implementation from pending or completed evaluation.
|
||||
Remove completed baseline items from [Future Work](future.md), retaining a
|
||||
concise link for deferred repair-aware retry and LLM-validator work.
|
||||
6. Run `git diff --check` and the full Stage 5 validation set once more after
|
||||
documentation and example changes.
|
||||
|
||||
Stage 6 is complete when current behavior is documented in its canonical
|
||||
owners, the feature roadmap accurately records implementation/evaluation
|
||||
status, deferred work remains unimplemented, and all required checks pass.
|
||||
|
||||
## Explicit Non-Goals
|
||||
|
||||
- Changing the framework-wide retry default or retry orchestration.
|
||||
- Supplying rejected candidates or validation feedback to retry attempts.
|
||||
- Structured validation-issue contracts or retryability classification.
|
||||
- An LLM-backed spell validator.
|
||||
- Spell-name mutation or catalog-aware normalization.
|
||||
- Multiple reference bindings per generic slot or list-valued reference
|
||||
configuration.
|
||||
- Spell descriptions, mechanics, unknown overlay levels, or class metadata.
|
||||
- A configuration-version bump, environment override, checkpoint migration,
|
||||
or checkpoint-format change.
|
||||
|
||||
## Open Questions
|
||||
|
||||
None. The feature roadmap contains enough evidence and policy decisions to
|
||||
implement every stage without additional testing-policy choices. If a new test
|
||||
reveals a production defect, that is a scope boundary rather than an open
|
||||
planning question: stop the affected stage and request explicit authorization
|
||||
before changing production behavior.
|
||||
None. The overlay transport, file contract, composition policy, preparation
|
||||
boundary, prompt content, validator behavior, retry baseline, provenance, and
|
||||
test ownership are fixed by this plan.
|
||||
|
||||
Reference in New Issue
Block a user