diff --git a/docs/roadmap/dnd-spell-extraction.md b/docs/roadmap/dnd-spell-extraction.md deleted file mode 100644 index 4c9c0bf..0000000 --- a/docs/roadmap/dnd-spell-extraction.md +++ /dev/null @@ -1,180 +0,0 @@ -# D&D Spell Extraction Quality - -## Status - -The accepted baseline implementation is complete. It provides effective -SRD-plus-overlay catalog composition, canonical-name prompt grounding, -deterministic catalog validation, the maintained `retries: 2` policy, and -provenance and checkpoint identity coverage through the assembled offline -workflow. Checkpoint identity includes independently scoped extractor and -validator fingerprints of the effective catalog, closing the earlier gap in -which raw overlay changes invalidated reuse but embedded catalog changes did -not. - -External quality evaluation is pending. This repository contains the -maintained example and offline fake-LLM coverage, but no approved -human-reviewed transcript corpus or authorized live-model evaluation was -available for this implementation run. The maintained example can be run -from the repository root with: - -```sh -go run ./cmd/notarius run dnd-session \ - --config examples/dnd-spells-production.config.yml \ - --input examples/seriatim-minimal-transcript.json \ - --chunk_cache bypass -``` - -That command requires the configured Scriptorium profile credential. When an -approved profile and reviewed corpus are available, record anonymized -aggregate false positives, false negatives, unknown-name rejections, -acceptance by attempt, and model-call cost here without committing transcripts -or raw prompts. Feedback-aware repair and semantic LLM validation remain -deferred until that baseline evaluation is available. - -## Objective - -Improve the precision and consistency of `dnd/spells` extraction by first -grounding the LLM in an effective spell catalog composed from the embedded SRD -5.1 catalog and configured overlays, rejecting invalid catalog names -deterministically, and measuring the existing retry mechanism. Preserve a clear -separation between response shape, domain policy, retry orchestration, and later -semantic review. - -## Accepted Baseline Scope - -### Ground extraction with a catalog input - -Provide the spell catalog to the extraction prompt as a separate input rather -than embedding all spell names as an enum in the private LLM response schema. -The response schema should continue to define transport shape, while the -catalog remains the single source of truth for recognized spell names. - -Build the prompt input from the effective catalog. Include canonical names -only; aliases remain available to deterministic lookup but do not belong in the -prompt. Omit levels and class memberships unless evaluation demonstrates that -they improve extraction. Instruct the model to return canonical catalog names -and to use reference material only for disambiguation, never as source -evidence. - -The effective catalog must support configured overlays from the first -implementation so non-SRD, sourcebook, and homebrew spells can participate in -the same grounding and validation path. Overlay composition must validate -canonical names, aliases, duplicates, conflicts, and provenance -deterministically. The prompt and validator must consume the same resolved -catalog; configuration and catalog identity must participate in effective -pipeline and checkpoint identity wherever they can change accepted output. - -This design avoids duplicating the catalog in a schema asset and allows -deterministic validation to produce a specific diagnostic for an unknown name. - -### Add deterministic catalog validation - -Add a typed deterministic validator for extracted spell names and include it in -the production default chain after shape validation. It should use the catalog -lookup rules so case, repeated whitespace, and supported apostrophe variants do -not cause false rejections. - -The validator should validate only; canonical display-name replacement belongs -in the future D&D spell normalizer. It must not silently discard an individual -unknown cast from an otherwise valid extraction result. - -The initial target chain is: - -```yaml -validators: - - generic/valid_json - - generic/valid_json_schema - - extract/dnd/spells/shape - - extract/dnd/spells/catalog - - extract/dnd/spells/source_refs - - extract/dnd/spells/source_relatedness -``` - -A name absent from the effective base-plus-overlay catalog should reject the -extraction result. Overlay support must not weaken this policy implicitly. - -### Measure ordinary extraction retries - -Configure the spell extraction binding with `retries: 2`. This means one -initial extraction attempt plus at most two additional attempts for a module -error, validator error, or validator rejection. The retry wraps extraction and -the complete extraction validator chain. - -Keep the framework-wide retry default at zero. The two-retry policy is specific -to the spell extraction workflow and should not silently apply to deterministic -stages or future LLM modules with different cost and failure characteristics. - -The baseline deliberately retains current retry behavior: each retry repeats -the same extraction request without the rejected candidate or validator -diagnostic. Measure this behavior before introducing corrective prompts so the -effects of catalog grounding, deterministic validation, and later repair-aware -retry can be distinguished. - -### Baseline evaluation policy - -Exercise the grounded extractor against a small human-reviewed transcript set. -Record false positives, false negatives, unknown-name rejections, retry -outcomes, acceptance by attempt, and model-call cost. Preserve representative -cases that can compare ordinary retry with a future repair-aware strategy. - -Do not add an LLM validator during this milestone. Finite catalog membership is -deterministic, and semantic LLM validation needs evidence of a specific failure -mode before its extra cost and nondeterminism are justified. - -## Target-State Acceptance Criteria - -- The extraction prompt receives canonical names from the effective catalog - without maintaining a second handwritten spell list or schema enum; aliases - are excluded from the prompt. -- Configured overlays compose deterministically with the embedded catalog, and - the extractor and validator consume the same effective catalog. -- Every accepted spell name resolves through the effective catalog, including - aliases recognized only by deterministic lookup. -- Unknown spell names produce a scoped deterministic rejection and can consume - the configured extraction retry budget. -- Two retries result in no more than three extraction calls for a rejected - chunk, excluding any later separately configured LLM validator calls. -- Discarded-attempt warnings are not promoted, and an exhausted rejection - remains a rejected pipeline outcome under existing runner semantics. -- The default production validator chain, maintained examples, and applicable - current-behavior documentation are updated alongside implementation. - -## Deferred Retry And Validation Work - -Reassess the following only after the baseline evaluation is available: - -1. Add bounded, field-specific structured validation issues rather than relying - only on a reason code and free-form message. Validators should report all - related issues they can safely identify in one pass. -2. Classify retryable outcomes. Provider failures and actionable validation - rejections may retry; cancellation must stop; configuration, schema-loading, - and internal invariant failures should fail without spending more LLM calls. -3. Add an optional typed repair capability to the extractor contract. The - framework should transport the rejected candidate and diagnostics without - constructing a domain prompt; each module should own its repair prompt. -4. Evaluate a hybrid two-retry policy: first repair the rejected candidate with - structured feedback, then use a fresh extraction without the candidate if - repair is still rejected. Compare it with repeated repair and the ordinary - retry baseline. -5. Consider a narrowly scoped LLM validator only if human review demonstrates - semantic failures that deterministic checks cannot resolve, such as - distinguishing a true cast from discussion, intent, table chatter, or an - effect inferred from general D&D knowledge. -6. Implement catalog-aware name canonicalization in the D&D normalizer after - catalog-validator behavior is stable; this is separate from retry repair. -7. Define the measurements and thresholds that would justify feedback-aware - repair or semantic LLM validation. The immediate baseline records evidence - but does not need to establish those gates. - -Repair responses should remain complete extraction replacements rather than -patches. Diagnostics must be treated as bounded data, particularly if a future -LLM-backed validator can contribute their text. - -## Resolved Scope Decisions - -- Catalog overlays are part of the immediate implementation, not a later - extension. -- Prompt grounding includes canonical spell names only. Aliases participate - only in deterministic lookup and validation. -- Measurements and decision thresholds for feedback-aware repair and semantic - LLM validation are deferred future work and do not block the baseline. diff --git a/docs/roadmap/dnd-spell-normalization.md b/docs/roadmap/dnd-spell-normalization.md new file mode 100644 index 0000000..e887fb3 --- /dev/null +++ b/docs/roadmap/dnd-spell-normalization.md @@ -0,0 +1,136 @@ +# D&D Spell Normalization + +## Status + +Proposed as the next D&D pipeline milestone. This feature completes the first +domain-specific normalize stage for the spell pipeline before work begins on +NPC and combat-turn artifacts. + +## Objective + +Replace the spell lane's no-op normalizer with a deterministic D&D normalizer +that emits canonical catalog names, removes only clearly identical duplicate +casts, preserves source provenance, and makes every mutation visible through +scoped warnings. + +The milestone should improve the consistency of durable spell output without +adding another LLM call or introducing fuzzy repair policy. + +## Target Behavior + +### Effective catalog construction + +The normalizer constructs the same immutable SRD-plus-overlay effective +catalog used by spell extraction and catalog validation. It declares the +optional `spell_catalog` normalize-stage reference slot and resolves it during +pipeline preparation, before source parsing or module execution. + +An overlay-capable pipeline binds the same catalog file independently at the +extract and normalize stages. This repetition is intentional: references are +stage-local, and the normalizer must not reach into a constructed extractor or +depend on extractor-private state. + +The normalizer contributes its effective catalog digest through +`pipeline.CheckpointFingerprintProvider`. Changing the embedded catalog, +composition policy, or normalize-stage overlay therefore invalidates reusable +normalize checkpoints. + +### Canonical spell names + +For every spell cast, look up the extracted name using the effective catalog's +existing case, whitespace, apostrophe, and alias rules. Replace a recognized +value with its canonical display name. This is the only spell-name repair in +the initial feature. + +Do not use edit distance, phonetic matching, model judgment, or another fuzzy +heuristic. If a value does not resolve, retain it unchanged and emit a scoped +warning; the configured normalize validator chain remains responsible for +acceptance or rejection. + +Emit a warning for each changed spell name. Diagnostics should identify the +artifact index and the original and canonical values without modifying other +fields. + +### Source-reference normalization + +Sort each cast's source references by source identity, start unit, and end +unit, then remove exact duplicate references. Do not merge adjacent or merely +overlapping ranges, because doing so could broaden the evidence attributed to +an event. + +The normalizer must not synthesize source references, alter source-unit +boundaries, or use auxiliary references as evidence. + +### Conservative duplicate collapse + +After name and source-reference canonicalization, treat two casts as the same +event only when all of the following match: + +- canonical spell name; +- caster after case folding and whitespace normalization; and +- the complete canonical source-reference set. + +Collapse each such group into its first occurrence, preserving stable pipeline +order. Retain the first cast's caster, effect, and narrative description. Do +not combine prose fields or select a winner based on length, confidence, or +model-like semantic judgment. The retained cast receives the group's already +canonical source-reference set. + +Emit one scoped warning per collapsed group, including the retained index and +the removed indices. Casts with different evidence remain distinct even when +their spell and caster match. In particular, adjacency at a chunk or scene +boundary is not sufficient evidence of duplication. + +### Production composition and validation + +Register the typed spell normalizer in the D&D family and make it the default +normalizer for the maintained production spell pipeline. Keep the artifact +kind and durable spell-list schema unchanged. + +Add a normalize-stage production validator chain using the existing generic +JSON and JSON Schema validators followed by the existing spell shape, catalog, +source-reference, and source-relatedness validators in their current order. +Explicit validator overrides remain authoritative. + +The maintained overlay-capable example should bind `spell_catalog` at both the +extract and normalize stages. The base-only example should continue to work +without a catalog reference. + +## Acceptance Criteria + +- Base and overlay aliases are written using canonical catalog display names. +- Unknown names are never guessed, deleted, or silently changed. +- Source references are deterministically sorted and exactly deduplicated. +- Only casts with matching canonical spell, normalized caster, and identical + evidence sets collapse; near or adjacent events remain separate. +- Normalization preserves the first cast's non-name fields and stable order. +- Every name change and duplicate collapse emits a scoped warning. +- Extract, normalize, and catalog-validation catalog identities agree for the + same bound references, and catalog changes invalidate normalize checkpoints. +- The maintained production pipeline and current-behavior documentation use + the D&D normalizer without changing the durable artifact schema. + +## Evaluation + +Maintain a small human-reviewed set of representative spell outputs covering +canonical names, aliases, repeated casts, duplicate model output, and scene +boundaries. Use it to review normalization behavior and warnings, not as a +claim that LLM extraction is deterministically correct. + +Record cases where likely duplicates remain because their evidence differs. +Those examples should inform later LLM-assisted deduplication work rather than +causing this deterministic milestone to adopt broader heuristics. + +## Deferred Work + +- Fuzzy correction of unknown spell names. +- Collapsing casts based only on overlapping, adjacent, or semantically similar + evidence. +- LLM-assisted duplicate proposals or prose reconciliation. +- A generic deduplication contract based on stable artifact-element IDs. +- Repair-aware extraction retries or LLM-backed semantic validation. +- Changes to the spell artifact schema, including stable cast IDs. + +An LLM-backed normalizer is explicitly outside this milestone. If later +evaluation justifies one, shared spell-catalog prompt material should be +designed at that time around the actual normalization or repair request. diff --git a/docs/roadmap/future.md b/docs/roadmap/future.md index 1088ae5..7b523f5 100644 --- a/docs/roadmap/future.md +++ b/docs/roadmap/future.md @@ -9,15 +9,12 @@ not as committed release dates. ### Solidify Spell Extraction -- Evaluate the implemented baseline in - [D&D Spell Extraction Quality](dnd-spell-extraction.md), then reconsider its - [deferred retry and validation work](dnd-spell-extraction.md#deferred-retry-and-validation-work). -- Replace the no-op spell normalizer with a D&D-specific implementation that - canonicalizes recognized spell names, performs only high-confidence repairs, - retains uncertain values for review, and emits scoped warnings. -- Conservatively collapse duplicate spell casts when canonical spell, caster, - and source evidence establish that they represent the same event. Merge and - canonicalize their source references rather than relying on spell name alone. +- Implement the deterministic catalog-aware normalizer defined in + [D&D Spell Normalization](dnd-spell-normalization.md), including conservative + exact-evidence duplicate collapse. +- Evaluate ordinary extraction retries and the completed normalization path + against a human-reviewed transcript set before adding repair-aware retries or + an LLM-backed semantic validator. - Maintain a small set of human-reviewed transcripts and outputs for prompt, validator, and normalizer development. Treat model-quality review as an iterative human evaluation aid, not a deterministic correctness gate. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index c4b5561..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,458 +0,0 @@ -# D&D Spell Extraction Quality Implementation Plan - -## Status - -The accepted baseline implementation is complete through the assembled -offline workflow. External quality evaluation remains pending as recorded in -[D&D Spell Extraction Quality](dnd-spell-extraction.md). This plan covers that -baseline, not its deferred repair-aware retry or semantic LLM-validation work. - -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. - -## Objective - -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. - -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. - -## Governing Documents - -Before changing code, read and follow: - -- [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 worktree changes. Do not implement anything listed under -the feature roadmap's deferred retry and validation section. - -## Fixed Design Decisions - -### 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 - ``` - -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/cli ./internal/modules/dnd/extract/spells ./internal/modules/dnd/validate/spells/catalog ./internal/modules/integration - ``` - -Stage 5 is complete when the assembled offline workflow protects catalog -grounding, validation, provenance, retry count, and checkpoint invalidation at -their appropriate test boundaries. - -## Stage 6 — Documentation, evaluation handoff, and completion - -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 overlay transport, file contract, composition policy, preparation -boundary, prompt content, validator behavior, retry baseline, provenance, and -test ownership are fixed by this plan. diff --git a/docs/roadmap/tests.md b/docs/roadmap/tests.md deleted file mode 100644 index 3334458..0000000 --- a/docs/roadmap/tests.md +++ /dev/null @@ -1,461 +0,0 @@ -# Test Suite Policy Review - -## Context - -Notarius now has a canonical [Testing Policy](../policy/testing.md). The policy -defines a risk-based approach that favors durable behavioral coverage while -removing redundant, brittle, misleading, or obsolete tests whose lifetime cost -exceeds their protective value. - -Much of the existing test suite predates that policy. Recent CLI and -configuration work has already applied several of its principles, but the suite -has not been reviewed consistently as a whole. This roadmap calls for that -review before additional domain capabilities materially expand the number of -tests and fixtures. - -The review is not based on a presumption that old tests are bad, that direct -tests of package-private behavior must be removed, or that the suite should be -made smaller at any cost. Existing tests should be retained when they provide -durable and nonredundant protection for a meaningful risk. The goal is a leaner -and clearer allocation of test ownership together with credible protection for -important behavior. - -## Objective - -Review the complete Notarius test suite against the Testing Policy, identify -both low-value coverage and meaningful protection gaps, and define the -justified target revisions. The ordered execution plan is maintained separately -in [Implementation](implementation.md). - -The review must answer: - -- Which important contracts, invariants, failure modes, and integration - boundaries does the current suite protect? -- Where do multiple tests protect the same behavior without providing distinct - failure isolation or integration confidence? -- Which tests are coupled to incidental implementation details, duplicated - policy literals, closed-world inventories, unstable formatting, or mock - choreography? -- Which important risks can still fail silently despite the existing suite? -- Which tests should be retained, consolidated, rewritten, or deleted, and - which new tests are warranted? - -## Review Scope - -Review all committed Go tests and their supporting fixtures. Organize the work -by behavioral layer rather than treating test count or package coverage as the -unit of quality. - -### CLI and configuration - -Review command parsing, configuration decoding and precedence, validation, -effective configuration, run controls, reference selection, output/cache/debug -state, production composition, and maintained examples. - -Pay particular attention to: - -- duplicated assertions across parser, resolver, CLI, and assembled-run tests; -- tests that restate complete defaults or registry contents rather than - protecting operator-visible behavior; -- exact error or output assertions broader than the documented CLI contract; -- fixture mutation that can silently stop establishing a test precondition; - and -- whether representative CLI workflows provide sufficient assembled coverage. - -### Framework and durable state - -Review pipeline preparation and execution, artifact contracts, validation and -retry behavior, LLM scheduling and transport seams, chunk-plan storage, -checkpoint compatibility, debug bundles, and file persistence. - -Presume durable protection is important for data integrity, serialization, -compatibility, cancellation, concurrency, cache correctness, resume behavior, -atomic or failure-safe persistence, and recovery. Look for opportunities to -replace many narrow structural tests with a smaller invariant, round-trip, or -behavior-level test only when protection is not weakened. - -### Modules and domain behavior - -Review generic, seriatim, and D&D module tests, including codecs, chunking, -extraction, merging, normalization, validators, registration, prompt assets, -and module integration. - -Confirm that domain rules and artifact schemas have clear test ownership. -Identify tests that merely reproduce schemas, prompt asset inventories, or -implementation structure, while preserving tests that protect compatibility, -source provenance, normalization, validation, or other consequential domain -invariants. - -### Cross-cutting suite quality - -Across all packages, evaluate: - -- deterministic, offline, credential-free execution; -- isolation from mutable machine and process-global state; -- appropriate use of real collaborators, fakes, stubs, and mocks; -- stable behavioral assertions and useful failure diagnostics; -- golden files and large snapshots; -- helper and fixture complexity; -- test runtime, race safety, repetition stability, and parallel-execution - assumptions; and -- semantic ownership and duplication across package, integration, and - end-to-end layers. - -Use coverage only as a diagnostic to locate unexpectedly untested critical -branches. Do not recommend tests solely to increase a percentage or make -coverage uniform across packages. - -## Review Method - -1. Read the Testing Policy and the canonical documentation for each subsystem - before judging its tests. -2. Establish a clean baseline with the repository validation commands, focused - race tests where concurrency or shared state is relevant, and a coverage - report used only for investigation. -3. Inventory tests and map each meaningful test or closely related group to the - contract, invariant, integration boundary, or regression it protects. -4. Inspect production code only as needed to understand the protected behavior, - identify the stable boundary, and detect untested risk. Do not infer a - contract merely from current implementation detail. -5. Evaluate marginal value across layers. Similar assertions are not redundant - when one owns a package contract and another distinctly proves production - wiring or end-to-end integration. -6. Record evidence for every proposed change. Name the affected test or fixture, - the realistic defect it currently catches or fails to catch, and why the - recommendation improves confidence or reduces unnecessary friction. -7. Check historical context when a test appears unusually specific. Preserve a - regression test when the underlying defect remains plausible and - consequential, even if its purpose is not obvious from the current code. - -Do not modify production code or tests during the review. If the review reveals -incorrect production behavior, report it separately from test-suite -harmonization rather than treating a changed test expectation as the fix. - -## Review Deliverable - -Produce an evidence-backed report organized by priority and subsystem. Each -finding must classify the proposed disposition as one of: - -- **retain:** valuable protection at an appropriate boundary; -- **consolidate:** overlapping protection that can be represented more simply; -- **rewrite:** meaningful protection expressed through a brittle or misleading - boundary; -- **delete:** no sufficient plausible defect or distinct protection justifies - the lifetime cost; or -- **add:** a consequential risk lacks credible protection. - -For consolidate, rewrite, delete, and add findings, describe the protected risk, -current evidence, recommended boundary, and expected effect on confidence and -maintenance. Do not produce a raw list of every test when a package or related -group shares one clear disposition. - -Distinguish required changes from optional cleanup. Absence of a finding is not -evidence that a package needs more tests. - -## Completion Criteria - -The review is complete when: - -- the full suite has been considered at an appropriate behavioral grouping; -- important test ownership and integration boundaries are mapped; -- every recommended change is supported by a concrete risk and evidence; -- high-risk behavior without credible protection is identified; -- redundant or brittle protection is distinguished from valuable intentional - overlap; -- production defects, if any, are reported separately; and -- every target revision is specific enough to support a decision-complete - implementation roadmap. - -## Revisions Needed - -### Review baseline - -The review was completed on 2026-07-18 against 90 committed Go test files, -536 named tests, approximately 21,900 lines of test code, and the committed -fixtures under `testdata/` and `examples/`. - -The baseline is clean: - -- `go test ./...`, `go vet ./...`, and `go build ./cmd/notarius` pass; -- `go test -race` passes for `internal/framework/pipeline`, - `internal/framework/llm`, `internal/cli`, and - `internal/modules/integration`; -- five shuffled repetitions of the full suite pass; and -- the default suite is offline, credential-free, and fast. The only HTTP - behavior uses local test servers or deliberately unreachable loopback - endpoints without making provider calls. - -Coverage was used only to investigate risk. It confirms broad behavioral -coverage in CLI, configuration, source, pipeline, and production module -packages. It also exposes the material checkpoint gap described below: most -filesystem loader paths and all source, merge, and normalize checkpoint -round trips are unexercised. No production defect was identified during this -review. - -### Test ownership that should be retained - -The following overlap is intentional and should remain: - -- CLI command, run, cache, reference, state-hardening, production-composition, - and maintained-example tests own exit classification, option mapping, - physical state placement, default production wiring, and representative - assembled workflows. Package tests continue to own the underlying parsing, - resolution, and persistence rules. -- Configuration contract tests own file decoding, precedence, validation, - redaction, and effective resolution. Pipeline resolver tests own module, - capability, typed-variant, validator-chain, and reference resolution after - configuration has produced a profile. -- Pipeline runner tests own bounded concurrency, deterministic ordering, - cancellation, retries, rejection propagation, checkpoint decisions, debug - recording, candidate/final encoding, and terminal failure behavior. The one - cross-family concurrency test remains valuable because it proves that the - framework worker bound and shared provider scheduler remain independent in - an assembled production-style pipeline. -- Source, chunk-plan store, file I/O, debug-bundle, output encoder, artifact - codec, and manifest tests own data integrity, confinement, serialization, - compatibility, atomic publication, redaction, and round trips. The exact - checkpoint schema identifier and exact artifact-schema digest tests are - compatibility tests, not incidental constant assertions, and must remain. -- Seriatim adapter, D&D scene chunker and spell extractor, D&D validators, - generic chunker and validators, prompt preparation, and durable spell codec - tests own their domain rules and external or durable schemas. Leaf - constructor/spec/registration tests remain justified because the extension - contract explicitly requires each leaf to be independently buildable. -- The centralized production import-boundary test remains a valuable - executable architecture rule. Its synthetic rejected fixture is necessary - to prove that the checker itself is active. - -Absence from the revisions below means the current behavioral group has an -appropriate owner and no justified change was found. - -### Required findings - -#### P0 - add filesystem checkpoint compatibility and recovery coverage - -**Disposition: add.** `internal/framework/checkpoint/recorder_test.go` currently -proves only that an empty successful extract checkpoint can be written and -loaded. Coverage confirms that `FilesystemLoader.Source`, `Merge`, `Normalize`, -manifest validation, payload digest validation, and most recorder status paths -are otherwise silent. CLI resume coverage proves root selection and one happy -reuse, while pipeline checkpoint tests use controlled collaborators; neither -owns the filesystem format. - -Add package-level tests at the real recorder/loader boundary that: - -- prove `NewIdentity` is deterministic under reordered lanes, references, and - fingerprints, changes when pipeline, source/input, selected lanes, runtime - overrides, references, or provenance change, and produces a confined stable - relative path; -- round-trip representative source, extract, merge, and normalize payloads, - including serialized artifact identity, warnings, extract rejections, - metadata, and dependency fingerprints; -- prove caller mutation cannot alter recorded or loaded values and retain the - existing restrictive-permission expectation through real files; and -- mutate one persisted artifact at a time to prove that missing or malformed - JSON, old or unknown workspace schema versions, wrong identity/stage/lane/ - module/status/dependencies, incomplete codec identity, invalid base64, and - content/output-digest mismatches yield a non-reused decision with useful - category context rather than a panic or silent reuse. - -Use relationships and category fragments rather than snapshotting complete -manifests or error sentences. This addition protects cache correctness, -compatibility, recovery, and sensitive durable state; it is not intended to -raise a coverage percentage. - -#### P1 - remove tests that exercise only their own fakes or obsolete APIs - -**Disposition: delete.** Remove the fake-behavior tests in -`internal/framework/contracts/contracts_test.go`: -`TestFakeExtractorReturnsTypedOutput`, `TestFakeChunkerReturnsSourcePlan`, -`TestFakeChunkerReceivesPerRunContext`, -`TestFakeExtractorReceivesChunkAndAmbientContext`, -`TestFakeMergeNormalizeAndOutputContracts`, and `TestReferenceSetDataTypes`. -They detect changes in test helpers or Go struct assignment, not defects in -production contracts. Retain the compile-time interface assertions and the -clone, JSON omission, and content-ownership tests, which protect type -compatibility and non-leakage invariants. - -**Disposition: delete.** Remove -`internal/framework/llm/scriptorium_api_test.go`. It was introduced as -pre-adapter API grounding and still mirrors unused Scriptorium request/result -fields, option constructors, and sentinel errors. Production compilation now -grounds the API actually used, while `scriptorium_client_test.go` and module -prompt-preparation tests protect adapter mapping, local integration, -cancellation, validation, profile selection, and credential redaction. - -**Disposition: delete.** Remove -`TestHelpersReturnValidationResults` from -`internal/framework/validate/validate_test.go`; it is a compile-only assignment -that adds no protection beyond the function signatures and the two behavioral -helper tests. - -**Disposition: delete.** Remove the direct -`TestTypedNormalizerPreservesValue` test for the generic no-op normalizer. Its -single assignment is already exercised through resolver, runner, production -composition, and maintained-example output tests. Retain the direct append-order -merger test because input ordering is a meaningful transformation invariant. - -#### P1 - remove misleading D&D integration claims - -**Disposition: delete.** In -`internal/modules/integration/dnd_spells_runner_test.go`, delete: - -- `TestRunnerDoesNotExtractSpellMentionedOnlyInPartyReference`, whose fake LLM - is programmed to return no spells and therefore cannot prove the claimed - extraction policy; -- `TestRunnerCarriesDNDSpellCastWithInvalidSourceRefToSerializedOutput`, which - omits the production validator chain and duplicates the extractor's explicit - invalid-evidence handoff test; and -- `TestRunnerRejectsMalformedDNDSpellsArtifactAtSerializationBoundary`, whose - final-codec invariant is already owned by the codec and runner candidate/ - final-encoding tests. - -Retain the representative Seriatim-to-spell assembled workflow and the test -that proves party/glossary references and provenance cross the production -module boundary. The policy that reference material is not source evidence is -durably protected by extractor mapping, prompt-input assertions, source-ref -validators, and the production validator-chain composition test; it cannot be -proved by controlling an LLM stub's semantic answer. - -#### P1 - consolidate superseded configuration and CLI state tests - -**Disposition: consolidate.** Delete `internal/core/config/v3_test.go` after -moving its two distinct protections into the current contract owners: - -- add the version-2 migration rejection case to the strict file-decoding cases - in `file_config_contract_test.go`; and -- ensure the positive per-user chunk-plan/checkpoint root separation remains - in `env_contract_test.go`. - -The remaining default, precedence, invalid-source, redaction, and removed-field -assertions are already more completely owned by `file_config_contract_test.go`, -`env_contract_test.go`, `validation_contract_test.go`, and -`redaction_test.go`. - -**Disposition: consolidate.** Delete `internal/cli/state_surfaces_test.go`. -Move the shared `emptyLookup` helper to -`contract_test_helpers_test.go`. Its debug flag syntax, allocation timing, -no-debug absence, version-3 validation, and removed-field cases are already -owned by `run_contract_test.go`, `state_hardening_test.go`, -`command_contract_test.go`, and the configuration contract tests. Preserve the -state matrix, failure retention, no-debug terminal-writer, and pre-resolution -debug allocation protections in those current owners. - -**Disposition: rewrite.** Keep -`TestDefaultCLICompositionResolvesMaintainedConfigurations` as a narrow default -composition smoke test, but run one representative config-validation command -through empty `Options` rather than revalidating every maintained example. -`example_contract_test.go` owns both maintained examples. Remove the repeated -maintained-example loops from `TestProductionCatalogCoversMaintainedConfigurations` -and `TestProductionConfigValidationCoversModuleAndVariantFailures`; retain the -production registry/codec/default-chain checks and the distinct production -failure cases. - -#### P1 - consolidate resolver and module-composition duplication - -**Disposition: delete.** Remove -`internal/framework/pipeline/default_modules_test.go`. Its large local catalog -only re-proves default binding selection, already owned by -`profile_test.go` and `effective_config_contract_test.go`; production default -keys and wiring are separately exercised by leaf registration, the production -catalog, default CLI composition, and the maintained end-to-end example. - -**Disposition: delete.** Remove -`internal/modules/seriatim/input/transcript/config_test.go`. It builds an -entire fake catalog to repeat generic resolver success, deterministic digest, -missing-capability, and unknown-lane behavior. Retain Seriatim parsing and leaf -registration tests, the Seriatim runner integration, D&D cross-family -capability tests, resolver contract tests, and production CLI examples. - -**Disposition: consolidate.** In -`internal/modules/integration/dnd_spells_config_test.go`, retain only the two -cross-family capability contracts: Seriatim must provide `source.transcript` -to the spell extractor, and the spell extractor must provide -`dnd.spell_casts` to append-order. Express them as one compact table over a -programmatic profile/catalog. Remove the generic successful-resolution, -stable-digest, and unknown-lane cases, which are already exercised by the -retained runner integration and resolver/configuration owners. - -#### P1 - replace brittle assertions with behavioral ones - -**Disposition: rewrite.** In `internal/core/source/source_test.go`, make -`ValidateRef` the owner of malformed reference categories. `ValidateDocument` -should retain one propagation/context case plus the document-only self-reference -invariant instead of repeating missing, foreign, and reversed-reference cases. -Replace complete internal error-sentence equality with the smallest stable -category and field fragments. Retain exact acceptance/rejection boundaries, -duplicate detection, deterministic digests, and reference ordering. - -**Disposition: rewrite.** Replace the decoded-schema-structure walk in -`internal/modules/dnd/chunk/scenes/schema_test.go` with representative JSON -Schema validation: accept a valid source-unit-boundary response and reject old -segment fields, non-positive bounds, invalid enums, empty caveats, and unknown -properties. Likewise, change the private spell response-schema assertion in -`internal/modules/dnd/extract/spells/schema_test.go` to accept the LLM transport -shape without `source_id` and reject a response that supplies canonical -`source_id`. Retain schema identity/hash, mutation safety, diagnostics -non-leakage, DTO decoding, and the codec's durable fixture tests. These rewrites -preserve schema regression protection without coupling tests to nested map -layout or unsafe type assertions. - -**Disposition: rewrite.** In `internal/framework/llm/schema_registry_test.go`, -assert that returned keys are sorted and required framework test schemas are -present without asserting that the registry contains exactly two entries. -Retain the explicit negative D&D lookup because it protects the framework/ -domain ownership boundary. - -#### P2 - make composition and architecture tests open to legitimate extension - -**Disposition: rewrite.** The family registrar tests in -`internal/modules/generic/register`, `internal/modules/seriatim/register`, and -`internal/modules/dnd/register` currently assert closed-world key and asset -inventories that duplicate the CLI production catalog. Change them to require -the family-owned registrations they need, permit unrelated future additions, -and prove representative entries can be built or prepared. Keep exact order -for the D&D default spell validator chain because that order is documented -production policy. Keep missing-dependency-before-mutation and contextual -duplicate-registration cases. - -**Disposition: consolidate.** Move the source/framework independence rules -from `internal/framework/chunkplan/import_boundaries_test.go` into the -centralized `internal/modules/import_boundaries_test.go` checker and delete the -second repository walker. Preserve both unique rules: `internal/core/source` -may import neither framework nor modules, and `internal/framework/chunkplan` -may not import modules. Add rule-level cases so a broken checker still fails. - -#### Documentation alignment - -**Disposition: rewrite.** When the affected tests move or are deleted, update -the `Tests To Inspect` sections in `docs/internal/state.md`, -`docs/internal/pipeline.md`, `docs/internal/modules.md`, and -`docs/internal/llm.md`. They currently name files such as -`internal/cli/state_surfaces_test.go` and `internal/cli/run_test.go` that will -be deleted or do not exist. Point each document at the retained contract, -state-hardening, production-composition, example, checkpoint, and integration -owners without recreating an exhaustive test inventory. - -### Optional cleanup - -No additional cleanup is recommended now. In particular, do not mechanically -convert the large resolver and runner suites to table-driven form, merge all -leaf registration tests into family registrars, add tests for trivial accessor -coverage, or introduce a golden-update framework. Those changes do not provide -enough additional confidence to justify their immediate cost. - -### Target state - -The revision is complete when: - -- every checkpoint stage has a real filesystem round trip and incompatible or - corrupted state is demonstrably recomputed rather than silently reused; -- all protection named as retained above remains present at its stated owner; -- the fake-only, obsolete, misleading, duplicate, and closed-world assertions - named in the required findings are removed or rewritten exactly as specified; -- maintained examples are each owned by one example contract plus one narrow - default-composition smoke path, rather than repeated across production tests; -- default tests remain deterministic, offline, credential-free, order - independent, and race-clean; -- failures identify the violated behavioral category without snapshotting - complete incidental diagnostics; -- internal documentation points to existing retained test owners; and -- the suite reaches this state without changing production behavior.