From ab0b4e350c3f3e491e98fcc6617fab6bbc7a0e96 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 22 Jul 2026 14:38:35 +0000 Subject: [PATCH] Relax private D&D response schemas --- docs/integrations/dnd-npc-artifacts.md | 5 ++ docs/integrations/dnd-spell-artifacts.md | 4 +- docs/internal/llm.md | 10 +-- docs/internal/modules.md | 11 +++ .../npcs/assets/schemas/dnd_npcs_llm.v1.json | 22 ++--- .../modules/dnd/extract/npcs/canonicalize.go | 27 ++---- .../dnd/extract/npcs/extractor_test.go | 30 +++++-- internal/modules/dnd/extract/npcs/model.go | 6 +- .../modules/dnd/extract/npcs/schema_test.go | 60 +++++++++++++ .../dnd/extract/npcs/test_helpers_test.go | 11 ++- .../assets/schemas/dnd_spells_llm.v1.json | 13 +-- .../dnd/extract/spells/canonicalize.go | 27 ++---- .../dnd/extract/spells/extractor_test.go | 19 +++- internal/modules/dnd/extract/spells/model.go | 6 +- .../modules/dnd/extract/spells/schema_test.go | 60 +++++++++++++ .../dnd/extract/spells/test_helpers_test.go | 13 +-- .../integration/dnd_npcs_runner_test.go | 86 ++++++++++++++----- .../integration/dnd_spells_helpers_test.go | 33 ++++--- .../integration/dnd_spells_runner_test.go | 66 ++++++++++---- 19 files changed, 355 insertions(+), 154 deletions(-) diff --git a/docs/integrations/dnd-npc-artifacts.md b/docs/integrations/dnd-npc-artifacts.md index 3ae545c..7c7381f 100644 --- a/docs/integrations/dnd-npc-artifacts.md +++ b/docs/integrations/dnd-npc-artifacts.md @@ -20,6 +20,11 @@ the Unicode-normalized, case-folded canonical name using the identity policy. The durable codec enforces the artifact shape and ID syntax; registry identity validation remains a separate deterministic concern. +The extractor's private LLM response schema is a separate structural transport +contract. It omits framework-assigned NPC and source IDs and admits semantic +candidates for the deterministic shape and source-reference validators; it is +not part of this durable contract. + ## Output Shape The payload is one object with a required top-level `npcs` array: diff --git a/docs/integrations/dnd-spell-artifacts.md b/docs/integrations/dnd-spell-artifacts.md index 8c757ee..f25ee73 100644 --- a/docs/integrations/dnd-spell-artifacts.md +++ b/docs/integrations/dnd-spell-artifacts.md @@ -18,7 +18,9 @@ The durable JSON Schema is owned by the D&D spell artifact codec. The extractor's private LLM response schema is a separate transport contract: its source-reference objects omit `source_id`, which the extractor assigns while mapping the response to the canonical artifact. The LLM DTO and transport -schema are not part of this durable contract. +schema are not part of this durable contract. The private schema owns required +fields, JSON types, object and array shapes, and unknown-field rejection; +deterministic validators own the durable artifact's semantic constraints. The output contains canonical spell casts derived from transcript evidence. Source IDs are assigned from the input identity; source-unit ranges identify diff --git a/docs/internal/llm.md b/docs/internal/llm.md index b837868..0babdb2 100644 --- a/docs/internal/llm.md +++ b/docs/internal/llm.md @@ -157,11 +157,11 @@ schemas remain package-owned. The spell, NPC, and combat extractors' package-owned prompts declare their structured JSON inputs and private response schemas. Each private response schema remains separate from its durable artifact codec schema; this work does -not use shared schema fragments or schema generation. The combat private schema -owns the transport envelope—required fields, JSON types, nullability, and -unknown-field rejection—while its deterministic validators own semantic -constraints such as enum membership, non-empty values and collections, and -positive numbers. The spell extractor's prompt declares a required +not use shared schema fragments or schema generation. Those private schemas own +the transport envelope—required fields, JSON types, nullability, and +unknown-field rejection—while deterministic validators own semantic constraints +such as enum membership, non-empty values and collections, and positive +numbers. The spell extractor's prompt declares a required `application/json` `spell_catalog` input and an optional `application/json` `npcs` input. The extractor generates the catalog input from its prepared diff --git a/docs/internal/modules.md b/docs/internal/modules.md index 98cfd30..57b3c93 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -192,6 +192,12 @@ decodes the model response, assigns the generic source identity to every source reference, canonicalizes duplicate references, orders spell casts by their earliest cited unit, and returns `dnd.SpellList`. +Its private response schema admits only the structural transport envelope: +required fields, JSON types, array and object shapes, and unknown-field +rejection. It maps integer source-unit candidates directly without repairing +semantic values, so the deterministic shape, catalog, and source-reference +validators own blank values, empty evidence, and invalid or unresolved ranges. + The extractor owns its private model-response DTO, embedded prompt, LLM response schema, strict option decoder, injected shared LLM client, and prompt/schema manifest metadata. During preparation it resolves the optional `spell_catalog` @@ -250,6 +256,11 @@ registry slot. Its prompt and private response schema are package-owned. The prompt follows the shared D&D extraction ordering and cache policy documented in [LLM Runtime](llm.md#dd-extraction-prompt-ordering-and-cache-boundaries). +The private response schema owns only structural transport validation and maps +integer source-unit candidates unchanged. Required semantic content, non-empty +evidence, and valid source ranges are rejected by the deterministic shape and +source-reference validators. + ### `internal/modules/dnd/extract/combatturns` The combat extractor prepares one structured request per supplied chunk using diff --git a/internal/modules/dnd/extract/npcs/assets/schemas/dnd_npcs_llm.v1.json b/internal/modules/dnd/extract/npcs/assets/schemas/dnd_npcs_llm.v1.json index 6015d9a..800eb6e 100644 --- a/internal/modules/dnd/extract/npcs/assets/schemas/dnd_npcs_llm.v1.json +++ b/internal/modules/dnd/extract/npcs/assets/schemas/dnd_npcs_llm.v1.json @@ -19,19 +19,16 @@ ], "properties": { "name": { - "type": "string", - "minLength": 1 + "type": "string" }, "aliases": { "type": "array", "items": { - "type": "string", - "minLength": 1 + "type": "string" } }, "description": { - "type": "string", - "minLength": 1 + "type": "string" }, "relationships": { "type": "array", @@ -41,31 +38,26 @@ "required": ["target", "relationship"], "properties": { "target": { - "type": "string", - "minLength": 1 + "type": "string" }, "relationship": { - "type": "string", - "minLength": 1 + "type": "string" } } } }, "source_refs": { "type": "array", - "minItems": 1, "items": { "type": "object", "additionalProperties": false, "required": ["start_unit_id", "end_unit_id"], "properties": { "start_unit_id": { - "type": "integer", - "minimum": 1 + "type": "integer" }, "end_unit_id": { - "type": "integer", - "minimum": 1 + "type": "integer" } } } diff --git a/internal/modules/dnd/extract/npcs/canonicalize.go b/internal/modules/dnd/extract/npcs/canonicalize.go index 4c5bd3a..e05c5d7 100644 --- a/internal/modules/dnd/extract/npcs/canonicalize.go +++ b/internal/modules/dnd/extract/npcs/canonicalize.go @@ -6,7 +6,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) func canonicalizeResponse(response *extractionResponse, doc *source.SourceDocument) { @@ -33,10 +32,6 @@ func canonicalizeNPC(npc *npcResponse) { if npc == nil { return } - for index := range npc.SourceRefs { - npc.SourceRefs[index].StartUnitID = canonicalUnitRef(npc.SourceRefs[index].StartUnitID) - npc.SourceRefs[index].EndUnitID = canonicalUnitRef(npc.SourceRefs[index].EndUnitID) - } sort.SliceStable(npc.SourceRefs, func(i, j int) bool { left := npc.SourceRefs[i] right := npc.SourceRefs[j] @@ -48,14 +43,6 @@ func canonicalizeNPC(npc *npcResponse) { npc.SourceRefs = dedupeSourceRefs(npc.SourceRefs) } -func canonicalUnitRef(ref shared.UnitRef) shared.UnitRef { - value := ref.Int() - if value <= 0 { - return ref - } - return shared.UnitRefFromInt(value) -} - func dedupeSourceRefs(refs []npcSourceRefResponse) []npcSourceRefResponse { if len(refs) < 2 { return refs @@ -73,16 +60,15 @@ func dedupeSourceRefs(refs []npcSourceRefResponse) []npcSourceRefResponse { } func sameSourceRef(left npcSourceRefResponse, right npcSourceRefResponse) bool { - return left.StartUnitID.Int() == right.StartUnitID.Int() && - left.EndUnitID.Int() == right.EndUnitID.Int() + return left.StartUnitID == right.StartUnitID && left.EndUnitID == right.EndUnitID } func earliestSourceIndex(doc *source.SourceDocument, npc npcResponse) (int, bool) { earliest := 0 found := false for _, ref := range npc.SourceRefs { - start := ref.StartUnitID.Int() - end := ref.EndUnitID.Int() + start := ref.StartUnitID + end := ref.EndUnitID if start > 0 && end > 0 { startIndex, startOK := source.UnitIndex(doc, start) endIndex, endOK := source.UnitIndex(doc, end) @@ -98,8 +84,7 @@ func earliestSourceIndex(doc *source.SourceDocument, npc npcResponse) (int, bool return earliest, found } -func unitSortValue(ref shared.UnitRef) int { - value := ref.Int() +func unitSortValue(value int) int { if value <= 0 { return int(^uint(0) >> 1) } @@ -150,8 +135,8 @@ func canonicalSourceRefs(values []npcSourceRefResponse, sourceID string) []sourc for index, value := range values { out[index] = source.SourceRef{ SourceID: sourceID, - StartUnitID: value.StartUnitID.Int(), - EndUnitID: value.EndUnitID.Int(), + StartUnitID: value.StartUnitID, + EndUnitID: value.EndUnitID, } } return out diff --git a/internal/modules/dnd/extract/npcs/extractor_test.go b/internal/modules/dnd/extract/npcs/extractor_test.go index f7ec8dc..dc48fe0 100644 --- a/internal/modules/dnd/extract/npcs/extractor_test.go +++ b/internal/modules/dnd/extract/npcs/extractor_test.go @@ -11,7 +11,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) func TestExtractReturnsCanonicalNPCListFromPrivateResponse(t *testing.T) { @@ -25,9 +24,9 @@ func TestExtractReturnsCanonicalNPCListFromPrivateResponse(t *testing.T) { Name: "Mira Thorn", Aliases: []string{"The Greencloak"}, Description: "A guarded ranger.", Relationships: []npcRelationshipResponse{{Target: "Captain Vale", Relationship: "commands"}}, SourceRefs: []npcSourceRefResponse{ - {StartUnitID: sharedUnitRef(2), EndUnitID: sharedUnitRef(2)}, - {StartUnitID: sharedUnitRef(1), EndUnitID: sharedUnitRef(2)}, - {StartUnitID: sharedUnitRef(1), EndUnitID: sharedUnitRef(2)}, + {StartUnitID: 2, EndUnitID: 2}, + {StartUnitID: 1, EndUnitID: 2}, + {StartUnitID: 1, EndUnitID: 2}, }, }, }}} @@ -65,8 +64,8 @@ func TestExtractOrdersNPCsBySourcePositionRatherThanUnitID(t *testing.T) { { Name: "Earlier NPC", Aliases: []string{}, Description: "Appears first.", Relationships: []npcRelationshipResponse{}, SourceRefs: []npcSourceRefResponse{ - {StartUnitID: sharedUnitRef(50), EndUnitID: sharedUnitRef(50)}, - {StartUnitID: sharedUnitRef(100), EndUnitID: sharedUnitRef(100)}, + {StartUnitID: 50, EndUnitID: 50}, + {StartUnitID: 100, EndUnitID: 100}, }, }, }}} @@ -111,7 +110,7 @@ func TestExtractPassesCampaignReferencesAsPromptInputs(t *testing.T) { func TestExtractPreservesMalformedCandidatesForValidators(t *testing.T) { client := &fakeNPCsLLMClient{response: extractionResponse{NPCs: []npcResponse{{ Name: "", Aliases: nil, Description: "", Relationships: nil, - SourceRefs: []npcSourceRefResponse{{StartUnitID: sharedUnitRef(99), EndUnitID: shared.UnitRefFromString("missing")}, {StartUnitID: sharedUnitRef(99), EndUnitID: shared.UnitRefFromInt(0)}}, + SourceRefs: []npcSourceRefResponse{{StartUnitID: 99, EndUnitID: 0}}, }}}} result, err := newExtractor(t, client).Extract(context.Background(), extractionRequest()) if err != nil { @@ -125,6 +124,21 @@ func TestExtractPreservesMalformedCandidatesForValidators(t *testing.T) { } } +func TestExtractMapsRawSemanticCandidatesWithoutRepair(t *testing.T) { + client := &fakeNPCsLLMClient{content: []byte(`{"npcs":[{"name":"","aliases":[],"description":"","relationships":[],"source_refs":[{"start_unit_id":0,"end_unit_id":-1}]}]}`)} + result, err := newExtractor(t, client).Extract(context.Background(), extractionRequest()) + if err != nil { + t.Fatalf("Extract() error = %v, want nil", err) + } + npc := result.Value.NPCs[0] + if npc.ID != "" || npc.Name != "" || npc.Description != "" { + t.Fatalf("NPC = %#v, want blank semantic values preserved", npc) + } + if refs := npc.SourceRefs; len(refs) != 1 || refs[0] != (source.SourceRef{SourceID: "session-alpha", StartUnitID: 0, EndUnitID: -1}) { + t.Fatalf("source refs = %#v, want raw nonpositive candidates preserved", refs) + } +} + func TestExtractHandlesCancellationAndProviderErrors(t *testing.T) { request := extractionRequest() extractor := newExtractor(t, &fakeNPCsLLMClient{response: extractionResponse{NPCs: []npcResponse{}}}) @@ -142,5 +156,3 @@ func TestExtractHandlesCancellationAndProviderErrors(t *testing.T) { t.Fatalf("provider Extract() error = %v, want contextual provider error", err) } } - -func sharedUnitRef(value int) shared.UnitRef { return shared.UnitRefFromInt(value) } diff --git a/internal/modules/dnd/extract/npcs/model.go b/internal/modules/dnd/extract/npcs/model.go index d04e1ac..fcff5a1 100644 --- a/internal/modules/dnd/extract/npcs/model.go +++ b/internal/modules/dnd/extract/npcs/model.go @@ -1,7 +1,5 @@ package npcs -import "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" - type extractionResponse struct { NPCs []npcResponse `json:"npcs"` } @@ -20,6 +18,6 @@ type npcRelationshipResponse struct { } type npcSourceRefResponse struct { - StartUnitID shared.UnitRef `json:"start_unit_id"` - EndUnitID shared.UnitRef `json:"end_unit_id"` + StartUnitID int `json:"start_unit_id"` + EndUnitID int `json:"end_unit_id"` } diff --git a/internal/modules/dnd/extract/npcs/schema_test.go b/internal/modules/dnd/extract/npcs/schema_test.go index 59e3fc6..2d88b0e 100644 --- a/internal/modules/dnd/extract/npcs/schema_test.go +++ b/internal/modules/dnd/extract/npcs/schema_test.go @@ -28,6 +28,66 @@ func TestLoadResponseSchemaUsesPrivateNPCSchema(t *testing.T) { if err := validateJSONSchema(validJSON, schema.JSONSchema); err != nil { t.Fatalf("valid private NPC response rejected: %v", err) } + for _, test := range []struct { + name string + response map[string]any + valid bool + }{ + { + name: "semantic blanks and empty collections", + response: map[string]any{"npcs": []any{map[string]any{ + "name": "", "aliases": []any{""}, "description": "", "relationships": []any{map[string]any{"target": "", "relationship": ""}}, "source_refs": []any{}, + }}}, + valid: true, + }, + { + name: "nonpositive unit candidates", + response: map[string]any{"npcs": []any{map[string]any{ + "name": "Mira Thorn", "aliases": []any{}, "description": "A ranger.", "relationships": []any{}, + "source_refs": []any{map[string]any{"start_unit_id": 0, "end_unit_id": -1}}, + }}}, + valid: true, + }, + { + name: "missing required field", + response: map[string]any{"npcs": []any{map[string]any{ + "aliases": []any{}, "description": "A ranger.", "relationships": []any{}, "source_refs": []any{}, + }}}, + }, + { + name: "unknown field", + response: map[string]any{"npcs": []any{map[string]any{ + "name": "Mira Thorn", "aliases": []any{}, "description": "A ranger.", "relationships": []any{}, "source_refs": []any{}, "id": "assigned later", + }}}, + }, + { + name: "wrong field type", + response: map[string]any{"npcs": []any{map[string]any{ + "name": 7, "aliases": []any{}, "description": "A ranger.", "relationships": []any{}, "source_refs": []any{}, + }}}, + }, + { + name: "noninteger source identifier", + response: map[string]any{"npcs": []any{map[string]any{ + "name": "Mira Thorn", "aliases": []any{}, "description": "A ranger.", "relationships": []any{}, + "source_refs": []any{map[string]any{"start_unit_id": 1.5, "end_unit_id": 2}}, + }}}, + }, + } { + t.Run(test.name, func(t *testing.T) { + content, err := json.Marshal(test.response) + if err != nil { + t.Fatal(err) + } + err = validateJSONSchema(content, schema.JSONSchema) + if (err == nil) != test.valid { + t.Fatalf("validateJSONSchema() error = %v, want valid=%t", err, test.valid) + } + }) + } + if err := validateJSONSchema([]byte(`{"npcs":`), schema.JSONSchema); err == nil { + t.Fatal("validateJSONSchema() error = nil, want malformed JSON rejected") + } withID := map[string]any{"npcs": []any{map[string]any{ "name": "Mira Thorn", "id": "assigned-later", "aliases": []any{}, "description": "A ranger.", "relationships": []any{}, "source_refs": []any{map[string]any{"start_unit_id": 1, "end_unit_id": 2}}, diff --git a/internal/modules/dnd/extract/npcs/test_helpers_test.go b/internal/modules/dnd/extract/npcs/test_helpers_test.go index 1295fdb..166502a 100644 --- a/internal/modules/dnd/extract/npcs/test_helpers_test.go +++ b/internal/modules/dnd/extract/npcs/test_helpers_test.go @@ -8,7 +8,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) func extractionRequest() contracts.TypedExtractionRequest { @@ -47,7 +46,7 @@ func sourceDocument() *source.SourceDocument { } func responseSourceRefs(startUnitID, endUnitID int) []npcSourceRefResponse { - return []npcSourceRefResponse{{StartUnitID: shared.UnitRefFromInt(startUnitID), EndUnitID: shared.UnitRefFromInt(endUnitID)}} + return []npcSourceRefResponse{{StartUnitID: startUnitID, EndUnitID: endUnitID}} } func newExtractor(t *testing.T, client contracts.StructuredLLMClient, references ...contracts.ReferenceSet) *Extractor { @@ -99,9 +98,13 @@ func (client *fakeNPCsLLMClient) CompleteStructured(_ context.Context, req contr if !ok { return contracts.StructuredCompletionResponse{}, errors.New("unexpected output target") } - *target = client.response content := append([]byte(nil), client.content...) - if len(content) == 0 { + if len(content) != 0 { + if err := json.Unmarshal(content, target); err != nil { + return contracts.StructuredCompletionResponse{}, err + } + } else { + *target = client.response var err error content, err = json.Marshal(client.response) if err != nil { diff --git a/internal/modules/dnd/extract/spells/assets/schemas/dnd_spells_llm.v1.json b/internal/modules/dnd/extract/spells/assets/schemas/dnd_spells_llm.v1.json index 2cbe1ef..2b6ab5e 100644 --- a/internal/modules/dnd/extract/spells/assets/schemas/dnd_spells_llm.v1.json +++ b/internal/modules/dnd/extract/spells/assets/schemas/dnd_spells_llm.v1.json @@ -20,40 +20,33 @@ "properties": { "caster": { "type": "string", - "minLength": 1, "description": "Canonical in-world character or creature that casts the spell, never the human player, transcript speaker, or GM when the in-world caster can be identified." }, "spell": { "type": "string", - "minLength": 1, "description": "Canonical spell name from the provided spell-name catalog." }, "effect": { "type": "string", - "minLength": 1, "description": "Concise immediate effect or resolution established by the cited transcript units; do not infer mechanics from general D&D rules knowledge or follow persistent downstream consequences." }, "narrative_description": { "type": "string", - "minLength": 1, "description": "Short session-grounded description of the casting declaration and immediate resolution, containing only details established by the cited transcript units." }, "source_refs": { "type": "array", - "minItems": 1, - "description": "One or more narrow transcript ranges that collectively support every factual claim about the casting declaration and immediate resolution in this spell-cast object.", + "description": "Transcript ranges offered as evidence for factual claims about the casting declaration and immediate resolution in this spell-cast object.", "items": { "type": "object", "additionalProperties": false, "required": ["start_unit_id", "end_unit_id"], "properties": { "start_unit_id": { - "type": "integer", - "minimum": 1 + "type": "integer" }, "end_unit_id": { - "type": "integer", - "minimum": 1 + "type": "integer" } } } diff --git a/internal/modules/dnd/extract/spells/canonicalize.go b/internal/modules/dnd/extract/spells/canonicalize.go index 2031e7e..69ced5f 100644 --- a/internal/modules/dnd/extract/spells/canonicalize.go +++ b/internal/modules/dnd/extract/spells/canonicalize.go @@ -5,7 +5,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) func canonicalizeResponse(response *extractionResponse) { @@ -29,14 +28,10 @@ func canonicalizeResponse(response *extractionResponse) { } func canonicalizeSpellCast(spell *spellCastResponse) { - for index := range spell.SourceRefs { - spell.SourceRefs[index].StartUnitID = canonicalUnitRef(spell.SourceRefs[index].StartUnitID) - spell.SourceRefs[index].EndUnitID = canonicalUnitRef(spell.SourceRefs[index].EndUnitID) - } sort.SliceStable(spell.SourceRefs, func(i, j int) bool { left := spell.SourceRefs[i] right := spell.SourceRefs[j] - if left.StartUnitID.Int() != right.StartUnitID.Int() { + if left.StartUnitID != right.StartUnitID { return unitSortValue(left.StartUnitID) < unitSortValue(right.StartUnitID) } return unitSortValue(left.EndUnitID) < unitSortValue(right.EndUnitID) @@ -44,14 +39,6 @@ func canonicalizeSpellCast(spell *spellCastResponse) { spell.SourceRefs = dedupeSourceRefs(spell.SourceRefs) } -func canonicalUnitRef(ref shared.UnitRef) shared.UnitRef { - value := ref.Int() - if value <= 0 { - return ref - } - return shared.UnitRefFromInt(value) -} - func dedupeSourceRefs(refs []spellSourceRefResponse) []spellSourceRefResponse { if len(refs) < 2 { return refs @@ -69,13 +56,12 @@ func dedupeSourceRefs(refs []spellSourceRefResponse) []spellSourceRefResponse { } func sameSourceRef(left spellSourceRefResponse, right spellSourceRefResponse) bool { - return left.StartUnitID.Int() == right.StartUnitID.Int() && - left.EndUnitID.Int() == right.EndUnitID.Int() + return left.StartUnitID == right.StartUnitID && left.EndUnitID == right.EndUnitID } func earliestSourceUnit(spell spellCastResponse) (int, bool) { for _, ref := range spell.SourceRefs { - start := ref.StartUnitID.Int() + start := ref.StartUnitID if start > 0 { return start, true } @@ -83,8 +69,7 @@ func earliestSourceUnit(spell spellCastResponse) (int, bool) { return 0, false } -func unitSortValue(ref shared.UnitRef) int { - value := ref.Int() +func unitSortValue(value int) int { if value <= 0 { return int(^uint(0) >> 1) } @@ -98,8 +83,8 @@ func canonicalSpellList(response extractionResponse, sourceID string) dnd.SpellL for refIndex, ref := range spell.SourceRefs { refs[refIndex] = source.SourceRef{ SourceID: sourceID, - StartUnitID: ref.StartUnitID.Int(), - EndUnitID: ref.EndUnitID.Int(), + StartUnitID: ref.StartUnitID, + EndUnitID: ref.EndUnitID, } } spellCasts[index] = dnd.SpellCast{ diff --git a/internal/modules/dnd/extract/spells/extractor_test.go b/internal/modules/dnd/extract/spells/extractor_test.go index 674ee00..40e8c9e 100644 --- a/internal/modules/dnd/extract/spells/extractor_test.go +++ b/internal/modules/dnd/extract/spells/extractor_test.go @@ -268,7 +268,7 @@ func TestExtractRejectsInvalidRequests(t *testing.T) { func TestExtractOrdersAndDeduplicatesEvidence(t *testing.T) { client := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{ {Caster: "Borin", Spell: "Fire Bolt", Effect: "Burns.", NarrativeDescription: "Second.", SourceRefs: responseSourceRefs(2, 2)}, - {Caster: "Aria", Spell: "Cure Wounds", Effect: "Heals.", NarrativeDescription: "First.", SourceRefs: []spellSourceRefResponse{{StartUnitID: shared.UnitRefFromInt(1), EndUnitID: shared.UnitRefFromInt(2)}, {StartUnitID: shared.UnitRefFromInt(1), EndUnitID: shared.UnitRefFromInt(2)}}}, + {Caster: "Aria", Spell: "Cure Wounds", Effect: "Heals.", NarrativeDescription: "First.", SourceRefs: []spellSourceRefResponse{{StartUnitID: 1, EndUnitID: 2}, {StartUnitID: 1, EndUnitID: 2}}}, {Caster: "Narrator", Spell: "Unknown", Effect: "Unknown.", NarrativeDescription: "Uncited."}, }}} result, err := newExtractor(t, client).Extract(context.Background(), extractionRequest()) @@ -286,7 +286,7 @@ func TestExtractOrdersAndDeduplicatesEvidence(t *testing.T) { func TestExtractPreservesInvalidEvidenceForValidators(t *testing.T) { client := &fakeSpellsLLMClient{response: extractionResponse{SpellCasts: []spellCastResponse{{ Caster: "Aria", Spell: "Cure Wounds", Effect: "Heals.", NarrativeDescription: "Aria heals.", - SourceRefs: []spellSourceRefResponse{{StartUnitID: shared.UnitRefFromInt(99), EndUnitID: shared.UnitRefFromString("missing")}}, + SourceRefs: []spellSourceRefResponse{{StartUnitID: 99, EndUnitID: 0}}, }}}} result, err := newExtractor(t, client).Extract(context.Background(), extractionRequest()) if err != nil { @@ -297,3 +297,18 @@ func TestExtractPreservesInvalidEvidenceForValidators(t *testing.T) { t.Fatalf("source ref = %#v, want canonical source with invalid range preserved", ref) } } + +func TestExtractMapsRawSemanticCandidatesWithoutRepair(t *testing.T) { + client := &fakeSpellsLLMClient{content: []byte(`{"spell_casts":[{"caster":"","spell":"Cure Wounds","effect":"","narrative_description":"","source_refs":[{"start_unit_id":0,"end_unit_id":-1}]}]}`)} + result, err := newExtractor(t, client).Extract(context.Background(), extractionRequest()) + if err != nil { + t.Fatalf("Extract() error = %v, want nil", err) + } + spell := result.Value.SpellCasts[0] + if spell.Caster != "" || spell.Effect != "" || spell.NarrativeDescription != "" { + t.Fatalf("spell = %#v, want blank semantic values preserved", spell) + } + if refs := spell.SourceRefs; len(refs) != 1 || refs[0] != (source.SourceRef{SourceID: "session-alpha", StartUnitID: 0, EndUnitID: -1}) { + t.Fatalf("source refs = %#v, want raw nonpositive candidates preserved", refs) + } +} diff --git a/internal/modules/dnd/extract/spells/model.go b/internal/modules/dnd/extract/spells/model.go index 933c6bb..c765f3f 100644 --- a/internal/modules/dnd/extract/spells/model.go +++ b/internal/modules/dnd/extract/spells/model.go @@ -1,7 +1,5 @@ package spells -import "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" - type extractionResponse struct { SpellCasts []spellCastResponse `json:"spell_casts"` } @@ -15,6 +13,6 @@ type spellCastResponse struct { } type spellSourceRefResponse struct { - StartUnitID shared.UnitRef `json:"start_unit_id"` - EndUnitID shared.UnitRef `json:"end_unit_id"` + StartUnitID int `json:"start_unit_id"` + EndUnitID int `json:"end_unit_id"` } diff --git a/internal/modules/dnd/extract/spells/schema_test.go b/internal/modules/dnd/extract/spells/schema_test.go index 98589cc..fde7976 100644 --- a/internal/modules/dnd/extract/spells/schema_test.go +++ b/internal/modules/dnd/extract/spells/schema_test.go @@ -29,6 +29,66 @@ func TestLoadResponseSchemaUsesExtractorOwnedLLMSchema(t *testing.T) { if err := validateJSONSchema(validJSON, schema.JSONSchema); err != nil { t.Fatalf("valid private spells response rejected: %v", err) } + for _, test := range []struct { + name string + response map[string]any + valid bool + }{ + { + name: "semantic blanks and empty evidence", + response: map[string]any{"spell_casts": []any{map[string]any{ + "caster": "", "spell": "", "effect": "", "narrative_description": "", "source_refs": []any{}, + }}}, + valid: true, + }, + { + name: "nonpositive unit candidates", + response: map[string]any{"spell_casts": []any{map[string]any{ + "caster": "Aria", "spell": "Cure Wounds", "effect": "Heals.", "narrative_description": "Aria heals.", + "source_refs": []any{map[string]any{"start_unit_id": 0, "end_unit_id": -1}}, + }}}, + valid: true, + }, + { + name: "missing required field", + response: map[string]any{"spell_casts": []any{map[string]any{ + "spell": "Cure Wounds", "effect": "Heals.", "narrative_description": "Aria heals.", "source_refs": []any{}, + }}}, + }, + { + name: "unknown field", + response: map[string]any{"spell_casts": []any{map[string]any{ + "caster": "Aria", "spell": "Cure Wounds", "effect": "Heals.", "narrative_description": "Aria heals.", "source_refs": []any{}, "id": "assigned later", + }}}, + }, + { + name: "wrong field type", + response: map[string]any{"spell_casts": []any{map[string]any{ + "caster": 7, "spell": "Cure Wounds", "effect": "Heals.", "narrative_description": "Aria heals.", "source_refs": []any{}, + }}}, + }, + { + name: "noninteger source identifier", + response: map[string]any{"spell_casts": []any{map[string]any{ + "caster": "Aria", "spell": "Cure Wounds", "effect": "Heals.", "narrative_description": "Aria heals.", + "source_refs": []any{map[string]any{"start_unit_id": 1.5, "end_unit_id": 2}}, + }}}, + }, + } { + t.Run(test.name, func(t *testing.T) { + content, err := json.Marshal(test.response) + if err != nil { + t.Fatal(err) + } + err = validateJSONSchema(content, schema.JSONSchema) + if (err == nil) != test.valid { + t.Fatalf("validateJSONSchema() error = %v, want valid=%t", err, test.valid) + } + }) + } + if err := validateJSONSchema([]byte(`{"spell_casts":`), schema.JSONSchema); err == nil { + t.Fatal("validateJSONSchema() error = nil, want malformed JSON rejected") + } withCanonicalSourceID := validSpellsResponse() withCanonicalSourceID["spell_casts"].([]any)[0].(map[string]any)["source_refs"].([]any)[0].(map[string]any)["source_id"] = "session-alpha" diff --git a/internal/modules/dnd/extract/spells/test_helpers_test.go b/internal/modules/dnd/extract/spells/test_helpers_test.go index ae7b6c5..e3bb322 100644 --- a/internal/modules/dnd/extract/spells/test_helpers_test.go +++ b/internal/modules/dnd/extract/spells/test_helpers_test.go @@ -8,7 +8,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" spellcatalog "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/spells/catalog" ) @@ -76,8 +75,8 @@ func mustJSON(t *testing.T, value any) string { func responseSourceRefs(startUnitID int, endUnitID int) []spellSourceRefResponse { return []spellSourceRefResponse{ { - StartUnitID: shared.UnitRefFromInt(startUnitID), - EndUnitID: shared.UnitRefFromInt(endUnitID), + StartUnitID: startUnitID, + EndUnitID: endUnitID, }, } } @@ -151,9 +150,13 @@ func (client *fakeSpellsLLMClient) CompleteStructured(_ context.Context, req con if !ok { return contracts.StructuredCompletionResponse{}, errors.New("unexpected output target") } - *target = client.response content := append([]byte(nil), client.content...) - if len(content) == 0 { + if len(content) != 0 { + if err := json.Unmarshal(content, target); err != nil { + return contracts.StructuredCompletionResponse{}, err + } + } else { + *target = client.response var err error content, err = json.Marshal(client.response) if err != nil { diff --git a/internal/modules/integration/dnd_npcs_runner_test.go b/internal/modules/integration/dnd_npcs_runner_test.go index 7bbefa9..d989e12 100644 --- a/internal/modules/integration/dnd_npcs_runner_test.go +++ b/internal/modules/integration/dnd_npcs_runner_test.go @@ -18,6 +18,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" dndregister "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/register" npcshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/shape" + npcsourcerefs "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/source_refs" genericregister "gitea.maximumdirect.net/eric/notarius/internal/modules/generic/register" "gitea.maximumdirect.net/eric/notarius/internal/modules/seriatim/input/transcript" seriatimregister "gitea.maximumdirect.net/eric/notarius/internal/modules/seriatim/register" @@ -131,7 +132,7 @@ func TestRunnerProcessesSeriatimInputWithProductionDNDNPCPipeline(t *testing.T) } } -func TestProductionNPCPipelineAttributesInvalidShapeBeforeSchemaValidation(t *testing.T) { +func TestProductionNPCPipelineRoutesSemanticCandidatesToDeterministicValidators(t *testing.T) { registries := productionNPCRegistries(t) effective, err := loadNPCPipelineConfig(t).Resolve(config.ResolveInput{ PipelineID: "dnd-npcs-fixture", @@ -140,23 +141,51 @@ func TestProductionNPCPipelineAttributesInvalidShapeBeforeSchemaValidation(t *te if err != nil { t.Fatalf("Resolve() error = %v, want nil", err) } - client := &fakeNPCProductionLLMClient{response: npcProductionResponse{NPCs: []npcProductionRecord{{ - Name: "", - Aliases: []string{}, - Description: "A participant.", - Relationships: []npcProductionRelationship{}, - SourceRefs: []npcProductionSourceRef{{StartUnitID: 1, EndUnitID: 1}}, - }}}} - output, err := runPreparedPipeline(t, registries, effective.ResolvedPipeline, client, pipeline.RunInput{RawInput: readNPCFixture(t)}) - if err != nil { - t.Fatalf("Run() error = %v, want non-fatal rejected output", err) - } - if len(client.requests) != 3 || len(output.Rejected) != 1 || len(output.NormalizeOutputs) != 0 { - t.Fatalf("LLM requests = %d rejected = %#v normalized = %#v, want exhausted shape rejection", len(client.requests), output.Rejected, output.NormalizeOutputs) - } - rejection := output.Rejected[0] - if rejection.ReasonCode != npcshape.ReasonCode || rejection.ValidatorName != npcshape.Key || rejection.AttemptCount != 3 { - t.Fatalf("rejection = %#v, want exhausted NPC shape rejection", rejection) + for _, test := range []struct { + name string + response []byte + reasonCode string + validatorName string + }{ + { + name: "blank string", + response: []byte(`{"npcs":[{"name":"","aliases":[],"description":"A participant.","relationships":[],"source_refs":[{"start_unit_id":1,"end_unit_id":1}]}]}`), + reasonCode: npcshape.ReasonCode, + validatorName: npcshape.Key, + }, + { + name: "empty evidence", + response: []byte(`{"npcs":[{"name":"Mira Thorn","aliases":[],"description":"A participant.","relationships":[],"source_refs":[]}]}`), + reasonCode: npcshape.ReasonCode, + validatorName: npcshape.Key, + }, + { + name: "nonpositive unit candidate", + response: []byte(`{"npcs":[{"name":"Mira Thorn","aliases":[],"description":"A participant.","relationships":[],"source_refs":[{"start_unit_id":0,"end_unit_id":1}]}]}`), + reasonCode: npcsourcerefs.ReasonCode, + validatorName: npcsourcerefs.Key, + }, + { + name: "unknown unit candidate", + response: []byte(`{"npcs":[{"name":"Mira Thorn","aliases":[],"description":"A participant.","relationships":[],"source_refs":[{"start_unit_id":99,"end_unit_id":99}]}]}`), + reasonCode: npcsourcerefs.ReasonCode, + validatorName: npcsourcerefs.Key, + }, + } { + t.Run(test.name, func(t *testing.T) { + client := &fakeNPCProductionLLMClient{rawResponses: [][]byte{test.response, test.response, test.response}} + output, err := runPreparedPipeline(t, registries, effective.ResolvedPipeline, client, pipeline.RunInput{RawInput: readNPCFixture(t)}) + if err != nil { + t.Fatalf("Run() error = %v, want non-fatal rejected output", err) + } + if len(client.requests) != 3 || len(output.Rejected) != 1 || len(output.NormalizeOutputs) != 0 { + t.Fatalf("LLM requests = %d rejected = %#v normalized = %#v, want exhausted rejection", len(client.requests), output.Rejected, output.NormalizeOutputs) + } + rejection := output.Rejected[0] + if rejection.ReasonCode != test.reasonCode || rejection.ValidatorName != test.validatorName || rejection.AttemptCount != 3 { + t.Fatalf("rejection = %#v, want exhausted %s rejection", rejection, test.validatorName) + } + }) } } @@ -183,15 +212,26 @@ type npcProductionSourceRef struct { } type fakeNPCProductionLLMClient struct { - response npcProductionResponse - requests []contracts.StructuredCompletionRequest + response npcProductionResponse + rawResponses [][]byte + requests []contracts.StructuredCompletionRequest } func (client *fakeNPCProductionLLMClient) CompleteStructured(_ context.Context, req contracts.StructuredCompletionRequest, out any) (contracts.StructuredCompletionResponse, error) { client.requests = append(client.requests, req) - content, err := json.Marshal(client.response) - if err != nil { - return contracts.StructuredCompletionResponse{}, err + var content []byte + if client.rawResponses != nil { + index := len(client.requests) - 1 + if index >= len(client.rawResponses) { + return contracts.StructuredCompletionResponse{}, fmt.Errorf("missing fake NPC response %d", index) + } + content = append([]byte(nil), client.rawResponses[index]...) + } else { + var err error + content, err = json.Marshal(client.response) + if err != nil { + return contracts.StructuredCompletionResponse{}, err + } } if err := json.Unmarshal(content, out); err != nil { return contracts.StructuredCompletionResponse{}, fmt.Errorf("populate NPC structured target: %w", err) diff --git a/internal/modules/integration/dnd_spells_helpers_test.go b/internal/modules/integration/dnd_spells_helpers_test.go index ffabc09..4e0ee31 100644 --- a/internal/modules/integration/dnd_spells_helpers_test.go +++ b/internal/modules/integration/dnd_spells_helpers_test.go @@ -22,24 +22,35 @@ type spellCastResponse struct { } type fakeSpellsLLMClient struct { - response extractionResponse - responses []extractionResponse - requests []contracts.StructuredCompletionRequest + response extractionResponse + responses []extractionResponse + rawResponses [][]byte + requests []contracts.StructuredCompletionRequest } func (client *fakeSpellsLLMClient) CompleteStructured(_ context.Context, req contracts.StructuredCompletionRequest, out any) (contracts.StructuredCompletionResponse, error) { client.requests = append(client.requests, cloneStructuredCompletionRequest(req)) - response := client.response - if client.responses != nil { + var content []byte + if client.rawResponses != nil { index := len(client.requests) - 1 - if index >= len(client.responses) { + if index >= len(client.rawResponses) { return contracts.StructuredCompletionResponse{}, fmt.Errorf("missing fake response %d", index) } - response = client.responses[index] - } - content, err := json.Marshal(response) - if err != nil { - return contracts.StructuredCompletionResponse{}, err + content = append([]byte(nil), client.rawResponses[index]...) + } else { + response := client.response + if client.responses != nil { + index := len(client.requests) - 1 + if index >= len(client.responses) { + return contracts.StructuredCompletionResponse{}, fmt.Errorf("missing fake response %d", index) + } + response = client.responses[index] + } + var err error + content, err = json.Marshal(response) + if err != nil { + return contracts.StructuredCompletionResponse{}, err + } } if err := json.Unmarshal(content, out); err != nil { return contracts.StructuredCompletionResponse{}, fmt.Errorf("populate structured target: %w", err) diff --git a/internal/modules/integration/dnd_spells_runner_test.go b/internal/modules/integration/dnd_spells_runner_test.go index 1770a6a..8e1936e 100644 --- a/internal/modules/integration/dnd_spells_runner_test.go +++ b/internal/modules/integration/dnd_spells_runner_test.go @@ -14,6 +14,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/extract/spells" spellnormalize "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/normalize/spells" spellshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/spells/shape" + spellsourcerefs "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/spells/source_refs" "gitea.maximumdirect.net/eric/notarius/internal/modules/seriatim/input/transcript" ) @@ -166,7 +167,7 @@ func TestRunnerPassesPartyAndGlossaryReferencesToDNDSpellsPrompt(t *testing.T) { } } -func TestProductionSpellPipelineAttributesInvalidShapeBeforeSchemaValidation(t *testing.T) { +func TestProductionSpellPipelineRoutesSemanticCandidatesToDeterministicValidators(t *testing.T) { registries := productionNPCRegistries(t) configValue := config.Default() configValue.Pipelines["dnd-spells-shape"] = pipeline.PipelineProfile{ @@ -186,24 +187,51 @@ func TestProductionSpellPipelineAttributesInvalidShapeBeforeSchemaValidation(t * if err != nil { t.Fatalf("Resolve() error = %v, want nil", err) } - response := extractionResponse{SpellCasts: []spellCastResponse{{ - Caster: "Aria", - Spell: "Cure Wounds", - Effect: "", - NarrativeDescription: "Aria casts the spell.", - SourceRefs: responseSourceRefs("spell-session", 1, 1), - }}} - client := &fakeSpellsLLMClient{responses: []extractionResponse{response, response, response}} - output, err := runPreparedPipeline(t, registries, effective.ResolvedPipeline, client, pipeline.RunInput{RawInput: readDNDSpellsFixture(t)}) - if err != nil { - t.Fatalf("Run() error = %v, want non-fatal rejected output", err) - } - if len(client.requests) != 3 || len(output.Rejected) != 1 || len(output.NormalizeOutputs) != 0 { - t.Fatalf("LLM requests = %d rejected = %#v normalized = %#v, want exhausted shape rejection", len(client.requests), output.Rejected, output.NormalizeOutputs) - } - rejection := output.Rejected[0] - if rejection.ReasonCode != spellshape.ReasonCode || rejection.ValidatorName != spellshape.Key || rejection.AttemptCount != 3 { - t.Fatalf("rejection = %#v, want exhausted spell shape rejection", rejection) + for _, test := range []struct { + name string + response []byte + reasonCode string + validatorName string + }{ + { + name: "blank string", + response: []byte(`{"spell_casts":[{"caster":"Aria","spell":"Cure Wounds","effect":"","narrative_description":"Aria casts the spell.","source_refs":[{"start_unit_id":1,"end_unit_id":1}]}]}`), + reasonCode: spellshape.ReasonCode, + validatorName: spellshape.Key, + }, + { + name: "empty evidence", + response: []byte(`{"spell_casts":[{"caster":"Aria","spell":"Cure Wounds","effect":"Heals.","narrative_description":"Aria casts the spell.","source_refs":[]}]}`), + reasonCode: spellshape.ReasonCode, + validatorName: spellshape.Key, + }, + { + name: "nonpositive unit candidate", + response: []byte(`{"spell_casts":[{"caster":"Aria","spell":"Cure Wounds","effect":"Heals.","narrative_description":"Aria casts the spell.","source_refs":[{"start_unit_id":0,"end_unit_id":1}]}]}`), + reasonCode: spellsourcerefs.ReasonCode, + validatorName: spellsourcerefs.Key, + }, + { + name: "unknown unit candidate", + response: []byte(`{"spell_casts":[{"caster":"Aria","spell":"Cure Wounds","effect":"Heals.","narrative_description":"Aria casts the spell.","source_refs":[{"start_unit_id":99,"end_unit_id":99}]}]}`), + reasonCode: spellsourcerefs.ReasonCode, + validatorName: spellsourcerefs.Key, + }, + } { + t.Run(test.name, func(t *testing.T) { + client := &fakeSpellsLLMClient{rawResponses: [][]byte{test.response, test.response, test.response}} + output, err := runPreparedPipeline(t, registries, effective.ResolvedPipeline, client, pipeline.RunInput{RawInput: readDNDSpellsFixture(t)}) + if err != nil { + t.Fatalf("Run() error = %v, want non-fatal rejected output", err) + } + if len(client.requests) != 3 || len(output.Rejected) != 1 || len(output.NormalizeOutputs) != 0 { + t.Fatalf("LLM requests = %d rejected = %#v normalized = %#v, want exhausted rejection", len(client.requests), output.Rejected, output.NormalizeOutputs) + } + rejection := output.Rejected[0] + if rejection.ReasonCode != test.reasonCode || rejection.ValidatorName != test.validatorName || rejection.AttemptCount != 3 { + t.Fatalf("rejection = %#v, want exhausted %s rejection", rejection, test.validatorName) + } + }) } }