diff --git a/internal/modules/extract/dnd/spells/assets/prompts/system.md b/internal/modules/extract/dnd/spells/assets/prompts/system.md index 78b5fe9..b7dff3d 100644 --- a/internal/modules/extract/dnd/spells/assets/prompts/system.md +++ b/internal/modules/extract/dnd/spells/assets/prompts/system.md @@ -6,4 +6,9 @@ Extract only spell casts that are supported by the provided source text. Do not infer spells from general D&D knowledge or from table chatter that does not identify a spell being cast. +Reference material, when present, is supporting context only. Use it only to +disambiguate names, aliases, speakers, campaign terms, or spell names already +present in the source text. Do not extract a spell cast solely because it appears +in reference material. + Source references must use the source-unit IDs exactly as provided. diff --git a/internal/modules/extract/dnd/spells/assets/prompts/user.md b/internal/modules/extract/dnd/spells/assets/prompts/user.md index 8cd1cf6..59d86d5 100644 --- a/internal/modules/extract/dnd/spells/assets/prompts/user.md +++ b/internal/modules/extract/dnd/spells/assets/prompts/user.md @@ -16,6 +16,19 @@ Source units: {{ end }} {{ end }} +{{ if hasreference "roster" }} +Roster reference material: +{{ reference "roster" }} + +{{ end }} +{{ if hasreference "glossary" }} +Glossary reference material: +{{ reference "glossary" }} + +{{ end }} Return only D&D spell-cast artifacts. For each spell cast, identify the in-world caster, spell name, effect, narrative description, and source references using source_id, start_unit_id, and end_unit_id. + +Use roster and glossary reference material only to clarify source text. Do not +return spells, casters, or effects that are mentioned only in reference material. diff --git a/internal/modules/extract/dnd/spells/extractor.go b/internal/modules/extract/dnd/spells/extractor.go index 7fa7f00..cc96a93 100644 --- a/internal/modules/extract/dnd/spells/extractor.go +++ b/internal/modules/extract/dnd/spells/extractor.go @@ -25,6 +25,19 @@ var providedCapabilities = []string{ "dnd.spell_casts", } +var referenceSlots = []contracts.ReferenceSlot{ + { + Name: "glossary", + Description: "Optional campaign glossary reference material used only for disambiguation.", + AcceptedMediaTypes: []string{"text/plain; charset=utf-8"}, + }, + { + Name: "roster", + Description: "Optional campaign roster or player-character reference material used only for disambiguation.", + AcceptedMediaTypes: []string{"text/plain; charset=utf-8"}, + }, +} + var _ contracts.Extractor = (*Extractor)(nil) type Extractor struct{} @@ -46,7 +59,7 @@ func (e *Extractor) SchemaVersion() string { } func (e *Extractor) ReferenceSlots() []contracts.ReferenceSlot { - return nil + return cloneReferenceSlots(referenceSlots) } func (e *Extractor) ManifestMetadata() map[string]any { @@ -140,10 +153,11 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.ExtractionRequest func ModuleSpec() pipeline.ModuleSpec { return pipeline.ModuleSpec{ - Key: Key, - Stage: pipeline.StageExtract, - Requires: append([]string(nil), requiredCapabilities...), - Provides: append([]string(nil), providedCapabilities...), + Key: Key, + Stage: pipeline.StageExtract, + Requires: append([]string(nil), requiredCapabilities...), + Provides: append([]string(nil), providedCapabilities...), + ReferenceSlots: cloneReferenceSlots(referenceSlots), } } @@ -165,3 +179,15 @@ func spellCastPayload(spellCast spellCastResponse) (json.RawMessage, error) { func extractorErrorf(format string, args ...any) error { return fmt.Errorf("dnd spells extractor: "+format, args...) } + +func cloneReferenceSlots(slots []contracts.ReferenceSlot) []contracts.ReferenceSlot { + if len(slots) == 0 { + return nil + } + out := make([]contracts.ReferenceSlot, len(slots)) + for i, slot := range slots { + slot.AcceptedMediaTypes = append([]string(nil), slot.AcceptedMediaTypes...) + out[i] = slot + } + return out +} diff --git a/internal/modules/extract/dnd/spells/extractor_test.go b/internal/modules/extract/dnd/spells/extractor_test.go index fcf1e9a..38fba50 100644 --- a/internal/modules/extract/dnd/spells/extractor_test.go +++ b/internal/modules/extract/dnd/spells/extractor_test.go @@ -116,6 +116,56 @@ func TestExtractorManifestMetadataIncludesPromptAndSchemaProvenance(t *testing.T } } +func TestExtractIncludesReferencesInPrompt(t *testing.T) { + client := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{}}} + req := extractionRequestWithClient(client) + req.References = contracts.ReferenceSet{ + Slots: map[string]contracts.ResolvedReferenceSlot{ + "roster": { + Slot: contracts.ReferenceSlot{Name: "roster"}, + Items: []contracts.ReferenceItem{ + {SlotName: "roster", Content: []byte("Aria Brightmantle: party cleric")}, + }, + }, + "glossary": { + Slot: contracts.ReferenceSlot{Name: "glossary"}, + Items: []contracts.ReferenceItem{ + {SlotName: "glossary", Content: []byte("Brightmantle: local temple name")}, + }, + }, + }, + } + + if _, err := New().Extract(context.Background(), req); err != nil { + t.Fatalf("Extract() error = %v, want nil", err) + } + + if len(client.requests) != 1 { + t.Fatalf("LLM calls = %d, want 1", len(client.requests)) + } + system := client.requests[0].Messages[0].Content + for _, want := range []string{ + "Reference material, when present, is supporting context only.", + "in reference material.", + } { + if !strings.Contains(system, want) { + t.Fatalf("system prompt = %q, want substring %q", system, want) + } + } + user := client.requests[0].Messages[1].Content + for _, want := range []string{ + "Roster reference material:", + "Aria Brightmantle: party cleric", + "Glossary reference material:", + "Brightmantle: local temple name", + "mentioned only in reference material.", + } { + if !strings.Contains(user, want) { + t.Fatalf("user prompt = %q, want substring %q", user, want) + } + } +} + func TestExtractReturnsNoCandidatesForEmptyResponse(t *testing.T) { client := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{}}} diff --git a/internal/modules/extract/dnd/spells/prompt.go b/internal/modules/extract/dnd/spells/prompt.go index 7b66473..41d51f7 100644 --- a/internal/modules/extract/dnd/spells/prompt.go +++ b/internal/modules/extract/dnd/spells/prompt.go @@ -32,11 +32,12 @@ var spellsPromptBundle = mustLoadPromptBundle() func mustLoadPromptBundle() *prompt.Bundle { bundle, err := prompt.LoadBundle(embeddedAssets, prompt.Definition{ - PromptID: PromptID, - Version: SchemaVersion, - EmbeddedPath: "assets/prompts", - SystemPath: "assets/prompts/system.md", - UserPath: "assets/prompts/user.md", + PromptID: PromptID, + Version: SchemaVersion, + EmbeddedPath: "assets/prompts", + SystemPath: "assets/prompts/system.md", + UserPath: "assets/prompts/user.md", + ReferenceSlots: cloneReferenceSlots(referenceSlots), }) if err != nil { panic(err) @@ -74,7 +75,7 @@ func renderPrompt(req contracts.ExtractionRequest) (system string, user string, if err != nil { return "", "", prompt.Metadata{}, err } - system, user, metadata, err = spellsPromptBundle.RenderUserSystem(data) + system, user, metadata, err = spellsPromptBundle.RenderUserSystemWithReferences(data, req.References) if err != nil { return "", "", prompt.Metadata{}, fmt.Errorf("dnd spells prompt: %w", err) } diff --git a/internal/modules/extract/dnd/spells/prompt_test.go b/internal/modules/extract/dnd/spells/prompt_test.go index 4bfea5b..770ac5e 100644 --- a/internal/modules/extract/dnd/spells/prompt_test.go +++ b/internal/modules/extract/dnd/spells/prompt_test.go @@ -101,6 +101,50 @@ func TestRenderPromptIncludesSourceContext(t *testing.T) { if metadata.EmbeddedPath != "assets/prompts" { t.Fatalf("metadata.EmbeddedPath = %q, want assets/prompts", metadata.EmbeddedPath) } + if strings.Contains(user, "Roster reference material") || strings.Contains(user, "Glossary reference material") { + t.Fatalf("user prompt = %q, want no optional reference sections without bindings", user) + } +} + +func TestRenderPromptIncludesBoundReferences(t *testing.T) { + req := promptExtractionRequest() + req.References = contracts.ReferenceSet{ + Slots: map[string]contracts.ResolvedReferenceSlot{ + "roster": { + Slot: contracts.ReferenceSlot{Name: "roster"}, + Items: []contracts.ReferenceItem{ + {SlotName: "roster", Content: []byte("Aria: cleric, also known as Sister Aria")}, + }, + }, + "glossary": { + Slot: contracts.ReferenceSlot{Name: "glossary"}, + Items: []contracts.ReferenceItem{ + {SlotName: "glossary", Content: []byte("Cure Wounds: healing spell")}, + }, + }, + }, + } + + _, user, metadata, err := renderPrompt(req) + if err != nil { + t.Fatalf("renderPrompt() error = %v, want nil", err) + } + + for _, want := range []string{ + "Roster reference material:", + "Aria: cleric, also known as Sister Aria", + "Glossary reference material:", + "Cure Wounds: healing spell", + "Use roster and glossary reference material only to clarify source text.", + "mentioned only in reference material.", + } { + if !strings.Contains(user, want) { + t.Fatalf("user prompt = %q, want substring %q", user, want) + } + } + if metadata.SHA256 != spellsPromptBundle.Metadata().SHA256 { + t.Fatalf("metadata.SHA256 = %q, want template hash %q", metadata.SHA256, spellsPromptBundle.Metadata().SHA256) + } } func TestBuildPromptDataRejectsMissingSourceContext(t *testing.T) { diff --git a/internal/modules/extract/dnd/spells/registry_test.go b/internal/modules/extract/dnd/spells/registry_test.go index b7c3204..59407d4 100644 --- a/internal/modules/extract/dnd/spells/registry_test.go +++ b/internal/modules/extract/dnd/spells/registry_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" ) @@ -36,6 +37,18 @@ func TestModuleSpec(t *testing.T) { Provides: []string{ "dnd.spell_casts", }, + ReferenceSlots: []contracts.ReferenceSlot{ + { + Name: "glossary", + Description: "Optional campaign glossary reference material used only for disambiguation.", + AcceptedMediaTypes: []string{"text/plain; charset=utf-8"}, + }, + { + Name: "roster", + Description: "Optional campaign roster or player-character reference material used only for disambiguation.", + AcceptedMediaTypes: []string{"text/plain; charset=utf-8"}, + }, + }, } if !reflect.DeepEqual(got, want) { t.Fatalf("ModuleSpec() = %#v, want %#v", got, want) @@ -43,6 +56,7 @@ func TestModuleSpec(t *testing.T) { got.Requires[0] = "changed" got.Provides[0] = "changed" + got.ReferenceSlots[0].AcceptedMediaTypes[0] = "changed" again := ModuleSpec() if !reflect.DeepEqual(again, want) { t.Fatalf("ModuleSpec() after caller mutation = %#v, want %#v", again, want) diff --git a/internal/modules/extract/dnd/spells/runner_test.go b/internal/modules/extract/dnd/spells/runner_test.go index 803a3a1..dc36b44 100644 --- a/internal/modules/extract/dnd/spells/runner_test.go +++ b/internal/modules/extract/dnd/spells/runner_test.go @@ -110,6 +110,96 @@ func TestRunnerProcessesSeriatimInputWithDNDSpellsExtractor(t *testing.T) { } } +func TestRunnerPassesRosterAndGlossaryReferencesToDNDSpellsPrompt(t *testing.T) { + raw := readDNDSpellsFixture(t) + expectedDoc := parseDNDSpellsFixture(t, raw) + resolved := resolveDNDSpellsPipeline(t) + resolved.ResolvedPipeline.ArtifactLanes[0].ReferenceSet = dndSpellsReferenceSet( + "Aria: party cleric\nBorin: fighter", + "Fire Bolt: evocation cantrip", + ) + llmClient := &fakeSpellsLLMClient{ + response: extractionResponse{ + SpellCasts: []spellCastResponse{ + { + Caster: "Borin", + Spell: "Fire Bolt", + Effect: "Scorches the wight.", + NarrativeDescription: "Borin hurls fire at the wight.", + SourceRefs: []source.SourceRef{ + {SourceID: expectedDoc.ID, StartUnitID: "seg-003", EndUnitID: "seg-003"}, + }, + }, + }, + }, + } + + output, err := pipeline.New(dndSpellsRunnerRegistries(t)).Run(context.Background(), pipeline.RunInput{ + Pipeline: resolved.ResolvedPipeline, + RawInput: raw, + LLMClient: llmClient, + }) + if err != nil { + t.Fatalf("Run() error = %v, want nil", err) + } + + if len(output.Approved) != 1 { + t.Fatalf("len(Approved) = %d, want 1", len(output.Approved)) + } + if len(output.Manifest.References) != 2 { + t.Fatalf("manifest references = %#v, want roster and glossary provenance", output.Manifest.References) + } + if len(llmClient.requests) != 1 { + t.Fatalf("LLM calls = %d, want 1", len(llmClient.requests)) + } + user := llmClient.requests[0].Messages[1].Content + for _, want := range []string{ + "Roster reference material:", + "Aria: party cleric", + "Glossary reference material:", + "Fire Bolt: evocation cantrip", + } { + if !strings.Contains(user, want) { + t.Fatalf("user prompt = %q, want substring %q", user, want) + } + } +} + +func TestRunnerDoesNotExtractSpellMentionedOnlyInRoster(t *testing.T) { + raw := readDNDSpellsFixture(t) + resolved := resolveDNDSpellsPipeline(t) + resolved.ResolvedPipeline.ArtifactLanes[0].ReferenceSet = dndSpellsReferenceSet( + "Mira: wizard who can cast Lightning Bolt", + "", + ) + llmClient := &fakeSpellsLLMClient{ + response: extractionResponse{SpellCasts: []spellCastResponse{}}, + } + + output, err := pipeline.New(dndSpellsRunnerRegistries(t)).Run(context.Background(), pipeline.RunInput{ + Pipeline: resolved.ResolvedPipeline, + RawInput: raw, + LLMClient: llmClient, + }) + if err != nil { + t.Fatalf("Run() error = %v, want nil", err) + } + + if len(output.Approved) != 0 { + t.Fatalf("approved artifacts = %#v, want no roster-only spell casts", output.Approved) + } + if len(llmClient.requests) != 1 { + t.Fatalf("LLM calls = %d, want 1", len(llmClient.requests)) + } + user := llmClient.requests[0].Messages[1].Content + if !strings.Contains(user, "Lightning Bolt") { + t.Fatalf("user prompt = %q, want roster-only spell in reference section", user) + } + if output.Manifest.ValidationStatus != "approved" { + t.Fatalf("ValidationStatus = %q, want approved empty extraction", output.Manifest.ValidationStatus) + } +} + func TestRunnerRejectsDNDSpellCastWithInvalidSourceRef(t *testing.T) { raw := readDNDSpellsFixture(t) resolved := resolveDNDSpellsPipeline(t) @@ -155,6 +245,43 @@ func TestRunnerRejectsDNDSpellCastWithInvalidSourceRef(t *testing.T) { } } +func dndSpellsReferenceSet(roster string, glossary string) contracts.ReferenceSet { + slots := make(map[string]contracts.ResolvedReferenceSlot) + if strings.TrimSpace(roster) != "" { + slots["roster"] = contracts.ResolvedReferenceSlot{ + Slot: contracts.ReferenceSlot{Name: "roster"}, + Items: []contracts.ReferenceItem{ + { + SlotName: "roster", + MediaType: "text/plain; charset=utf-8", + Content: []byte(roster), + Digest: "sha256:roster", + Origin: contracts.ReferenceOrigin{Type: "file", URI: "file:///tmp/roster.txt"}, + SizeBytes: int64(len(roster)), + BindingSource: contracts.ReferenceBindingSourceConfig, + }, + }, + } + } + if strings.TrimSpace(glossary) != "" { + slots["glossary"] = contracts.ResolvedReferenceSlot{ + Slot: contracts.ReferenceSlot{Name: "glossary"}, + Items: []contracts.ReferenceItem{ + { + SlotName: "glossary", + MediaType: "text/plain; charset=utf-8", + Content: []byte(glossary), + Digest: "sha256:glossary", + Origin: contracts.ReferenceOrigin{Type: "file", URI: "file:///tmp/glossary.txt"}, + SizeBytes: int64(len(glossary)), + BindingSource: contracts.ReferenceBindingSourceConfig, + }, + }, + } + } + return contracts.ReferenceSet{Slots: slots} +} + func TestRunnerFailsWhenDNDSpellsExtractorReturnsMalformedOutput(t *testing.T) { raw := readDNDSpellsFixture(t) resolved := resolveDNDSpellsPipeline(t) diff --git a/internal/modules/extract/dnd/spells/validator.go b/internal/modules/extract/dnd/spells/validator.go index 4c760e6..1b33ef6 100644 --- a/internal/modules/extract/dnd/spells/validator.go +++ b/internal/modules/extract/dnd/spells/validator.go @@ -20,6 +20,7 @@ const ( reasonMissingRequiredField = "missing_required_field" reasonMissingSourceRef = "missing_source_ref" reasonInvalidSourceRef = "invalid_source_ref" + reasonSpellNotNearSource = "spell_not_near_source" ) var _ contracts.Validator = ShapeValidator{} @@ -54,12 +55,15 @@ func (validator SourceRefValidator) Validate(ctx context.Context, req contracts. } decisions := make([]contracts.ValidationDecision, 0, len(req.Candidates)) + var warnings []contracts.Warning for _, candidate := range req.Candidates { decisions = append(decisions, validateSourceRefs(req.Source, candidate)) + warnings = append(warnings, sourceRelatednessWarnings(req.Source, candidate)...) } return contracts.ValidationResult{ ValidatorName: validator.Name(), Decisions: decisions, + Warnings: warnings, }, nil } @@ -88,6 +92,66 @@ func validateSourceRefs(doc *source.SourceDocument, candidate artifacts.Artifact return validate.Approved(candidate.Index) } +func sourceRelatednessWarnings(doc *source.SourceDocument, candidate artifacts.ArtifactCandidate) []contracts.Warning { + if doc == nil || len(candidate.SourceRefs) == 0 { + return nil + } + var payload SpellCast + if err := json.Unmarshal(candidate.Payload, &payload); err != nil { + return nil + } + spell := strings.TrimSpace(payload.Spell) + if spell == "" { + return nil + } + + needle := strings.ToLower(spell) + for _, ref := range candidate.SourceRefs { + text, ok := sourceRefText(doc, ref) + if !ok { + continue + } + if strings.Contains(strings.ToLower(text), needle) { + return nil + } + } + + return []contracts.Warning{ + { + Scope: fmt.Sprintf("candidate.%d", candidate.Index), + ReasonCode: reasonSpellNotNearSource, + Message: fmt.Sprintf("spell %q was not found in the cited source text", spell), + }, + } +} + +func sourceRefText(doc *source.SourceDocument, ref source.SourceRef) (string, bool) { + if err := source.ValidateRef(doc, ref); err != nil { + return "", false + } + start := -1 + end := -1 + for i, unit := range doc.Units { + if unit.ID == ref.StartUnitID { + start = i + } + if unit.ID == ref.EndUnitID { + end = i + } + } + if start < 0 || end < start { + return "", false + } + var b strings.Builder + for i := start; i <= end; i++ { + if b.Len() > 0 { + b.WriteString("\n") + } + b.WriteString(doc.Units[i].Text) + } + return b.String(), true +} + func requiredSpellCastFields(payload SpellCast) []struct { name string value string diff --git a/internal/modules/extract/dnd/spells/validator_test.go b/internal/modules/extract/dnd/spells/validator_test.go index f5b7160..10e3182 100644 --- a/internal/modules/extract/dnd/spells/validator_test.go +++ b/internal/modules/extract/dnd/spells/validator_test.go @@ -51,6 +51,9 @@ func TestValidatorsApproveValidCandidate(t *testing.T) { t.Fatalf("SourceRefValidator.Validate() error = %v, want nil", err) } assertSingleDecision(t, sourceRefResult, sourceRefValidatorName, 7, true, validate.ReasonApproved) + if len(sourceRefResult.Warnings) != 0 { + t.Fatalf("warnings = %#v, want none", sourceRefResult.Warnings) + } } func TestShapeValidatorRejectsMalformedPayload(t *testing.T) { @@ -167,6 +170,29 @@ func TestSourceRefValidatorRejectsInvalidRefs(t *testing.T) { } } +func TestSourceRefValidatorWarnsWhenSpellNameIsNotInCitedSource(t *testing.T) { + candidate := validSpellCandidate(31) + payload := validSpellPayload() + payload.Spell = "Shield" + candidate.Payload = mustSpellPayload(t, payload) + + result, err := SourceRefValidator{}.Validate(context.Background(), contracts.ValidationRequest{ + Source: promptSourceDocument(), + Candidates: []artifacts.ArtifactCandidate{candidate}, + }) + if err != nil { + t.Fatalf("SourceRefValidator.Validate() error = %v, want nil", err) + } + assertSingleDecision(t, result, sourceRefValidatorName, 31, true, validate.ReasonApproved) + if len(result.Warnings) != 1 { + t.Fatalf("warnings = %#v, want one relatedness warning", result.Warnings) + } + warning := result.Warnings[0] + if warning.ReasonCode != reasonSpellNotNearSource || !strings.Contains(warning.Message, "Shield") { + t.Fatalf("warning = %#v, want spell relatedness warning", warning) + } +} + func TestSourceRefValidatorRequiresSourceDocument(t *testing.T) { _, err := SourceRefValidator{}.Validate(context.Background(), contracts.ValidationRequest{ Candidates: []artifacts.ArtifactCandidate{validSpellCandidate(19)},