diff --git a/docs/internal/modules.md b/docs/internal/modules.md index b0091f7..6c8993b 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -189,6 +189,11 @@ Seriatim registrars own their production leaf registrations. The D&D registrar owns D&D leaf registrations, the spell default-validator chain, and D&D prompt/schema asset collection. +Concrete implementation packages do not import generic implementation +packages directly. A concrete family's `register` package is its composition +point for specializing reusable generic implementations, while the generic +registrar composes only generic children. + Framework packages must not import production extensions. Tests may compose registries and catalogs directly with fakes. diff --git a/internal/modules/import_boundaries_test.go b/internal/modules/import_boundaries_test.go index 9affb16..50cb8fa 100644 --- a/internal/modules/import_boundaries_test.go +++ b/internal/modules/import_boundaries_test.go @@ -41,69 +41,145 @@ func TestImportBoundaryFixtureIsRejected(t *testing.T) { fixture := filepath.Join(repositoryRoot, "internal", "modules", "generic", "testdata", "importboundaries", "imports_dnd.go") err := checkImportBoundaries(repositoryRoot, fixture) if err == nil { - t.Fatal("fixture import was accepted, want generic-to-D&D violation") + t.Fatal("fixture import was accepted, want generic-to-concrete violation") } - if !strings.Contains(err.Error(), "generic packages must not import D&D packages") { - t.Fatalf("fixture error = %q, want generic-to-D&D violation", err) + if !strings.Contains(err.Error(), "generic family must not import concrete family") { + t.Fatalf("fixture error = %q, want generic-to-concrete violation", err) } } func TestImportBoundaryRules(t *testing.T) { tests := []struct { - name string - filename string - importPath string - wantError bool + name string + filename string + sourcePackage string + importPath string + wantError bool }{ { - name: "D&D implementation cannot import Seriatim", - filename: "internal/modules/dnd/extract/example/extractor.go", - importPath: moduleImportPrefix + "seriatim/input/transcript", - wantError: true, + name: "concrete implementation cannot import peer concrete family", + filename: "internal/modules/dnd/extract/example/extractor.go", + sourcePackage: "example", + importPath: moduleImportPrefix + "seriatim/input/transcript", + wantError: true, }, { - name: "Seriatim implementation cannot import D&D", - filename: "internal/modules/seriatim/input/example/adapter.go", - importPath: moduleImportPrefix + "dnd/shared", - wantError: true, + name: "future concrete family cannot import current concrete family", + filename: "internal/modules/almanac/extract/example/extractor.go", + sourcePackage: "example", + importPath: moduleImportPrefix + "dnd/shared", + wantError: true, }, { - name: "generic implementation cannot import D&D", - filename: "internal/modules/generic/merge/example/merger.go", - importPath: moduleImportPrefix + "dnd/shared", - wantError: true, + name: "current concrete family cannot import future concrete family", + filename: "internal/modules/seriatim/input/example/adapter.go", + sourcePackage: "example", + importPath: moduleImportPrefix + "almanac/shared", + wantError: true, }, { - name: "domain root cannot import child", - filename: "internal/modules/dnd/types.go", - importPath: moduleImportPrefix + "dnd/extract/spells", - wantError: true, + name: "generic implementation cannot import current concrete family", + filename: "internal/modules/generic/merge/example/merger.go", + sourcePackage: "example", + importPath: moduleImportPrefix + "dnd/shared", + wantError: true, }, { - name: "D&D implementation may import generic implementation", - filename: "internal/modules/dnd/extract/example/extractor.go", - importPath: moduleImportPrefix + "generic/normalize/noop", + name: "generic implementation cannot import future concrete family", + filename: "internal/modules/generic/merge/example/merger.go", + sourcePackage: "example", + importPath: moduleImportPrefix + "almanac/shared", + wantError: true, }, { - name: "domain registrar may compose child packages", - filename: "internal/modules/dnd/register/register.go", - importPath: moduleImportPrefix + "dnd/extract/spells", + name: "concrete implementation cannot import generic implementation", + filename: "internal/modules/dnd/extract/example/extractor.go", + sourcePackage: "example", + importPath: moduleImportPrefix + "generic/normalize/noop", + wantError: true, }, { - name: "CLI may compose registrars", - filename: "internal/cli/catalog.go", - importPath: moduleImportPrefix + "dnd/register", + name: "concrete white-box test follows production rules", + filename: "internal/modules/dnd/extract/example/extractor_test.go", + sourcePackage: "example", + importPath: moduleImportPrefix + "generic/normalize/noop", + wantError: true, }, { - name: "external integration test may compose domains", - filename: "internal/modules/integration/example_test.go", - importPath: moduleImportPrefix + "dnd/extract/spells", + name: "concrete registrar may compose generic implementation", + filename: "internal/modules/almanac/register/register.go", + sourcePackage: "register", + importPath: moduleImportPrefix + "generic/normalize/noop", + }, + { + name: "family root cannot import child implementation", + filename: "internal/modules/almanac/types.go", + sourcePackage: "almanac", + importPath: moduleImportPrefix + "almanac/extract/events", + wantError: true, + }, + { + name: "family root cannot import registrar", + filename: "internal/modules/almanac/types.go", + sourcePackage: "almanac", + importPath: moduleImportPrefix + "almanac/register", + wantError: true, + }, + { + name: "child may import family root", + filename: "internal/modules/almanac/extract/events/extractor.go", + sourcePackage: "events", + importPath: moduleImportPrefix + "almanac", + }, + { + name: "child may import same-family sibling", + filename: "internal/modules/almanac/extract/events/extractor.go", + sourcePackage: "events", + importPath: moduleImportPrefix + "almanac/shared", + }, + { + name: "concrete registrar may compose own child", + filename: "internal/modules/almanac/register/register.go", + sourcePackage: "register", + importPath: moduleImportPrefix + "almanac/extract/events", + }, + { + name: "generic registrar may compose generic child", + filename: "internal/modules/generic/register/register.go", + sourcePackage: "register", + importPath: moduleImportPrefix + "generic/output/json", + }, + { + name: "application composition root may compose registrars", + filename: "internal/cli/catalog.go", + sourcePackage: "cli", + importPath: moduleImportPrefix + "almanac/register", + }, + { + name: "black-box integration test may compose families", + filename: "internal/modules/integration/example_test.go", + sourcePackage: "integration_test", + importPath: moduleImportPrefix + "almanac/extract/events", + }, + { + name: "white-box integration test is not exempt", + filename: "internal/modules/integration/example_test.go", + sourcePackage: "integration", + importPath: moduleImportPrefix + "almanac/extract/events", + wantError: true, + }, + { + name: "integration production file is not exempt", + filename: "internal/modules/integration/example.go", + sourcePackage: "integration", + importPath: moduleImportPrefix + "almanac/extract/events", + wantError: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateImport(tt.filename, tt.importPath) + err := validateImport(tt.filename, tt.sourcePackage, tt.importPath) if tt.wantError && err == nil { t.Fatal("validateImport() error = nil, want boundary violation") } @@ -129,68 +205,77 @@ func checkImportBoundaries(repositoryRoot string, filename string) error { if err != nil { return fmt.Errorf("parse import in %s: %w", relative, err) } - if err := validateImport(relative, importPath); err != nil { + if err := validateImport(relative, parsed.Name.Name, importPath); err != nil { return fmt.Errorf("%s imports %s: %w", relative, importPath, err) } } return nil } -func validateImport(filename string, importPath string) error { - if isExternalIntegrationTest(filename) { +func validateImport(filename string, sourcePackage string, importPath string) error { + if isBlackBoxIntegrationTest(filename, sourcePackage) { return nil } - sourceDomain, sourceRoot := domainForFile(filename) - targetDomain, targetChild := domainForImport(importPath) - if sourceDomain == "" || targetDomain == "" { + targetFamily, targetChild := moduleFamilyForImport(importPath) + if targetFamily == "" { return nil } - if sourceRoot && sourceDomain == targetDomain && targetChild { - return fmt.Errorf("domain root packages must not import child implementations") + if isIntegrationFile(filename) { + return fmt.Errorf("module integration composition is allowed only in black-box tests") } - if sourceDomain == "generic" && targetDomain == "dnd" { - return fmt.Errorf("generic packages must not import D&D packages") + sourceFamily, sourceRoot, sourceRegistrar := moduleFamilyForFile(filename) + if sourceFamily == "" { + return nil } - if sourceDomain == "dnd" && targetDomain == "seriatim" { - return fmt.Errorf("D&D packages must not import Seriatim packages") + if sourceRoot && sourceFamily == targetFamily && targetChild { + return fmt.Errorf("family root must not import child packages") } - if sourceDomain == "seriatim" && targetDomain == "dnd" { - return fmt.Errorf("Seriatim packages must not import D&D packages") + if sourceFamily == targetFamily { + return nil } - return nil + if sourceFamily == "generic" { + return fmt.Errorf("generic family must not import concrete family %q", targetFamily) + } + if targetFamily == "generic" { + if sourceRegistrar { + return nil + } + return fmt.Errorf("concrete family %q may import generic implementations only from its registrar", sourceFamily) + } + return fmt.Errorf("concrete family %q must not import concrete family %q", sourceFamily, targetFamily) } -func domainForFile(filename string) (domain string, root bool) { +func moduleFamilyForFile(filename string) (family string, root bool, registrar bool) { const prefix = "internal/modules/" if !strings.HasPrefix(filename, prefix) { - return "", false + return "", false, false } remainder := strings.TrimPrefix(filename, prefix) parts := strings.Split(remainder, "/") - if len(parts) < 2 || !isDomain(parts[0]) { - return "", false + if len(parts) < 2 || parts[0] == "integration" { + return "", false, false } - return parts[0], len(parts) == 2 + return parts[0], len(parts) == 2, len(parts) > 2 && parts[1] == "register" } -func domainForImport(importPath string) (domain string, child bool) { +func moduleFamilyForImport(importPath string) (family string, child bool) { if !strings.HasPrefix(importPath, moduleImportPrefix) { return "", false } remainder := strings.TrimPrefix(importPath, moduleImportPrefix) parts := strings.Split(remainder, "/") - if len(parts) == 0 || !isDomain(parts[0]) { + if len(parts) == 0 || parts[0] == "" || parts[0] == "integration" { return "", false } return parts[0], len(parts) > 1 } -func isDomain(name string) bool { - return name == "dnd" || name == "generic" || name == "seriatim" +func isIntegrationFile(filename string) bool { + return strings.HasPrefix(filename, "internal/modules/integration/") } -func isExternalIntegrationTest(filename string) bool { - return strings.HasPrefix(filename, "internal/modules/integration/") && strings.HasSuffix(filename, "_test.go") +func isBlackBoxIntegrationTest(filename string, sourcePackage string) bool { + return isIntegrationFile(filename) && strings.HasSuffix(filename, "_test.go") && sourcePackage == "integration_test" } func testRepositoryRoot(t *testing.T) string { diff --git a/internal/modules/seriatim/input/transcript/config_test.go b/internal/modules/seriatim/input/transcript/config_test.go index d26b6c2..8671de2 100644 --- a/internal/modules/seriatim/input/transcript/config_test.go +++ b/internal/modules/seriatim/input/transcript/config_test.go @@ -12,8 +12,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" - "gitea.maximumdirect.net/eric/notarius/internal/modules/generic/merge/appendorder" - "gitea.maximumdirect.net/eric/notarius/internal/modules/generic/normalize/noop" ) func TestPipelineConfigLoadsAndResolvesWithSeriatimInput(t *testing.T) { @@ -192,12 +190,7 @@ func mustRegisterExtractor(t *testing.T, registry *pipeline.ExtractorRegistry, s func mustRegisterMerger(t *testing.T, registry *pipeline.MergerRegistry, spec pipeline.ModuleSpec) { t.Helper() if err := pipeline.RegisterMerger[seriatimArtifact](registry, spec, func() (contracts.Merger[seriatimArtifact], error) { - return appendorder.NewTyped(func(values []seriatimArtifact) (seriatimArtifact, error) { - if len(values) == 0 { - return seriatimArtifact{}, nil - } - return values[0], nil - }) + return fakeMerger{}, nil }); err != nil { t.Fatalf("register merger: %v", err) } @@ -206,7 +199,7 @@ func mustRegisterMerger(t *testing.T, registry *pipeline.MergerRegistry, spec pi func mustRegisterNormalizer(t *testing.T, registry *pipeline.NormalizerRegistry, spec pipeline.ModuleSpec) { t.Helper() if err := pipeline.RegisterNormalizer[seriatimArtifact](registry, spec, func() (contracts.Normalizer[seriatimArtifact], error) { - return noop.NewTyped[seriatimArtifact](), nil + return fakeNormalizer{}, nil }); err != nil { t.Fatalf("register normalizer: %v", err) } @@ -241,6 +234,27 @@ func (fakeExtractor) Extract(ctx context.Context, req contracts.TypedExtractionR return contracts.TypedExtractionResult[seriatimArtifact]{}, nil } +type fakeMerger struct{} + +func (fakeMerger) Key() string { return pipeline.DefaultMergeModule } + +func (fakeMerger) Merge(ctx context.Context, req contracts.TypedMergeRequest[seriatimArtifact]) (contracts.TypedMergeResult[seriatimArtifact], error) { + if len(req.ExtractOutputs) == 0 { + return contracts.TypedMergeResult[seriatimArtifact]{}, nil + } + return contracts.TypedMergeResult[seriatimArtifact]{Value: req.ExtractOutputs[0].Value}, nil +} + +type fakeNormalizer struct{} + +func (fakeNormalizer) Key() string { return pipeline.DefaultNormalizeModule } + +func (fakeNormalizer) ReferenceSlots() []contracts.ReferenceSlot { return nil } + +func (fakeNormalizer) Normalize(ctx context.Context, req contracts.TypedNormalizeRequest[seriatimArtifact]) (contracts.TypedNormalizeResult[seriatimArtifact], error) { + return contracts.TypedNormalizeResult[seriatimArtifact]{Value: req.MergeOutput.Value}, nil +} + type fakeOutput struct{} func (fakeOutput) Key() string { return pipeline.DefaultOutputModule } diff --git a/internal/modules/seriatim/input/transcript/runner_test.go b/internal/modules/seriatim/input/transcript/runner_test.go index 1bd58dd..aad1e72 100644 --- a/internal/modules/seriatim/input/transcript/runner_test.go +++ b/internal/modules/seriatim/input/transcript/runner_test.go @@ -11,8 +11,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" - "gitea.maximumdirect.net/eric/notarius/internal/modules/generic/merge/appendorder" - "gitea.maximumdirect.net/eric/notarius/internal/modules/generic/normalize/noop" ) func runPreparedPipeline(t *testing.T, registries pipeline.Registries, resolved pipeline.ResolvedPipeline, llmClient contracts.StructuredLLMClient, input pipeline.RunInput) (pipeline.RunOutput, error) { @@ -138,15 +136,14 @@ func seriatimRunnerRegistries(t *testing.T, extractor contracts.Extractor[seriat }); err != nil { t.Fatalf("register extractor: %v", err) } - if err := appendorder.RegisterTyped(mergers, seriatimArtifactKind, func(values []seriatimArtifact) (seriatimArtifact, error) { - if len(values) == 0 { - return seriatimArtifact{}, nil - } - return values[0], nil + if err := pipeline.RegisterMerger[seriatimArtifact](mergers, pipeline.ModuleSpec{Key: pipeline.DefaultMergeModule, Stage: pipeline.StageMerge, ArtifactKind: seriatimArtifactKind}, func() (contracts.Merger[seriatimArtifact], error) { + return fakeMerger{}, nil }); err != nil { t.Fatalf("register merger: %v", err) } - if err := noop.RegisterTyped[seriatimArtifact](normalizers, seriatimArtifactKind); err != nil { + if err := pipeline.RegisterNormalizer[seriatimArtifact](normalizers, pipeline.ModuleSpec{Key: pipeline.DefaultNormalizeModule, Stage: pipeline.StageNormalize, ArtifactKind: seriatimArtifactKind}, func() (contracts.Normalizer[seriatimArtifact], error) { + return fakeNormalizer{}, nil + }); err != nil { t.Fatalf("register normalizer: %v", err) } if err := outputs.Register(pipeline.DefaultOutputModule, func() (contracts.OutputEncoder, error) {