From e2cb0d901aca1f7ff730e257b20add922704d509 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 24 Jul 2026 14:38:42 +0000 Subject: [PATCH] Centralize D&D extraction request preparation --- .../dnd/extract/combatturns/extractor.go | 17 +--- .../dnd/extract/combatturns/extractor_test.go | 18 +--- .../dnd/extract/npcinteractions/extractor.go | 17 +--- .../extract/npcinteractions/extractor_test.go | 28 ++---- .../modules/dnd/extract/npcs/extractor.go | 17 +--- .../dnd/extract/npcs/extractor_test.go | 28 ++++-- .../dnd/extract/npcs/test_helpers_test.go | 5 - .../extract/scenedescriptions/extractor.go | 17 +--- .../scenedescriptions/extractor_test.go | 21 ++-- .../scenedescriptions/test_helpers_test.go | 5 - .../modules/dnd/extract/spells/extractor.go | 17 +--- .../dnd/extract/spells/extractor_test.go | 13 +-- .../dnd/extract/spells/test_helpers_test.go | 5 - .../modules/dnd/shared/extraction_inputs.go | 22 ++++- .../dnd/shared/extraction_inputs_test.go | 99 ++++++++++++++----- 15 files changed, 144 insertions(+), 185 deletions(-) diff --git a/internal/modules/dnd/extract/combatturns/extractor.go b/internal/modules/dnd/extract/combatturns/extractor.go index 694e7f7..2436943 100644 --- a/internal/modules/dnd/extract/combatturns/extractor.go +++ b/internal/modules/dnd/extract/combatturns/extractor.go @@ -144,22 +144,7 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe if e.llm == nil { return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("LLM client must not be nil") } - if ctx == nil { - return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("context must not be nil") - } - if err := ctx.Err(); err != nil { - return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("context error before extraction: %w", err) - } - if req.Source == nil { - return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("source must not be nil") - } - if req.Chunk == nil { - return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("chunk must not be nil") - } - if len(req.Chunk.Units) == 0 { - return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("chunk %q units must not be empty", req.Chunk.ID) - } - sourceInput, err := shared.ChunkPromptMaterial(req) + sourceInput, err := shared.PrepareChunkExtraction(ctx, req) if err != nil { return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("%w", err) } diff --git a/internal/modules/dnd/extract/combatturns/extractor_test.go b/internal/modules/dnd/extract/combatturns/extractor_test.go index b63de0e..e6aab81 100644 --- a/internal/modules/dnd/extract/combatturns/extractor_test.go +++ b/internal/modules/dnd/extract/combatturns/extractor_test.go @@ -222,9 +222,8 @@ func TestNewRejectsMalformedNPCRegistryBeforeLLMCallWithoutContent(t *testing.T) func TestExtractRejectsInvalidRequestsAndWrapsProviderFailures(t *testing.T) { validReq := extractionRequest() - canceledCtx, cancel := context.WithCancel(context.Background()) - cancel() validExtractor := newExtractor(t, &fakeCombatTurnsLLMClient{response: extractionResponse{CombatTurns: []combatTurnResponse{}}}) + var nilExtractor *Extractor tests := []struct { name string extractor *Extractor @@ -232,13 +231,9 @@ func TestExtractRejectsInvalidRequestsAndWrapsProviderFailures(t *testing.T) { req contracts.TypedExtractionRequest want string }{ - {name: "nil extractor", ctx: context.Background(), req: validReq, want: "extractor"}, - {name: "nil context", extractor: validExtractor, req: validReq, want: "context"}, - {name: "canceled context", extractor: validExtractor, ctx: canceledCtx, req: validReq, want: "context"}, - {name: "nil source", extractor: validExtractor, ctx: context.Background(), req: contracts.TypedExtractionRequest{Chunk: validReq.Chunk}, want: "source"}, - {name: "nil chunk", extractor: validExtractor, ctx: context.Background(), req: contracts.TypedExtractionRequest{Source: validReq.Source}, want: "chunk"}, - {name: "empty chunk units", extractor: validExtractor, ctx: context.Background(), req: emptyChunkRequest(validReq), want: "units"}, - {name: "source input mismatch", extractor: validExtractor, ctx: context.Background(), req: mismatchedSourceInputRequest(validReq), want: "must match chunk"}, + {name: "nil extractor", extractor: nilExtractor, ctx: context.Background(), req: validReq, want: "extractor"}, + {name: "nil LLM client", extractor: &Extractor{}, ctx: context.Background(), req: validReq, want: "LLM client"}, + {name: "wrapped preflight failure", extractor: validExtractor, ctx: context.Background(), req: mismatchedSourceInputRequest(validReq), want: "must match chunk"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -339,11 +334,6 @@ func combatSourceDocument() *source.SourceDocument { } } -func emptyChunkRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { - req.Chunk = &source.Chunk{ID: req.Chunk.ID, SourceID: req.Chunk.SourceID, Index: req.Chunk.Index} - return req -} - func mismatchedSourceInputRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { req.SourceInput = contracts.NewLLMInputMaterial("source", "application/json", []byte(`{"other":true}`), "sha256:other", "") return req diff --git a/internal/modules/dnd/extract/npcinteractions/extractor.go b/internal/modules/dnd/extract/npcinteractions/extractor.go index a6e3f18..d74e338 100644 --- a/internal/modules/dnd/extract/npcinteractions/extractor.go +++ b/internal/modules/dnd/extract/npcinteractions/extractor.go @@ -143,22 +143,7 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe if e.llm == nil { return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("LLM client must not be nil") } - if ctx == nil { - return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("context must not be nil") - } - if err := ctx.Err(); err != nil { - return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("context error before extraction: %w", err) - } - if req.Source == nil { - return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("source must not be nil") - } - if req.Chunk == nil { - return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("chunk must not be nil") - } - if len(req.Chunk.Units) == 0 { - return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("chunk %q units must not be empty", req.Chunk.ID) - } - sourceInput, err := shared.ChunkPromptMaterial(req) + sourceInput, err := shared.PrepareChunkExtraction(ctx, req) if err != nil { return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("%w", err) } diff --git a/internal/modules/dnd/extract/npcinteractions/extractor_test.go b/internal/modules/dnd/extract/npcinteractions/extractor_test.go index 8e40de1..979e073 100644 --- a/internal/modules/dnd/extract/npcinteractions/extractor_test.go +++ b/internal/modules/dnd/extract/npcinteractions/extractor_test.go @@ -204,24 +204,21 @@ func TestExtractRejectsInvalidRequestsAndProviderFailures(t *testing.T) { references := requiredRegistryReferences(t, "Mira Thorn") valid := extractionRequest() valid.References = references - canceled, cancel := context.WithCancel(context.Background()) - cancel() extractor := newExtractor(t, &fakeInteractionsLLMClient{}, references) + var nilExtractor *Extractor for _, test := range []struct { - name string - ctx context.Context - req contracts.TypedExtractionRequest - want string + name string + extractor *Extractor + ctx context.Context + req contracts.TypedExtractionRequest + want string }{ - {"nil context", nil, valid, "context"}, - {"canceled context", canceled, valid, "context"}, - {"nil source", context.Background(), contracts.TypedExtractionRequest{Chunk: valid.Chunk, References: references}, "source"}, - {"nil chunk", context.Background(), contracts.TypedExtractionRequest{Source: valid.Source, References: references}, "chunk"}, - {"empty chunk", context.Background(), emptyChunkRequest(valid), "units"}, - {"source input mismatch", context.Background(), mismatchedSourceInputRequest(valid), "must match chunk"}, + {"nil extractor", nilExtractor, context.Background(), valid, "extractor"}, + {"nil LLM client", &Extractor{}, context.Background(), valid, "LLM client"}, + {"wrapped preflight failure", extractor, context.Background(), mismatchedSourceInputRequest(valid), "must match chunk"}, } { t.Run(test.name, func(t *testing.T) { - if _, err := extractor.Extract(test.ctx, test.req); err == nil || !strings.Contains(err.Error(), test.want) { + if _, err := test.extractor.Extract(test.ctx, test.req); err == nil || !strings.Contains(err.Error(), test.want) { t.Fatalf("Extract() error = %v, want %q", err, test.want) } }) @@ -361,11 +358,6 @@ func interactionKinds(value dnd.NPCInteractionList) []dnd.NPCInteractionKind { return kinds } -func emptyChunkRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { - req.Chunk = &source.Chunk{ID: req.Chunk.ID, SourceID: req.Chunk.SourceID, Index: req.Chunk.Index} - return req -} - func mismatchedSourceInputRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { req.SourceInput = contracts.NewLLMInputMaterial("source", "application/json", []byte(`{"different":true}`), "sha256:other", "") return req diff --git a/internal/modules/dnd/extract/npcs/extractor.go b/internal/modules/dnd/extract/npcs/extractor.go index b5865f0..b813855 100644 --- a/internal/modules/dnd/extract/npcs/extractor.go +++ b/internal/modules/dnd/extract/npcs/extractor.go @@ -110,22 +110,7 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe if e.llm == nil { return contracts.TypedExtractionResult[dnd.NPCList]{}, extractorErrorf("LLM client must not be nil") } - if ctx == nil { - return contracts.TypedExtractionResult[dnd.NPCList]{}, extractorErrorf("context must not be nil") - } - if err := ctx.Err(); err != nil { - return contracts.TypedExtractionResult[dnd.NPCList]{}, extractorErrorf("context error before extraction: %w", err) - } - if req.Source == nil { - return contracts.TypedExtractionResult[dnd.NPCList]{}, extractorErrorf("source must not be nil") - } - if req.Chunk == nil { - return contracts.TypedExtractionResult[dnd.NPCList]{}, extractorErrorf("chunk must not be nil") - } - if len(req.Chunk.Units) == 0 { - return contracts.TypedExtractionResult[dnd.NPCList]{}, extractorErrorf("chunk %q units must not be empty", req.Chunk.ID) - } - sourceInput, err := shared.ChunkPromptMaterial(req) + sourceInput, err := shared.PrepareChunkExtraction(ctx, req) if err != nil { return contracts.TypedExtractionResult[dnd.NPCList]{}, extractorErrorf("%w", err) } diff --git a/internal/modules/dnd/extract/npcs/extractor_test.go b/internal/modules/dnd/extract/npcs/extractor_test.go index 7efd9ce..ef1a2be 100644 --- a/internal/modules/dnd/extract/npcs/extractor_test.go +++ b/internal/modules/dnd/extract/npcs/extractor_test.go @@ -173,19 +173,27 @@ func TestExtractMapsRawSemanticCandidatesWithoutRepair(t *testing.T) { } } -func TestExtractHandlesCancellationAndProviderErrors(t *testing.T) { +func TestExtractRetainsLocalErrorContextAndProviderFailures(t *testing.T) { request := extractionRequest() extractor := newExtractor(t, &fakeNPCsLLMClient{response: extractionResponse{NPCs: []npcResponse{}}}) - canceled, cancel := context.WithCancel(context.Background()) - cancel() - if _, err := extractor.Extract(canceled, request); err == nil || !strings.Contains(err.Error(), "context") { - t.Fatalf("canceled Extract() error = %v, want context error", err) + var nilExtractor *Extractor + for _, test := range []struct { + name string + extractor *Extractor + req contracts.TypedExtractionRequest + want string + }{ + {name: "nil extractor", extractor: nilExtractor, req: request, want: "extractor"}, + {name: "nil LLM client", extractor: &Extractor{}, req: request, want: "LLM client"}, + {name: "wrapped preflight failure", extractor: extractor, req: mismatchedSourceInputRequest(request), want: "must match chunk"}, + } { + t.Run(test.name, func(t *testing.T) { + if _, err := test.extractor.Extract(context.Background(), test.req); err == nil || !strings.Contains(err.Error(), "dnd npcs") || !strings.Contains(err.Error(), test.want) { + t.Fatalf("Extract() error = %v, want contextual local error", err) + } + }) } - _, err := extractor.Extract(context.Background(), mismatchedSourceInputRequest(request)) - if err == nil || !strings.Contains(err.Error(), "dnd npcs") || !strings.Contains(err.Error(), "must match chunk") { - t.Fatalf("source input error = %v, want contextual source input error", err) - } - _, err = newExtractor(t, &fakeNPCsLLMClient{err: errors.New("provider unavailable")}).Extract(context.Background(), request) + _, err := newExtractor(t, &fakeNPCsLLMClient{err: errors.New("provider unavailable")}).Extract(context.Background(), request) if err == nil || !strings.Contains(err.Error(), "dnd npcs") || !strings.Contains(err.Error(), "provider unavailable") { t.Fatalf("provider Extract() error = %v, want contextual provider error", err) } diff --git a/internal/modules/dnd/extract/npcs/test_helpers_test.go b/internal/modules/dnd/extract/npcs/test_helpers_test.go index 166502a..59c709b 100644 --- a/internal/modules/dnd/extract/npcs/test_helpers_test.go +++ b/internal/modules/dnd/extract/npcs/test_helpers_test.go @@ -58,11 +58,6 @@ func newExtractor(t *testing.T, client contracts.StructuredLLMClient, references return extractor } -func emptyChunkRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { - req.Chunk = &source.Chunk{ID: req.Chunk.ID, SourceID: req.Chunk.SourceID, Index: req.Chunk.Index} - return req -} - func mismatchedSourceInputRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { req.SourceInput = contracts.NewLLMInputMaterial("source", "application/json", []byte(`{"different":true}`), "sha256:other", "file:///other.json") return req diff --git a/internal/modules/dnd/extract/scenedescriptions/extractor.go b/internal/modules/dnd/extract/scenedescriptions/extractor.go index c593e47..070bfad 100644 --- a/internal/modules/dnd/extract/scenedescriptions/extractor.go +++ b/internal/modules/dnd/extract/scenedescriptions/extractor.go @@ -113,22 +113,7 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe if e.llm == nil { return contracts.TypedExtractionResult[dnd.SceneDescriptionList]{}, extractorErrorf("LLM client must not be nil") } - if ctx == nil { - return contracts.TypedExtractionResult[dnd.SceneDescriptionList]{}, extractorErrorf("context must not be nil") - } - if err := ctx.Err(); err != nil { - return contracts.TypedExtractionResult[dnd.SceneDescriptionList]{}, extractorErrorf("context error before extraction: %w", err) - } - if req.Source == nil { - return contracts.TypedExtractionResult[dnd.SceneDescriptionList]{}, extractorErrorf("source must not be nil") - } - if req.Chunk == nil { - return contracts.TypedExtractionResult[dnd.SceneDescriptionList]{}, extractorErrorf("chunk must not be nil") - } - if len(req.Chunk.Units) == 0 { - return contracts.TypedExtractionResult[dnd.SceneDescriptionList]{}, extractorErrorf("chunk %q units must not be empty", req.Chunk.ID) - } - sourceInput, err := shared.ChunkPromptMaterial(req) + sourceInput, err := shared.PrepareChunkExtraction(ctx, req) if err != nil { return contracts.TypedExtractionResult[dnd.SceneDescriptionList]{}, extractorErrorf("%w", err) } diff --git a/internal/modules/dnd/extract/scenedescriptions/extractor_test.go b/internal/modules/dnd/extract/scenedescriptions/extractor_test.go index 10fec7f..57fec1f 100644 --- a/internal/modules/dnd/extract/scenedescriptions/extractor_test.go +++ b/internal/modules/dnd/extract/scenedescriptions/extractor_test.go @@ -87,26 +87,23 @@ func TestExtractPreservesTheModelKindWithoutRepair(t *testing.T) { func TestExtractValidatesRequestsAndSurfacesProviderFailures(t *testing.T) { request := extractionRequest() extractor := newExtractor(t, &fakeSceneDescriptionsLLMClient{response: extractionResponse{Kind: dnd.SceneKindMeta, Title: "Table talk", Summary: "The group discusses rules."}}) + var nilExtractor *Extractor for _, test := range []struct { - name string - req contracts.TypedExtractionRequest + name string + extractor *Extractor + req contracts.TypedExtractionRequest + want string }{ - {name: "nil source", req: func() contracts.TypedExtractionRequest { r := request; r.Source = nil; return r }()}, - {name: "nil chunk", req: func() contracts.TypedExtractionRequest { r := request; r.Chunk = nil; return r }()}, - {name: "empty chunk", req: emptyChunkRequest(request)}, - {name: "mismatched source input", req: mismatchedSourceInputRequest(request)}, + {name: "nil extractor", extractor: nilExtractor, req: request, want: "extractor"}, + {name: "nil LLM client", extractor: &Extractor{}, req: request, want: "LLM client"}, + {name: "wrapped preflight failure", extractor: extractor, req: mismatchedSourceInputRequest(request), want: "must match chunk"}, } { t.Run(test.name, func(t *testing.T) { - if _, err := extractor.Extract(context.Background(), test.req); err == nil || !strings.Contains(err.Error(), "dnd scene descriptions") { + if _, err := test.extractor.Extract(context.Background(), test.req); err == nil || !strings.Contains(err.Error(), "dnd scene descriptions") || !strings.Contains(err.Error(), test.want) { t.Fatalf("Extract() error = %v, want contextual validation error", err) } }) } - canceled, cancel := context.WithCancel(context.Background()) - cancel() - if _, err := extractor.Extract(canceled, request); err == nil || !strings.Contains(err.Error(), "context") { - t.Fatalf("canceled Extract() error = %v, want context error", err) - } _, err := newExtractor(t, &fakeSceneDescriptionsLLMClient{err: errors.New("provider unavailable")}).Extract(context.Background(), request) if err == nil || !strings.Contains(err.Error(), "dnd scene descriptions") || !strings.Contains(err.Error(), "provider unavailable") { t.Fatalf("provider Extract() error = %v, want contextual provider error", err) diff --git a/internal/modules/dnd/extract/scenedescriptions/test_helpers_test.go b/internal/modules/dnd/extract/scenedescriptions/test_helpers_test.go index 1a992e0..8a26738 100644 --- a/internal/modules/dnd/extract/scenedescriptions/test_helpers_test.go +++ b/internal/modules/dnd/extract/scenedescriptions/test_helpers_test.go @@ -54,11 +54,6 @@ func newExtractor(t *testing.T, client contracts.StructuredLLMClient, references return extractor } -func emptyChunkRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { - req.Chunk = &source.Chunk{ID: req.Chunk.ID, SourceID: req.Chunk.SourceID, Index: req.Chunk.Index} - return req -} - func mismatchedSourceInputRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { req.SourceInput = contracts.NewLLMInputMaterial("source", "application/json", []byte(`{"different":true}`), "sha256:other", "file:///other.json") return req diff --git a/internal/modules/dnd/extract/spells/extractor.go b/internal/modules/dnd/extract/spells/extractor.go index 978f03c..422bbfa 100644 --- a/internal/modules/dnd/extract/spells/extractor.go +++ b/internal/modules/dnd/extract/spells/extractor.go @@ -166,22 +166,7 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe if e.llm == nil { return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("LLM client must not be nil") } - if ctx == nil { - return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("context must not be nil") - } - if err := ctx.Err(); err != nil { - return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("context error before extraction: %w", err) - } - if req.Source == nil { - return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("source must not be nil") - } - if req.Chunk == nil { - return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("chunk must not be nil") - } - if len(req.Chunk.Units) == 0 { - return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("chunk %q units must not be empty", req.Chunk.ID) - } - sourceInput, err := shared.ChunkPromptMaterial(req) + sourceInput, err := shared.PrepareChunkExtraction(ctx, req) if err != nil { return contracts.TypedExtractionResult[dnd.SpellList]{}, extractorErrorf("%w", err) } diff --git a/internal/modules/dnd/extract/spells/extractor_test.go b/internal/modules/dnd/extract/spells/extractor_test.go index 0514572..8d0702d 100644 --- a/internal/modules/dnd/extract/spells/extractor_test.go +++ b/internal/modules/dnd/extract/spells/extractor_test.go @@ -238,9 +238,8 @@ func TestExtractWrapsLLMClientError(t *testing.T) { func TestExtractRejectsInvalidRequests(t *testing.T) { validReq := extractionRequest() - canceledCtx, cancel := context.WithCancel(context.Background()) - cancel() validExtractor := newExtractor(t, &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{}}}) + var nilExtractor *Extractor tests := []struct { name string extractor *Extractor @@ -248,13 +247,9 @@ func TestExtractRejectsInvalidRequests(t *testing.T) { req contracts.TypedExtractionRequest want string }{ - {name: "nil extractor", ctx: context.Background(), req: validReq, want: "extractor"}, - {name: "nil context", extractor: validExtractor, req: validReq, want: "context"}, - {name: "canceled context", extractor: validExtractor, ctx: canceledCtx, req: validReq, want: "context"}, - {name: "nil source", extractor: validExtractor, ctx: context.Background(), req: contracts.TypedExtractionRequest{Chunk: validReq.Chunk}, want: "source"}, - {name: "nil chunk", extractor: validExtractor, ctx: context.Background(), req: contracts.TypedExtractionRequest{Source: validReq.Source}, want: "chunk"}, - {name: "empty chunk units", extractor: validExtractor, ctx: context.Background(), req: emptyChunkRequest(validReq), want: "units"}, - {name: "source input mismatches chunk", extractor: validExtractor, ctx: context.Background(), req: mismatchedSourceInputRequest(validReq), want: "must match chunk"}, + {name: "nil extractor", extractor: nilExtractor, ctx: context.Background(), req: validReq, want: "extractor"}, + {name: "nil LLM client", extractor: &Extractor{}, ctx: context.Background(), req: validReq, want: "LLM client"}, + {name: "wrapped preflight failure", extractor: validExtractor, ctx: context.Background(), req: mismatchedSourceInputRequest(validReq), want: "must match chunk"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/internal/modules/dnd/extract/spells/test_helpers_test.go b/internal/modules/dnd/extract/spells/test_helpers_test.go index e3bb322..b95bb1f 100644 --- a/internal/modules/dnd/extract/spells/test_helpers_test.go +++ b/internal/modules/dnd/extract/spells/test_helpers_test.go @@ -99,11 +99,6 @@ func extractionRequest() contracts.TypedExtractionRequest { return req } -func emptyChunkRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { - req.Chunk = &source.Chunk{ID: req.Chunk.ID, SourceID: req.Chunk.SourceID, Index: req.Chunk.Index} - return req -} - func mismatchedSourceInputRequest(req contracts.TypedExtractionRequest) contracts.TypedExtractionRequest { req.SourceInput = spellSourceInput() return req diff --git a/internal/modules/dnd/shared/extraction_inputs.go b/internal/modules/dnd/shared/extraction_inputs.go index 3189069..5a9a442 100644 --- a/internal/modules/dnd/shared/extraction_inputs.go +++ b/internal/modules/dnd/shared/extraction_inputs.go @@ -2,14 +2,30 @@ package shared import ( "bytes" + "context" "fmt" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" ) -// ChunkPromptMaterial prepares the chunk-scoped source material used by D&D -// extractors when constructing their prompt inputs. -func ChunkPromptMaterial(req contracts.TypedExtractionRequest) (contracts.LLMInputMaterial, error) { +// PrepareChunkExtraction validates common D&D extraction prerequisites and +// prepares owned chunk-scoped source material for prompt inputs. +func PrepareChunkExtraction(ctx context.Context, req contracts.TypedExtractionRequest) (contracts.LLMInputMaterial, error) { + if ctx == nil { + return contracts.LLMInputMaterial{}, fmt.Errorf("context must not be nil") + } + if err := ctx.Err(); err != nil { + return contracts.LLMInputMaterial{}, fmt.Errorf("context error before extraction: %w", err) + } + if req.Source == nil { + return contracts.LLMInputMaterial{}, fmt.Errorf("source must not be nil") + } + if req.Chunk == nil { + return contracts.LLMInputMaterial{}, fmt.Errorf("chunk must not be nil") + } + if len(req.Chunk.Units) == 0 { + return contracts.LLMInputMaterial{}, fmt.Errorf("chunk %q units must not be empty", req.Chunk.ID) + } material := req.SourceInput.Clone() if len(material.Content) == 0 { material = contracts.NewLLMInputMaterial("source", req.Chunk.MediaType, req.Chunk.Content, "", "") diff --git a/internal/modules/dnd/shared/extraction_inputs_test.go b/internal/modules/dnd/shared/extraction_inputs_test.go index d37e011..e6b8c87 100644 --- a/internal/modules/dnd/shared/extraction_inputs_test.go +++ b/internal/modules/dnd/shared/extraction_inputs_test.go @@ -1,6 +1,7 @@ package shared import ( + "context" "reflect" "strings" "testing" @@ -9,48 +10,78 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" ) -func TestChunkPromptMaterial(t *testing.T) { - chunk := &source.Chunk{ - ID: "session-alpha:chunk:0", - Content: []byte(`{"units":[1,2]}`), - MediaType: "application/json", - } +func TestPrepareChunkExtraction(t *testing.T) { + canceled, cancel := context.WithCancel(context.Background()) + cancel() tests := []struct { name string + ctx context.Context + configure func(*contracts.TypedExtractionRequest) sourceInput contracts.LLMInputMaterial want contracts.LLMInputMaterial wantErr string mutateOutput bool }{ { - name: "fallback to chunk content", - want: contracts.NewLLMInputMaterial("source", chunk.MediaType, chunk.Content, "", ""), + name: "nil context", + wantErr: "context must not be nil", + }, + { + name: "canceled context", + ctx: canceled, + wantErr: "context error before extraction", + }, + { + name: "nil source", + configure: func(req *contracts.TypedExtractionRequest) { + req.Source = nil + }, + wantErr: "source must not be nil", + }, + { + name: "nil chunk", + configure: func(req *contracts.TypedExtractionRequest) { + req.Chunk = nil + }, + wantErr: "chunk must not be nil", + }, + { + name: "empty chunk units", + configure: func(req *contracts.TypedExtractionRequest) { + req.Chunk = &source.Chunk{ID: req.Chunk.ID} + }, + wantErr: "units must not be empty", + }, + { + name: "fallback to chunk content", + want: contracts.NewLLMInputMaterial("source", "application/json", []byte(`{"units":[1,2]}`), "", ""), + mutateOutput: true, }, { name: "clone isolation", - sourceInput: contracts.NewLLMInputMaterial("source", chunk.MediaType, chunk.Content, "sha256:source", "file:///source.json"), - want: contracts.NewLLMInputMaterial("source", chunk.MediaType, chunk.Content, "sha256:source", "file:///source.json"), + sourceInput: contracts.NewLLMInputMaterial("source", "application/json", []byte(`{"units":[1,2]}`), "sha256:source", "file:///source.json"), + want: contracts.NewLLMInputMaterial("source", "application/json", []byte(`{"units":[1,2]}`), "sha256:source", "file:///source.json"), mutateOutput: true, }, { name: "mismatched content", - sourceInput: contracts.NewLLMInputMaterial("source", chunk.MediaType, []byte(`{"units":[9]}`), "sha256:other", "file:///other.json"), + sourceInput: contracts.NewLLMInputMaterial("source", "application/json", []byte(`{"units":[9]}`), "sha256:other", "file:///other.json"), wantErr: "source input must match chunk", }, { name: "default fields", sourceInput: contracts.LLMInputMaterial{ - Content: append([]byte(nil), chunk.Content...), + Content: []byte(`{"units":[1,2]}`), Digest: "sha256:source", OriginURI: "file:///source.json", }, want: contracts.LLMInputMaterial{ Name: "source", - MediaType: chunk.MediaType, - Content: append([]byte(nil), chunk.Content...), + MediaType: "application/json", + Content: []byte(`{"units":[1,2]}`), Digest: "sha256:source", OriginURI: "file:///source.json", - SizeBytes: int64(len(chunk.Content)), + SizeBytes: int64(len(`{"units":[1,2]}`)), }, }, { @@ -58,7 +89,7 @@ func TestChunkPromptMaterial(t *testing.T) { sourceInput: contracts.LLMInputMaterial{ Name: "transcript", MediaType: "text/plain", - Content: append([]byte(nil), chunk.Content...), + Content: []byte(`{"units":[1,2]}`), Digest: "sha256:explicit", OriginURI: "file:///explicit.txt", SizeBytes: 42, @@ -66,7 +97,7 @@ func TestChunkPromptMaterial(t *testing.T) { want: contracts.LLMInputMaterial{ Name: "transcript", MediaType: "text/plain", - Content: append([]byte(nil), chunk.Content...), + Content: []byte(`{"units":[1,2]}`), Digest: "sha256:explicit", OriginURI: "file:///explicit.txt", SizeBytes: 42, @@ -76,26 +107,46 @@ func TestChunkPromptMaterial(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - req := contracts.TypedExtractionRequest{Chunk: chunk, SourceInput: test.sourceInput} - got, err := ChunkPromptMaterial(req) + req := extractionRequest() + req.SourceInput = test.sourceInput + if test.configure != nil { + test.configure(&req) + } + ctx := test.ctx + if ctx == nil && test.wantErr != "context must not be nil" { + ctx = context.Background() + } + got, err := PrepareChunkExtraction(ctx, req) if test.wantErr != "" { if err == nil || !strings.Contains(err.Error(), test.wantErr) { - t.Fatalf("ChunkPromptMaterial() error = %v, want %q", err, test.wantErr) + t.Fatalf("PrepareChunkExtraction() error = %v, want %q", err, test.wantErr) } return } if err != nil { - t.Fatalf("ChunkPromptMaterial() error = %v, want nil", err) + t.Fatalf("PrepareChunkExtraction() error = %v, want nil", err) } if !reflect.DeepEqual(got, test.want) { - t.Fatalf("ChunkPromptMaterial() = %#v, want %#v", got, test.want) + t.Fatalf("PrepareChunkExtraction() = %#v, want %#v", got, test.want) } if test.mutateOutput { got.Content[0] = 'x' - if string(test.sourceInput.Content) != string(chunk.Content) { - t.Fatalf("ChunkPromptMaterial() output shares content with source input") + if string(req.SourceInput.Content) != string(test.sourceInput.Content) || string(req.Chunk.Content) != `{"units":[1,2]}` { + t.Fatal("PrepareChunkExtraction() output shares request content") } } }) } } + +func extractionRequest() contracts.TypedExtractionRequest { + doc := &source.SourceDocument{ID: "session-alpha"} + chunk := &source.Chunk{ + ID: "session-alpha:chunk:0", + SourceID: doc.ID, + Content: []byte(`{"units":[1,2]}`), + MediaType: "application/json", + Units: []source.SourceUnit{{ID: 1}}, + } + return contracts.TypedExtractionRequest{Source: doc, Chunk: chunk} +}