From b949e9bbc09f5d8085f2332c911e1494a4725cd0 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 17 Jul 2026 06:30:56 +0000 Subject: [PATCH] Construct universal modules with decoded options --- docs/config.md | 4 ++ docs/internal/llm.md | 10 ++- docs/internal/modules.md | 25 +++++--- docs/internal/overview.md | 10 +-- docs/internal/pipeline.md | 17 +++-- internal/cli/compatibility_test.go | 16 ++++- internal/cli/run.go | 1 + .../framework/contracts/composition_test.go | 16 +---- internal/framework/contracts/contracts.go | 4 -- .../framework/contracts/contracts_test.go | 9 ++- internal/framework/pipeline/debug.go | 9 +++ internal/framework/pipeline/runner.go | 4 -- internal/framework/pipeline/runner_test.go | 40 ++++++++---- internal/modules/dnd/chunk/scenes/chunker.go | 45 +++++++++---- .../modules/dnd/chunk/scenes/chunker_test.go | 64 +++++++++++-------- .../chunk/scenes/scriptorium_assets_test.go | 2 +- .../modules/generic/chunk/units/chunker.go | 62 ++++++++++-------- .../generic/chunk/units/chunker_test.go | 46 +++++++------ .../modules/generic/output/json/encoder.go | 29 +++++++-- .../generic/output/json/encoder_test.go | 3 + .../seriatim/input/transcript/adapter.go | 29 +++++++-- .../input/transcript/registry_test.go | 3 + 22 files changed, 290 insertions(+), 158 deletions(-) diff --git a/docs/config.md b/docs/config.md index fba1bac..5decacf 100644 --- a/docs/config.md +++ b/docs/config.md @@ -236,6 +236,10 @@ Binding fields: chain; set a non-empty list to use exactly those validators in configured order. +During resolution, each selected module's registered option validator runs. +Production input, chunk, and output bindings reject unknown or invalid options +with the affected binding context. + Validator bindings use the same shorthand or object module-binding form, but only these fields are supported: diff --git a/docs/internal/llm.md b/docs/internal/llm.md index 000eb73..af9f07c 100644 --- a/docs/internal/llm.md +++ b/docs/internal/llm.md @@ -33,10 +33,16 @@ without exposing Scriptorium types through stage contracts. 2. creating a `ScriptoriumClient` from the effective profile source; 3. attaching an `LLMProfileRecorder`; 4. creating a scheduler from the effective concurrency limit; -5. returning a `ScheduledClient` wrapper; and -6. injecting that one shared client into complete pipeline preparation before +5. returning a `ScheduledClient` wrapper; +6. decorating that shared client before preparation when debug recording is + enabled; and +7. injecting that one shared client into complete pipeline preparation before the source file is read or the runner is invoked. +The D&D scene chunker retains this injected client and uses it for every scene +completion. Later legacy LLM-backed operations still receive the same shared +client through their operation requests. + The CLI separately gathers explicit profile IDs from resolved LLM-capable stage and validator bindings. It prepares a small internal check prompt for each ID so missing or invalid profiles fail before pipeline execution. The runtime profile diff --git a/docs/internal/modules.md b/docs/internal/modules.md index 87ee151..8af17e6 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -17,13 +17,15 @@ validator registry. Package-family registrars compose those leaf registrations into the production catalog and own family-level policy such as default validator chains and prompt asset collection. -Production extract, merge, normalize, and validator packages currently use the -explicit legacy raw registration APIs. Typed registration is framework-ready, -but no production artifact kind or codec is registered yet. All selected -production implementations are constructed during pipeline preparation through -temporary adapters around their existing zero-argument constructors. Their raw -option maps and LLM clients remain operation inputs until each implementation -migrates to its construction-owned decoder and injected dependencies. +Production input, chunk, and output packages register strict option decoders and +run-local builders. Preparation decodes their options into implementation-owned +values and injects dependencies; their operation requests contain run context, +not raw option maps or LLM clients. Production extract, merge, normalize, and +validator packages still use the explicit legacy raw registration APIs and +temporary adapters around zero-argument constructors. Their raw option maps and +LLM clients remain operation inputs while that part of the catalog migrates. +Typed registration is framework-ready, but no production artifact kind or +codec is registered yet. Specs expose capability and execution metadata without constructing an implementation. Registry entries separately expose option validation and @@ -51,8 +53,8 @@ evidence. The resolver and materializer behavior is described in The adapter decodes the supported transcript JSON, selects the source identity, computes canonical source provenance, validates segments, and maps each segment into a generic source unit with a self-reference plus speaker and timestamp -metadata. Its spec advertises the transcript capabilities consumed by D&D -modules. +metadata. It accepts no module options. Its spec advertises the transcript +capabilities consumed by D&D modules. Parsing is strict about required values and duplicate unit IDs but deliberately ignores unrelated Seriatim fields. The external format and derived-identity @@ -72,6 +74,8 @@ canonical source reference from those unit references. The accepted options and defaults are defined in [Configuration](../config.md#implemented-production-modules). Generic framework validation canonicalizes the returned unit slices before extraction. +The chunker decodes its options during construction and retains only the typed +window settings used by `Chunk`. ### `internal/modules/dnd/chunk/scenes` @@ -79,6 +83,9 @@ The scene chunker prepares a structured Scriptorium request from the full transcript, session, and optional D&D reference inputs. It validates the model's scene boundaries against source-unit IDs and converts them into deterministic chunks with canonical source references spanning each scene's units. +Preparation injects the shared structured LLM client into the chunker; `Chunk` +supplies only the run-specific profile, session, source, references, and +metadata. Scene validation requires sequential, contiguous, non-overlapping coverage from the first source unit through the last. Each chunk contains JSON scene content diff --git a/docs/internal/overview.md b/docs/internal/overview.md index 35441bc..ddcc97a 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -60,10 +60,12 @@ type equality across the lane, and records schema identity in the resolved lane and pipeline digest. Registry entries carry separate option-validation and run-local construction closures. Preparation injects shared dependencies and constructs input, chunk, validators, ordered lanes, and output before source -parsing. Production module families do not register typed variants yet and -continue through explicitly named legacy raw registrations and temporary -zero-argument constructor adapters. The current runner rejects a typed prepared -lane instead of routing it through raw execution. +parsing. Production input, chunk, and output modules use strict construction-time +option decoding, and the LLM-backed scene chunker retains the injected shared +client. Production artifact-lane modules and validators do not register typed +variants yet and continue through explicitly named legacy raw registrations and +temporary zero-argument constructor adapters. The current runner rejects a +typed prepared lane instead of routing it through raw execution. ## Production Extensions diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index c2efe81..4a3c77a 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -74,17 +74,18 @@ support chunks, artifacts, or both. Duplicate variants and exact Go-type mismatches are rejected deterministically. Production composition initializes the artifact codec registry without adding -codec entries, and production modules use the explicitly named legacy raw -registration APIs. A raw registration cannot satisfy a typed lane. +codec entries, and production artifact-lane modules use the explicitly named +legacy raw registration APIs. A raw registration cannot satisfy a typed lane. A `ModuleSpec` declares its stage plus required and provided capabilities. Chunk, extract, merge, and normalize specs may also declare reference slots. Registry implementations defensively copy spec metadata, reject duplicate keys, and verify that a constructed implementation reports the registered key. Builder registrations accept `ModuleDependencies` and cloned raw options through -one `BuildRequest`. Existing production registrations are adapted from their -zero-argument constructors while their implementation-owned option decoders are -migrated separately. +one `BuildRequest`. Production input, chunk, and output builders decode those +options and retain typed values or injected dependencies in the constructed +implementation. Remaining production raw-stage registrations are adapted from +their zero-argument constructors during migration. A `ValidatorSpec` declares a validator key and execution class. Resolution uses the execution class to reject incompatible profile bindings before execution. @@ -106,7 +107,11 @@ session and profile metadata, and checkpoint/debug collaborators. The runner parses source bytes through the already constructed input adapter. Later stage requests receive the generic source model; extract requests receive chunk-scoped input material, while chunk, merge, and normalize requests retain -access to the original source material. +access to the original source material. Input, chunk, and output operation +requests do not carry raw module options. The chunk request also does not carry +an LLM client; an LLM-backed chunker receives the shared client during +preparation. Their operation requests retain run-specific source, reference, +profile, session, and metadata context as applicable. Typed lanes can be composed, resolved, and prepared but are not executed by the current raw runner. The runner rejects such input before source work; diff --git a/internal/cli/compatibility_test.go b/internal/cli/compatibility_test.go index c3fdf47..3a5060d 100644 --- a/internal/cli/compatibility_test.go +++ b/internal/cli/compatibility_test.go @@ -111,7 +111,7 @@ func TestProductionCompatibilitySnapshot(t *testing.T) { }) identitySnapshot := map[string]map[string]any{ - "scenes": scenes.New().ManifestMetadata(), + "scenes": sceneManifestMetadata(t), "spells": spells.New().ManifestMetadata(), } for name, metadata := range identitySnapshot { @@ -340,7 +340,10 @@ func TestProductionLLMCallersShareScheduledClient(t *testing.T) { errs := make(chan error, 2) go func() { started.Done() - _, err := scenes.New().Chunk(context.Background(), contracts.ChunkRequest{Source: doc, LLMClient: client}) + chunker, err := scenes.New(client, scenes.Options{}) + if err == nil { + _, err = chunker.Chunk(context.Background(), contracts.ChunkRequest{Source: doc}) + } errs <- err }() go func() { @@ -508,6 +511,15 @@ func (client *blockingProductionLLMClient) CompleteStructured(ctx context.Contex return contracts.StructuredCompletionResponse{Content: payload}, nil } +func sceneManifestMetadata(t *testing.T) map[string]any { + t.Helper() + chunker, err := scenes.New(orderingProductionLLMClient{}, scenes.Options{}) + if err != nil { + t.Fatalf("construct scene chunker: %v", err) + } + return chunker.ManifestMetadata() +} + func assertAssetNames(t *testing.T, getFS func() (fs.FS, error), want []string) { t.Helper() fSys, err := getFS() diff --git a/internal/cli/run.go b/internal/cli/run.go index c47260f..c947cbc 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -277,6 +277,7 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i if err != nil { return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("create LLM client for profile %q: %w", factoryProfileID, err)) } + llmClient = pipeline.WithDebugLLMRecording(llmClient, debugRecorder) prepared, err := pipeline.Prepare(effective.ResolvedPipeline, registries, pipeline.ModuleDependencies{LLM: llmClient}) if err != nil { return failPipelineCommand(stderr, runDir, cfg.Diagnostics.Retention, fmt.Errorf("prepare pipeline %q: %w", pipelineID, err)) diff --git a/internal/framework/contracts/composition_test.go b/internal/framework/contracts/composition_test.go index d6f01bc..f1430c0 100644 --- a/internal/framework/contracts/composition_test.go +++ b/internal/framework/contracts/composition_test.go @@ -17,7 +17,6 @@ var _ contracts.LegacyRawExtractor = compositionExtractor{} var _ contracts.LegacyRawMerger = compositionMerger{} var _ contracts.LegacyRawNormalizer = compositionNormalizer{} var _ contracts.LegacyRawValidator = compositionValidator{} -var _ contracts.StructuredLLMClient = compositionLLMClient{} var _ contracts.OutputEncoder = compositionOutputEncoder{} func TestContractsComposeAcrossPackages(t *testing.T) { @@ -38,9 +37,8 @@ func TestContractsComposeAcrossPackages(t *testing.T) { } chunking, err := chunker.Chunk(ctx, contracts.ChunkRequest{ - Source: doc, - LLMClient: compositionLLMClient{}, - Metadata: map[string]any{"max_units": 2}, + Source: doc, + Metadata: map[string]any{"request": "test"}, }) if err != nil { t.Fatalf("Chunk() error = %v, want nil", err) @@ -136,10 +134,6 @@ func (chunker compositionChunker) Chunk(ctx context.Context, req contracts.Chunk if req.Source == nil { return contracts.ChunkResult{}, errors.New("source document is required") } - if req.LLMClient == nil { - return contracts.ChunkResult{}, errors.New("structured llm client is required") - } - return contracts.ChunkResult{ Chunks: []source.Chunk{ { @@ -160,12 +154,6 @@ func (chunker compositionChunker) Chunk(ctx context.Context, req contracts.Chunk }, nil } -type compositionLLMClient struct{} - -func (client compositionLLMClient) CompleteStructured(ctx context.Context, req contracts.StructuredCompletionRequest, out any) (contracts.StructuredCompletionResponse, error) { - return contracts.StructuredCompletionResponse{}, nil -} - type compositionExtractor struct{} func (extractor compositionExtractor) Key() string { diff --git a/internal/framework/contracts/contracts.go b/internal/framework/contracts/contracts.go index 5255daf..4964764 100644 --- a/internal/framework/contracts/contracts.go +++ b/internal/framework/contracts/contracts.go @@ -129,7 +129,6 @@ type ParseRequest struct { Path string `json:"path,omitempty"` Raw []byte `json:"-"` LLMProfile string `json:"llm_profile,omitempty"` - Options map[string]any `json:"options,omitempty"` Metadata map[string]any `json:"metadata,omitempty"` } @@ -143,9 +142,7 @@ type ChunkRequest struct { SourceInput LLMInputMaterial `json:"source_input,omitempty"` SessionID string `json:"session_id,omitempty"` References ReferenceSet `json:"references,omitempty"` - LLMClient StructuredLLMClient `json:"-"` LLMProfile string `json:"llm_profile,omitempty"` - Options map[string]any `json:"options,omitempty"` Metadata map[string]any `json:"metadata,omitempty"` } @@ -377,7 +374,6 @@ type OutputRequest struct { Rejected []RejectedOutput `json:"rejected,omitempty"` Warnings []Warning `json:"warnings,omitempty"` LLMProfile string `json:"llm_profile,omitempty"` - Options map[string]any `json:"options,omitempty"` Metadata map[string]any `json:"metadata,omitempty"` } diff --git a/internal/framework/contracts/contracts_test.go b/internal/framework/contracts/contracts_test.go index 004332f..230a092 100644 --- a/internal/framework/contracts/contracts_test.go +++ b/internal/framework/contracts/contracts_test.go @@ -97,7 +97,7 @@ func TestFakeChunkerReturnsSourceChunks(t *testing.T) { } } -func TestFakeChunkerReceivesLLMClient(t *testing.T) { +func TestFakeChunkerReceivesPerRunContext(t *testing.T) { doc := &source.SourceDocument{ ID: "source-1", Kind: "document", @@ -107,14 +107,13 @@ func TestFakeChunkerReceivesLLMClient(t *testing.T) { {ID: 1, Kind: "section", Text: "Source text."}, }, } - client := fakeLLMClient{} chunker := &recordingChunker{key: "llm-chunker"} - if _, err := chunker.Chunk(context.Background(), ChunkRequest{Source: doc, LLMClient: client}); err != nil { + if _, err := chunker.Chunk(context.Background(), ChunkRequest{Source: doc, SessionID: "session", LLMProfile: "profile"}); err != nil { t.Fatalf("Chunk() error = %v, want nil", err) } - if chunker.request.LLMClient == nil { - t.Fatal("ChunkRequest.LLMClient = nil, want structured LLM client") + if chunker.request.SessionID != "session" || chunker.request.LLMProfile != "profile" { + t.Fatalf("ChunkRequest = %#v, want per-run session and profile", chunker.request) } } diff --git a/internal/framework/pipeline/debug.go b/internal/framework/pipeline/debug.go index 6c3180e..cd96a72 100644 --- a/internal/framework/pipeline/debug.go +++ b/internal/framework/pipeline/debug.go @@ -238,9 +238,18 @@ func wrapDebugLLMClient(client contracts.StructuredLLMClient, recorder DebugReco if client == nil || recorder == nil || !recorder.Enabled() { return client } + if _, ok := client.(*debugLLMClient); ok { + return client + } return &debugLLMClient{inner: client, recorder: recorder} } +// WithDebugLLMRecording decorates a shared LLM client so calls made by +// construction-injected modules participate in the run's debug recording. +func WithDebugLLMRecording(client contracts.StructuredLLMClient, recorder DebugRecorder) contracts.StructuredLLMClient { + return wrapDebugLLMClient(client, recorder) +} + func (client *debugLLMClient) CompleteStructured(ctx context.Context, req contracts.StructuredCompletionRequest, out any) (contracts.StructuredCompletionResponse, error) { client.mu.Lock() client.counter++ diff --git a/internal/framework/pipeline/runner.go b/internal/framework/pipeline/runner.go index 0695f72..e0220c3 100644 --- a/internal/framework/pipeline/runner.go +++ b/internal/framework/pipeline/runner.go @@ -133,7 +133,6 @@ func (r *Runner) Run(ctx context.Context, input RunInput) (output RunOutput, err Path: input.Path, Raw: input.RawInput, LLMProfile: input.pipeline.Input.LLMProfile, - Options: cloneOptions(input.pipeline.Input.Options), Metadata: input.Metadata, }) if err != nil { @@ -206,9 +205,7 @@ func (r *Runner) Run(ctx context.Context, input RunInput) (output RunOutput, err SourceInput: sourceInput.Clone(), SessionID: sessionID, References: CloneReferenceSet(input.pipeline.ChunkReferences.ReferenceSet), - LLMClient: input.llmClient, LLMProfile: input.pipeline.Chunk.LLMProfile, - Options: cloneOptions(input.pipeline.Chunk.Options), Metadata: input.Metadata, }) if err != nil { @@ -352,7 +349,6 @@ func (r *Runner) Run(ctx context.Context, input RunInput) (output RunOutput, err Rejected: cloneRejectedOutputs(output.Rejected), Warnings: output.Warnings, LLMProfile: input.pipeline.Output.LLMProfile, - Options: cloneOptions(input.pipeline.Output.Options), Metadata: input.Metadata, }) output.Warnings = append(output.Warnings, encoded.Warnings...) diff --git a/internal/framework/pipeline/runner_test.go b/internal/framework/pipeline/runner_test.go index ccdee98..42c8d4b 100644 --- a/internal/framework/pipeline/runner_test.go +++ b/internal/framework/pipeline/runner_test.go @@ -55,6 +55,7 @@ func newPreparedRunner(t *testing.T, registries Registries) preparedRunnerHarnes func (h preparedRunnerHarness) Run(ctx context.Context, input RunInput) (RunOutput, error) { h.t.Helper() + input.llmClient = WithDebugLLMRecording(input.llmClient, input.Debug) prepared, err := Prepare(input.pipeline, h.registries, ModuleDependencies{LLM: input.llmClient}) if err != nil { return RunOutput{}, err @@ -541,7 +542,7 @@ func TestRunPreservesChunkMetadataDuringCanonicalization(t *testing.T) { } } -func TestRunExecutesChunksAndPassesChunkAndLLMClient(t *testing.T) { +func TestRunExecutesChunksAndUsesConstructedLLMClient(t *testing.T) { modules := defaultRunnerModules() llmClient := fakeLLMClient{} @@ -558,8 +559,8 @@ func TestRunExecutesChunksAndPassesChunkAndLLMClient(t *testing.T) { if !reflect.DeepEqual(extractor.seenChunkIDs, []string{"chunk-0", "chunk-1"}) { t.Fatalf("seen chunks = %#v, want both chunks", extractor.seenChunkIDs) } - if len(modules.chunker.requests) != 1 || modules.chunker.requests[0].LLMClient == nil { - t.Fatalf("chunker LLM client = %#v, want client on chunk request", modules.chunker.requests) + if len(modules.chunker.requests) != 1 || modules.chunker.llmClient == nil { + t.Fatalf("chunker LLM client = %#v, want injected client", modules.chunker.llmClient) } if len(extractor.seenLLMClients) != 2 || extractor.seenLLMClients[0] == nil || extractor.seenLLMClients[1] == nil { t.Fatalf("seen LLM clients = %#v, want client for each chunk", extractor.seenLLMClients) @@ -721,14 +722,14 @@ func TestRunPassesModuleBindingConfigToStageRequests(t *testing.T) { if got := modules.input.requests[0].LLMProfile; got != "input-profile" { t.Fatalf("input LLMProfile = %q, want input-profile", got) } - if got := modules.input.requests[0].Options["input_option"]; got != "input-value" { - t.Fatalf("input Options = %#v, want input option", modules.input.requests[0].Options) + if got := modules.input.buildOptions["input_option"]; got != "input-value" { + t.Fatalf("input construction options = %#v, want input option", modules.input.buildOptions) } if got := modules.chunker.requests[0].LLMProfile; got != "chunk-profile" { t.Fatalf("chunk LLMProfile = %q, want chunk-profile", got) } - if got := modules.chunker.requests[0].Options["chunk_option"]; got != "chunk-value" { - t.Fatalf("chunk Options = %#v, want chunk option", modules.chunker.requests[0].Options) + if got := modules.chunker.buildOptions["chunk_option"]; got != "chunk-value" { + t.Fatalf("chunk construction options = %#v, want chunk option", modules.chunker.buildOptions) } if got := modules.extractors["extract-alpha"].requests[0].LLMProfile; got != "extract-profile" { t.Fatalf("extract LLMProfile = %q, want extract-profile", got) @@ -751,8 +752,8 @@ func TestRunPassesModuleBindingConfigToStageRequests(t *testing.T) { if got := modules.output.requests[0].LLMProfile; got != "output-profile" { t.Fatalf("output LLMProfile = %q, want output-profile", got) } - if got := modules.output.requests[0].Options["output_option"]; got != "output-value" { - t.Fatalf("output Options = %#v, want output option", modules.output.requests[0].Options) + if got := modules.output.buildOptions["output_option"]; got != "output-value" { + t.Fatalf("output construction options = %#v, want output option", modules.output.buildOptions) } } @@ -2034,18 +2035,22 @@ func newRunnerRegistries(t *testing.T, modules *runnerModules) Registries { ValidatorChains: NewValidatorChainRegistry(), Outputs: NewOutputEncoderRegistry(), } - if err := registries.Inputs.Register("input", func() (contracts.InputAdapter, error) { + allowOptions := func(map[string]any) error { return nil } + if err := registries.Inputs.RegisterBuilderWithSpec(defaultModuleSpec("input", StageInput), allowOptions, func(request BuildRequest) (contracts.InputAdapter, error) { if modules.inputBuildErr != nil { return nil, modules.inputBuildErr } + modules.input.buildOptions = cloneOptions(request.Options) return modules.input, nil }); err != nil { t.Fatalf("register input: %v", err) } - if err := registries.Chunkers.Register("chunk", func() (contracts.Chunker, error) { + if err := registries.Chunkers.RegisterBuilderWithSpec(defaultModuleSpec("chunk", StageChunk), allowOptions, func(request BuildRequest) (contracts.Chunker, error) { if modules.chunkerBuildErr != nil { return nil, modules.chunkerBuildErr } + modules.chunker.buildOptions = cloneOptions(request.Options) + modules.chunker.llmClient = request.Dependencies.LLM return modules.chunker, nil }); err != nil { t.Fatalf("register chunker: %v", err) @@ -2075,7 +2080,10 @@ func newRunnerRegistries(t *testing.T, modules *runnerModules) Registries { t.Fatalf("register validator %q: %v", key, err) } } - if err := registries.Outputs.Register("output", func() (contracts.OutputEncoder, error) { return modules.output, nil }); err != nil { + if err := registries.Outputs.RegisterBuilderWithSpec(defaultModuleSpec("output", StageOutput), allowOptions, func(request BuildRequest) (contracts.OutputEncoder, error) { + modules.output.buildOptions = cloneOptions(request.Options) + return modules.output, nil + }); err != nil { t.Fatalf("register output: %v", err) } return registries @@ -2087,6 +2095,7 @@ type runnerInputAdapter struct { err error manifestMetadata map[string]any requests []contracts.ParseRequest + buildOptions map[string]any } func (adapter *runnerInputAdapter) Key() string { @@ -2113,6 +2122,8 @@ type runnerChunker struct { llmPromptID string manifestMetadata map[string]any requests []contracts.ChunkRequest + buildOptions map[string]any + llmClient contracts.StructuredLLMClient } func (chunker *runnerChunker) Key() string { @@ -2133,13 +2144,13 @@ func (chunker *runnerChunker) Chunk(ctx context.Context, req contracts.ChunkRequ } return contracts.ChunkResult{}, err } - if chunker.callLLM && req.LLMClient != nil { + if chunker.callLLM && chunker.llmClient != nil { promptID := strings.TrimSpace(chunker.llmPromptID) if promptID == "" { promptID = "runner.chunk" } var out map[string]any - if _, err := req.LLMClient.CompleteStructured(ctx, contracts.StructuredCompletionRequest{ + if _, err := chunker.llmClient.CompleteStructured(ctx, contracts.StructuredCompletionRequest{ StageName: chunker.key, PromptID: promptID, ProfileID: req.LLMProfile, @@ -2402,6 +2413,7 @@ type runnerOutputEncoder struct { err error manifestMetadata map[string]any requests []contracts.OutputRequest + buildOptions map[string]any } func (encoder *runnerOutputEncoder) Key() string { diff --git a/internal/modules/dnd/chunk/scenes/chunker.go b/internal/modules/dnd/chunk/scenes/chunker.go index f10c195..ccdefdb 100644 --- a/internal/modules/dnd/chunk/scenes/chunker.go +++ b/internal/modules/dnd/chunk/scenes/chunker.go @@ -33,10 +33,17 @@ var referenceSlotDescriptions = shared.ReferenceSlotDescriptions{ var _ contracts.Chunker = (*Chunker)(nil) var _ contracts.ManifestMetadataProvider = (*Chunker)(nil) -type Chunker struct{} +type Options struct{} -func New() *Chunker { - return &Chunker{} +type Chunker struct { + llm contracts.StructuredLLMClient +} + +func New(llmClient contracts.StructuredLLMClient, _ Options) (*Chunker, error) { + if llmClient == nil { + return nil, chunkerErrorf("LLM client must not be nil") + } + return &Chunker{llm: llmClient}, nil } func (c *Chunker) Key() string { @@ -71,6 +78,9 @@ func (c *Chunker) Chunk(ctx context.Context, req contracts.ChunkRequest) (contra if c == nil { return contracts.ChunkResult{}, chunkerErrorf("chunker must not be nil") } + if c.llm == nil { + return contracts.ChunkResult{}, chunkerErrorf("LLM client must not be nil") + } if ctx == nil { return contracts.ChunkResult{}, chunkerErrorf("context must not be nil") } @@ -86,15 +96,8 @@ func (c *Chunker) Chunk(ctx context.Context, req contracts.ChunkRequest) (contra if err := source.ValidateDocument(req.Source); err != nil { return contracts.ChunkResult{}, chunkerErrorf("validate source document: %w", err) } - if req.LLMClient == nil { - return contracts.ChunkResult{}, chunkerErrorf("LLM client must not be nil") - } - if len(req.Options) > 0 { - return contracts.ChunkResult{}, chunkerErrorf("options are not supported") - } - var response chunkResponse - if _, err := req.LLMClient.CompleteStructured(ctx, contracts.StructuredCompletionRequest{ + if _, err := c.llm.CompleteStructured(ctx, contracts.StructuredCompletionRequest{ StageName: Key, PromptID: PromptID, PromptVersion: ResponseSchemaVersion, @@ -130,11 +133,27 @@ func ModuleSpec() pipeline.ModuleSpec { } func Register(registry *pipeline.ChunkerRegistry) error { - return registry.RegisterWithSpec(ModuleSpec(), func() (contracts.Chunker, error) { - return New(), nil + return registry.RegisterBuilderWithSpec(ModuleSpec(), validateOptions, func(request pipeline.BuildRequest) (contracts.Chunker, error) { + options, err := DecodeOptions(request.Options) + if err != nil { + return nil, err + } + return New(request.Dependencies.LLM, options) }) } +func validateOptions(options map[string]any) error { + _, err := DecodeOptions(options) + return err +} + +func DecodeOptions(options map[string]any) (Options, error) { + if err := pipeline.RejectUnknownOptions(options); err != nil { + return Options{}, chunkerErrorf("%w", err) + } + return Options{}, nil +} + func chunksFromResponse(doc *source.SourceDocument, response chunkResponse) ([]source.Chunk, error) { if response.Scenes == nil { return nil, fmt.Errorf("scenes must be present") diff --git a/internal/modules/dnd/chunk/scenes/chunker_test.go b/internal/modules/dnd/chunk/scenes/chunker_test.go index d4b884c..5788f76 100644 --- a/internal/modules/dnd/chunk/scenes/chunker_test.go +++ b/internal/modules/dnd/chunk/scenes/chunker_test.go @@ -15,10 +15,8 @@ import ( ) func TestNewModuleSpecAndRegister(t *testing.T) { - chunker := New() - if chunker == nil { - t.Fatal("New() = nil, want chunker") - } + client := &fakeScenesLLMClient{} + chunker := newChunker(t, client) if chunker.Key() != Key { t.Fatalf("Key() = %q, want %q", chunker.Key(), Key) } @@ -52,7 +50,7 @@ func TestNewModuleSpecAndRegister(t *testing.T) { if !reflect.DeepEqual(registered, want) { t.Fatalf("registered spec = %#v, want %#v", registered, want) } - built, err := registry.Build(Key) + built, err := registry.BuildWithRequest(Key, pipeline.BuildRequest{Dependencies: pipeline.ModuleDependencies{LLM: client}}) if err != nil { t.Fatalf("Build(%q) error = %v, want nil", Key, err) } @@ -62,6 +60,18 @@ func TestNewModuleSpecAndRegister(t *testing.T) { if slots := built.ReferenceSlots(); !reflect.DeepEqual(slots, want.ReferenceSlots) { t.Fatalf("ReferenceSlots() = %#v, want %#v", slots, want.ReferenceSlots) } + if err := registry.ValidateOptions(Key, map[string]any{"unexpected": true}); err == nil || !strings.Contains(err.Error(), "unknown option") { + t.Fatalf("ValidateOptions() error = %v, want unknown option error", err) + } +} + +func TestConstructionRejectsMissingDependencyAndUnknownOptions(t *testing.T) { + if _, err := New(nil, Options{}); err == nil || !strings.Contains(err.Error(), "LLM client") { + t.Fatalf("New(nil) error = %v, want LLM client error", err) + } + if _, err := DecodeOptions(map[string]any{"unexpected": true}); err == nil || !strings.Contains(err.Error(), "unknown option") { + t.Fatalf("DecodeOptions() error = %v, want unknown option error", err) + } } func TestRegisterNilRegistryReturnsError(t *testing.T) { @@ -129,7 +139,7 @@ func TestChunkReturnsSceneChunksFromStructuredOutput(t *testing.T) { }, } - result, err := New().Chunk(context.Background(), chunkRequestWithClient(client)) + result, err := newChunker(t, client).Chunk(context.Background(), chunkRequest()) if err != nil { t.Fatalf("Chunk() error = %v, want nil", err) } @@ -221,7 +231,7 @@ func TestChunkPassesReferencesAsPromptInputs(t *testing.T) { }, }, }} - req := chunkRequestWithClient(client) + req := chunkRequest() req.References = contracts.ReferenceSet{ Slots: map[string]contracts.ResolvedReferenceSlot{ "players": { @@ -245,7 +255,7 @@ func TestChunkPassesReferencesAsPromptInputs(t *testing.T) { }, } - if _, err := New().Chunk(context.Background(), req); err != nil { + if _, err := newChunker(t, client).Chunk(context.Background(), req); err != nil { t.Fatalf("Chunk() error = %v, want nil", err) } request := client.requests[0] @@ -293,7 +303,7 @@ func TestChunkRejectsWhitespaceOnlyBoundaryCaveats(t *testing.T) { }, } - _, err := New().Chunk(context.Background(), chunkRequestWithClient(client)) + _, err := newChunker(t, client).Chunk(context.Background(), chunkRequest()) if err == nil { t.Fatal("Chunk() error = nil, want malformed structured output error") } @@ -306,12 +316,11 @@ func TestChunkDefensivelyCopiesSourceUnitsAndMetadata(t *testing.T) { doc := sceneSourceDocument() client := &fakeScenesLLMClient{response: validSceneResponse()} - result, err := New().Chunk(context.Background(), contracts.ChunkRequest{ + result, err := newChunker(t, client).Chunk(context.Background(), contracts.ChunkRequest{ Source: doc, SourceInput: sceneSourceInput(), SessionID: "session-123", LLMProfile: "profile-scenes", - LLMClient: client, }) if err != nil { t.Fatalf("Chunk() error = %v, want nil", err) @@ -338,7 +347,7 @@ func TestChunkDefensivelyCopiesSourceUnitsAndMetadata(t *testing.T) { } func TestChunkerManifestMetadataIncludesPromptAndSchemaProvenance(t *testing.T) { - metadata := New().ManifestMetadata() + metadata := newChunker(t, &fakeScenesLLMClient{}).ManifestMetadata() tests := map[string]string{ "prompt_id": PromptID, @@ -368,7 +377,7 @@ func TestChunkerManifestMetadataIncludesPromptAndSchemaProvenance(t *testing.T) func TestChunkRejectsInvalidRequests(t *testing.T) { validClient := &fakeScenesLLMClient{response: validSceneResponse()} - validReq := chunkRequestWithClient(validClient) + validReq := chunkRequest() canceledCtx, cancel := context.WithCancel(context.Background()) cancel() invalidDoc := sceneSourceDocument() @@ -384,13 +393,11 @@ func TestChunkRejectsInvalidRequests(t *testing.T) { want string }{ {name: "nil chunker", chunker: nil, ctx: context.Background(), req: validReq, want: "chunker"}, - {name: "nil context", chunker: New(), ctx: nil, req: validReq, want: "context"}, - {name: "canceled context", chunker: New(), ctx: canceledCtx, req: validReq, want: "context"}, - {name: "nil source", chunker: New(), ctx: context.Background(), req: contracts.ChunkRequest{LLMClient: validClient}, want: "source"}, - {name: "empty source units", chunker: New(), ctx: context.Background(), req: contracts.ChunkRequest{Source: emptyDoc, LLMClient: validClient}, want: "units"}, - {name: "invalid source", chunker: New(), ctx: context.Background(), req: contracts.ChunkRequest{Source: invalidDoc, LLMClient: validClient}, want: "validate source document"}, - {name: "nil LLM client", chunker: New(), ctx: context.Background(), req: contracts.ChunkRequest{Source: sceneSourceDocument()}, want: "LLM client"}, - {name: "unsupported options", chunker: New(), ctx: context.Background(), req: requestWithOptions(validReq), want: "options"}, + {name: "nil context", chunker: newChunker(t, validClient), ctx: nil, req: validReq, want: "context"}, + {name: "canceled context", chunker: newChunker(t, validClient), ctx: canceledCtx, req: validReq, want: "context"}, + {name: "nil source", chunker: newChunker(t, validClient), ctx: context.Background(), req: contracts.ChunkRequest{}, want: "source"}, + {name: "empty source units", chunker: newChunker(t, validClient), ctx: context.Background(), req: contracts.ChunkRequest{Source: emptyDoc}, want: "units"}, + {name: "invalid source", chunker: newChunker(t, validClient), ctx: context.Background(), req: contracts.ChunkRequest{Source: invalidDoc}, want: "validate source document"}, } for _, tt := range tests { @@ -488,7 +495,7 @@ func TestChunkRejectsMalformedStructuredOutput(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { client := &fakeScenesLLMClient{response: tt.response} - _, err := New().Chunk(context.Background(), chunkRequestWithClient(client)) + _, err := newChunker(t, client).Chunk(context.Background(), chunkRequest()) if err == nil { t.Fatal("Chunk() error = nil, want error") } @@ -502,7 +509,7 @@ func TestChunkRejectsMalformedStructuredOutput(t *testing.T) { func TestChunkWrapsLLMClientError(t *testing.T) { client := &fakeScenesLLMClient{err: errors.New("provider unavailable")} - _, err := New().Chunk(context.Background(), chunkRequestWithClient(client)) + _, err := newChunker(t, client).Chunk(context.Background(), chunkRequest()) if err == nil { t.Fatal("Chunk() error = nil, want LLM error") } @@ -511,13 +518,12 @@ func TestChunkWrapsLLMClientError(t *testing.T) { } } -func chunkRequestWithClient(client contracts.StructuredLLMClient) contracts.ChunkRequest { +func chunkRequest() contracts.ChunkRequest { return contracts.ChunkRequest{ Source: sceneSourceDocument(), SourceInput: sceneSourceInput(), SessionID: "session-123", LLMProfile: "profile-scenes", - LLMClient: client, } } @@ -527,9 +533,13 @@ func sceneSourceInput() contracts.LLMInputMaterial { return contracts.NewLLMInputMaterial("source", "application/json", []byte(sceneTranscriptJSON), "sha256:transcript", "file:///session-alpha.json") } -func requestWithOptions(req contracts.ChunkRequest) contracts.ChunkRequest { - req.Options = map[string]any{"max_units": 2} - return req +func newChunker(t *testing.T, client contracts.StructuredLLMClient) *Chunker { + t.Helper() + chunker, err := New(client, Options{}) + if err != nil { + t.Fatalf("New() error = %v, want nil", err) + } + return chunker } func sceneSourceDocument() *source.SourceDocument { diff --git a/internal/modules/dnd/chunk/scenes/scriptorium_assets_test.go b/internal/modules/dnd/chunk/scenes/scriptorium_assets_test.go index e480bf2..eca2c51 100644 --- a/internal/modules/dnd/chunk/scenes/scriptorium_assets_test.go +++ b/internal/modules/dnd/chunk/scenes/scriptorium_assets_test.go @@ -47,7 +47,7 @@ func TestScriptoriumPromptPreparesTranscriptAndTaskMessages(t *testing.T) { func TestScriptoriumPromptDiagnosticsOmitRawMaterials(t *testing.T) { transcript := []byte(`{"secret":"source text"}`) prepared := prepareScenesPrompt(t, transcript, "private player note", "private party note", "private glossary note") - metadata := New().ManifestMetadata() + metadata := newChunker(t, &fakeScenesLLMClient{}).ManifestMetadata() payload, err := json.Marshal(map[string]any{ "prepared": map[string]any{ diff --git a/internal/modules/generic/chunk/units/chunker.go b/internal/modules/generic/chunk/units/chunker.go index 0db19fb..5be3dcb 100644 --- a/internal/modules/generic/chunk/units/chunker.go +++ b/internal/modules/generic/chunk/units/chunker.go @@ -21,10 +21,17 @@ const ( var _ contracts.Chunker = (*Chunker)(nil) -type Chunker struct{} +type Options struct { + MaxUnits int + OverlapUnits int +} -func New() *Chunker { - return &Chunker{} +type Chunker struct { + options Options +} + +func New(options Options) *Chunker { + return &Chunker{options: options} } func (c *Chunker) Key() string { @@ -39,6 +46,9 @@ func (c *Chunker) Chunk(ctx context.Context, req contracts.ChunkRequest) (contra if c == nil { return contracts.ChunkResult{}, chunkerErrorf("chunker must not be nil") } + if c.options.MaxUnits <= 0 || c.options.OverlapUnits < 0 || c.options.OverlapUnits >= c.options.MaxUnits { + return contracts.ChunkResult{}, chunkerErrorf("chunker options must be initialized by construction") + } if ctx == nil { return contracts.ChunkResult{}, chunkerErrorf("context must not be nil") } @@ -55,15 +65,10 @@ func (c *Chunker) Chunk(ctx context.Context, req contracts.ChunkRequest) (contra return contracts.ChunkResult{}, chunkerErrorf("validate source document: %w", err) } - opts, err := chunkOptionsFrom(req.Options) - if err != nil { - return contracts.ChunkResult{}, err - } - - step := opts.maxUnits - opts.overlapUnits + step := c.options.MaxUnits - c.options.OverlapUnits chunks := make([]source.Chunk, 0, (len(req.Source.Units)+step-1)/step) for start := 0; start < len(req.Source.Units); start += step { - end := start + opts.maxUnits + end := start + c.options.MaxUnits if end > len(req.Source.Units) { end = len(req.Source.Units) } @@ -119,36 +124,43 @@ func ModuleSpec() pipeline.ModuleSpec { } func Register(registry *pipeline.ChunkerRegistry) error { - return registry.RegisterWithSpec(ModuleSpec(), func() (contracts.Chunker, error) { - return New(), nil + return registry.RegisterBuilderWithSpec(ModuleSpec(), validateOptions, func(request pipeline.BuildRequest) (contracts.Chunker, error) { + options, err := DecodeOptions(request.Options) + if err != nil { + return nil, err + } + return New(options), nil }) } -type chunkOptions struct { - maxUnits int - overlapUnits int +func validateOptions(options map[string]any) error { + _, err := DecodeOptions(options) + return err } -func chunkOptionsFrom(options map[string]any) (chunkOptions, error) { - opts := chunkOptions{ - maxUnits: defaultMaxUnits, - overlapUnits: defaultOverlapUnits, +func DecodeOptions(options map[string]any) (Options, error) { + if err := pipeline.RejectUnknownOptions(options, "max_units", "overlap_units"); err != nil { + return Options{}, chunkerErrorf("%w", err) + } + opts := Options{ + MaxUnits: defaultMaxUnits, + OverlapUnits: defaultOverlapUnits, } var err error if value, ok := options["max_units"]; ok { - opts.maxUnits, err = positiveIntOption("max_units", value) + opts.MaxUnits, err = positiveIntOption("max_units", value) if err != nil { - return chunkOptions{}, err + return Options{}, err } } if value, ok := options["overlap_units"]; ok { - opts.overlapUnits, err = nonNegativeIntOption("overlap_units", value) + opts.OverlapUnits, err = nonNegativeIntOption("overlap_units", value) if err != nil { - return chunkOptions{}, err + return Options{}, err } } - if opts.overlapUnits >= opts.maxUnits { - return chunkOptions{}, chunkerErrorf("overlap_units must be less than max_units") + if opts.OverlapUnits >= opts.MaxUnits { + return Options{}, chunkerErrorf("overlap_units must be less than max_units") } return opts, nil } diff --git a/internal/modules/generic/chunk/units/chunker_test.go b/internal/modules/generic/chunk/units/chunker_test.go index 94c1cbc..08050c1 100644 --- a/internal/modules/generic/chunk/units/chunker_test.go +++ b/internal/modules/generic/chunk/units/chunker_test.go @@ -44,10 +44,18 @@ func TestModuleSpecAndRegister(t *testing.T) { if slots := chunker.ReferenceSlots(); len(slots) != 0 { t.Fatalf("ReferenceSlots() = %#v, want none", slots) } + configured, err := registry.BuildWithRequest(Key, pipeline.BuildRequest{Options: map[string]any{"max_units": 1}}) + if err != nil { + t.Fatalf("BuildWithRequest(%q) error = %v, want nil", Key, err) + } + result, err := configured.Chunk(context.Background(), contracts.ChunkRequest{Source: testSource(2)}) + if err != nil || len(result.Chunks) != 2 { + t.Fatalf("constructed chunker result = %#v, %v; want two chunks", result, err) + } } func TestChunkUsesDefaultsForSingleChunk(t *testing.T) { - result, err := New().Chunk(context.Background(), contracts.ChunkRequest{Source: testSource(3), Options: nil}) + result, err := newChunker(t, nil).Chunk(context.Background(), contracts.ChunkRequest{Source: testSource(3)}) if err != nil { t.Fatalf("Chunk() error = %v, want nil", err) } @@ -74,10 +82,7 @@ func TestChunkUsesDefaultsForSingleChunk(t *testing.T) { } func TestChunkExactBoundaries(t *testing.T) { - result, err := New().Chunk(context.Background(), contracts.ChunkRequest{ - Source: testSource(6), - Options: map[string]any{"max_units": 2}, - }) + result, err := newChunker(t, map[string]any{"max_units": 2}).Chunk(context.Background(), contracts.ChunkRequest{Source: testSource(6)}) if err != nil { t.Fatalf("Chunk() error = %v, want nil", err) } @@ -93,10 +98,7 @@ func TestChunkExactBoundaries(t *testing.T) { } func TestChunkOverlap(t *testing.T) { - result, err := New().Chunk(context.Background(), contracts.ChunkRequest{ - Source: testSource(7), - Options: map[string]any{"max_units": 3, "overlap_units": 1}, - }) + result, err := newChunker(t, map[string]any{"max_units": 3, "overlap_units": 1}).Chunk(context.Background(), contracts.ChunkRequest{Source: testSource(7)}) if err != nil { t.Fatalf("Chunk() error = %v, want nil", err) } @@ -123,19 +125,17 @@ func TestChunkRejectsInvalidOptions(t *testing.T) { {name: "overlap negative", options: map[string]any{"overlap_units": -1}, want: "non-negative"}, {name: "overlap too large", options: map[string]any{"max_units": 2, "overlap_units": 2}, want: "less than"}, {name: "json number", options: map[string]any{"max_units": json.Number("bad")}, want: "integer"}, + {name: "unknown", options: map[string]any{"unexpected": true}, want: "unknown option"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, err := New().Chunk(context.Background(), contracts.ChunkRequest{ - Source: testSource(3), - Options: test.options, - }) + _, err := DecodeOptions(test.options) if err == nil { - t.Fatal("Chunk() error = nil, want error") + t.Fatal("DecodeOptions() error = nil, want error") } if !strings.Contains(err.Error(), "generic chunker") || !strings.Contains(err.Error(), test.want) { - t.Fatalf("Chunk() error = %q, want module context and %q", err.Error(), test.want) + t.Fatalf("DecodeOptions() error = %q, want module context and %q", err.Error(), test.want) } }) } @@ -145,7 +145,7 @@ func TestChunkRejectsEmptySource(t *testing.T) { doc := testSource(1) doc.Units = nil - _, err := New().Chunk(context.Background(), contracts.ChunkRequest{Source: doc}) + _, err := newChunker(t, nil).Chunk(context.Background(), contracts.ChunkRequest{Source: doc}) if err == nil { t.Fatal("Chunk() error = nil, want empty source error") } @@ -157,10 +157,7 @@ func TestChunkRejectsEmptySource(t *testing.T) { func TestChunkDefensivelyCopiesUnits(t *testing.T) { doc := testSource(2) - result, err := New().Chunk(context.Background(), contracts.ChunkRequest{ - Source: doc, - Options: map[string]any{"max_units": 1}, - }) + result, err := newChunker(t, map[string]any{"max_units": 1}).Chunk(context.Background(), contracts.ChunkRequest{Source: doc}) if err != nil { t.Fatalf("Chunk() error = %v, want nil", err) } @@ -183,6 +180,15 @@ func TestChunkDefensivelyCopiesUnits(t *testing.T) { } } +func newChunker(t *testing.T, rawOptions map[string]any) *Chunker { + t.Helper() + options, err := DecodeOptions(rawOptions) + if err != nil { + t.Fatalf("DecodeOptions() error = %v, want nil", err) + } + return New(options) +} + func testSource(count int) *source.SourceDocument { units := make([]source.SourceUnit, 0, count) for i := 1; i <= count; i++ { diff --git a/internal/modules/generic/output/json/encoder.go b/internal/modules/generic/output/json/encoder.go index 95a420e..2d4891e 100644 --- a/internal/modules/generic/output/json/encoder.go +++ b/internal/modules/generic/output/json/encoder.go @@ -21,10 +21,15 @@ var safeOutputFileChar = regexp.MustCompile(`[^A-Za-z0-9._-]`) var _ contracts.OutputEncoder = (*Encoder)(nil) -type Encoder struct{} +type Options struct{} + +type Encoder struct { + options Options +} func New() *Encoder { - return &Encoder{} + options, _ := DecodeOptions(nil) + return &Encoder{options: options} } func (e *Encoder) Key() string { @@ -59,11 +64,27 @@ func ModuleSpec() pipeline.ModuleSpec { } func Register(registry *pipeline.OutputEncoderRegistry) error { - return registry.RegisterWithSpec(ModuleSpec(), func() (contracts.OutputEncoder, error) { - return New(), nil + return registry.RegisterBuilderWithSpec(ModuleSpec(), validateOptions, func(request pipeline.BuildRequest) (contracts.OutputEncoder, error) { + options, err := DecodeOptions(request.Options) + if err != nil { + return nil, err + } + return &Encoder{options: options}, nil }) } +func validateOptions(options map[string]any) error { + _, err := DecodeOptions(options) + return err +} + +func DecodeOptions(options map[string]any) (Options, error) { + if err := pipeline.RejectUnknownOptions(options); err != nil { + return Options{}, encoderErrorf("%w", err) + } + return Options{}, nil +} + type indexFile struct { ManifestFile string `json:"manifest_file"` OutputFiles []outputFileIndex `json:"output_files"` diff --git a/internal/modules/generic/output/json/encoder_test.go b/internal/modules/generic/output/json/encoder_test.go index dd37f21..41c21fd 100644 --- a/internal/modules/generic/output/json/encoder_test.go +++ b/internal/modules/generic/output/json/encoder_test.go @@ -34,6 +34,9 @@ func TestModuleSpecAndRegister(t *testing.T) { if !reflect.DeepEqual(spec, want) { t.Fatalf("registered spec = %#v, want %#v", spec, want) } + if err := registry.ValidateOptions(Key, map[string]any{"unexpected": true}); err == nil || !strings.Contains(err.Error(), "unknown option") { + t.Fatalf("ValidateOptions() error = %v, want unknown option error", err) + } } func TestEncodeReturnsLogicalFilesForNormalizedOutputs(t *testing.T) { diff --git a/internal/modules/seriatim/input/transcript/adapter.go b/internal/modules/seriatim/input/transcript/adapter.go index 11b9e5a..7411b5f 100644 --- a/internal/modules/seriatim/input/transcript/adapter.go +++ b/internal/modules/seriatim/input/transcript/adapter.go @@ -31,10 +31,15 @@ var providedCapabilities = []string{ var _ contracts.InputAdapter = (*Adapter)(nil) -type Adapter struct{} +type Options struct{} + +type Adapter struct { + options Options +} func New() *Adapter { - return &Adapter{} + options, _ := DecodeOptions(nil) + return &Adapter{options: options} } func (a *Adapter) Key() string { @@ -96,11 +101,27 @@ func ModuleSpec() pipeline.ModuleSpec { } func Register(registry *pipeline.InputAdapterRegistry) error { - return registry.RegisterWithSpec(ModuleSpec(), func() (contracts.InputAdapter, error) { - return New(), nil + return registry.RegisterBuilderWithSpec(ModuleSpec(), validateOptions, func(request pipeline.BuildRequest) (contracts.InputAdapter, error) { + options, err := DecodeOptions(request.Options) + if err != nil { + return nil, err + } + return &Adapter{options: options}, nil }) } +func validateOptions(options map[string]any) error { + _, err := DecodeOptions(options) + return err +} + +func DecodeOptions(options map[string]any) (Options, error) { + if err := pipeline.RejectUnknownOptions(options); err != nil { + return Options{}, inputErrorf("%w", err) + } + return Options{}, nil +} + func sourceUnit(sourceID string, segment segment, index int, seen map[int]struct{}) (source.SourceUnit, error) { segmentLabel := fmt.Sprintf("segment[%d]", index) if segment.ID <= 0 { diff --git a/internal/modules/seriatim/input/transcript/registry_test.go b/internal/modules/seriatim/input/transcript/registry_test.go index 8c45eb6..e76ecc4 100644 --- a/internal/modules/seriatim/input/transcript/registry_test.go +++ b/internal/modules/seriatim/input/transcript/registry_test.go @@ -56,6 +56,9 @@ func TestRegisterMakesAdapterBuildable(t *testing.T) { if adapter.Key() != Key { t.Fatalf("adapter.Key() = %q, want %q", adapter.Key(), Key) } + if err := registry.ValidateOptions(Key, map[string]any{"unexpected": true}); err == nil || !strings.Contains(err.Error(), "unknown option") { + t.Fatalf("ValidateOptions() error = %v, want unknown option error", err) + } } func TestRegisterStoresModuleSpec(t *testing.T) {