Apply spell normalization follow-up fixes

2026-07-20 19:44:35 -05:00
parent 3eb68baca6
commit 8b5a4e0efd
8 changed files with 268 additions and 550 deletions
--- 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{{
--- 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 {
--- 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")}