Implement NPC extraction follow-up fixes

This commit is contained in:
2026-07-20 23:10:25 -05:00
parent 20cfbfd311
commit c6f330eb06
13 changed files with 133 additions and 757 deletions

View File

@@ -17,8 +17,8 @@ func canonicalizeResponse(response *extractionResponse, doc *source.SourceDocume
canonicalizeNPC(&response.NPCs[index])
}
sort.SliceStable(response.NPCs, func(i, j int) bool {
left, leftOK := earliestSourceUnit(doc, response.NPCs[i])
right, rightOK := earliestSourceUnit(doc, response.NPCs[j])
left, leftOK := earliestSourceIndex(doc, response.NPCs[i])
right, rightOK := earliestSourceIndex(doc, response.NPCs[j])
if leftOK != rightOK {
return leftOK
}
@@ -77,7 +77,9 @@ func sameSourceRef(left npcSourceRefResponse, right npcSourceRefResponse) bool {
left.EndUnitID.Int() == right.EndUnitID.Int()
}
func earliestSourceUnit(doc *source.SourceDocument, npc npcResponse) (int, bool) {
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()
@@ -87,10 +89,13 @@ func earliestSourceUnit(doc *source.SourceDocument, npc npcResponse) (int, bool)
if !startOK || !endOK || startIndex > endIndex {
continue
}
return start, true
if !found || startIndex < earliest {
earliest = startIndex
found = true
}
}
}
return 0, false
return earliest, found
}
func unitSortValue(ref shared.UnitRef) int {

View File

@@ -56,6 +56,38 @@ func TestExtractReturnsCanonicalNPCListFromPrivateResponse(t *testing.T) {
}
}
func TestExtractOrdersNPCsBySourcePositionRatherThanUnitID(t *testing.T) {
client := &fakeNPCsLLMClient{response: extractionResponse{NPCs: []npcResponse{
{
Name: "Later NPC", Aliases: []string{}, Description: "Appears later.", Relationships: []npcRelationshipResponse{},
SourceRefs: responseSourceRefs(10, 10),
},
{
Name: "Earlier NPC", Aliases: []string{}, Description: "Appears first.", Relationships: []npcRelationshipResponse{},
SourceRefs: []npcSourceRefResponse{
{StartUnitID: sharedUnitRef(50), EndUnitID: sharedUnitRef(50)},
{StartUnitID: sharedUnitRef(100), EndUnitID: sharedUnitRef(100)},
},
},
}}}
req := extractionRequest()
req.Source.Units = []source.SourceUnit{
{ID: 100, Kind: "transcript_segment", Text: "Earlier NPC appears."},
{ID: 10, Kind: "transcript_segment", Text: "Later NPC appears."},
{ID: 50, Kind: "transcript_segment", Text: "Earlier NPC appears again."},
}
req.Chunk.Units = append([]source.SourceUnit(nil), req.Source.Units...)
req.Chunk.Ref = source.SourceRef{SourceID: req.Source.ID, StartUnitID: 100, EndUnitID: 50}
result, err := newExtractor(t, client).Extract(context.Background(), req)
if err != nil {
t.Fatalf("Extract() error = %v, want nil", err)
}
if len(result.Value.NPCs) != 2 || result.Value.NPCs[0].Name != "Earlier NPC" || result.Value.NPCs[1].Name != "Later NPC" {
t.Fatalf("NPC order = %#v, want source-document order", result.Value.NPCs)
}
}
func TestExtractPassesCampaignReferencesAsPromptInputs(t *testing.T) {
client := &fakeNPCsLLMClient{response: extractionResponse{NPCs: []npcResponse{}}}
req := extractionRequest()

View File

@@ -9,6 +9,7 @@ import (
"gitea.maximumdirect.net/eric/notarius/internal/framework/contracts"
npccodec "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/codec/npcs"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/diagnostics"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity"
)
@@ -56,14 +57,14 @@ func resolveNPCRegistry(references contracts.ReferenceSet) (npcRegistryPromptInp
codec := npccodec.New()
value, err := codec.Decode(item.Content)
if err != nil {
return npcRegistryPromptInput{}, fmt.Errorf("decode NPC registry: %w", err)
return npcRegistryPromptInput{}, fmt.Errorf("decode NPC registry: invalid approved NPC JSON")
}
if issues := identity.ValidateList(value); len(issues) > 0 {
return npcRegistryPromptInput{}, fmt.Errorf("validate NPC registry identity: %s", formatNPCIdentityIssues(issues))
return npcRegistryPromptInput{}, fmt.Errorf("%s", formatNPCIdentityIssues(issues))
}
content, err := codec.Encode(value)
if err != nil {
return npcRegistryPromptInput{}, fmt.Errorf("encode canonical NPC registry: %w", err)
return npcRegistryPromptInput{}, fmt.Errorf("encode canonical NPC registry: approved NPC value could not be encoded")
}
digest := semanticNPCRegistryDigest(content)
@@ -83,7 +84,11 @@ func semanticNPCRegistryDigest(content []byte) string {
func formatNPCIdentityIssues(issues []identity.Issue) string {
parts := make([]string, len(issues))
for index, issue := range issues {
parts[index] = fmt.Sprintf("%s at record %d", issue.Code, issue.RecordIndex)
location := fmt.Sprintf("record %d", issue.RecordIndex)
if issue.AliasIndex >= 0 {
location += fmt.Sprintf(" alias %d", issue.AliasIndex)
}
parts[index] = fmt.Sprintf("%s at %s", issue.Code, location)
}
return strings.Join(parts, ", ")
return diagnostics.Aggregate("validate NPC registry identity", parts)
}

View File

@@ -4,14 +4,17 @@ import (
"bytes"
"context"
"encoding/json"
"fmt"
"strings"
"testing"
"unicode/utf8"
"gitea.maximumdirect.net/eric/notarius/internal/core/source"
"gitea.maximumdirect.net/eric/notarius/internal/framework/contracts"
"gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd"
npccodec "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/codec/npcs"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/diagnostics"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity"
)
@@ -90,12 +93,13 @@ func TestResolveNPCRegistryRejectsInvalidBoundaryValues(t *testing.T) {
name string
reference contracts.ReferenceSet
wantError string
forbidden []string
}{
{name: "zero items", reference: contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{NPCRegistryReferenceSlot: {Items: []contracts.ReferenceItem{}}}}, wantError: "exactly one"},
{name: "multiple", reference: contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{NPCRegistryReferenceSlot: {Items: []contracts.ReferenceItem{{Content: []byte(`{"npcs":[]}`)}, {Content: []byte(`{"npcs":[]}`)}}}}}, wantError: "exactly one"},
{name: "wrong media type", reference: npcRegistryReferenceWithMedia([]byte(`{"npcs":[]}`), "text/plain"), wantError: "must be application/json"},
{name: "malformed JSON", reference: npcRegistryReference([]byte(`{"npcs":[`), "file:///private.json"), wantError: "decode NPC registry"},
{name: "unknown field", reference: npcRegistryReference([]byte(`{"npcs":[],"unexpected":true}`), "file:///private.json"), wantError: "unknown field"},
{name: "malformed JSON", reference: npcRegistryReference([]byte(`{"npcs":[],"MALFORMED_REGISTRY_SECRET":`), "file:///private.json"), wantError: "invalid approved NPC JSON", forbidden: []string{"MALFORMED_REGISTRY_SECRET"}},
{name: "unknown field", reference: npcRegistryReference([]byte(`{"npcs":[],"UNKNOWN_FIELD_SECRET":true}`), "file:///private.json"), wantError: "invalid approved NPC JSON", forbidden: []string{"UNKNOWN_FIELD_SECRET"}},
{name: "invalid ID", reference: npcRegistryReference(marshalNPCRegistry(t, invalidID), "file:///private.json"), wantError: "decode NPC registry"},
{name: "alias collision", reference: npcRegistryReference(encodeNPCRegistry(t, valueWithAliasCollision), "file:///private.json"), wantError: string(identity.IssueAliasOwnershipCollision)},
{name: "byte limit", reference: npcRegistryReference(bytes.Repeat([]byte("x"), NPCRegistryMaxBytes+1), "file:///private.json"), wantError: "limit"},
@@ -106,13 +110,52 @@ func TestResolveNPCRegistryRejectsInvalidBoundaryValues(t *testing.T) {
if err == nil || !strings.Contains(err.Error(), test.wantError) {
t.Fatalf("resolveNPCRegistry() error = %v, want %q", err, test.wantError)
}
if strings.Contains(err.Error(), "Mira Thorn") || strings.Contains(err.Error(), "The Greencloak") || strings.Contains(err.Error(), "private.json") {
t.Fatalf("error leaked registry content or provenance: %v", err)
for _, forbidden := range append(test.forbidden, "Mira Thorn", "The Greencloak", "private.json") {
if strings.Contains(err.Error(), forbidden) {
t.Fatalf("error leaked registry content or provenance %q: %v", forbidden, err)
}
}
})
}
}
func TestResolveNPCRegistryBoundsIdentityDiagnosticsWithoutContent(t *testing.T) {
const recordCount = 30
value := dnd.NPCList{NPCs: make([]dnd.NPC, recordCount)}
for index := range value.NPCs {
value.NPCs[index] = dnd.NPC{
ID: "npc:sha256:0000000000000000000000000000000000000000000000000000000000000000",
Name: fmt.Sprintf("PRIVATE NPC %d", index),
Aliases: []string{"PRIVATE SHARED ALIAS"},
Description: "PRIVATE DESCRIPTION",
Relationships: []dnd.NPCRelationship{},
SourceRefs: []source.SourceRef{{SourceID: "private-source", StartUnitID: 1, EndUnitID: 1}},
}
}
issues := identity.ValidateList(value)
if len(issues) <= diagnostics.MaxIssues {
t.Fatalf("identity issues = %d, want more than display limit", len(issues))
}
_, err := resolveNPCRegistry(npcRegistryReference(marshalNPCRegistry(t, value), "file:///private-registry.json"))
if err == nil {
t.Fatal("resolveNPCRegistry() error = nil, want bounded identity rejection")
}
message := err.Error()
if !utf8.ValidString(message) || len([]byte(message)) > diagnostics.MaxMessageBytes {
t.Fatalf("identity error has invalid encoding or size: bytes=%d message=%q", len([]byte(message)), message)
}
wantOmitted := fmt.Sprintf("%d additional issue(s) omitted", len(issues)-diagnostics.MaxIssues)
if !strings.Contains(message, wantOmitted) {
t.Fatalf("identity error = %q, want %q", message, wantOmitted)
}
for _, forbidden := range []string{"PRIVATE NPC", "PRIVATE SHARED ALIAS", "PRIVATE DESCRIPTION", "private-source", "private-registry.json"} {
if strings.Contains(message, forbidden) {
t.Fatalf("identity error leaked %q: %s", forbidden, message)
}
}
}
func TestNPCRegistryFingerprintIsSemanticAndDefensive(t *testing.T) {
value := validNPCRegistryList()
canonical := encodeNPCRegistry(t, value)

View File

@@ -13,8 +13,8 @@ import (
"gitea.maximumdirect.net/eric/notarius/internal/framework/contracts"
"gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/diagnostics"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/diagnostics"
)
const (

View File

@@ -1,5 +1,5 @@
// Package diagnostics provides bounded, safe text for deterministic NPC
// validator decisions and warnings.
// decisions and warnings.
package diagnostics
import (

View File

@@ -0,0 +1,28 @@
package diagnostics
import (
"fmt"
"strings"
"testing"
"unicode/utf8"
)
func TestAggregateEnforcesByteBudgetAndReportsOmissions(t *testing.T) {
issues := make([]string, MaxIssues)
for index := range issues {
issues[index] = fmt.Sprintf("issue-%d-%s", index, strings.Repeat("火", MaxDisplayedRunes))
}
message := Aggregate("invalid NPC data", issues)
if !utf8.ValidString(message) || len([]byte(message)) > MaxMessageBytes {
t.Fatalf("Aggregate() returned invalid or oversized message: bytes=%d message=%q", len([]byte(message)), message)
}
displayed := strings.Count(message, "issue-")
if displayed == 0 || displayed >= len(issues) {
t.Fatalf("Aggregate() displayed %d issues, want byte-budget omission", displayed)
}
wantOmitted := fmt.Sprintf("%d additional issue(s) omitted", len(issues)-displayed)
if !strings.Contains(message, wantOmitted) {
t.Fatalf("Aggregate() = %q, want %q", message, wantOmitted)
}
}

View File

@@ -8,8 +8,8 @@ import (
"gitea.maximumdirect.net/eric/notarius/internal/framework/contracts"
"gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/diagnostics"
domainidentity "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/diagnostics"
npcshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/shape"
)

View File

@@ -8,7 +8,7 @@ import (
"gitea.maximumdirect.net/eric/notarius/internal/framework/contracts"
"gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/diagnostics"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/diagnostics"
)
const (

View File

@@ -8,7 +8,7 @@ import (
"gitea.maximumdirect.net/eric/notarius/internal/framework/contracts"
"gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/diagnostics"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/diagnostics"
npcshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/shape"
)

View File

@@ -9,8 +9,8 @@ import (
"gitea.maximumdirect.net/eric/notarius/internal/framework/contracts"
"gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/diagnostics"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity"
"gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/diagnostics"
npcshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcs/shape"
)