diff --git a/internal/cli/cache_contract_test.go b/internal/cli/cache_contract_test.go index 841b9a6..90ac4ab 100644 --- a/internal/cli/cache_contract_test.go +++ b/internal/cli/cache_contract_test.go @@ -207,17 +207,17 @@ func TestRunAutoReusesPlanWhenRunInputsChange(t *testing.T) { if err != nil { t.Fatal(err) } - configText := strings.Replace(string(data), " chunk: test/chunk\n", ` chunk: + configText := replaceRequiredOnce(t, string(data), " chunk: test/chunk\n", ` chunk: module: test/chunk options: strategy: first -`, 1) - configText = strings.Replace(configText, " output: test/output\n", ` other: +`) + configText = replaceRequiredOnce(t, configText, " output: test/output\n", ` other: extract: test/extract merge: test/merge normalize: test/normalize output: test/output -`, 1) +`) if err := os.WriteFile(roots.config, []byte(configText), 0o600); err != nil { t.Fatal(err) } @@ -235,7 +235,7 @@ func TestRunAutoReusesPlanWhenRunInputsChange(t *testing.T) { if err != nil { t.Fatal(err) } - configText = strings.Replace(string(configData), "strategy: first", "strategy: second", 1) + configText = replaceRequiredOnce(t, string(configData), "strategy: first", "strategy: second") if err := os.WriteFile(roots.config, []byte(configText), 0o600); err != nil { t.Fatal(err) } @@ -341,11 +341,7 @@ func replaceStateTestConfigLine(t *testing.T, path, old, new string) { if err != nil { t.Fatal(err) } - text := string(data) - if !strings.Contains(text, old) { - t.Fatalf("config %q does not contain %q", path, old) - } - text = strings.Replace(text, old, new, 1) + text := replaceRequiredOnce(t, string(data), old, new) if err := os.WriteFile(path, []byte(text), 0o600); err != nil { t.Fatal(err) } diff --git a/internal/cli/contract_test_helpers_test.go b/internal/cli/contract_test_helpers_test.go new file mode 100644 index 0000000..2613ef6 --- /dev/null +++ b/internal/cli/contract_test_helpers_test.go @@ -0,0 +1,14 @@ +package cli + +import ( + "strings" + "testing" +) + +func replaceRequiredOnce(t *testing.T, input, old, replacement string) string { + t.Helper() + if count := strings.Count(input, old); count != 1 { + t.Fatalf("replacement marker %q occurs %d times, want exactly once", old, count) + } + return strings.Replace(input, old, replacement, 1) +} diff --git a/internal/cli/production_contract_test.go b/internal/cli/production_contract_test.go index 90cd858..c7d4ff8 100644 --- a/internal/cli/production_contract_test.go +++ b/internal/cli/production_contract_test.go @@ -31,13 +31,13 @@ func TestProductionCatalogCoversMaintainedConfigurations(t *testing.T) { components := productionTestComponents(t) registries := components.registries - assertProductionKeys(t, "inputs", registries.Inputs.RegisteredKeys(), []string{"seriatim"}) - assertProductionKeys(t, "chunkers", registries.Chunkers.RegisteredKeys(), []string{"dnd/scenes", "generic"}) - assertProductionKeys(t, "extractors", registries.Extractors.RegisteredKeys(), []string{"dnd/spells"}) - assertProductionKeys(t, "mergers", registries.Mergers.RegisteredKeys(), []string{"appendorder"}) - assertProductionKeys(t, "normalizers", registries.Normalizers.RegisteredKeys(), []string{"noop"}) - assertProductionKeys(t, "outputs", registries.Outputs.RegisteredKeys(), []string{"json"}) - assertProductionKeys(t, "validators", registries.Validators.RegisteredKeys(), []string{ + assertProductionContains(t, "inputs", registries.Inputs.RegisteredKeys(), []string{"seriatim"}) + assertProductionContains(t, "chunkers", registries.Chunkers.RegisteredKeys(), []string{"dnd/scenes", "generic"}) + assertProductionContains(t, "extractors", registries.Extractors.RegisteredKeys(), []string{"dnd/spells"}) + assertProductionContains(t, "mergers", registries.Mergers.RegisteredKeys(), []string{"appendorder"}) + assertProductionContains(t, "normalizers", registries.Normalizers.RegisteredKeys(), []string{"noop"}) + assertProductionContains(t, "outputs", registries.Outputs.RegisteredKeys(), []string{"json"}) + assertProductionContains(t, "validators", registries.Validators.RegisteredKeys(), []string{ "extract/dnd/spells/shape", "extract/dnd/spells/source_refs", "extract/dnd/spells/source_relatedness", @@ -46,15 +46,9 @@ func TestProductionCatalogCoversMaintainedConfigurations(t *testing.T) { "generic/valid_json", "generic/valid_json_schema", }) - if got := registries.ArtifactCodecs.RegisteredKinds(); !reflect.DeepEqual(got, []contracts.ArtifactKind{dnd.SpellListKind}) { - t.Fatalf("artifact codec kinds = %#v, want [%q]", got, dnd.SpellListKind) - } - if got := registries.Mergers.RegisteredArtifactKinds(pipeline.DefaultMergeModule); !reflect.DeepEqual(got, []contracts.ArtifactKind{dnd.SpellListKind}) { - t.Fatalf("merger variants = %#v, want [%q]", got, dnd.SpellListKind) - } - if got := registries.Normalizers.RegisteredArtifactKinds(pipeline.DefaultNormalizeModule); !reflect.DeepEqual(got, []contracts.ArtifactKind{dnd.SpellListKind}) { - t.Fatalf("normalizer variants = %#v, want [%q]", got, dnd.SpellListKind) - } + assertProductionContains(t, "artifact codec kinds", registries.ArtifactCodecs.RegisteredKinds(), []contracts.ArtifactKind{dnd.SpellListKind}) + assertProductionContains(t, "merger variants", registries.Mergers.RegisteredArtifactKinds(pipeline.DefaultMergeModule), []contracts.ArtifactKind{dnd.SpellListKind}) + assertProductionContains(t, "normalizer variants", registries.Normalizers.RegisteredArtifactKinds(pipeline.DefaultNormalizeModule), []contracts.ArtifactKind{dnd.SpellListKind}) wantChain := []pipeline.ModuleBinding{ pipeline.Binding("generic/valid_json"), @@ -68,7 +62,7 @@ func TestProductionCatalogCoversMaintainedConfigurations(t *testing.T) { } assetNames := productionAssetNames(t, components.assets.PromptFS) - wantAssets := []string{ + requiredAssets := []string{ "dnd.scenes/dnd.scenes.yaml", "dnd.scenes/instructions.md", "dnd.scenes/sharedassets/common-dnd-references.md", @@ -82,9 +76,7 @@ func TestProductionCatalogCoversMaintainedConfigurations(t *testing.T) { "dnd.spells/sharedassets/common-dnd-transcript.md", "dnd.spells/task.md", } - if !reflect.DeepEqual(assetNames, wantAssets) { - t.Fatalf("production prompt assets = %#v, want %#v", assetNames, wantAssets) - } + assertProductionContains(t, "production prompt assets", assetNames, requiredAssets) catalog := catalogFromRegistries(registries) converted := registriesFromCatalog(catalog) @@ -156,6 +148,62 @@ func TestProductionPromptAssetsPrepareWithoutProviderCredentials(t *testing.T) { } } +func TestProductionLLMClientFactoriesBuildOfflineRuntime(t *testing.T) { + components := productionTestComponents(t) + factories := []struct { + name string + factory LLMClientFactory + }{ + {name: "default production assets", factory: productionLLMClientFactory}, + {name: "provided production assets", factory: productionLLMClientFactoryWithAssets(components.assets)}, + } + for _, tt := range factories { + t.Run(tt.name, func(t *testing.T) { + client, manifests, err := tt.factory(context.Background(), config.Default(), "test-profile") + if err != nil { + t.Fatalf("build production LLM runtime: %v", err) + } + if client == nil { + t.Fatal("production LLM runtime returned a nil client") + } + if len(manifests) != 0 { + t.Fatalf("eager profile manifests = %#v, want none", manifests) + } + if _, ok := client.(contracts.LLMProfileManifestProvider); !ok { + t.Fatalf("production LLM client %T does not provide profile manifests", client) + } + }) + } +} + +func TestProductionLLMClientFactoriesRejectInvalidConstruction(t *testing.T) { + t.Run("canceled context", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + client, manifests, err := productionLLMClientFactory(ctx, config.Default(), "test-profile") + if !errors.Is(err, context.Canceled) || client != nil || len(manifests) != 0 { + t.Fatalf("client=%T manifests=%#v error=%v, want canceled construction", client, manifests, err) + } + }) + + t.Run("nil assets", func(t *testing.T) { + client, manifests, err := productionLLMClientFactoryWithAssets(nil)(context.Background(), config.Default(), "test-profile") + if err == nil || !strings.Contains(err.Error(), "asset registry must not be nil") || client != nil || len(manifests) != 0 { + t.Fatalf("client=%T manifests=%#v error=%v, want nil-assets failure", client, manifests, err) + } + }) + + t.Run("invalid scheduler concurrency", func(t *testing.T) { + components := productionTestComponents(t) + cfg := config.Default() + cfg.Concurrency.TotalLLM = 0 + client, manifests, err := productionLLMClientFactoryWithAssets(components.assets)(context.Background(), cfg, "test-profile") + if err == nil || !strings.Contains(err.Error(), "create LLM scheduler") || !strings.Contains(err.Error(), "greater than zero") || client != nil || len(manifests) != 0 { + t.Fatalf("client=%T manifests=%#v error=%v, want scheduler-construction failure", client, manifests, err) + } + }) +} + func TestProductionConfigValidationCoversModuleAndVariantFailures(t *testing.T) { base := string(readRepositoryFile(t, "examples", "dnd-spells.config.yml")) validPath := writeProductionContractConfig(t, base) @@ -180,13 +228,13 @@ func TestProductionConfigValidationCoversModuleAndVariantFailures(t *testing.T) }{ { name: "unknown module", - content: strings.Replace(base, " input: seriatim\n", " input: missing/input\n", 1), + content: replaceRequiredOnce(t, base, " input: seriatim\n", " input: missing/input\n"), options: productionCLIOptions(t), fragments: []string{"pipeline \"dnd-session\"", "input", "missing/input"}, }, { name: "unknown validator", - content: strings.Replace(base, " extract: dnd/spells\n", " extract:\n module: dnd/spells\n validators:\n - module: missing/validator\n", 1), + content: replaceRequiredOnce(t, base, " extract: dnd/spells\n", " extract:\n module: dnd/spells\n validators:\n - module: missing/validator\n"), options: productionCLIOptions(t), fragments: []string{"validator", "missing/validator"}, }, @@ -198,7 +246,7 @@ func TestProductionConfigValidationCoversModuleAndVariantFailures(t *testing.T) }, { name: "deterministic validator with profile", - content: strings.Replace(base, " extract: dnd/spells\n", " extract:\n module: dnd/spells\n validators:\n - module: generic/valid_json\n llm_profile: forbidden-profile\n", 1), + content: replaceRequiredOnce(t, base, " extract: dnd/spells\n", " extract:\n module: dnd/spells\n validators:\n - module: generic/valid_json\n llm_profile: forbidden-profile\n"), options: productionCLIOptions(t), fragments: []string{"deterministic validator", "llm_profile"}, }, @@ -383,10 +431,20 @@ func productionAssetNames(t *testing.T, getFS func() (fs.FS, error)) []string { return names } -func assertProductionKeys(t *testing.T, name string, got, want []string) { +func assertProductionContains[T comparable](t *testing.T, name string, got, required []T) { t.Helper() - if !reflect.DeepEqual(got, want) { - t.Fatalf("%s = %#v, want %#v", name, got, want) + available := make(map[T]struct{}, len(got)) + for _, entry := range got { + available[entry] = struct{}{} + } + var missing []T + for _, entry := range required { + if _, ok := available[entry]; !ok { + missing = append(missing, entry) + } + } + if len(missing) > 0 { + t.Fatalf("%s missing required entries %#v; registered entries are %#v", name, missing, got) } } diff --git a/internal/cli/run_contract_test.go b/internal/cli/run_contract_test.go index adc6cd5..9d13f33 100644 --- a/internal/cli/run_contract_test.go +++ b/internal/cli/run_contract_test.go @@ -104,7 +104,7 @@ func TestRunOnlyExecutesSelectedLanes(t *testing.T) { if err != nil { t.Fatal(err) } - data = bytes.Replace(data, []byte(" output: test/output\n"), []byte(" other:\n extract: test/extract\n output: test/output\n"), 1) + data = []byte(replaceRequiredOnce(t, string(data), " output: test/output\n", " other:\n extract: test/extract\n output: test/output\n")) if err := os.WriteFile(roots.config, data, 0o600); err != nil { t.Fatal(err) } @@ -170,8 +170,8 @@ func TestRunStateRootsHonorEnvironmentFlagsAndDefaults(t *testing.T) { t.Fatal(err) } text := string(data) - text = strings.Replace(text, fmt.Sprintf(" directory: %q\n", roots.output), "", 1) - text = strings.Replace(text, fmt.Sprintf(" directory: %q\n", roots.debug), "", 1) + text = replaceRequiredOnce(t, text, fmt.Sprintf(" directory: %q\n", roots.output), "") + text = replaceRequiredOnce(t, text, fmt.Sprintf(" directory: %q\n", roots.debug), "") if err := os.WriteFile(roots.config, []byte(text), 0o600); err != nil { t.Fatal(err) } @@ -266,17 +266,23 @@ func TestRunLLMProfileOverrideAndValidationUseInjectedBoundaries(t *testing.T) { func TestEffectiveLLMProfileIDsAreSortedDeduplicatedAndLLMOnly(t *testing.T) { resolved := pipeline.ResolvedPipeline{ + Input: pipeline.ModuleBinding{LLMProfile: "input-profile"}, Chunk: pipeline.ModuleBinding{LLMProfile: " zeta "}, ArtifactLanes: []pipeline.ResolvedArtifactLane{ - {Extract: pipeline.ModuleBinding{LLMProfile: "alpha"}, Merge: pipeline.ModuleBinding{LLMProfile: "zeta"}}, + { + Extract: pipeline.ModuleBinding{LLMProfile: "alpha"}, + Merge: pipeline.ModuleBinding{LLMProfile: "zeta"}, + Normalize: pipeline.ModuleBinding{LLMProfile: " gamma "}, + }, }, ValidatorChains: []pipeline.ResolvedValidatorChain{{Validators: []pipeline.ResolvedValidator{ {Binding: pipeline.ModuleBinding{LLMProfile: "deterministic-profile"}, ExecutionClass: contracts.ExecutionClassDeterministic}, {Binding: pipeline.ModuleBinding{LLMProfile: "beta"}, ExecutionClass: contracts.ExecutionClassLLMBacked}, }}}, + Output: pipeline.ModuleBinding{LLMProfile: "output-profile"}, } got := effectiveLLMProfileIDs(resolved) - want := []string{"alpha", "beta", "zeta"} + want := []string{"alpha", "beta", "gamma", "zeta"} if strings.Join(got, ",") != strings.Join(want, ",") { t.Fatalf("effective profiles = %#v, want %#v", got, want) } @@ -335,7 +341,7 @@ func TestRunFactoryAndPreparationFailuresAreProcessFailures(t *testing.T) { if err != nil { t.Fatal(err) } - data = bytes.Replace(data, []byte("extract: test/extract"), []byte("extract: test/failing-extract"), 1) + data = []byte(replaceRequiredOnce(t, string(data), "extract: test/extract", "extract: test/failing-extract")) if err := os.WriteFile(roots.config, data, 0o600); err != nil { t.Fatal(err) } diff --git a/internal/cli/state_hardening_test.go b/internal/cli/state_hardening_test.go index 0dd7399..2c293cd 100644 --- a/internal/cli/state_hardening_test.go +++ b/internal/cli/state_hardening_test.go @@ -256,7 +256,7 @@ func TestRunRedactsSensitiveModuleOptionsFromConfigAndPipelineSummaries(t *testi if err != nil { t.Fatal(err) } - configText := strings.Replace(string(data), " input: test/input\n", ` input: + configText := replaceRequiredOnce(t, string(data), " input: test/input\n", ` input: module: test/input options: api_key: CONFIG_SUMMARY_SECRET_SENTINEL @@ -264,7 +264,7 @@ func TestRunRedactsSensitiveModuleOptionsFromConfigAndPipelineSummaries(t *testi nested: - - password: PIPELINE_SUMMARY_SECRET_SENTINEL neighbor: SAFE_NESTED_OPTION_SENTINEL -`, 1) +`) if err := os.WriteFile(roots.config, []byte(configText), 0o600); err != nil { t.Fatal(err) }