diff --git a/internal/cli/catalog.go b/internal/cli/catalog.go index 72a53a2..e3db919 100644 --- a/internal/cli/catalog.go +++ b/internal/cli/catalog.go @@ -16,6 +16,13 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/merge/appendorder" "gitea.maximumdirect.net/eric/notarius/internal/modules/normalize/noop" jsonoutput "gitea.maximumdirect.net/eric/notarius/internal/modules/output/json" + spellshape "gitea.maximumdirect.net/eric/notarius/internal/validators/extract/dnd/spells/shape" + spellsourcerefs "gitea.maximumdirect.net/eric/notarius/internal/validators/extract/dnd/spells/source_refs" + spellrelatedness "gitea.maximumdirect.net/eric/notarius/internal/validators/extract/dnd/spells/source_relatedness" + alwaysaccept "gitea.maximumdirect.net/eric/notarius/internal/validators/generic/always_accept" + alwaysreject "gitea.maximumdirect.net/eric/notarius/internal/validators/generic/always_reject" + validjson "gitea.maximumdirect.net/eric/notarius/internal/validators/generic/valid_json" + validjsonschema "gitea.maximumdirect.net/eric/notarius/internal/validators/generic/valid_json_schema" ) func productionRegistries() (pipeline.Registries, error) { @@ -47,12 +54,56 @@ func productionRegistries() (pipeline.Registries, error) { if err := noop.Register(registries.Normalizers); err != nil { return pipeline.Registries{}, fmt.Errorf("register noop normalizer: %w", err) } + if err := registerProductionValidators(registries.Validators); err != nil { + return pipeline.Registries{}, err + } + if err := registerProductionValidatorChains(registries.ValidatorChains); err != nil { + return pipeline.Registries{}, err + } if err := jsonoutput.Register(registries.Outputs); err != nil { return pipeline.Registries{}, fmt.Errorf("register json output encoder: %w", err) } return registries, nil } +func registerProductionValidators(registry *pipeline.ValidatorRegistry) error { + registrations := []struct { + name string + register func(*pipeline.ValidatorRegistry) error + }{ + {name: "generic always accept validator", register: alwaysaccept.Register}, + {name: "generic always reject validator", register: alwaysreject.Register}, + {name: "generic valid json validator", register: validjson.Register}, + {name: "generic valid json schema validator", register: validjsonschema.Register}, + {name: "dnd spell shape validator", register: spellshape.Register}, + {name: "dnd spell source references validator", register: spellsourcerefs.Register}, + {name: "dnd spell source relatedness validator", register: spellrelatedness.Register}, + } + for _, registration := range registrations { + if err := registration.register(registry); err != nil { + return fmt.Errorf("register %s: %w", registration.name, err) + } + } + return nil +} + +func registerProductionValidatorChains(registry *pipeline.ValidatorChainRegistry) error { + if err := registry.Register(pipeline.ValidatorChainMapping{ + Stage: pipeline.StageExtract, + Module: spells.Key, + Validators: []pipeline.ModuleBinding{ + pipeline.Binding(validjson.Key), + pipeline.Binding(validjsonschema.Key), + pipeline.Binding(spellshape.Key), + pipeline.Binding(spellsourcerefs.Key), + pipeline.Binding(spellrelatedness.Key), + }, + }); err != nil { + return fmt.Errorf("register dnd spells validator chain: %w", err) + } + return nil +} + func productionCatalog() (pipeline.ModuleCatalog, error) { registries, err := productionRegistries() if err != nil { diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index d4334f0..e5fda5c 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -27,6 +27,13 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/merge/appendorder" "gitea.maximumdirect.net/eric/notarius/internal/modules/normalize/noop" jsonoutput "gitea.maximumdirect.net/eric/notarius/internal/modules/output/json" + spellshape "gitea.maximumdirect.net/eric/notarius/internal/validators/extract/dnd/spells/shape" + spellsourcerefs "gitea.maximumdirect.net/eric/notarius/internal/validators/extract/dnd/spells/source_refs" + spellrelatedness "gitea.maximumdirect.net/eric/notarius/internal/validators/extract/dnd/spells/source_relatedness" + alwaysaccept "gitea.maximumdirect.net/eric/notarius/internal/validators/generic/always_accept" + alwaysreject "gitea.maximumdirect.net/eric/notarius/internal/validators/generic/always_reject" + validjson "gitea.maximumdirect.net/eric/notarius/internal/validators/generic/valid_json" + validjsonschema "gitea.maximumdirect.net/eric/notarius/internal/validators/generic/valid_json_schema" "gitea.maximumdirect.net/eric/scriptorium" ) @@ -121,13 +128,13 @@ func TestRunConfigValidateSuccessWithFakeCatalog(t *testing.T) { } } -func TestProductionCatalogIncludesDefaultModules(t *testing.T) { +func TestProductionCatalogIncludesProductionModulesValidatorsAndDefaults(t *testing.T) { catalog, err := productionCatalog() if err != nil { t.Fatalf("productionCatalog() error = %v, want nil", err) } - tests := []struct { + moduleTests := []struct { name string got func() (pipeline.ModuleSpec, bool) want pipeline.ModuleSpec @@ -169,7 +176,7 @@ func TestProductionCatalogIncludesDefaultModules(t *testing.T) { }, } - for _, test := range tests { + for _, test := range moduleTests { t.Run(test.name, func(t *testing.T) { got, ok := test.got() if !ok { @@ -180,6 +187,42 @@ func TestProductionCatalogIncludesDefaultModules(t *testing.T) { } }) } + + validatorTests := []pipeline.ValidatorSpec{ + alwaysaccept.Spec(), + alwaysreject.Spec(), + validjson.Spec(), + validjsonschema.Spec(), + spellshape.Spec(), + spellsourcerefs.Spec(), + spellrelatedness.Spec(), + } + for _, want := range validatorTests { + t.Run("validator "+want.Key, func(t *testing.T) { + got, ok := catalog.Validators.Spec(want.Key) + if !ok { + t.Fatalf("validator spec %q ok = false, want true", want.Key) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("validator spec = %#v, want %#v", got, want) + } + }) + } + + gotChain := catalog.ValidatorChains.Validators(pipeline.StageExtract, spells.Key) + wantChain := []pipeline.ModuleBinding{ + pipeline.Binding(validjson.Key), + pipeline.Binding(validjsonschema.Key), + pipeline.Binding(spellshape.Key), + pipeline.Binding(spellsourcerefs.Key), + pipeline.Binding(spellrelatedness.Key), + } + if !reflect.DeepEqual(gotChain, wantChain) { + t.Fatalf("dnd spell default validator chain = %#v, want %#v", gotChain, wantChain) + } + if got := catalog.ValidatorChains.Validators(pipeline.StageChunk, generic.Key); len(got) != 0 { + t.Fatalf("generic chunker default validator chain = %#v, want empty", got) + } } func TestProductionPromptAssetsRegisterAndPrepareDndPrompts(t *testing.T) { @@ -336,6 +379,50 @@ func TestRunConfigValidateUnknownProductionModuleIncludesContext(t *testing.T) { } } +func TestRunConfigValidateRejectsUnknownProductionValidator(t *testing.T) { + configPath := writeTestConfig(t, mvpConfigYAMLWithExtractValidators("dnd-session", "\n - missing/validator\n")) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunWithOptions([]string{"config", "validate", "--config", configPath, "--pipeline", "dnd-session"}, &stdout, &stderr, Options{}) + + if code != 1 { + t.Fatalf("RunWithOptions() code = %d, want 1", code) + } + if got := stderr.String(); !strings.Contains(got, "unknown validator") || !strings.Contains(got, "missing/validator") { + t.Fatalf("stderr = %q, want unknown validator", got) + } +} + +func TestRunConfigValidateRejectsLLMProfileForDeterministicProductionValidator(t *testing.T) { + configPath := writeTestConfig(t, `version: 2 +pipelines: + dnd-session: + input: seriatim + artifacts: + spells: + extract: + module: dnd/spells + validators: + - module: generic/valid_json + llm_profile: review +`) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunWithOptions([]string{"config", "validate", "--config", configPath, "--pipeline", "dnd-session"}, &stdout, &stderr, Options{}) + + if code != 1 { + t.Fatalf("RunWithOptions() code = %d, want 1", code) + } + got := stderr.String() + for _, want := range []string{"llm_profile", "deterministic", validjson.Key} { + if !strings.Contains(got, want) { + t.Fatalf("stderr = %q, want substring %q", got, want) + } + } +} + func TestRunConfigValidateReportsParseErrors(t *testing.T) { configPath := writeFile(t, "config.yml", "version: 1\n") var stdout bytes.Buffer @@ -743,7 +830,7 @@ func TestRunPipelineLLMFactoryFailure(t *testing.T) { } } -func TestRunPipelineCarriesInvalidLLMSourceRefsAsRawOutput(t *testing.T) { +func TestRunPipelineDefaultDNDSpellValidatorsRejectInvalidSourceRefs(t *testing.T) { configPath := writeTestConfig(t, mvpConfigYAML("dnd-session", "dnd/spells")) inputPath := writeSeriatimInput(t) outputDir := t.TempDir() @@ -756,11 +843,107 @@ func TestRunPipelineCarriesInvalidLLMSourceRefsAsRawOutput(t *testing.T) { LLMClientFactory: fakeLLMFactory(client, nil), }) + if code != 0 { + t.Fatalf("RunWithOptions() code = %d, stderr=%q", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "outputs=0") || !strings.Contains(stdout.String(), "rejected=1") { + t.Fatalf("stdout = %q, want rejected output count", stdout.String()) + } + + var manifest artifacts.RunManifest + readJSONFile(t, filepath.Join(onlyChildDir(t, outputDir), "manifest.json"), &manifest) + if manifest.ValidationStatus != "rejected" { + t.Fatalf("validation status = %q, want rejected", manifest.ValidationStatus) + } + if len(manifest.RejectedOutputs) != 1 { + t.Fatalf("rejected outputs = %#v, want one rejection", manifest.RejectedOutputs) + } + rejection := manifest.RejectedOutputs[0] + if rejection.ValidatorName != spellsourcerefs.Key || rejection.ReasonCode != spellsourcerefs.ReasonCode { + t.Fatalf("rejection = %#v, want source reference validator rejection", rejection) + } + gotChain := manifestValidatorChain(t, manifest, pipeline.StageExtract, "spells", spells.Key) + wantKeys := []string{validjson.Key, validjsonschema.Key, spellshape.Key, spellsourcerefs.Key, spellrelatedness.Key} + if got := manifestValidatorKeys(gotChain); !reflect.DeepEqual(got, wantKeys) { + t.Fatalf("validator chain keys = %#v, want %#v", got, wantKeys) + } +} + +func TestRunPipelineExplicitEmptyValidatorOverrideDisablesDNDSpellDefaults(t *testing.T) { + configPath := writeTestConfig(t, mvpConfigYAMLWithExtractValidators("dnd-session", " []\n")) + inputPath := writeSeriatimInput(t) + outputDir := t.TempDir() + diagnosticsDir := t.TempDir() + client := newFakeRunLLMClient(true) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunWithOptions([]string{"run", "dnd-session", "--config", configPath, "--input", inputPath, "--output-dir", outputDir, "--diagnostics-dir", diagnosticsDir}, &stdout, &stderr, Options{ + LLMClientFactory: fakeLLMFactory(client, nil), + }) + if code != 0 { t.Fatalf("RunWithOptions() code = %d, stderr=%q", code, stderr.String()) } if !strings.Contains(stdout.String(), "outputs=1") || !strings.Contains(stdout.String(), "rejected=0") { - t.Fatalf("stdout = %q, want raw output count", stdout.String()) + t.Fatalf("stdout = %q, want accepted output count", stdout.String()) + } + var manifest artifacts.RunManifest + readJSONFile(t, filepath.Join(onlyChildDir(t, outputDir), "manifest.json"), &manifest) + gotChain := manifestValidatorChain(t, manifest, pipeline.StageExtract, "spells", spells.Key) + if len(gotChain.Validators) != 0 { + t.Fatalf("validator chain = %#v, want explicit empty chain", gotChain) + } +} + +func TestRunPipelineExplicitValidatorOverrideReplacesDNDSpellDefaults(t *testing.T) { + configPath := writeTestConfig(t, mvpConfigYAMLWithExtractValidators("dnd-session", "\n - "+alwaysaccept.Key+"\n")) + inputPath := writeSeriatimInput(t) + outputDir := t.TempDir() + diagnosticsDir := t.TempDir() + client := newFakeRunLLMClient(true) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunWithOptions([]string{"run", "dnd-session", "--config", configPath, "--input", inputPath, "--output-dir", outputDir, "--diagnostics-dir", diagnosticsDir}, &stdout, &stderr, Options{ + LLMClientFactory: fakeLLMFactory(client, nil), + }) + + if code != 0 { + t.Fatalf("RunWithOptions() code = %d, stderr=%q", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "outputs=1") || !strings.Contains(stdout.String(), "rejected=0") { + t.Fatalf("stdout = %q, want accepted output count", stdout.String()) + } + var manifest artifacts.RunManifest + readJSONFile(t, filepath.Join(onlyChildDir(t, outputDir), "manifest.json"), &manifest) + gotChain := manifestValidatorChain(t, manifest, pipeline.StageExtract, "spells", spells.Key) + if got := manifestValidatorKeys(gotChain); !reflect.DeepEqual(got, []string{alwaysaccept.Key}) { + t.Fatalf("validator chain keys = %#v, want explicit override", got) + } +} + +func TestRunPipelineConfiguredValidatorOrderIsPreserved(t *testing.T) { + configPath := writeTestConfig(t, mvpConfigYAMLWithExtractValidators("dnd-session", "\n - "+alwaysaccept.Key+"\n - "+validjson.Key+"\n")) + inputPath := writeSeriatimInput(t) + outputDir := t.TempDir() + diagnosticsDir := t.TempDir() + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunWithOptions([]string{"run", "dnd-session", "--config", configPath, "--input", inputPath, "--output-dir", outputDir, "--diagnostics-dir", diagnosticsDir}, &stdout, &stderr, Options{ + LLMClientFactory: fakeLLMFactory(newFakeRunLLMClient(false), nil), + }) + + if code != 0 { + t.Fatalf("RunWithOptions() code = %d, stderr=%q", code, stderr.String()) + } + var manifest artifacts.RunManifest + readJSONFile(t, filepath.Join(onlyChildDir(t, outputDir), "manifest.json"), &manifest) + gotChain := manifestValidatorChain(t, manifest, pipeline.StageExtract, "spells", spells.Key) + wantKeys := []string{alwaysaccept.Key, validjson.Key} + if got := manifestValidatorKeys(gotChain); !reflect.DeepEqual(got, wantKeys) { + t.Fatalf("validator chain keys = %#v, want %#v", got, wantKeys) } } @@ -816,6 +999,42 @@ pipelines: } } +func TestRunConfigValidateChecksExplicitLLMValidatorProfileIDs(t *testing.T) { + profilePath := writeScriptoriumProfileFile(t, "known", "http://profile.test/v1", "test-model") + configPath := writeTestConfig(t, `version: 2 +scriptorium: + profile_file: `+profilePath+` +pipelines: + example: + input: fake/input + artifacts: + events: + extract: + module: fake/extract + validators: + - module: fake/llm-validator + llm_profile: missing +`) + catalog := fakeCatalog(t) + mustRegisterValidator(t, catalog.Validators, pipeline.ValidatorSpec{ + Key: "fake/llm-validator", + ExecutionClass: contracts.ExecutionClassLLMBacked, + }) + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunWithOptions([]string{"config", "validate", "--config", configPath, "--pipeline", "example"}, &stdout, &stderr, Options{ + Catalog: catalog, + }) + + if code != 1 { + t.Fatalf("RunWithOptions() code = %d, want 1", code) + } + if !strings.Contains(stderr.String(), "Scriptorium profile") || !strings.Contains(stderr.String(), "missing") { + t.Fatalf("stderr = %q, want unknown validator Scriptorium profile", stderr.String()) + } +} + func TestRunConfigValidateIncludesMergeAndIgnoresNonLLMStageScriptoriumProfiles(t *testing.T) { profilePath := writeScriptoriumProfileFile(t, "known", "http://profile.test/v1", "test-model") configPath := writeTestConfig(t, `version: 2 @@ -2270,18 +2489,18 @@ func TestExampleFixtureFailureCoverage(t *testing.T) { wantStderr: "completion unavailable", }, { - name: "malformed LLM response carried as raw output", + name: "malformed LLM response rejected by validation", args: []string{"run", "dnd-session", "--config", configPath, "--input", inputPath}, factory: fakeLLMFactory(newMalformedRunLLMClient(), nil), wantCode: 0, - wantOutputStatus: "approved", + wantOutputStatus: "rejected", }, { - name: "invalid source reference raw output", + name: "invalid source reference rejected by validation", args: []string{"run", "dnd-session", "--config", configPath, "--input", inputPath}, factory: fakeLLMFactory(newFakeRunLLMClient(true), nil), wantCode: 0, - wantOutputStatus: "approved", + wantOutputStatus: "rejected", }, } @@ -2440,6 +2659,18 @@ pipelines: ` } +func mvpConfigYAMLWithExtractValidators(pipelineID string, validators string) string { + return `version: 2 +pipelines: + ` + pipelineID + `: + input: seriatim + artifacts: + spells: + extract: + module: dnd/spells + validators:` + validators +} + func mvpConfigYAMLWithChunk(pipelineID string, chunker string, extractor string) string { return `version: 2 pipelines: @@ -2902,6 +3133,25 @@ func resolvedArtifactLane(t *testing.T, resolved pipeline.ResolvedPipeline, lane return pipeline.ResolvedArtifactLane{} } +func manifestValidatorChain(t *testing.T, manifest artifacts.RunManifest, stage pipeline.ModuleStage, laneID string, module string) artifacts.ValidatorChainManifest { + t.Helper() + for _, chain := range manifest.ValidatorChains { + if chain.Stage == string(stage) && chain.LaneID == laneID && chain.ModuleKey == module { + return chain + } + } + t.Fatalf("validator chain %s/%s/%s not found in %#v", stage, laneID, module, manifest.ValidatorChains) + return artifacts.ValidatorChainManifest{} +} + +func manifestValidatorKeys(chain artifacts.ValidatorChainManifest) []string { + keys := make([]string, 0, len(chain.Validators)) + for _, validator := range chain.Validators { + keys = append(keys, validator.Key) + } + return keys +} + func assertNoTemporaryFiles(t *testing.T, root string) { t.Helper() if err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { @@ -3000,6 +3250,32 @@ func mustRegisterOutput(t *testing.T, registry *pipeline.OutputEncoderRegistry, } } +func mustRegisterValidator(t *testing.T, registry *pipeline.ValidatorRegistry, spec pipeline.ValidatorSpec) { + t.Helper() + if err := registry.RegisterWithSpec(spec, func() (contracts.Validator, error) { + return fakeConfigValidator{name: spec.Key, executionClass: spec.ExecutionClass}, nil + }); err != nil { + t.Fatalf("register validator: %v", err) + } +} + +type fakeConfigValidator struct { + name string + executionClass contracts.ExecutionClass +} + +func (validator fakeConfigValidator) Name() string { + return validator.name +} + +func (validator fakeConfigValidator) ExecutionClass() contracts.ExecutionClass { + return validator.executionClass +} + +func (validator fakeConfigValidator) Validate(ctx context.Context, req contracts.ValidationRequest) (contracts.ValidationResult, error) { + return contracts.ValidationResult{Approved: true}, nil +} + func mapLookup(values map[string]string) func(string) (string, bool) { return func(key string) (string, bool) { value, ok := values[key]