Ground item occurrences by canonical names
This commit is contained in:
@@ -22,9 +22,11 @@ func canonicalizeResponse(response *extractionResponse, order shared.SourceRefOr
|
||||
}
|
||||
ordered := make([]orderedItemOccurrenceResponse, len(response.Occurrences))
|
||||
for index := range response.Occurrences {
|
||||
if err := validateRegistryPair(index, response.Occurrences[index], registry); err != nil {
|
||||
return err
|
||||
canonical, found := registry.Lookup(response.Occurrences[index].Name)
|
||||
if !found {
|
||||
return fmt.Errorf("occurrences[%d].name is not in the item registry", index)
|
||||
}
|
||||
response.Occurrences[index].Name = canonical.Name
|
||||
earliest, hasEvidence := canonicalizeItemOccurrence(&response.Occurrences[index], order, sourceID)
|
||||
ordered[index] = orderedItemOccurrenceResponse{
|
||||
value: response.Occurrences[index],
|
||||
@@ -56,26 +58,19 @@ func canonicalizeItemOccurrence(occurrence *itemOccurrenceResponse, order shared
|
||||
return order.EarliestValid(refs)
|
||||
}
|
||||
|
||||
func validateRegistryPair(index int, occurrence itemOccurrenceResponse, registry *itemregistry.Registry) error {
|
||||
item, found := registry.LookupID(occurrence.ItemID)
|
||||
if !found {
|
||||
return fmt.Errorf("occurrences[%d].item_id is not in the item registry", index)
|
||||
}
|
||||
if item.Name != occurrence.Name {
|
||||
return fmt.Errorf("occurrences[%d].name does not match item_id", index)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func canonicalItemOccurrenceList(response extractionResponse, sourceID string) dnd.ItemOccurrenceList {
|
||||
func canonicalItemOccurrenceList(response extractionResponse, sourceID string, registry *itemregistry.Registry) (dnd.ItemOccurrenceList, error) {
|
||||
if response.Occurrences == nil {
|
||||
return dnd.ItemOccurrenceList{}
|
||||
return dnd.ItemOccurrenceList{}, nil
|
||||
}
|
||||
occurrences := make([]dnd.ItemOccurrence, 0, len(response.Occurrences))
|
||||
for _, occurrence := range response.Occurrences {
|
||||
for index, occurrence := range response.Occurrences {
|
||||
item, found := registry.Lookup(occurrence.Name)
|
||||
if !found {
|
||||
return dnd.ItemOccurrenceList{}, fmt.Errorf("occurrences[%d].name is not in the item registry", index)
|
||||
}
|
||||
occurrences = append(occurrences, dnd.ItemOccurrence{
|
||||
ItemID: occurrence.ItemID,
|
||||
Name: occurrence.Name,
|
||||
ItemID: item.ID,
|
||||
Name: item.Name,
|
||||
Kind: dnd.ItemOccurrenceKind(occurrence.Kind),
|
||||
Quantity: cloneQuantity(occurrence.Quantity),
|
||||
From: occurrence.From,
|
||||
@@ -83,7 +78,7 @@ func canonicalItemOccurrenceList(response extractionResponse, sourceID string) d
|
||||
SourceRefs: itemOccurrenceSourceRefs(occurrence.SourceRefs, sourceID),
|
||||
})
|
||||
}
|
||||
return dnd.ItemOccurrenceList{Occurrences: occurrences}
|
||||
return dnd.ItemOccurrenceList{Occurrences: occurrences}, nil
|
||||
}
|
||||
|
||||
func itemOccurrenceSourceRefs(refs []itemOccurrenceSourceRefResponse, sourceID string) []source.SourceRef {
|
||||
|
||||
@@ -20,7 +20,7 @@ const (
|
||||
ItemRegistryMaxBytes = itemregistry.MaxBytes
|
||||
)
|
||||
|
||||
const mappingPolicy = "dnd.item_occurrences.extract_mapping.v1"
|
||||
const mappingPolicy = "dnd.item_occurrences.extract_mapping.v2"
|
||||
|
||||
var requiredCapabilities = []string{
|
||||
"chunks",
|
||||
@@ -164,7 +164,11 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe
|
||||
if err := canonicalizeResponse(&response, order, req.Source.ID, registry); err != nil {
|
||||
return contracts.TypedExtractionResult[dnd.ItemOccurrenceList]{}, extractorErrorf("map item occurrence response: %w", err)
|
||||
}
|
||||
return contracts.TypedExtractionResult[dnd.ItemOccurrenceList]{Value: canonicalItemOccurrenceList(response, req.Source.ID)}, nil
|
||||
value, err := canonicalItemOccurrenceList(response, req.Source.ID, registry)
|
||||
if err != nil {
|
||||
return contracts.TypedExtractionResult[dnd.ItemOccurrenceList]{}, extractorErrorf("resolve canonical item occurrence names: %w", err)
|
||||
}
|
||||
return contracts.TypedExtractionResult[dnd.ItemOccurrenceList]{Value: value}, nil
|
||||
}
|
||||
|
||||
func ModuleSpec() pipeline.ModuleSpec {
|
||||
|
||||
@@ -12,7 +12,7 @@ import (
|
||||
func TestExtractGroundsOccurrencesInRequiredRegistry(t *testing.T) {
|
||||
id := itemidentity.DeriveID("Torch")
|
||||
client := &fakeItemOccurrencesLLMClient{response: extractionResponse{Occurrences: []itemOccurrenceResponse{
|
||||
{ItemID: id, Name: "Torch", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)},
|
||||
{Name: "Torch", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)},
|
||||
}}}
|
||||
req := extractionRequest()
|
||||
req.References = itemRegistryReferences(t)
|
||||
@@ -24,55 +24,38 @@ func TestExtractGroundsOccurrencesInRequiredRegistry(t *testing.T) {
|
||||
t.Fatalf("occurrences = %#v", result.Value.Occurrences)
|
||||
}
|
||||
input := client.requests[0].Inputs[ItemRegistryReferenceSlot]
|
||||
if input.Name != ItemRegistryReferenceSlot || string(input.Content) == "" {
|
||||
t.Fatalf("registry prompt input = %#v", input)
|
||||
if input.Name != ItemRegistryReferenceSlot || string(input.Content) != `{"items":[{"name":"Torch"}]}` || strings.Contains(string(input.Content), "item:sha256:") {
|
||||
t.Fatalf("registry prompt input = %#v, want names-only projection", input)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractRejectsInvalidRegistryPairs(t *testing.T) {
|
||||
id := itemidentity.DeriveID("Torch")
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
occurrence itemOccurrenceResponse
|
||||
wantError string
|
||||
}{
|
||||
{
|
||||
name: "unknown item ID",
|
||||
occurrence: itemOccurrenceResponse{ItemID: "unknown", Name: "Torch", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)},
|
||||
wantError: "occurrences[0].item_id is not in the item registry",
|
||||
},
|
||||
{
|
||||
name: "mismatched item name",
|
||||
occurrence: itemOccurrenceResponse{ItemID: id, Name: "Lantern", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)},
|
||||
wantError: "occurrences[0].name does not match item_id",
|
||||
},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
client := &fakeItemOccurrencesLLMClient{response: extractionResponse{Occurrences: []itemOccurrenceResponse{test.occurrence}}}
|
||||
req := extractionRequest()
|
||||
req.References = itemRegistryReferences(t)
|
||||
result, err := newExtractor(t, client, req.References).Extract(context.Background(), req)
|
||||
if err == nil || !strings.Contains(err.Error(), test.wantError) {
|
||||
t.Fatalf("Extract() error = %v, want %q", err, test.wantError)
|
||||
}
|
||||
if len(result.Value.Occurrences) != 0 {
|
||||
t.Fatalf("Extract() returned partial occurrences: %#v", result.Value.Occurrences)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractRejectsResponseWithInvalidRegistryPairAfterValidOccurrence(t *testing.T) {
|
||||
func TestExtractCanonicalizesComparisonEquivalentNames(t *testing.T) {
|
||||
id := itemidentity.DeriveID("Torch")
|
||||
client := &fakeItemOccurrencesLLMClient{response: extractionResponse{Occurrences: []itemOccurrenceResponse{
|
||||
{ItemID: id, Name: "Torch", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)},
|
||||
{ItemID: "unknown", Name: "Unknown", Kind: "lost", From: "party", SourceRefs: responseRefs(2, 2)},
|
||||
{Name: " tORCH ", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)},
|
||||
}}}
|
||||
req := extractionRequest()
|
||||
req.References = itemRegistryReferences(t)
|
||||
result, err := newExtractor(t, client, req.References).Extract(context.Background(), req)
|
||||
if err == nil || !strings.Contains(err.Error(), "occurrences[1].item_id") {
|
||||
t.Fatalf("Extract() error = %v, want occurrence index and item ID", err)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
occurrence := result.Value.Occurrences[0]
|
||||
if occurrence.ItemID != id || occurrence.Name != "Torch" {
|
||||
t.Fatalf("canonical occurrence = %#v", occurrence)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractRejectsUnknownNameAfterValidOccurrence(t *testing.T) {
|
||||
client := &fakeItemOccurrencesLLMClient{response: extractionResponse{Occurrences: []itemOccurrenceResponse{
|
||||
{Name: "Torch", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)},
|
||||
{Name: "Unknown", Kind: "lost", From: "party", SourceRefs: responseRefs(2, 2)},
|
||||
}}}
|
||||
req := extractionRequest()
|
||||
req.References = itemRegistryReferences(t)
|
||||
result, err := newExtractor(t, client, req.References).Extract(context.Background(), req)
|
||||
if err == nil || !strings.Contains(err.Error(), "occurrences[1].name is not in the item registry") {
|
||||
t.Fatalf("Extract() error = %v, want occurrence index and unknown name", err)
|
||||
}
|
||||
if len(result.Value.Occurrences) != 0 {
|
||||
t.Fatalf("Extract() returned partial occurrences: %#v", result.Value.Occurrences)
|
||||
@@ -86,9 +69,36 @@ func TestExtractRequiresItemRegistry(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractAcceptsOnlyEmptyResponseForEmptyRegistry(t *testing.T) {
|
||||
references := emptyItemRegistryReferences(t)
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
response extractionResponse
|
||||
wantErr bool
|
||||
}{
|
||||
{name: "empty", response: extractionResponse{Occurrences: []itemOccurrenceResponse{}}},
|
||||
{name: "selection", response: extractionResponse{Occurrences: []itemOccurrenceResponse{{Name: "Torch", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)}}}, wantErr: true},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
client := &fakeItemOccurrencesLLMClient{response: test.response}
|
||||
req := extractionRequest()
|
||||
req.References = references
|
||||
result, err := newExtractor(t, client, references).Extract(context.Background(), req)
|
||||
if test.wantErr {
|
||||
if err == nil || !strings.Contains(err.Error(), "name is not in the item registry") || len(result.Value.Occurrences) != 0 {
|
||||
t.Fatalf("Extract() = %#v, %v; want no accepted occurrences", result, err)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil || result.Value.Occurrences == nil || len(result.Value.Occurrences) != 0 {
|
||||
t.Fatalf("Extract() = %#v, %v; want present empty occurrences", result, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractPreservesNullableFields(t *testing.T) {
|
||||
id := itemidentity.DeriveID("Torch")
|
||||
client := &fakeItemOccurrencesLLMClient{content: []byte(`{"occurrences":[{"item_id":"` + id + `","name":"Torch","kind":"discovered","quantity":null,"from":null,"to":null,"source_refs":[{"start_segment":1,"end_segment":1}]}]}`)}
|
||||
client := &fakeItemOccurrencesLLMClient{content: []byte(`{"occurrences":[{"name":"Torch","kind":"discovered","quantity":null,"from":null,"to":null,"source_refs":[{"start_segment":1,"end_segment":1}]}]}`)}
|
||||
req := extractionRequest()
|
||||
req.References = itemRegistryReferences(t)
|
||||
result, err := newExtractor(t, client, req.References).Extract(context.Background(), req)
|
||||
|
||||
@@ -5,7 +5,6 @@ type extractionResponse struct {
|
||||
}
|
||||
|
||||
type itemOccurrenceResponse struct {
|
||||
ItemID string `json:"item_id"`
|
||||
Name string `json:"name"`
|
||||
Kind string `json:"kind"`
|
||||
Quantity *int `json:"quantity,omitempty"`
|
||||
|
||||
@@ -34,7 +34,7 @@ func TestPromptAssetsPrepareItemOccurrencePrompt(t *testing.T) {
|
||||
"players": promptkit.Inline("item-occurrence-player"),
|
||||
"party": promptkit.Inline(" "),
|
||||
"glossary": promptkit.Inline(" "),
|
||||
"item_registry": promptkit.Inline(`{"items":[{"id":"item:sha256:test","name":"Torch"}]}`),
|
||||
"item_registry": promptkit.Inline(`{"items":[{"name":"Torch"}]}`),
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
|
||||
@@ -21,6 +21,15 @@ func itemRegistryReferences(t *testing.T) contracts.ReferenceSet {
|
||||
return contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{ItemRegistryReferenceSlot: {Slot: contracts.ReferenceSlot{Name: ItemRegistryReferenceSlot}, Items: []contracts.ReferenceItem{{SlotName: ItemRegistryReferenceSlot, Content: content, MediaType: "application/json", ArtifactKind: dnd.ItemRegistryKind}}}}}
|
||||
}
|
||||
|
||||
func emptyItemRegistryReferences(t *testing.T) contracts.ReferenceSet {
|
||||
t.Helper()
|
||||
content, err := itemcodec.New().Encode(dnd.ItemRegistry{Items: []dnd.Item{}})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{ItemRegistryReferenceSlot: {Slot: contracts.ReferenceSlot{Name: ItemRegistryReferenceSlot}, Items: []contracts.ReferenceItem{{SlotName: ItemRegistryReferenceSlot, Content: content, MediaType: "application/json", ArtifactKind: dnd.ItemRegistryKind}}}}}
|
||||
}
|
||||
|
||||
func testSourceRefs() []source.SourceRef {
|
||||
return []source.SourceRef{{SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 1}}
|
||||
}
|
||||
|
||||
@@ -19,11 +19,11 @@ func TestResponseSchemaIsStrictlyStructuralAndPrivate(t *testing.T) {
|
||||
}
|
||||
valid := map[string]any{"occurrences": []any{
|
||||
map[string]any{
|
||||
"item_id": "item:sha256:test", "name": "", "kind": "unsupported", "quantity": 0, "from": "party", "to": "Party",
|
||||
"name": "", "kind": "unsupported", "quantity": 0, "from": "party", "to": "Party",
|
||||
"source_refs": []any{map[string]any{"start_segment": 0, "end_segment": -1}},
|
||||
},
|
||||
map[string]any{
|
||||
"item_id": "item:sha256:test", "name": "Hidden Cache", "kind": "discovered", "quantity": nil, "from": nil, "to": nil,
|
||||
"name": "Hidden Cache", "kind": "discovered", "quantity": nil, "from": nil, "to": nil,
|
||||
"source_refs": []any{map[string]any{"start_segment": 1, "end_segment": 1}},
|
||||
},
|
||||
}}
|
||||
@@ -41,6 +41,7 @@ func TestResponseSchemaIsStrictlyStructuralAndPrivate(t *testing.T) {
|
||||
{"missing occurrences", map[string]any{}},
|
||||
{"missing occurrence name", map[string]any{"occurrences": []any{withoutField(responseOccurrence(), "name")}}},
|
||||
{"missing nullable field", map[string]any{"occurrences": []any{withoutField(responseOccurrence(), "quantity")}}},
|
||||
{"opaque item identifier", map[string]any{"occurrences": []any{withField(responseOccurrence(), "item_id", "item:sha256:opaque")}}},
|
||||
{"unknown occurrence field", map[string]any{"occurrences": []any{withField(responseOccurrence(), "extra", true)}}},
|
||||
{"unknown reference field", map[string]any{"occurrences": []any{withField(responseOccurrence(), "source_refs", []any{map[string]any{"start_segment": 1, "end_segment": 1, "extra": true}})}}},
|
||||
{"noninteger range", map[string]any{"occurrences": []any{withField(responseOccurrence(), "source_refs", []any{map[string]any{"start_segment": 1.5, "end_segment": 1}})}}},
|
||||
@@ -68,7 +69,7 @@ func TestResponseSchemaIsStrictlyStructuralAndPrivate(t *testing.T) {
|
||||
|
||||
func responseOccurrence() map[string]any {
|
||||
return map[string]any{
|
||||
"item_id": "item:sha256:test", "name": "Ring", "kind": "acquired", "quantity": nil, "from": nil, "to": "party", "source_refs": []any{},
|
||||
"name": "Ring", "kind": "acquired", "quantity": nil, "from": nil, "to": "party", "source_refs": []any{},
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user