diff --git a/docs/integrations/dnd-spell-artifacts.md b/docs/integrations/dnd-spell-artifacts.md index d1f0588..cf9cbef 100644 --- a/docs/integrations/dnd-spell-artifacts.md +++ b/docs/integrations/dnd-spell-artifacts.md @@ -101,6 +101,13 @@ normalizer uses these reason codes: | `source_references_normalized` | Reference order changed or exact duplicate references were removed. | | `duplicate_spell_cast_collapsed` | A later cast matched the retained cast's complete duplicate identity. | +Only warnings from an accepted normalize attempt are promoted to +`warnings.json`. If an unresolved name reaches the default normalize validator +chain, the catalog validator rejects the candidate with `unknown_spell`; the +`spell_name_unresolved` warning remains in the attempt's debug artifact. An +explicit validator override that accepts the candidate promotes the unresolved +warning normally. + ## Manifest Metadata The extractor adds prompt and response-schema provenance under the artifact lane diff --git a/docs/roadmap/dnd-spell-normalization.md b/docs/roadmap/dnd-spell-normalization.md deleted file mode 100644 index 37eb859..0000000 --- a/docs/roadmap/dnd-spell-normalization.md +++ /dev/null @@ -1,151 +0,0 @@ -# D&D Spell Normalization - -## Status - -Implemented deterministic feature. Implementation details are maintained -separately in `docs/roadmap/implementation.md`. - -## 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 normalizer improves 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. It also records catalog base ID, digest, and overlay IDs -as manifest metadata. Neither identity surface includes catalog contents or -reference paths. - -### 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 -this 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. Diagnostics are deterministic and bounded: user-controlled spell names -are truncated to 128 Unicode code points before display. - -### 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. Reordering references or -removing exact duplicates emits a scoped warning for the affected input cast. - -### 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, which must be non-empty and - valid against the source document. - -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. - -Unknown spell names, empty evidence sets, and invalid source references cannot -establish duplicate identity. They remain in the artifact for the configured -normalize validators to accept or reject. A duplicate warning displays at most -20 removed input indices and reports the exact omitted count. - -### 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 - -- [x] Base and overlay aliases are written using canonical catalog display names. -- [x] Unknown names are never guessed, deleted, or silently changed. -- [x] Source references are deterministically sorted and exactly deduplicated. -- [x] Only casts with matching canonical spell, normalized caster, and identical - non-empty valid evidence sets collapse; near, invalid, unknown, or adjacent - events remain separate. -- [x] Normalization preserves the first cast's non-name fields and stable order. -- [x] Every name change, unresolved name, source-reference mutation, and duplicate - collapse emits a scoped warning. -- [x] Extract, normalize, and catalog-validation catalog identities agree for the - same bound references, and catalog changes invalidate normalize checkpoints. -- [x] The maintained production pipeline and current-behavior documentation use - the D&D normalizer without changing the durable artifact schema. - -## Evaluation - -Status: pending qualitative review. No approved human-reviewed transcript -corpus is available locally under repository policy, so no sensitive transcript -content is committed and no claims about real transcript quality are made. - -A compact deterministic fixture set covers canonical names, exact source -reference normalization, repeated casts, duplicate output, and distinct -evidence. It exercises the normalizer contract without requiring paid or -network LLM calls. - -Record cases where likely duplicates remain because their evidence differs. -Those examples should inform later LLM-assisted deduplication work rather than -causing this deterministic feature 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 feature. 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/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index b504b04..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,388 +0,0 @@ -# D&D Spell Normalization Implementation Plan - -## Status - -Ready for implementation. - -This plan implements the accepted target state in -[`dnd-spell-normalization.md`](dnd-spell-normalization.md). That document owns -feature intent and policy; this document owns implementation order and concrete -engineering decisions. Implement each stage in sequence and keep every stage -buildable and testable before proceeding. - -## Scope and invariants - -Add a deterministic `dnd/spells` normalize-stage module. It materializes the -same effective embedded-base-plus-overlay spell catalog used by extraction, -canonicalizes catalog-backed spell names, canonicalizes exact source-reference -sets, and conservatively collapses duplicate spell casts. It does not call an -LLM and does not add a prompt asset. - -Preserve these boundaries throughout the work: - -- Keep domain behavior under `internal/modules/dnd`; do not add D&D concepts to - the pipeline framework or generic contracts. -- Keep the fixed pipeline architecture unchanged. This work implements an - existing normalize stage; it does not introduce branches or DAG behavior. -- Keep `dnd.SpellList` and its JSON schema unchanged. Do not add artifact IDs, - schema constraints, or fields used only for normalization. -- Keep existing extractor, merger, validator, retry, and checkpoint formats - unchanged unless this plan explicitly identifies an integration adjustment. -- Do not add fuzzy spell matching, caster entity resolution, source-range - merging, prose reconciliation, generic duplicate normalization, or LLM-backed - validation. -- Do not mutate request values or slices that may share backing storage with an - upstream result. Construct an independent normalized result. -- Preserve nil and empty spell/reference collections rather than repairing - malformed shape. Validators own shape and source-validity rejection. -- Keep warnings stable, deterministic, bounded, and free of catalog content, - reference paths, or other sensitive material. - -The normalize-stage public selector is `dnd/spells`. Stage-specific registries -make this unambiguous despite the extractor using the same selector. - -## Stage 1: Typed normalizer and catalog contract - -Create `internal/modules/dnd/normalize/spells` as a typed normalize module. -Follow the construction and registration patterns used by the existing D&D -spell extractor and deterministic normalizers. - -### Module construction - -- Define an empty `Options` type and use the repository's strict option decoder - so unknown configuration fields are rejected. -- Expose a constructor that accepts decoded options and the prepared - `contracts.ReferenceSet`. -- Resolve the effective catalog during module construction with the existing - `internal/modules/dnd/spells/catalog` resolver. Do not reimplement catalog - parsing, alias handling, or digest calculation. -- Declare one optional reference slot named `spell_catalog`, matching the - extractor's media type and size limit. Do not declare campaign references the - normalizer does not consume. -- Make malformed, duplicated, oversized, or otherwise invalid catalog - references fail during pipeline preparation, before run-state or checkpoint - composition. -- Register the module as `pipeline.StageNormalize`, artifact kind - `dnd.SpellListKind`, under selector `dnd/spells`. -- Do not require or consume an LLM profile. The normalizer remains fully - deterministic and offline. - -### Prepared identity and audit metadata - -Implement `pipeline.CheckpointFingerprintProvider`. Contribute exactly one -local fingerprint: - -- name: `effective_catalog` -- value: the effective catalog's semantic digest - -Preparation scopes this fingerprint by the normalize-stage component identity. -Return defensive data according to the provider contract. - -Also implement `contracts.ManifestMetadataProvider` with the same non-secret -audit fields used by catalog-backed extraction components: - -- `catalog_base_id` -- `catalog_digest` -- `catalog_overlay_ids` - -Return defensive copies of collection values. Do not include catalog entries, -raw reference content, or local paths in either identity or metadata. - -### Name canonicalization - -For each input spell cast, call the effective catalog's existing `Lookup` -operation. This intentionally inherits its case, whitespace, apostrophe, and -explicit-alias policy. - -- If lookup succeeds, write the catalog's canonical display name to the output. -- If the input is already byte-for-byte equal to the canonical name, emit no - name warning. -- If lookup succeeds and changes the value, emit reason code - `spell_name_canonicalized` at scope `spell_casts[N]`, where `N` is the merged - input index. The message identifies the input index, bounded original value, - and canonical replacement. -- If lookup fails, preserve the original value exactly and emit reason code - `spell_name_unresolved` at scope `spell_casts[N]`. Do not guess, drop, or - rewrite it. - -Before quoting either an extracted name or an overlay-provided canonical name -in a warning, limit it to 128 Unicode code points by retaining the first 127 and -appending a visible ellipsis. Never truncate by byte position or produce invalid -UTF-8. Use normal Go quoting so control characters cannot corrupt diagnostic -structure. - -### Source-reference canonicalization - -For every cast, copy and sort references by this exact tuple: - -1. `SourceID`, compared as an exact string; -2. `StartUnitID`; -3. `EndUnitID`. - -Then remove exact structural duplicates. Do not trim or rewrite `SourceID`, -alter boundaries, merge adjacent or overlapping ranges, or consult auxiliary -reference text. Preserve nil as nil and an empty non-nil slice as empty. - -If sorting changes order or exact duplicates are removed, emit one warning with -reason code `source_references_normalized` at `spell_casts[N]`. Its message -reports the input index, original count, final count, whether order changed, and -the duplicate-removal count. Do not emit it when the canonical result is -structurally identical to the input. - -### Stage 1 tests - -Add focused package tests that establish: - -- strict option decoding and the exact module/reference-slot contract; -- successful embedded-base and overlay catalog construction; -- preparation-time failure for invalid catalog references; -- checkpoint fingerprint and manifest metadata values and defensive copying; -- canonical, case/whitespace/apostrophe, and overlay-alias lookups; -- canonicalization and unresolved-name warning semantics; -- safe truncation of long Unicode names and quoting of control characters; -- reference tuple ordering and exact de-duplication; -- no merging of adjacent or overlapping ranges; -- source-normalization warning semantics; -- preservation of nil versus empty collections and input immutability; and -- the repository's standard nil receiver, nil context, and already-canceled - context behavior for module calls. - -Do not assert entire warning prose when reason code, scope, and semantic message -fragments are the durable contract. - -## Stage 2: Conservative duplicate collapse - -Perform duplicate detection after name and source-reference canonicalization. -Use original merged input positions as stable indices for warnings and retained -entry selection. - -### Duplicate identity - -A cast is eligible for duplicate grouping only when all of these hold: - -- its spell name resolved successfully through the effective catalog; -- its canonical source-reference set is non-empty; and -- every canonical source reference passes `source.ValidateRef` against the - normalize request's source document. - -Eligible casts are duplicates only when all of these keys are exactly equal: - -- canonical catalog display name; -- normalized caster key; and -- complete canonical source-reference set. - -Build the caster key by applying `strings.Fields`, joining with one ASCII space, -and then applying `golang.org/x/text/cases.Fold`. This implements Unicode case -folding rather than locale-specific lowercasing. Make `golang.org/x/text` a -direct module dependency when importing it; it is already present indirectly. -Leave output caster text untouched. Do not remove punctuation, apply aliases, -or attempt entity resolution. - -Compare source-reference sets only after the deterministic sorting and exact -de-duplication from Stage 1. Exact equality includes source identity and both -boundaries. Unknown names, empty evidence, or any invalid source reference make -a cast ineligible for duplicate grouping; the cast remains in output so the -configured normalize validators can decide its validity. - -### Collapse behavior - -- Retain the first input occurrence of each duplicate group and preserve stable - output order. -- The retained artifact is its normalized Stage 1 copy. Preserve that first - occurrence's caster, effect, and narrative verbatim. -- Do not union sources, combine prose, select a “better” occurrence, or use - adjacency/overlap as duplicate evidence. -- Preserve all casts that do not meet the complete duplicate identity. - -Emit one `duplicate_spell_cast_collapsed` warning per collapsed group. Scope it -to the retained input cast (`spell_casts[R]`) and identify the retained input -index and removed input indices. Order group warnings by retained input index. -Display at most 20 removed indices and report the exact omitted count when more -exist. - -Warning order for the complete normalizer is: - -1. Walk input casts in input order. For each cast, emit its name warning, if - any, followed by its source-reference warning, if any. -2. Emit duplicate-group warnings ordered by retained input index. - -Per-cast warnings remain present even when that input cast is later removed by -duplicate collapse; warning scopes deliberately refer to merged input indices. - -### Stage 2 tests - -Add table-driven and focused behavioral tests for: - -- two- and three-member duplicate groups; -- caster case and whitespace normalization without output caster rewriting; -- first-occurrence retention, stable output ordering, and preservation of the - first occurrence's non-identity fields; -- distinct canonical spells, casters, or evidence sets remaining separate; -- exact evidence equality across differently ordered or duplicated input refs; -- unknown spells, empty evidence, and invalid refs never collapsing; -- no collapse based only on adjacent or overlapping evidence; -- no source union or prose combination; -- deterministic warning ordering and input-index semantics; -- the 20-index warning display bound and exact omitted count; and -- output/input slice independence under mutation checks. - -Include an idempotence test: normalizing an already normalized successful result -does not change artifacts or emit new mutation warnings. - -## Stage 3: D&D defaults and maintained configurations - -Update the D&D registrar without changing global framework defaults. - -### Registration and validator composition - -- Add the spell normalizer package to the D&D registrar and register its - normalize-stage variant. -- Register this default validator chain for stage `normalize`, module - `dnd/spells`, in this exact order: `json`, `schema`, - `extract/dnd/spells/shape`, `extract/dnd/spells/catalog`, - `extract/dnd/spells/source_refs`, and - `extract/dnd/spells/source_relatedness`. -- Reuse the existing validator implementations and selectors. Do not rename - them merely because they are now also used after normalization. -- Leave the extractor's default validator chain unchanged. -- Preserve explicit pipeline validator overrides as authoritative; defaults are - used only when configuration does not supply an override. -- Leave the global/default `noop` normalizer behavior unchanged for all other - pipelines and lanes. - -The relatedness validator may conservatively warn when a transcript uses an -alias while the artifact contains the canonical spelling. That warning is -non-fatal and does not justify changing validator behavior in this scope. - -### Maintained examples - -Update maintained D&D spell configurations as follows: - -- The embedded-base example explicitly selects `normalize: dnd/spells` and - requires no normalize-stage catalog reference. -- The overlay example independently binds the same catalog document under - `pipelines..artifacts..normalize.references.spell_catalog` as - it does under extraction. Do not rely on implicit cross-stage reference - sharing. - -Ensure both maintained examples pass strict configuration loading and effective -pipeline materialization. Do not add an LLM profile to the deterministic -normalizer. - -### Stage 3 tests - -Add or update registration and configuration tests to prove: - -- `dnd/spells` resolves as the typed spell normalizer; -- its exact normalize default validator order is stable; -- existing extract defaults remain unchanged; -- explicit normalize validator overrides remain authoritative; -- the base example works without `spell_catalog` at normalize; and -- the overlay example binds its catalog independently to both extraction and - normalization. - -Prefer asserting semantic scope and values over global literal counts that -become brittle whenever another prepared component contributes catalog -identity. - -## Stage 4: Assembled execution, provenance, and checkpoint identity - -Add an offline assembled-pipeline test using deterministic or fake upstream -output. The fixture must exercise records crossing chunk/merge boundaries -without an external LLM call. Verify that the normalizer: - -- receives merged spell casts; -- canonicalizes a catalog alias or spelling variant; -- normalizes reference order; -- collapses only an exact supported duplicate; -- leaves different evidence as a distinct event; -- returns warnings through the existing run result and manifest path; and -- produces output accepted by the default normalize validator chain. - -Add a separate assertion that an explicit normalize validator override remains -in force in an assembled pipeline. - -Extend checkpoint and provenance coverage to show: - -- the normalizer contributes its independently scoped `effective_catalog` - fingerprint; -- catalog-backed extractor and validators continue to contribute their own - independently scoped identities; -- all components materialized from the same reference set report the same - semantic digest; -- changing effective catalog semantics changes run/checkpoint identity and - prevents normalize-checkpoint reuse; -- semantically equivalent catalog material retains the semantic digest under - the existing catalog rules; -- raw overlay reference provenance remains present independently of the - semantic catalog fingerprint; and -- the manifest contains the normalizer's base ID, digest, and overlay IDs. - -Do not change checkpoint schema, layout, compatibility, or the component -fingerprint framework. If an existing test assumes an exact total fingerprint -count, replace that brittle assertion with scoped name/value assertions while -retaining checks for LLM-profile and reference provenance. - -## Stage 5: Documentation and evaluation fixtures - -Update current-state documentation only as behavior becomes implemented: - -- `docs/config.md`: document `dnd/spells` as a normalize selector, its optional - stage-local `spell_catalog` reference, and its default validator chain. -- `docs/internal/modules.md` and `docs/internal/overview.md`: add the typed - normalizer, deterministic behavior, catalog dependency, metadata, and - checkpoint identity where each document's current-behavior scope requires it. -- `docs/integrations/dnd-spell-artifacts.md`: document canonical-name behavior, - exact source-reference normalization, conservative duplicate identity, - preservation rules, and warning reason codes. -- Maintained examples: ensure their comments explain base-only versus - independently bound overlay behavior. -- `docs/roadmap/dnd-spell-normalization.md`: mark implemented acceptance items - complete while leaving any unevaluated human-review claims explicitly - pending. -- `docs/roadmap/future.md`: remove or revise only entries made obsolete by this - completed feature. Leave deferred LLM, fuzzy, generic-normalizer, and richer - reconciliation work in the future roadmap. - -Do not create -`internal/modules/dnd/shared/assets/prompts/common-dnd-spells.md` in this -implementation. No component in scope makes an LLM call, so adding the prompt -would create an unused contract and would not improve backend cache reuse. - -Add a compact deterministic fixture set covering the normalizer's accepted -input/output behavior. Do not fabricate claims about real transcript quality or -make paid or network LLM calls. If an approved human-reviewed transcript corpus -is available locally and repository policy permits its use, record aggregate -observations in the feature roadmap without committing sensitive transcript -content. Otherwise leave the qualitative evaluation item pending and state -why. - -## Verification gate - -At the end of every stage, run the narrowest directly affected package tests. -Before declaring the implementation complete, run: - -```text -git diff --check -go test ./... -go vet ./... -go build ./cmd/notarius -``` - -Run targeted race detection for the affected execution surfaces: - -```text -go test -race ./internal/modules/dnd/... ./internal/framework/pipeline ./internal/cli ./internal/modules/integration -``` - -All tests must remain offline and deterministic. This targeted command -supplements, rather than changes, the repository-wide validation contract in -`docs/development.md`. - -Review the final diff for accidental framework expansion, schema changes, -prompt assets, generated output, local paths, or unrelated worktree edits. - -## Open Questions - -None. The feature roadmap and the decisions above are sufficient to implement -the work without further product or architecture choices. diff --git a/internal/cli/assembled_spell_pipeline_contract_test.go b/internal/cli/assembled_spell_pipeline_contract_test.go index 999d50c..61eeffe 100644 --- a/internal/cli/assembled_spell_pipeline_contract_test.go +++ b/internal/cli/assembled_spell_pipeline_contract_test.go @@ -22,7 +22,7 @@ import ( const assembledSpellExtractorKey = "test/dnd/spell-casts" func TestAssembledSpellPipelineNormalizesMergedCasts(t *testing.T) { - registries, resolved, extractor := assembledSpellPipeline(t, false) + registries, resolved, extractor := assembledSpellPipeline(t, assembledSpellPipelineOptions{}) prepared, err := pipeline.Prepare(resolved, registries, pipeline.ModuleDependencies{}) if err != nil { t.Fatalf("Prepare() error = %v, want nil", err) @@ -102,7 +102,7 @@ func TestAssembledSpellPipelineNormalizesMergedCasts(t *testing.T) { } func TestAssembledSpellPipelineHonorsNormalizeValidatorOverride(t *testing.T) { - registries, resolved, _ := assembledSpellPipeline(t, true) + registries, resolved, _ := assembledSpellPipeline(t, assembledSpellPipelineOptions{normalizeValidatorOverride: true}) var normalizeChain *pipeline.ResolvedValidatorChain for index := range resolved.ValidatorChains { chain := &resolved.ValidatorChains[index] @@ -134,10 +134,84 @@ func TestAssembledSpellPipelineHonorsNormalizeValidatorOverride(t *testing.T) { } } -func assembledSpellPipeline(t *testing.T, override bool) (pipeline.Registries, pipeline.ResolvedPipeline, *assembledSpellExtractor) { +func TestAssembledSpellPipelineRejectsUnknownSpellWithoutPromotingAttemptWarning(t *testing.T) { + registries, resolved, _ := assembledSpellPipeline(t, assembledSpellPipelineOptions{unknownSpell: true}) + prepared, err := pipeline.Prepare(resolved, registries, pipeline.ModuleDependencies{}) + if err != nil { + t.Fatalf("Prepare() error = %v, want nil", err) + } + output, err := pipeline.New().Run(context.Background(), pipeline.RunInput{ + Prepared: prepared, + RawInput: readRepositoryFile(t, "examples", "seriatim-minimal-transcript.json"), + ChunkCacheMode: pipeline.ChunkCacheBypass, + }) + if err != nil { + t.Fatalf("Run() error = %v, want nil", err) + } + if output.Manifest.ValidationStatus != "rejected" || len(output.NormalizeOutputs) != 0 || len(output.Rejected) != 1 { + t.Fatalf("run output = %#v, want one rejected normalize candidate and no normalized output", output) + } + rejection := output.Rejected[0] + if rejection.Stage != string(pipeline.StageNormalize) || rejection.LaneID != "spells" || rejection.ModuleKey != spellnormalize.Key || rejection.ValidatorName != "extract/dnd/spells/catalog" || rejection.ReasonCode != "unknown_spell" { + t.Fatalf("rejection = %#v, want durable normalize catalog rejection", rejection) + } + rejectedFile := decodeAssembledOutput[struct { + Rejected []contracts.RejectedOutput `json:"rejected"` + }](t, output.OutputFiles, "rejected.json") + if !reflect.DeepEqual(rejectedFile.Rejected, output.Rejected) { + t.Fatalf("rejected file = %#v, run rejections = %#v, want durable rejection diagnostic", rejectedFile.Rejected, output.Rejected) + } + for _, warning := range output.Warnings { + if warning.ReasonCode == spellnormalize.ReasonCodeSpellNameUnresolved { + t.Fatalf("warnings = %#v, want rejected-attempt warning to remain non-durable", output.Warnings) + } + } +} + +func TestAssembledSpellPipelinePromotesUnknownSpellWarningWhenOverrideAccepts(t *testing.T) { + registries, resolved, _ := assembledSpellPipeline(t, assembledSpellPipelineOptions{normalizeValidatorOverride: true, unknownSpell: true}) + prepared, err := pipeline.Prepare(resolved, registries, pipeline.ModuleDependencies{}) + if err != nil { + t.Fatalf("Prepare() error = %v, want nil", err) + } + output, err := pipeline.New().Run(context.Background(), pipeline.RunInput{ + Prepared: prepared, + RawInput: readRepositoryFile(t, "examples", "seriatim-minimal-transcript.json"), + ChunkCacheMode: pipeline.ChunkCacheBypass, + }) + if err != nil { + t.Fatalf("Run() error = %v, want nil", err) + } + if output.Manifest.ValidationStatus != "approved" || len(output.Rejected) != 0 || len(output.NormalizeOutputs) != 1 { + t.Fatalf("run output = %#v, want accepted unknown spell with explicit validator override", output) + } + var normalized dnd.SpellList + if err := json.Unmarshal(output.NormalizeOutputs[0].Artifact.Content, &normalized); err != nil { + t.Fatalf("decode normalized output: %v", err) + } + if len(normalized.SpellCasts) != 1 || normalized.SpellCasts[0].Spell != "Mysterious Burst" { + t.Fatalf("normalized casts = %#v, want unresolved name preserved", normalized.SpellCasts) + } + if len(output.Warnings) != 1 || output.Warnings[0].ReasonCode != spellnormalize.ReasonCodeSpellNameUnresolved || output.Warnings[0].Scope != "spell_casts[0]" { + t.Fatalf("warnings = %#v, want promoted scoped unresolved-name warning", output.Warnings) + } + warningsFile := decodeAssembledOutput[struct { + Warnings []contracts.Warning `json:"warnings"` + }](t, output.OutputFiles, "warnings.json") + if !reflect.DeepEqual(warningsFile.Warnings, output.Warnings) { + t.Fatalf("warnings file = %#v, run warnings = %#v, want durable unresolved-name warning", warningsFile.Warnings, output.Warnings) + } +} + +type assembledSpellPipelineOptions struct { + normalizeValidatorOverride bool + unknownSpell bool +} + +func assembledSpellPipeline(t *testing.T, options assembledSpellPipelineOptions) (pipeline.Registries, pipeline.ResolvedPipeline, *assembledSpellExtractor) { t.Helper() components := productionTestComponents(t) - extractor := &assembledSpellExtractor{} + extractor := &assembledSpellExtractor{unknownSpell: options.unknownSpell} if err := pipeline.RegisterExtractor[dnd.SpellList](components.registries.Extractors, pipeline.ModuleSpec{ Key: assembledSpellExtractorKey, Stage: pipeline.StageExtract, @@ -151,7 +225,7 @@ func assembledSpellPipeline(t *testing.T, override bool) (pipeline.Registries, p } normalize := pipeline.Binding(spellnormalize.Key) - if override { + if options.normalizeValidatorOverride { normalize.Validators = pipeline.ValidatorOverride{ Set: true, Validators: []pipeline.ModuleBinding{pipeline.Binding("generic/always_accept")}, @@ -175,6 +249,7 @@ func assembledSpellPipeline(t *testing.T, override bool) (pipeline.Registries, p type assembledSpellExtractor struct { mu sync.Mutex chunkIndexes []int + unknownSpell bool } func (e *assembledSpellExtractor) Key() string { return assembledSpellExtractorKey } @@ -193,6 +268,14 @@ func (e *assembledSpellExtractor) Extract(ctx context.Context, req contracts.Typ e.mu.Unlock() refOne := source.SourceRef{SourceID: req.Source.ID, StartUnitID: 1, EndUnitID: 1} refTwo := source.SourceRef{SourceID: req.Source.ID, StartUnitID: 2, EndUnitID: 2} + if e.unknownSpell { + if req.Chunk.Index == 0 { + return contracts.TypedExtractionResult[dnd.SpellList]{Value: dnd.SpellList{SpellCasts: []dnd.SpellCast{{ + Caster: "Aria", Spell: "Mysterious Burst", Effect: "an unknown magical effect", NarrativeDescription: "Aria produces a mysterious burst.", SourceRefs: []source.SourceRef{refOne}, + }}}}, nil + } + return contracts.TypedExtractionResult[dnd.SpellList]{Value: dnd.SpellList{SpellCasts: []dnd.SpellCast{}}}, nil + } switch req.Chunk.Index { case 0: return contracts.TypedExtractionResult[dnd.SpellList]{Value: dnd.SpellList{SpellCasts: []dnd.SpellCast{{ diff --git a/internal/cli/production_contract_test.go b/internal/cli/production_contract_test.go index 39b4e99..70a4110 100644 --- a/internal/cli/production_contract_test.go +++ b/internal/cli/production_contract_test.go @@ -161,6 +161,130 @@ func TestProductionSpellValidatorsPrepareFromMaterializedCatalog(t *testing.T) { } } +func TestProductionSpellNormalizerRejectsInvalidCatalogReferencesBeforeExecution(t *testing.T) { + components := productionTestComponents(t) + configPath := repositoryPath("examples", "dnd-spells-production.config.yml") + resolve := func(t *testing.T) pipeline.ResolvedPipeline { + t.Helper() + effective, err := loadMaintainedExample(t, configPath).Resolve(resolveInputForMaintainedExample(components, "dnd-session")) + if err != nil { + t.Fatalf("resolve production spell configuration: %v", err) + } + return effective.ResolvedPipeline + } + materialize := func(resolved pipeline.ResolvedPipeline) (pipeline.ResolvedPipeline, error) { + materialized, _, err := pipeline.MaterializeReferences(resolved, catalogFromRegistries(components.registries), pipeline.ReferenceMaterializationOptions{ + ConfigPath: configPath, + WorkingDir: filepath.Dir(configPath), + }) + return materialized, err + } + + t.Run("malformed catalog fails preparation", func(t *testing.T) { + catalogPath := filepath.Join(t.TempDir(), "malformed.json") + if err := os.WriteFile(catalogPath, []byte(`{"schema_version":`), 0o600); err != nil { + t.Fatal(err) + } + resolved := resolve(t) + setNormalizeSpellCatalogSource(t, &resolved, catalogPath) + materialized, err := materialize(resolved) + if err != nil { + t.Fatalf("MaterializeReferences() error = %v, want malformed JSON to reach preparation", err) + } + _, err = pipeline.Prepare(materialized, components.registries, pipeline.ModuleDependencies{LLM: &productionFakeLLMClient{}}) + for _, fragment := range []string{`pipeline "dnd-session"`, `lane "spells"`, "normalize", `module "dnd/spells"`, "decode spell catalog overlay"} { + if err == nil || !strings.Contains(err.Error(), fragment) { + t.Fatalf("Prepare() error = %v, want context fragment %q", err, fragment) + } + } + }) + + t.Run("multiple catalog items fail preparation", func(t *testing.T) { + materialized, err := materialize(resolve(t)) + if err != nil { + t.Fatal(err) + } + slot := materialized.ArtifactLanes[0].NormalizeReferences.ReferenceSet.Slots["spell_catalog"] + slot.Items = append(slot.Items, slot.Items[0]) + materialized.ArtifactLanes[0].NormalizeReferences.ReferenceSet.Slots["spell_catalog"] = slot + _, err = pipeline.Prepare(materialized, components.registries, pipeline.ModuleDependencies{LLM: &productionFakeLLMClient{}}) + for _, fragment := range []string{"normalize", `module "dnd/spells"`, "zero or one item"} { + if err == nil || !strings.Contains(err.Error(), fragment) { + t.Fatalf("Prepare() error = %v, want context fragment %q", err, fragment) + } + } + }) + + t.Run("oversized catalog fails materialization", func(t *testing.T) { + catalogPath := filepath.Join(t.TempDir(), "oversized.json") + if err := os.WriteFile(catalogPath, []byte(strings.Repeat("x", 1048577)), 0o600); err != nil { + t.Fatal(err) + } + checkpointRoot := filepath.Join(t.TempDir(), "checkpoints") + content := string(readRepositoryFile(t, "examples", "dnd-spells-production.config.yml")) + content = replaceRequiredOnce(t, content, "./dnd-spells-roster.txt", repositoryPath("examples", "dnd-spells-roster.txt")) + content = replaceRequiredOnce(t, content, "./dnd-spells-glossary.txt", repositoryPath("examples", "dnd-spells-glossary.txt")) + content = strings.Replace(content, "./dnd-spells-catalog.json", repositoryPath("examples", "dnd-spells-catalog.json"), 1) + content = replaceRequiredOnce(t, content, "./dnd-spells-catalog.json", catalogPath) + content = replaceRequiredOnce(t, content, " enabled: false\n directory: /var/cache/notarius/checkpoints", " enabled: true\n directory: "+checkpointRoot) + configFile := filepath.Join(t.TempDir(), "config.yml") + if err := os.WriteFile(configFile, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + + llmConstructed := false + chunkStoreConstructed := false + options := Options{ + Catalog: catalogFromRegistries(components.registries), + Registries: components.registries, + LLMClientFactory: func(context.Context, config.Config, string) (contracts.StructuredLLMClient, []artifacts.LLMProfileManifest, error) { + llmConstructed = true + return nil, nil, errors.New("LLM client must not be constructed") + }, + ChunkPlanStoreFactory: func(string) (pipeline.ChunkPlanStore, error) { + chunkStoreConstructed = true + return nil, errors.New("chunk-plan store must not be constructed") + }, + } + var stdout, stderr strings.Builder + code := RunWithOptions([]string{ + "run", "dnd-session", "--config", configFile, + "--input", repositoryPath("examples", "seriatim-minimal-transcript.json"), + }, &stdout, &stderr, options) + errText := stderr.String() + for _, fragment := range []string{"normalize", `lane "spells"`, `reference slot "spell_catalog"`, "1048577 bytes", "limit 1048576"} { + if code == 0 || !strings.Contains(errText, fragment) { + t.Fatalf("RunWithOptions() code = %d stderr = %q, want context fragment %q", code, errText, fragment) + } + } + if llmConstructed || chunkStoreConstructed { + t.Fatalf("runtime construction = LLM %t, chunk store %t; want materialization failure first", llmConstructed, chunkStoreConstructed) + } + if _, err := os.Stat(checkpointRoot); !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("checkpoint root stat error = %v, want no checkpoint allocation", err) + } + }) +} + +func setNormalizeSpellCatalogSource(t *testing.T, resolved *pipeline.ResolvedPipeline, sourcePath string) { + t.Helper() + if resolved == nil || len(resolved.ArtifactLanes) != 1 { + t.Fatalf("resolved pipeline = %#v, want one artifact lane", resolved) + } + bindings := resolved.ArtifactLanes[0].NormalizeReferences.Bindings + matches := 0 + for index := range bindings { + if bindings[index].SlotName == "spell_catalog" { + bindings[index].Source = sourcePath + matches++ + } + } + if matches != 1 { + t.Fatalf("normalize reference bindings = %#v, want exactly one spell_catalog binding", bindings) + } + resolved.ArtifactLanes[0].NormalizeReferences.Bindings = bindings +} + func TestProductionLLMClientFactoriesBuildOfflineRuntime(t *testing.T) { components := productionTestComponents(t) factories := []struct { diff --git a/internal/cli/spell_catalog_identity_contract_test.go b/internal/cli/spell_catalog_identity_contract_test.go index e376870..8a04c03 100644 --- a/internal/cli/spell_catalog_identity_contract_test.go +++ b/internal/cli/spell_catalog_identity_contract_test.go @@ -190,8 +190,8 @@ func TestSemanticSpellCatalogFingerprintChangesCheckpointIdentityWithoutReferenc return identity } first := identityFor(fingerprints) - changed := append([]pipeline.CheckpointFingerprint(nil), fingerprints...) - changed[0].Value = "sha256:changed-effective-catalog" + changed := replaceCheckpointFingerprintValue(t, fingerprints, normalizeSpellCatalogFingerprintName(), "sha256:changed-effective-catalog") + assertOnlyCheckpointFingerprintChanged(t, fingerprints, changed, normalizeSpellCatalogFingerprintName()) second := identityFor(changed) if first.Digest == second.Digest || reflect.DeepEqual(first.ReferenceDigests, nil) || !reflect.DeepEqual(first.ReferenceDigests, second.ReferenceDigests) { t.Fatalf("identities = %#v / %#v, want semantic invalidation with unchanged reference provenance", first, second) @@ -251,8 +251,8 @@ func TestChangedSemanticSpellCatalogFingerprintCannotResumeRecordedCheckpoint(t if restored, decision := sameLoader.Normalize("spells", spellnormalize.Key, normalizeDependencies); !decision.Reused || string(restored.Output.Artifact.Content) != `{"spell_casts":[]}` { t.Fatalf("same normalize checkpoint = %#v, decision=%#v, want reuse", restored, decision) } - changed := append([]pipeline.CheckpointFingerprint(nil), fingerprints...) - changed[0].Value = "sha256:changed-effective-catalog" + changed := replaceCheckpointFingerprintValue(t, fingerprints, normalizeSpellCatalogFingerprintName(), "sha256:changed-effective-catalog") + assertOnlyCheckpointFingerprintChanged(t, fingerprints, changed, normalizeSpellCatalogFingerprintName()) _, changedLoader, err := checkpointHandlersForRun(settings, Options{}, materialized, changed, []byte("same input"), nil, nil, "", "", true) if err != nil { t.Fatal(err) @@ -265,6 +265,49 @@ func TestChangedSemanticSpellCatalogFingerprintCannotResumeRecordedCheckpoint(t } } +func normalizeSpellCatalogFingerprintName() string { + return "normalize:spells:" + spellnormalize.Key + ":effective_catalog" +} + +func replaceCheckpointFingerprintValue(t *testing.T, fingerprints []pipeline.CheckpointFingerprint, name, value string) []pipeline.CheckpointFingerprint { + t.Helper() + changed := append([]pipeline.CheckpointFingerprint(nil), fingerprints...) + matches := 0 + for index := range changed { + if changed[index].Name == name { + changed[index].Value = value + matches++ + } + } + if matches != 1 { + t.Fatalf("checkpoint fingerprints = %#v, want exactly one fingerprint named %q", fingerprints, name) + } + return changed +} + +func assertOnlyCheckpointFingerprintChanged(t *testing.T, before, after []pipeline.CheckpointFingerprint, changedName string) { + t.Helper() + if len(before) != len(after) { + t.Fatalf("fingerprint lengths = %d and %d, want equal", len(before), len(after)) + } + changes := 0 + for index := range before { + if before[index].Name != after[index].Name { + t.Fatalf("fingerprint[%d] name changed from %q to %q", index, before[index].Name, after[index].Name) + } + if before[index].Value == after[index].Value { + continue + } + changes++ + if before[index].Name != changedName { + t.Fatalf("fingerprint %q changed unexpectedly", before[index].Name) + } + } + if changes != 1 { + t.Fatalf("fingerprints changed %d values, want exactly %q", changes, changedName) + } +} + func TestMaintainedProductionOverlayRunAlignsGroundingValidationAndProvenance(t *testing.T) { outputRoot := filepath.Join(t.TempDir(), "output") fake := &productionFakeLLMClient{spellResponse: productionSpellResponse("Aegis of Emberfall")} diff --git a/internal/modules/dnd/normalize/spells/normalizer.go b/internal/modules/dnd/normalize/spells/normalizer.go index bda4cac..72e0468 100644 --- a/internal/modules/dnd/normalize/spells/normalizer.go +++ b/internal/modules/dnd/normalize/spells/normalizer.go @@ -345,7 +345,7 @@ func DecodeOptions(options map[string]any) (Options, error) { func referenceSlots() []contracts.ReferenceSlot { return []contracts.ReferenceSlot{{ Name: spellcatalog.SpellCatalogReferenceSlot, - Description: "Optional canonical spell-name catalog used for extraction grounding.", + Description: "Optional canonical spell-name catalog used for normalization and duplicate identity.", AcceptedMediaTypes: []string{"application/json"}, MaxBytes: 1048576, }} diff --git a/internal/modules/dnd/normalize/spells/normalizer_test.go b/internal/modules/dnd/normalize/spells/normalizer_test.go index dcb21c6..a82007b 100644 --- a/internal/modules/dnd/normalize/spells/normalizer_test.go +++ b/internal/modules/dnd/normalize/spells/normalizer_test.go @@ -31,7 +31,7 @@ func TestModuleContractAndStrictOptions(t *testing.T) { ArtifactKind: dnd.SpellListKind, ReferenceSlots: []contracts.ReferenceSlot{{ Name: spellcatalog.SpellCatalogReferenceSlot, - Description: "Optional canonical spell-name catalog used for extraction grounding.", + Description: "Optional canonical spell-name catalog used for normalization and duplicate identity.", AcceptedMediaTypes: []string{"application/json"}, MaxBytes: 1048576, }},