diff --git a/docs/internal/llm.md b/docs/internal/llm.md index e9b8234..cc0c1b0 100644 --- a/docs/internal/llm.md +++ b/docs/internal/llm.md @@ -141,7 +141,7 @@ handling of debug data is defined in [Operations](../operations.md#debug). `schema_registry_test.go`: asset composition, validation, and defensive copies. - `internal/framework/llm/secrets_test.go`: provider-error redaction. -- `internal/cli/run_test.go`: profile validation, production client wiring, - manifest recording, and debug integration. +- `internal/cli/run_contract_test.go`: profile validation, production client + wiring, manifest recording, and debug integration. - Module-local `scriptorium_assets_test.go` files: prompt inputs and package asset registration. diff --git a/docs/internal/modules.md b/docs/internal/modules.md index 39344ea..9e63d71 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -238,10 +238,11 @@ does not inventory implementations. - Package-local `*_test.go` files under the module or validator being changed. - `internal/framework/pipeline/typed_resolution_test.go`: typed registry, spec, and heterogeneous artifact composition. -- `internal/framework/pipeline/default_modules_test.go`: framework binding - defaults. -- `internal/cli/run_test.go`: production catalog, config resolution, and - end-to-end CLI composition. +- `internal/framework/pipeline/profile_test.go`: framework binding defaults and + profile resolution. +- `internal/cli/production_contract_test.go`: production catalog, config + resolution, and composition smoke coverage. +- `internal/cli/example_contract_test.go`: maintained example ownership. - `internal/framework/promptfs/*_test.go` and `internal/modules/dnd/shared/*_test.go`: shared prompt and reference assembly. - `internal/modules/integration/*_test.go`: black-box composition across diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index dcbb5b4..d141df8 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -321,8 +321,12 @@ stage, resolved lane, and source chunk rather than completion time. construction order, dependency failures, and the before-source-work boundary. - `internal/framework/pipeline/references_test.go`: target resolution and materialization. -- `internal/cli/run_test.go`: production stage transitions, retries, rejections, - warnings, debug hooks, manifests, and end-to-end composition. +- `internal/cli/run_contract_test.go`: production run transitions, retries, + rejections, warnings, debug hooks, and manifests. +- `internal/cli/production_contract_test.go`: production composition and + configuration-resolution smoke coverage. +- `internal/cli/example_contract_test.go`: maintained example resolution and + execution ownership. - `internal/modules/integration/*_test.go` and `internal/modules/seriatim/input/transcript/runner_test.go`: typed runner composition across concrete module families. diff --git a/docs/internal/state.md b/docs/internal/state.md index 371ce6d..b401bf8 100644 --- a/docs/internal/state.md +++ b/docs/internal/state.md @@ -52,10 +52,13 @@ separately and never replace the command's primary error. ## Tests To Inspect -- `internal/cli/state_surfaces_test.go`: debug allocation and configuration - boundaries. +- `internal/cli/run_contract_test.go`: command-owned state allocation, + terminalization, and output/report boundaries. - `internal/cli/state_hardening_test.go`: independent roots, reuse, failures, permissions, cleanup, and redaction. +- `internal/cli/production_contract_test.go`: production composition and + configuration validation at the CLI boundary. +- `internal/cli/example_contract_test.go`: maintained example ownership. - `internal/core/debugbundle/*_test.go`: bundle allocation and summary writes. - `internal/framework/checkpoint/*_test.go`: checkpoint serialization and reuse. diff --git a/internal/framework/chunkplan/import_boundaries_test.go b/internal/framework/chunkplan/import_boundaries_test.go deleted file mode 100644 index 183b77f..0000000 --- a/internal/framework/chunkplan/import_boundaries_test.go +++ /dev/null @@ -1,62 +0,0 @@ -package chunkplan - -import ( - "go/parser" - "go/token" - "path/filepath" - "runtime" - "strconv" - "strings" - "testing" -) - -const repositoryImportPrefix = "gitea.maximumdirect.net/eric/notarius/internal/" - -func TestPlanStoreAndSourceImportBoundaries(t *testing.T) { - repositoryRoot := repositoryRoot(t) - for _, tc := range []struct { - name string - directory string - forbidden []string - }{ - {name: "source is framework and module independent", directory: "internal/core/source", forbidden: []string{"framework/", "modules/"}}, - {name: "plan store is module independent", directory: "internal/framework/chunkplan", forbidden: []string{"modules/"}}, - } { - t.Run(tc.name, func(t *testing.T) { - files, err := filepath.Glob(filepath.Join(repositoryRoot, tc.directory, "*.go")) - if err != nil { - t.Fatal(err) - } - for _, filename := range files { - if strings.HasSuffix(filename, "_test.go") { - continue - } - parsed, err := parser.ParseFile(token.NewFileSet(), filename, nil, parser.ImportsOnly) - if err != nil { - t.Fatal(err) - } - for _, item := range parsed.Imports { - path, err := strconv.Unquote(item.Path.Value) - if err != nil { - t.Fatal(err) - } - path = strings.TrimPrefix(path, repositoryImportPrefix) - for _, prefix := range tc.forbidden { - if strings.HasPrefix(path, prefix) { - t.Fatalf("%s imports %q, forbidden by %s boundary", filepath.Base(filename), path, tc.name) - } - } - } - } - }) - } -} - -func repositoryRoot(t *testing.T) string { - t.Helper() - _, filename, _, ok := runtime.Caller(0) - if !ok { - t.Fatal("resolve test location") - } - return filepath.Clean(filepath.Join(filepath.Dir(filename), "..", "..", "..")) -} diff --git a/internal/modules/dnd/register/register_test.go b/internal/modules/dnd/register/register_test.go index 0d2e043..816db3b 100644 --- a/internal/modules/dnd/register/register_test.go +++ b/internal/modules/dnd/register/register_test.go @@ -10,6 +10,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/llm" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/extract/spells" ) @@ -19,12 +20,10 @@ func TestRegisterAddsDNDFamily(t *testing.T) { if err := Register(registries, assets); err != nil { t.Fatalf("Register() error = %v, want nil", err) } - assertKeys(t, "chunkers", registries.Chunkers.RegisteredKeys(), []string{"dnd/scenes"}) - assertKeys(t, "extractors", registries.Extractors.RegisteredKeys(), []string{"dnd/spells"}) - if got := registries.ArtifactCodecs.RegisteredKinds(); !reflect.DeepEqual(got, []contracts.ArtifactKind{"dnd/spell-list"}) { - t.Fatalf("artifact codec kinds = %#v, want dnd/spell-list", got) - } - assertKeys(t, "validators", registries.Validators.RegisteredKeys(), []string{ + assertContainsKeys(t, "chunkers", registries.Chunkers.RegisteredKeys(), []string{"dnd/scenes"}) + assertContainsKeys(t, "extractors", registries.Extractors.RegisteredKeys(), []string{"dnd/spells"}) + assertContainsArtifactKinds(t, registries.ArtifactCodecs.RegisteredKinds(), []contracts.ArtifactKind{dnd.SpellListKind}) + assertContainsKeys(t, "validators", registries.Validators.RegisteredKeys(), []string{ "extract/dnd/spells/shape", "extract/dnd/spells/source_refs", "extract/dnd/spells/source_relatedness", @@ -41,7 +40,7 @@ func TestRegisterAddsDNDFamily(t *testing.T) { if got := registries.ValidatorChains.Validators(pipeline.StageExtract, spells.Key); !reflect.DeepEqual(got, wantChain) { t.Fatalf("spell validator chain = %#v, want %#v", got, wantChain) } - assertAssetNames(t, assets.PromptFS, []string{ + assertAssetNamesContain(t, assets.PromptFS, []string{ "dnd.scenes/dnd.scenes.yaml", "dnd.scenes/instructions.md", "dnd.scenes/sharedassets/common-dnd-references.md", @@ -55,10 +54,16 @@ func TestRegisterAddsDNDFamily(t *testing.T) { "dnd.spells/sharedassets/common-dnd-transcript.md", "dnd.spells/task.md", }) - assertAssetNames(t, assets.SchemaFS, []string{ + assertAssetNamesContain(t, assets.SchemaFS, []string{ "dnd_scenes.v1.json", "dnd_spells_llm.v1.json", }) + if spec, ok := registries.Chunkers.Spec("dnd/scenes"); !ok || spec.Key != "dnd/scenes" { + t.Fatalf("scene chunker spec = %#v, present = %t; want family-owned spec", spec, ok) + } + if spec, ok := registries.Extractors.Spec(spells.Key); !ok || spec.ArtifactKind != dnd.SpellListKind { + t.Fatalf("spell extractor spec = %#v, present = %t; want dnd spell-list artifact", spec, ok) + } } func TestRegisterRejectsMissingDNDDependenciesBeforeMutation(t *testing.T) { @@ -118,14 +123,33 @@ func completeRegistries() pipeline.Registries { } } -func assertKeys(t *testing.T, name string, got, want []string) { +func assertContainsKeys(t *testing.T, name string, got, want []string) { t.Helper() - if !reflect.DeepEqual(got, want) { - t.Fatalf("%s keys = %#v, want %#v", name, got, want) + seen := make(map[string]struct{}, len(got)) + for _, key := range got { + seen[key] = struct{}{} + } + for _, key := range want { + if _, ok := seen[key]; !ok { + t.Fatalf("%s keys = %#v, want required key %q", name, got, key) + } } } -func assertAssetNames(t *testing.T, getFS func() (fs.FS, error), want []string) { +func assertContainsArtifactKinds(t *testing.T, got, want []contracts.ArtifactKind) { + t.Helper() + seen := make(map[contracts.ArtifactKind]struct{}, len(got)) + for _, kind := range got { + seen[kind] = struct{}{} + } + for _, kind := range want { + if _, ok := seen[kind]; !ok { + t.Fatalf("artifact codec kinds = %#v, want required kind %q", got, kind) + } + } +} + +func assertAssetNamesContain(t *testing.T, getFS func() (fs.FS, error), want []string) { t.Helper() fSys, err := getFS() if err != nil { @@ -141,7 +165,13 @@ func assertAssetNames(t *testing.T, getFS func() (fs.FS, error), want []string) t.Fatalf("walk assets: %v", err) } sort.Strings(got) - if !reflect.DeepEqual(got, want) { - t.Fatalf("asset names = %#v, want %#v", got, want) + seen := make(map[string]struct{}, len(got)) + for _, name := range got { + seen[name] = struct{}{} + } + for _, name := range want { + if _, ok := seen[name]; !ok { + t.Fatalf("asset names = %#v, want required asset %q", got, name) + } } } diff --git a/internal/modules/generic/register/register_test.go b/internal/modules/generic/register/register_test.go index 6a34e4d..9d33fc1 100644 --- a/internal/modules/generic/register/register_test.go +++ b/internal/modules/generic/register/register_test.go @@ -1,7 +1,6 @@ package register import ( - "reflect" "strings" "testing" @@ -13,21 +12,23 @@ func TestRegisterAddsGenericFamily(t *testing.T) { if err := Register(registries, nil); err != nil { t.Fatalf("Register() error = %v, want nil", err) } - assertKeys(t, "chunkers", registries.Chunkers.RegisteredKeys(), []string{"generic"}) - assertKeys(t, "mergers", registries.Mergers.RegisteredKeys(), nil) - assertKeys(t, "normalizers", registries.Normalizers.RegisteredKeys(), nil) - assertKeys(t, "validators", registries.Validators.RegisteredKeys(), []string{ + assertContainsKeys(t, "chunkers", registries.Chunkers.RegisteredKeys(), []string{"generic"}) + assertContainsKeys(t, "validators", registries.Validators.RegisteredKeys(), []string{ "generic/always_accept", "generic/always_reject", "generic/valid_json", "generic/valid_json_schema", }) - assertKeys(t, "outputs", registries.Outputs.RegisteredKeys(), []string{"json"}) - if got := registries.Inputs.RegisteredKeys(); len(got) != 0 { - t.Fatalf("input keys = %#v, want generic registrar to leave inputs unchanged", got) + assertContainsKeys(t, "outputs", registries.Outputs.RegisteredKeys(), []string{"json"}) + assertNoKeys(t, "inputs", registries.Inputs.RegisteredKeys(), "generic registrar to leave inputs unchanged") + assertNoKeys(t, "extractors", registries.Extractors.RegisteredKeys(), "generic registrar to leave extractors unchanged") + assertNoKeys(t, "mergers", registries.Mergers.RegisteredKeys(), "generic registrar to leave mergers for typed family composition") + assertNoKeys(t, "normalizers", registries.Normalizers.RegisteredKeys(), "generic registrar to leave normalizers for typed family composition") + if chunker, err := registries.Chunkers.Build("generic"); err != nil || chunker.Key() != "generic" { + t.Fatalf("build generic chunker = %v, %v; want generic implementation", chunker, err) } - if got := registries.Extractors.RegisteredKeys(); len(got) != 0 { - t.Fatalf("extractor keys = %#v, want generic registrar to leave extractors unchanged", got) + if output, err := registries.Outputs.Build("json"); err != nil || output.Key() != "json" { + t.Fatalf("build json output = %v, %v; want json implementation", output, err) } } @@ -83,9 +84,22 @@ func completeRegistries() pipeline.Registries { } } -func assertKeys(t *testing.T, name string, got, want []string) { +func assertContainsKeys(t *testing.T, name string, got, want []string) { t.Helper() - if !reflect.DeepEqual(got, want) { - t.Fatalf("%s keys = %#v, want %#v", name, got, want) + seen := make(map[string]struct{}, len(got)) + for _, key := range got { + seen[key] = struct{}{} + } + for _, key := range want { + if _, ok := seen[key]; !ok { + t.Fatalf("%s keys = %#v, want required key %q", name, got, key) + } + } +} + +func assertNoKeys(t *testing.T, name string, got []string, reason string) { + t.Helper() + if len(got) != 0 { + t.Fatalf("%s keys = %#v, want %s", name, got, reason) } } diff --git a/internal/modules/import_boundaries_test.go b/internal/modules/import_boundaries_test.go index 2027807..01e52b4 100644 --- a/internal/modules/import_boundaries_test.go +++ b/internal/modules/import_boundaries_test.go @@ -13,6 +13,7 @@ import ( ) const moduleImportPrefix = "gitea.maximumdirect.net/eric/notarius/internal/modules/" +const internalImportPrefix = "gitea.maximumdirect.net/eric/notarius/internal/" func TestProductionImportBoundaries(t *testing.T) { repositoryRoot := testRepositoryRoot(t) @@ -252,6 +253,39 @@ func TestImportBoundaryRules(t *testing.T) { importPath: moduleImportPrefix + "generic/chunk/units", wantError: true, }, + { + name: "source production may import another core package", + filename: "internal/core/source/source.go", + sourcePackage: "source", + importPath: internalImportPrefix + "artifacts", + }, + { + name: "source production cannot import framework", + filename: "internal/core/source/source.go", + sourcePackage: "source", + importPath: internalImportPrefix + "framework/contracts", + wantError: true, + }, + { + name: "source production cannot import modules", + filename: "internal/core/source/source.go", + sourcePackage: "source", + importPath: moduleImportPrefix + "dnd", + wantError: true, + }, + { + name: "chunkplan production may import framework", + filename: "internal/framework/chunkplan/store.go", + sourcePackage: "chunkplan", + importPath: internalImportPrefix + "framework/contracts", + }, + { + name: "chunkplan production cannot import modules", + filename: "internal/framework/chunkplan/store.go", + sourcePackage: "chunkplan", + importPath: moduleImportPrefix + "dnd", + wantError: true, + }, { name: "framework test may import module implementation", filename: "internal/framework/pipeline/compatibility_test.go", @@ -345,6 +379,9 @@ func checkImportBoundaries(repositoryRoot string, filename string) error { } func validateImport(filename string, sourcePackage string, importPath string) error { + if !strings.HasSuffix(filename, "_test.go") && strings.HasPrefix(filename, "internal/core/source/") && strings.HasPrefix(importPath, internalImportPrefix+"framework/") { + return importBoundaryViolation(filename, importPath, "core/source production code must not import framework or module implementations") + } target, ok := moduleTargetForImport(importPath) if !ok { return nil @@ -384,6 +421,12 @@ func validateImport(filename string, sourcePackage string, importPath string) er } return importBoundaryViolation(filename, importPath, "direct module imports from non-module tests are allowed only in CLI, core, and framework compatibility-test roots") } + if strings.HasPrefix(filename, "internal/core/source/") && strings.HasPrefix(importPath, moduleImportPrefix) { + return importBoundaryViolation(filename, importPath, "core/source production code must not import framework or module implementations") + } + if strings.HasPrefix(filename, "internal/framework/chunkplan/") && strings.HasPrefix(importPath, moduleImportPrefix) { + return importBoundaryViolation(filename, importPath, "framework/chunkplan production code must not import module implementations") + } if strings.HasPrefix(filename, "internal/framework/") || strings.HasPrefix(filename, "internal/core/") { return importBoundaryViolation(filename, importPath, "core and framework production code must not import module implementations") } diff --git a/internal/modules/seriatim/register/register_test.go b/internal/modules/seriatim/register/register_test.go index 59792e8..2df31e6 100644 --- a/internal/modules/seriatim/register/register_test.go +++ b/internal/modules/seriatim/register/register_test.go @@ -1,7 +1,6 @@ package register import ( - "reflect" "strings" "testing" @@ -13,8 +12,22 @@ func TestRegisterAddsSeriatimFamily(t *testing.T) { if err := Register(registries, nil); err != nil { t.Fatalf("Register() error = %v, want nil", err) } - if got, want := registries.Inputs.RegisteredKeys(), []string{"seriatim"}; !reflect.DeepEqual(got, want) { - t.Fatalf("input keys = %#v, want %#v", got, want) + assertContainsKeys(t, "inputs", registries.Inputs.RegisteredKeys(), []string{"seriatim"}) + if adapter, err := registries.Inputs.Build("seriatim"); err != nil || adapter.Key() != "seriatim" { + t.Fatalf("build seriatim input = %v, %v; want seriatim implementation", adapter, err) + } +} + +func assertContainsKeys(t *testing.T, name string, got, want []string) { + t.Helper() + seen := make(map[string]struct{}, len(got)) + for _, key := range got { + seen[key] = struct{}{} + } + for _, key := range want { + if _, ok := seen[key]; !ok { + t.Fatalf("%s keys = %#v, want required key %q", name, got, key) + } } }