From 8e0b029f5f557f090dd12756d297695dd925594f Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 24 Jul 2026 14:33:34 +0000 Subject: [PATCH] Order combat and NPC interaction extraction by document position --- .../dnd/extract/combatturns/canonicalize.go | 80 +++++-------------- .../dnd/extract/combatturns/extractor.go | 5 +- .../dnd/extract/combatturns/extractor_test.go | 37 +++++++++ .../extract/npcinteractions/canonicalize.go | 76 +++++------------- .../dnd/extract/npcinteractions/extractor.go | 5 +- .../extract/npcinteractions/extractor_test.go | 39 +++++++++ internal/modules/dnd/shared/unit_refs.go | 12 --- internal/modules/dnd/shared/unit_refs_test.go | 22 ----- 8 files changed, 118 insertions(+), 158 deletions(-) diff --git a/internal/modules/dnd/extract/combatturns/canonicalize.go b/internal/modules/dnd/extract/combatturns/canonicalize.go index ec90982..754dc7a 100644 --- a/internal/modules/dnd/extract/combatturns/canonicalize.go +++ b/internal/modules/dnd/extract/combatturns/canonicalize.go @@ -5,18 +5,19 @@ 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, doc *source.SourceDocument) { +func canonicalizeResponse(response *extractionResponse, order shared.SourceRefOrder, sourceID string) { if response == nil { return } for index := range response.CombatTurns { - canonicalizeCombatTurn(&response.CombatTurns[index]) + canonicalizeCombatTurn(&response.CombatTurns[index], order, sourceID) } sort.SliceStable(response.CombatTurns, func(i, j int) bool { - left, leftOK := earliestSourcePosition(doc, response.CombatTurns[i]) - right, rightOK := earliestSourcePosition(doc, response.CombatTurns[j]) + left, leftOK := order.EarliestValid(canonicalSourceRefs(response.CombatTurns[i].SourceRefs, sourceID)) + right, rightOK := order.EarliestValid(canonicalSourceRefs(response.CombatTurns[j].SourceRefs, sourceID)) if leftOK != rightOK { return leftOK } @@ -27,67 +28,11 @@ func canonicalizeResponse(response *extractionResponse, doc *source.SourceDocume }) } -func canonicalizeCombatTurn(turn *combatTurnResponse) { +func canonicalizeCombatTurn(turn *combatTurnResponse, order shared.SourceRefOrder, sourceID string) { if turn == nil { return } - sort.SliceStable(turn.SourceRefs, func(i, j int) bool { - left := turn.SourceRefs[i] - right := turn.SourceRefs[j] - if unitSortValue(left.StartUnitID) != unitSortValue(right.StartUnitID) { - return unitSortValue(left.StartUnitID) < unitSortValue(right.StartUnitID) - } - return unitSortValue(left.EndUnitID) < unitSortValue(right.EndUnitID) - }) - turn.SourceRefs = dedupeSourceRefs(turn.SourceRefs) -} - -func dedupeSourceRefs(refs []combatSourceRefResponse) []combatSourceRefResponse { - if len(refs) < 2 { - return refs - } - out := refs[:0] - var previous combatSourceRefResponse - for index, ref := range refs { - if index > 0 && sameSourceRef(previous, ref) { - continue - } - out = append(out, ref) - previous = ref - } - return out -} - -func sameSourceRef(left combatSourceRefResponse, right combatSourceRefResponse) bool { - return left == right -} - -func earliestSourcePosition(doc *source.SourceDocument, turn combatTurnResponse) (int, bool) { - if doc == nil { - return 0, false - } - earliest := 0 - found := false - for _, ref := range turn.SourceRefs { - candidate := source.SourceRef{SourceID: doc.ID, StartUnitID: ref.StartUnitID, EndUnitID: ref.EndUnitID} - if err := source.ValidateRef(doc, candidate); err != nil { - continue - } - index, ok := source.UnitIndex(doc, candidate.StartUnitID) - if !ok || (found && index >= earliest) { - continue - } - earliest = index - found = true - } - return earliest, found -} - -func unitSortValue(value int) int { - if value <= 0 { - return int(^uint(0) >> 1) - } - return value + turn.SourceRefs = combatResponseRefs(order.Canonicalize(canonicalSourceRefs(turn.SourceRefs, sourceID))) } func canonicalCombatTurnList(response extractionResponse, sourceID string) dnd.CombatTurnList { @@ -115,3 +60,14 @@ func canonicalSourceRefs(refs []combatSourceRefResponse, sourceID string) []sour } return out } + +func combatResponseRefs(refs []source.SourceRef) []combatSourceRefResponse { + if refs == nil { + return nil + } + values := make([]combatSourceRefResponse, len(refs)) + for index, ref := range refs { + values[index] = combatSourceRefResponse{StartUnitID: ref.StartUnitID, EndUnitID: ref.EndUnitID} + } + return values +} diff --git a/internal/modules/dnd/extract/combatturns/extractor.go b/internal/modules/dnd/extract/combatturns/extractor.go index af99573..694e7f7 100644 --- a/internal/modules/dnd/extract/combatturns/extractor.go +++ b/internal/modules/dnd/extract/combatturns/extractor.go @@ -15,7 +15,7 @@ import ( const ( Key = "dnd/combat-turns" ArtifactType = "dnd.combat_turn" - mappingPolicy = "dnd.combat_turns.extract_mapping.v1" + mappingPolicy = "dnd.combat_turns.extract_mapping.v2" ) const ( @@ -163,6 +163,7 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe if err != nil { return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("%w", err) } + order := shared.NewSourceRefOrder(req.Source) npcRegistry, err := e.npcResolver.Resolve(req.References) if err != nil { return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("resolve NPC registry: %w", err) @@ -181,7 +182,7 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe }, &response); err != nil { return contracts.TypedExtractionResult[dnd.CombatTurnList]{}, extractorErrorf("complete structured output: %w", err) } - canonicalizeResponse(&response, req.Source) + canonicalizeResponse(&response, order, req.Source.ID) return contracts.TypedExtractionResult[dnd.CombatTurnList]{Value: canonicalCombatTurnList(response, req.Source.ID)}, nil } diff --git a/internal/modules/dnd/extract/combatturns/extractor_test.go b/internal/modules/dnd/extract/combatturns/extractor_test.go index d37ff80..b63de0e 100644 --- a/internal/modules/dnd/extract/combatturns/extractor_test.go +++ b/internal/modules/dnd/extract/combatturns/extractor_test.go @@ -82,6 +82,43 @@ func TestExtractPreservesInvalidCandidatesForValidators(t *testing.T) { } } +func TestExtractUsesDocumentOrderForReferencesAndTurns(t *testing.T) { + client := &fakeCombatTurnsLLMClient{response: extractionResponse{CombatTurns: []combatTurnResponse{ + {Actor: "Later", TurnKind: "turn", SourceRefs: []combatSourceRefResponse{{StartUnitID: 10, EndUnitID: 10}}}, + {Actor: "First", TurnKind: "reaction", SourceRefs: []combatSourceRefResponse{ + {StartUnitID: 10, EndUnitID: 10}, + {StartUnitID: 30, EndUnitID: 30}, + {StartUnitID: 30, EndUnitID: 30}, + {StartUnitID: 999, EndUnitID: 0}, + }}, + {Actor: "Second", TurnKind: "other", SourceRefs: []combatSourceRefResponse{{StartUnitID: 30, EndUnitID: 30}}}, + }}} + req := extractionRequest() + req.Source.Units = []source.SourceUnit{{ID: 30}, {ID: 10}} + req.Chunk.Units = append([]source.SourceUnit(nil), req.Source.Units...) + req.Chunk.Ref = source.SourceRef{SourceID: req.Source.ID, StartUnitID: 30, EndUnitID: 10} + + result, err := newExtractor(t, client).Extract(context.Background(), req) + if err != nil { + t.Fatalf("Extract() error = %v", err) + } + if got := []string{result.Value.CombatTurns[0].Actor, result.Value.CombatTurns[1].Actor, result.Value.CombatTurns[2].Actor}; !reflect.DeepEqual(got, []string{"First", "Second", "Later"}) { + t.Fatalf("turn order = %#v, want document chronology with stable equal-evidence ties", got) + } + refs := result.Value.CombatTurns[0].SourceRefs + if got := []int{refs[0].StartUnitID, refs[1].StartUnitID, refs[2].StartUnitID}; !reflect.DeepEqual(got, []int{30, 10, 999}) { + t.Fatalf("source refs = %#v, want document order with exact duplicate removed", refs) + } + refs[0].StartUnitID = 777 + for _, turn := range client.response.CombatTurns { + for _, ref := range turn.SourceRefs { + if ref.StartUnitID == 777 { + t.Fatal("result source references alias the model response") + } + } + } +} + func TestExtractPassesReferencesAndNPCGroundingWithoutUsingItAsEvidence(t *testing.T) { client := &fakeCombatTurnsLLMClient{response: extractionResponse{CombatTurns: []combatTurnResponse{}}} references := contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{ diff --git a/internal/modules/dnd/extract/npcinteractions/canonicalize.go b/internal/modules/dnd/extract/npcinteractions/canonicalize.go index d1ae7f1..d785f1b 100644 --- a/internal/modules/dnd/extract/npcinteractions/canonicalize.go +++ b/internal/modules/dnd/extract/npcinteractions/canonicalize.go @@ -5,18 +5,19 @@ 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, doc *source.SourceDocument) { +func canonicalizeResponse(response *extractionResponse, order shared.SourceRefOrder, sourceID string) { if response == nil { return } for index := range response.Interactions { - canonicalizeInteraction(&response.Interactions[index]) + canonicalizeInteraction(&response.Interactions[index], order, sourceID) } sort.SliceStable(response.Interactions, func(i, j int) bool { - left, leftOK := earliestSourcePosition(doc, response.Interactions[i]) - right, rightOK := earliestSourcePosition(doc, response.Interactions[j]) + left, leftOK := order.EarliestValid(canonicalSourceRefs(response.Interactions[i].SourceRefs, sourceID)) + right, rightOK := order.EarliestValid(canonicalSourceRefs(response.Interactions[j].SourceRefs, sourceID)) if leftOK != rightOK { return leftOK } @@ -27,63 +28,11 @@ func canonicalizeResponse(response *extractionResponse, doc *source.SourceDocume }) } -func canonicalizeInteraction(interaction *interactionResponse) { +func canonicalizeInteraction(interaction *interactionResponse, order shared.SourceRefOrder, sourceID string) { if interaction == nil { return } - sort.SliceStable(interaction.SourceRefs, func(i, j int) bool { - left := interaction.SourceRefs[i] - right := interaction.SourceRefs[j] - if unitSortValue(left.StartUnitID) != unitSortValue(right.StartUnitID) { - return unitSortValue(left.StartUnitID) < unitSortValue(right.StartUnitID) - } - return unitSortValue(left.EndUnitID) < unitSortValue(right.EndUnitID) - }) - interaction.SourceRefs = dedupeSourceRefs(interaction.SourceRefs) -} - -func dedupeSourceRefs(refs []interactionSourceRefResponse) []interactionSourceRefResponse { - if len(refs) < 2 { - return refs - } - out := refs[:0] - var previous interactionSourceRefResponse - for index, ref := range refs { - if index > 0 && previous == ref { - continue - } - out = append(out, ref) - previous = ref - } - return out -} - -func earliestSourcePosition(doc *source.SourceDocument, interaction interactionResponse) (int, bool) { - if doc == nil { - return 0, false - } - found := false - earliest := 0 - for _, ref := range interaction.SourceRefs { - candidate := source.SourceRef{SourceID: doc.ID, StartUnitID: ref.StartUnitID, EndUnitID: ref.EndUnitID} - if err := source.ValidateRef(doc, candidate); err != nil { - continue - } - index, ok := source.UnitIndex(doc, candidate.StartUnitID) - if !ok || (found && index >= earliest) { - continue - } - earliest = index - found = true - } - return earliest, found -} - -func unitSortValue(value int) int { - if value <= 0 { - return int(^uint(0) >> 1) - } - return value + interaction.SourceRefs = interactionResponseRefs(order.Canonicalize(canonicalSourceRefs(interaction.SourceRefs, sourceID))) } func canonicalInteractionList(response extractionResponse, sourceID string) dnd.NPCInteractionList { @@ -111,3 +60,14 @@ func canonicalSourceRefs(refs []interactionSourceRefResponse, sourceID string) [ } return out } + +func interactionResponseRefs(refs []source.SourceRef) []interactionSourceRefResponse { + if refs == nil { + return nil + } + values := make([]interactionSourceRefResponse, len(refs)) + for index, ref := range refs { + values[index] = interactionSourceRefResponse{StartUnitID: ref.StartUnitID, EndUnitID: ref.EndUnitID} + } + return values +} diff --git a/internal/modules/dnd/extract/npcinteractions/extractor.go b/internal/modules/dnd/extract/npcinteractions/extractor.go index 8bdcacd..a6e3f18 100644 --- a/internal/modules/dnd/extract/npcinteractions/extractor.go +++ b/internal/modules/dnd/extract/npcinteractions/extractor.go @@ -14,7 +14,7 @@ import ( const ( Key = "dnd/npc-interactions" - mappingPolicy = "dnd.npc_interactions.extract_mapping.v1" + mappingPolicy = "dnd.npc_interactions.extract_mapping.v2" ) const ( @@ -162,6 +162,7 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe if err != nil { return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("%w", err) } + order := shared.NewSourceRefOrder(req.Source) npcRegistry, err := e.npcResolver.Resolve(req.References) if err != nil { return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("resolve NPC registry: %w", err) @@ -183,7 +184,7 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe }, &response); err != nil { return contracts.TypedExtractionResult[dnd.NPCInteractionList]{}, extractorErrorf("complete structured output: %w", err) } - canonicalizeResponse(&response, req.Source) + canonicalizeResponse(&response, order, req.Source.ID) return contracts.TypedExtractionResult[dnd.NPCInteractionList]{Value: canonicalInteractionList(response, req.Source.ID)}, nil } diff --git a/internal/modules/dnd/extract/npcinteractions/extractor_test.go b/internal/modules/dnd/extract/npcinteractions/extractor_test.go index b06c2fc..8e40de1 100644 --- a/internal/modules/dnd/extract/npcinteractions/extractor_test.go +++ b/internal/modules/dnd/extract/npcinteractions/extractor_test.go @@ -59,6 +59,45 @@ func TestExtractMapsEveryKindAndOrdersBySourcePosition(t *testing.T) { } } +func TestExtractUsesDocumentOrderForReferencesAndInteractions(t *testing.T) { + client := &fakeInteractionsLLMClient{response: extractionResponse{Interactions: []interactionResponse{ + {Name: "Later", Kind: "dialogue", SourceRefs: interactionRefs(10, 10)}, + {Name: "First", Kind: "mentioned", SourceRefs: []interactionSourceRefResponse{ + {StartUnitID: 10, EndUnitID: 10}, + {StartUnitID: 30, EndUnitID: 30}, + {StartUnitID: 30, EndUnitID: 30}, + {StartUnitID: 999, EndUnitID: 0}, + }}, + {Name: "Second", Kind: "other", SourceRefs: interactionRefs(30, 30)}, + }}} + references := requiredRegistryReferences(t, "Later", "First", "Second") + req := extractionRequest() + req.References = references + req.Source.Units = []source.SourceUnit{{ID: 30}, {ID: 10}} + req.Chunk.Units = append([]source.SourceUnit(nil), req.Source.Units...) + req.Chunk.Ref = source.SourceRef{SourceID: req.Source.ID, StartUnitID: 30, EndUnitID: 10} + + result, err := newExtractor(t, client, references).Extract(context.Background(), req) + if err != nil { + t.Fatalf("Extract() error = %v", err) + } + if got := interactionNames(result.Value); !reflect.DeepEqual(got, []string{"First", "Second", "Later"}) { + t.Fatalf("interaction order = %#v, want document chronology with stable equal-evidence ties", got) + } + refs := result.Value.Interactions[0].SourceRefs + if got := []int{refs[0].StartUnitID, refs[1].StartUnitID, refs[2].StartUnitID}; !reflect.DeepEqual(got, []int{30, 10, 999}) { + t.Fatalf("source refs = %#v, want document order with exact duplicate removed", refs) + } + refs[0].StartUnitID = 777 + for _, interaction := range client.response.Interactions { + for _, ref := range interaction.SourceRefs { + if ref.StartUnitID == 777 { + t.Fatal("result source references alias the model response") + } + } + } +} + func TestNewRequiresLLMAndRejectsAmbiguousReferenceSets(t *testing.T) { if _, err := New(nil, Options{}); err == nil || !strings.Contains(err.Error(), "LLM client") { t.Fatalf("New(nil) error = %v", err) diff --git a/internal/modules/dnd/shared/unit_refs.go b/internal/modules/dnd/shared/unit_refs.go index 465046e..60e558a 100644 --- a/internal/modules/dnd/shared/unit_refs.go +++ b/internal/modules/dnd/shared/unit_refs.go @@ -87,18 +87,6 @@ func ResolveUnitID(doc *source.SourceDocument, field string, ref UnitRef) (int, return ref.value, nil } -func SourceRefCandidate(doc *source.SourceDocument, ref SourceRefResponse) source.SourceRef { - return source.SourceRef{ - SourceID: strings.TrimSpace(ref.SourceID), - StartUnitID: unitIDCandidate(ref.StartUnitID), - EndUnitID: unitIDCandidate(ref.EndUnitID), - } -} - -func unitIDCandidate(ref UnitRef) int { - return ref.value -} - func parseUnitRefNumber(value string) (int, error) { trimmed := strings.TrimSpace(value) if trimmed == "" { diff --git a/internal/modules/dnd/shared/unit_refs_test.go b/internal/modules/dnd/shared/unit_refs_test.go index 93276f2..08ffd5a 100644 --- a/internal/modules/dnd/shared/unit_refs_test.go +++ b/internal/modules/dnd/shared/unit_refs_test.go @@ -74,28 +74,6 @@ func TestResolveUnitIDRejectsMissingUnit(t *testing.T) { } } -func TestSourceRefCandidateCanonicalizesValidRefsAndPreservesInvalidRefs(t *testing.T) { - doc := unitRefSourceDocument(1, 2) - - valid := SourceRefCandidate(doc, SourceRefResponse{ - SourceID: " session-alpha ", - StartUnitID: UnitRefFromInt(1), - EndUnitID: UnitRefFromInt(2), - }) - if valid != (source.SourceRef{SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2}) { - t.Fatalf("valid candidate = %#v, want canonical source ref", valid) - } - - invalid := SourceRefCandidate(doc, SourceRefResponse{ - SourceID: "session-alpha", - StartUnitID: UnitRefFromInt(9), - EndUnitID: UnitRefFromString("missing"), - }) - if invalid != (source.SourceRef{SourceID: "session-alpha", StartUnitID: 9, EndUnitID: 0}) { - t.Fatalf("invalid candidate = %#v, want unresolved values for validator", invalid) - } -} - func unitRefSourceDocument(ids ...int) *source.SourceDocument { doc := &source.SourceDocument{ ID: "session-alpha",