From 7b2fb0880d4d09fd0837ec83ed6a93cefe4de36a Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 22 Jul 2026 14:30:20 +0000 Subject: [PATCH] Align D&D validator ordering --- docs/internal/modules.md | 9 +++- docs/internal/pipeline.md | 7 +++ internal/cli/dnd_npc_contract_test.go | 4 +- internal/cli/production_contract_test.go | 8 ++-- .../spell_catalog_identity_contract_test.go | 4 +- internal/modules/dnd/register/chains.go | 8 ++-- .../modules/dnd/register/register_test.go | 6 +-- .../integration/dnd_npcs_runner_test.go | 30 +++++++++++++ .../integration/dnd_spells_helpers_test.go | 15 +++++-- .../integration/dnd_spells_runner_test.go | 43 +++++++++++++++++++ 10 files changed, 115 insertions(+), 19 deletions(-) diff --git a/docs/internal/modules.md b/docs/internal/modules.md index 9d43961..98cfd30 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -29,6 +29,12 @@ variants. The D&D production registrar registers the canonical typed spell, NPC, and combat implementations, including their kind-specific merge and normalize behavior. +For D&D artifact defaults, generic JSON syntax validation runs first. Rejecting +domain validators then own semantic diagnostics before generic JSON Schema +validation provides the final rejecting representation backstop; warning-only +relatedness validators run last. This default composition does not reorder an +explicitly configured validator chain. + Prepared extractors, extract validators, and codecs may be reused concurrently by the run-wide extract pool. Production implementations are immutable after construction: they retain only typed options, immutable assets, or the shared @@ -410,7 +416,8 @@ validator owns display normalization, comparison-unique targets, canonical source-reference order, chronology, and exact duplicate identity; it defers shape and source-reference failures. All four validators are deterministic and expose local policy fingerprints. The D&D registrar orders them after generic -JSON and response-schema validation at extraction and normalization. +JSON validation and before response-schema validation at extraction and +normalization. ## Production Registration diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index 74b0892..e57db52 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -298,6 +298,13 @@ canonical chunk JSON or artifact codec bytes. Validators execute in resolved order and stop at the first error or rejection. An empty chain approves the result. +Production D&D artifact chains keep generic JSON syntax validation first, then +run every rejecting domain validator before generic JSON Schema validation. The +domain validator therefore owns expected semantic diagnostics; the generic +schema validator remains the final rejecting representation backstop, before +warning-only relatedness validation. Explicitly configured validator chains +retain their configured order. + `runWithRetry` applies the effective retry policy around module execution and its complete validation chain. A module or validator error becomes a framework error when attempts are exhausted. A rejection becomes a recorded diff --git a/internal/cli/dnd_npc_contract_test.go b/internal/cli/dnd_npc_contract_test.go index 15f8e0f..210bf5e 100644 --- a/internal/cli/dnd_npc_contract_test.go +++ b/internal/cli/dnd_npc_contract_test.go @@ -50,17 +50,17 @@ func TestProductionNPCConfigurationResolvesTypedLane(t *testing.T) { wantExtractChain := []pipeline.ModuleBinding{ pipeline.Binding("generic/valid_json"), - pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/npcs/shape"), pipeline.Binding("extract/dnd/npcs/source_refs"), + pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/npcs/source_relatedness"), } wantNormalizeChain := []pipeline.ModuleBinding{ pipeline.Binding("generic/valid_json"), - pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/npcs/shape"), pipeline.Binding("normalize/dnd/npcs/identity"), pipeline.Binding("extract/dnd/npcs/source_refs"), + pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/npcs/source_relatedness"), } if got := validatorChain(effective.ResolvedPipeline, pipeline.StageExtract, npcextract.Key); !reflect.DeepEqual(got, wantExtractChain) { diff --git a/internal/cli/production_contract_test.go b/internal/cli/production_contract_test.go index 300fa96..a86ec22 100644 --- a/internal/cli/production_contract_test.go +++ b/internal/cli/production_contract_test.go @@ -63,10 +63,10 @@ func TestProductionCatalogCoversMaintainedConfigurations(t *testing.T) { wantChain := []pipeline.ModuleBinding{ pipeline.Binding("generic/valid_json"), - pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/spells/shape"), pipeline.Binding("extract/dnd/spells/catalog"), pipeline.Binding("extract/dnd/spells/source_refs"), + pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/spells/source_relatedness"), } if got := registries.ValidatorChains.Validators(pipeline.StageExtract, spells.Key); !reflect.DeepEqual(got, wantChain) { @@ -438,7 +438,7 @@ func TestProductionConfigValidationCoversModuleAndVariantFailures(t *testing.T) func TestProductionNormalizeValidatorOverrideRemainsAuthoritative(t *testing.T) { base := string(readRepositoryFile(t, "examples", "dnd-spells.config.yml")) - content := replaceRequiredOnce(t, base, " normalize: dnd/spells\n", " normalize:\n module: dnd/spells\n validators:\n - module: generic/always_accept\n") + content := replaceRequiredOnce(t, base, " normalize: dnd/spells\n", " normalize:\n module: dnd/spells\n validators:\n - module: generic/always_accept\n - module: generic/valid_json\n") path := writeProductionContractConfig(t, content) components := productionTestComponents(t) effective, err := loadMaintainedExample(t, path).Resolve(resolveInputForMaintainedExample(components, "dnd-session")) @@ -449,8 +449,8 @@ func TestProductionNormalizeValidatorOverrideRemainsAuthoritative(t *testing.T) if chain.Stage != pipeline.StageNormalize || chain.ModuleKey != spellnormalize.Key { continue } - if len(chain.Validators) != 1 || chain.Validators[0].Binding.Module != "generic/always_accept" { - t.Fatalf("normalize validator chain = %#v, want explicit always-accept override", chain) + if len(chain.Validators) != 2 || chain.Validators[0].Binding.Module != "generic/always_accept" || chain.Validators[1].Binding.Module != "generic/valid_json" { + t.Fatalf("normalize validator chain = %#v, want explicit validator order", chain) } return } diff --git a/internal/cli/spell_catalog_identity_contract_test.go b/internal/cli/spell_catalog_identity_contract_test.go index 78ab800..b98dc12 100644 --- a/internal/cli/spell_catalog_identity_contract_test.go +++ b/internal/cli/spell_catalog_identity_contract_test.go @@ -154,9 +154,9 @@ func TestSemanticSpellCatalogFingerprintChangesCheckpointIdentityWithoutReferenc fingerprints := prepared.CheckpointFingerprints() wantNames := map[string]struct{}{ "extract:spells:" + spells.Key + ":effective_catalog": {}, - "extract:spells:" + spells.Key + ":validator:3:extract/dnd/spells/catalog:effective_catalog": {}, + "extract:spells:" + spells.Key + ":validator:2:extract/dnd/spells/catalog:effective_catalog": {}, "normalize:spells:" + spellnormalize.Key + ":effective_catalog": {}, - "normalize:spells:" + spellnormalize.Key + ":validator:3:extract/dnd/spells/catalog:effective_catalog": {}, + "normalize:spells:" + spellnormalize.Key + ":validator:2:extract/dnd/spells/catalog:effective_catalog": {}, } seen := make(map[string]string, len(fingerprints)) for _, fingerprint := range fingerprints { diff --git a/internal/modules/dnd/register/chains.go b/internal/modules/dnd/register/chains.go index dd54e89..dfb1a75 100644 --- a/internal/modules/dnd/register/chains.go +++ b/internal/modules/dnd/register/chains.go @@ -32,10 +32,10 @@ func registerDefaultChains(registry *pipeline.ValidatorChainRegistry) error { Module: spellextract.Key, Validators: []pipeline.ModuleBinding{ pipeline.Binding(validjson.Key), - pipeline.Binding(validjsonschema.Key), pipeline.Binding(spellshape.Key), pipeline.Binding(spellcatalog.Key), pipeline.Binding(spellsourcerefs.Key), + pipeline.Binding(validjsonschema.Key), pipeline.Binding(spellrelatedness.Key), }, }) @@ -46,10 +46,10 @@ func registerDefaultChains(registry *pipeline.ValidatorChainRegistry) error { Module: spellnormalize.Key, Validators: []pipeline.ModuleBinding{ pipeline.Binding(validjson.Key), - pipeline.Binding(validjsonschema.Key), pipeline.Binding(spellshape.Key), pipeline.Binding(spellcatalog.Key), pipeline.Binding(spellsourcerefs.Key), + pipeline.Binding(validjsonschema.Key), pipeline.Binding(spellrelatedness.Key), }, }) @@ -60,9 +60,9 @@ func registerDefaultChains(registry *pipeline.ValidatorChainRegistry) error { Module: npcextract.Key, Validators: []pipeline.ModuleBinding{ pipeline.Binding(validjson.Key), - pipeline.Binding(validjsonschema.Key), pipeline.Binding(npcshape.Key), pipeline.Binding(npcsourcerefs.Key), + pipeline.Binding(validjsonschema.Key), pipeline.Binding(npcrelatedness.Key), }, }) @@ -73,10 +73,10 @@ func registerDefaultChains(registry *pipeline.ValidatorChainRegistry) error { Module: npcnormalize.Key, Validators: []pipeline.ModuleBinding{ pipeline.Binding(validjson.Key), - pipeline.Binding(validjsonschema.Key), pipeline.Binding(npcshape.Key), pipeline.Binding(npcidentity.Key), pipeline.Binding(npcsourcerefs.Key), + pipeline.Binding(validjsonschema.Key), pipeline.Binding(npcrelatedness.Key), }, }) diff --git a/internal/modules/dnd/register/register_test.go b/internal/modules/dnd/register/register_test.go index f1b5670..2701b66 100644 --- a/internal/modules/dnd/register/register_test.go +++ b/internal/modules/dnd/register/register_test.go @@ -52,10 +52,10 @@ func TestRegisterAddsDNDFamily(t *testing.T) { }) wantChain := []pipeline.ModuleBinding{ pipeline.Binding("generic/valid_json"), - pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/spells/shape"), pipeline.Binding("extract/dnd/spells/catalog"), pipeline.Binding("extract/dnd/spells/source_refs"), + pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/spells/source_relatedness"), } if got := registries.ValidatorChains.Validators(pipeline.StageExtract, spells.Key); !reflect.DeepEqual(got, wantChain) { @@ -66,9 +66,9 @@ func TestRegisterAddsDNDFamily(t *testing.T) { } npcExtractChain := []pipeline.ModuleBinding{ pipeline.Binding("generic/valid_json"), - pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/npcs/shape"), pipeline.Binding("extract/dnd/npcs/source_refs"), + pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/npcs/source_relatedness"), } if got := registries.ValidatorChains.Validators(pipeline.StageExtract, npcextract.Key); !reflect.DeepEqual(got, npcExtractChain) { @@ -76,10 +76,10 @@ func TestRegisterAddsDNDFamily(t *testing.T) { } npcNormalizeChain := []pipeline.ModuleBinding{ pipeline.Binding("generic/valid_json"), - pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/npcs/shape"), pipeline.Binding("normalize/dnd/npcs/identity"), pipeline.Binding("extract/dnd/npcs/source_refs"), + pipeline.Binding("generic/valid_json_schema"), pipeline.Binding("extract/dnd/npcs/source_relatedness"), } if got := registries.ValidatorChains.Validators(pipeline.StageNormalize, npcnormalize.Key); !reflect.DeepEqual(got, npcNormalizeChain) { diff --git a/internal/modules/integration/dnd_npcs_runner_test.go b/internal/modules/integration/dnd_npcs_runner_test.go index 0611d99..7bbefa9 100644 --- a/internal/modules/integration/dnd_npcs_runner_test.go +++ b/internal/modules/integration/dnd_npcs_runner_test.go @@ -17,6 +17,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/extract/npcs" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" dndregister "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/register" + npcshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/shape" genericregister "gitea.maximumdirect.net/eric/notarius/internal/modules/generic/register" "gitea.maximumdirect.net/eric/notarius/internal/modules/seriatim/input/transcript" seriatimregister "gitea.maximumdirect.net/eric/notarius/internal/modules/seriatim/register" @@ -130,6 +131,35 @@ func TestRunnerProcessesSeriatimInputWithProductionDNDNPCPipeline(t *testing.T) } } +func TestProductionNPCPipelineAttributesInvalidShapeBeforeSchemaValidation(t *testing.T) { + registries := productionNPCRegistries(t) + effective, err := loadNPCPipelineConfig(t).Resolve(config.ResolveInput{ + PipelineID: "dnd-npcs-fixture", + Catalog: moduleCatalog(registries), + }) + if err != nil { + t.Fatalf("Resolve() error = %v, want nil", err) + } + client := &fakeNPCProductionLLMClient{response: npcProductionResponse{NPCs: []npcProductionRecord{{ + Name: "", + Aliases: []string{}, + Description: "A participant.", + Relationships: []npcProductionRelationship{}, + SourceRefs: []npcProductionSourceRef{{StartUnitID: 1, EndUnitID: 1}}, + }}}} + output, err := runPreparedPipeline(t, registries, effective.ResolvedPipeline, client, pipeline.RunInput{RawInput: readNPCFixture(t)}) + if err != nil { + t.Fatalf("Run() error = %v, want non-fatal rejected output", err) + } + if len(client.requests) != 3 || len(output.Rejected) != 1 || len(output.NormalizeOutputs) != 0 { + t.Fatalf("LLM requests = %d rejected = %#v normalized = %#v, want exhausted shape rejection", len(client.requests), output.Rejected, output.NormalizeOutputs) + } + rejection := output.Rejected[0] + if rejection.ReasonCode != npcshape.ReasonCode || rejection.ValidatorName != npcshape.Key || rejection.AttemptCount != 3 { + t.Fatalf("rejection = %#v, want exhausted NPC shape rejection", rejection) + } +} + type npcProductionResponse struct { NPCs []npcProductionRecord `json:"npcs"` } diff --git a/internal/modules/integration/dnd_spells_helpers_test.go b/internal/modules/integration/dnd_spells_helpers_test.go index 276db48..ffabc09 100644 --- a/internal/modules/integration/dnd_spells_helpers_test.go +++ b/internal/modules/integration/dnd_spells_helpers_test.go @@ -22,13 +22,22 @@ type spellCastResponse struct { } type fakeSpellsLLMClient struct { - response extractionResponse - requests []contracts.StructuredCompletionRequest + response extractionResponse + responses []extractionResponse + requests []contracts.StructuredCompletionRequest } func (client *fakeSpellsLLMClient) CompleteStructured(_ context.Context, req contracts.StructuredCompletionRequest, out any) (contracts.StructuredCompletionResponse, error) { client.requests = append(client.requests, cloneStructuredCompletionRequest(req)) - content, err := json.Marshal(client.response) + response := client.response + if client.responses != nil { + index := len(client.requests) - 1 + if index >= len(client.responses) { + return contracts.StructuredCompletionResponse{}, fmt.Errorf("missing fake response %d", index) + } + response = client.responses[index] + } + content, err := json.Marshal(response) if err != nil { return contracts.StructuredCompletionResponse{}, err } diff --git a/internal/modules/integration/dnd_spells_runner_test.go b/internal/modules/integration/dnd_spells_runner_test.go index af45ab0..1770a6a 100644 --- a/internal/modules/integration/dnd_spells_runner_test.go +++ b/internal/modules/integration/dnd_spells_runner_test.go @@ -12,6 +12,8 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/extract/spells" + spellnormalize "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/normalize/spells" + spellshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/spells/shape" "gitea.maximumdirect.net/eric/notarius/internal/modules/seriatim/input/transcript" ) @@ -164,6 +166,47 @@ func TestRunnerPassesPartyAndGlossaryReferencesToDNDSpellsPrompt(t *testing.T) { } } +func TestProductionSpellPipelineAttributesInvalidShapeBeforeSchemaValidation(t *testing.T) { + registries := productionNPCRegistries(t) + configValue := config.Default() + configValue.Pipelines["dnd-spells-shape"] = pipeline.PipelineProfile{ + Input: pipeline.Binding(transcript.Key), + Chunk: pipeline.ModuleBinding{Module: pipeline.DefaultChunkModule, Options: map[string]any{"max_units": 100}}, + Artifacts: map[string]pipeline.ArtifactLaneProfile{ + "spells": { + Extract: pipeline.ModuleBinding{Module: spells.Key, Retries: 2}, + Normalize: pipeline.Binding(spellnormalize.Key), + }, + }, + } + effective, err := configValue.Resolve(config.ResolveInput{ + PipelineID: "dnd-spells-shape", + Catalog: moduleCatalog(registries), + }) + if err != nil { + t.Fatalf("Resolve() error = %v, want nil", err) + } + response := extractionResponse{SpellCasts: []spellCastResponse{{ + Caster: "Aria", + Spell: "Cure Wounds", + Effect: "", + NarrativeDescription: "Aria casts the spell.", + SourceRefs: responseSourceRefs("spell-session", 1, 1), + }}} + client := &fakeSpellsLLMClient{responses: []extractionResponse{response, response, response}} + output, err := runPreparedPipeline(t, registries, effective.ResolvedPipeline, client, pipeline.RunInput{RawInput: readDNDSpellsFixture(t)}) + if err != nil { + t.Fatalf("Run() error = %v, want non-fatal rejected output", err) + } + if len(client.requests) != 3 || len(output.Rejected) != 1 || len(output.NormalizeOutputs) != 0 { + t.Fatalf("LLM requests = %d rejected = %#v normalized = %#v, want exhausted shape rejection", len(client.requests), output.Rejected, output.NormalizeOutputs) + } + rejection := output.Rejected[0] + if rejection.ReasonCode != spellshape.ReasonCode || rejection.ValidatorName != spellshape.Key || rejection.AttemptCount != 3 { + t.Fatalf("rejection = %#v, want exhausted spell shape rejection", rejection) + } +} + func dndSpellsReferenceSet(party string, glossary string) contracts.ReferenceSet { slots := make(map[string]contracts.ResolvedReferenceSlot) if strings.TrimSpace(party) != "" {