From f4c05c34efd30f1410f6bb128045049dcae9250e Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 4 Aug 2026 19:52:47 +0000 Subject: [PATCH] Make item event responses compatible with strict schemas --- .../itemevents/assets/prompts/instructions.md | 15 ++-- .../schemas/dnd_item_events_llm.v1.json | 8 +- .../dnd/extract/itemevents/extractor_test.go | 15 ++++ .../dnd/extract/itemevents/schema_test.go | 39 +++++++-- .../dnd/register/schema_compatibility_test.go | 85 +++++++++++++++++++ 5 files changed, 143 insertions(+), 19 deletions(-) create mode 100644 internal/modules/dnd/register/schema_compatibility_test.go diff --git a/internal/modules/dnd/extract/itemevents/assets/prompts/instructions.md b/internal/modules/dnd/extract/itemevents/assets/prompts/instructions.md index a7cdcb1..2bbb421 100644 --- a/internal/modules/dnd/extract/itemevents/assets/prompts/instructions.md +++ b/internal/modules/dnd/extract/itemevents/assets/prompts/instructions.md @@ -1,7 +1,7 @@ Return one event only when the transcript establishes a meaningful item or currency occurrence. Use a concise observed item name and preserve the stated -currency denomination; include quantity only when the transcript explicitly -states it. +currency denomination. Set `quantity` to the explicitly stated integer, or to +`null` when the transcript does not state one. Use `discovered` when the party learns of or encounters an item without establishing possession. Use `acquired` when the party or a party member gains @@ -13,11 +13,12 @@ when the transcript explicitly describes it being physically destroyed or expended as a non-payment component. Use `transferred` only when possession moves between two distinct named party members. -For `discovered`, omit both holders. For `acquired`, provide only `to`; for -`lost` and `consumed`, provide only `from`; and for `transferred`, provide both -`from` and `to`. Use `party` only for collective or unresolved party possession, -never for either side of a transfer. Do not emit a transfer for a gift, sale, or -payment outside the party. +Return both `from` and `to` for every event, using `null` when a holder does not +apply. For `discovered`, set both holders to `null`. For `acquired`, set `from` +to `null` and provide `to`; for `lost` and `consumed`, provide `from` and set +`to` to `null`; and for `transferred`, provide both holders. Use `party` only +for collective or unresolved party possession, never for either side of a +transfer. Do not emit a transfer for a gift, sale, or payment outside the party. Ordinary non-depleting use is not an event. Do not infer acquisition from a discovery, or discovery from an acquisition: emit both only when each is diff --git a/internal/modules/dnd/extract/itemevents/assets/schemas/dnd_item_events_llm.v1.json b/internal/modules/dnd/extract/itemevents/assets/schemas/dnd_item_events_llm.v1.json index 6defd07..509d823 100644 --- a/internal/modules/dnd/extract/itemevents/assets/schemas/dnd_item_events_llm.v1.json +++ b/internal/modules/dnd/extract/itemevents/assets/schemas/dnd_item_events_llm.v1.json @@ -10,13 +10,13 @@ "items": { "type": "object", "additionalProperties": false, - "required": ["name", "kind", "source_refs"], + "required": ["name", "kind", "quantity", "from", "to", "source_refs"], "properties": { "name": {"type": "string"}, "kind": {"type": "string"}, - "quantity": {"type": "integer"}, - "from": {"type": "string"}, - "to": {"type": "string"}, + "quantity": {"type": ["integer", "null"]}, + "from": {"type": ["string", "null"]}, + "to": {"type": ["string", "null"]}, "source_refs": { "type": "array", "items": { diff --git a/internal/modules/dnd/extract/itemevents/extractor_test.go b/internal/modules/dnd/extract/itemevents/extractor_test.go index 8c2cb54..712a393 100644 --- a/internal/modules/dnd/extract/itemevents/extractor_test.go +++ b/internal/modules/dnd/extract/itemevents/extractor_test.go @@ -81,6 +81,21 @@ func TestExtractPreservesInvalidCandidatesAndEmptyResults(t *testing.T) { } } +func TestExtractMapsNullableResponseFieldsToAbsentArtifactFields(t *testing.T) { + client := &fakeItemEventsLLMClient{content: []byte(`{"events":[{"name":"Hidden Cache","kind":"discovered","quantity":null,"from":null,"to":null,"source_refs":[{"start_segment":1,"end_segment":1}]}]}`)} + result, err := newExtractor(t, client).Extract(context.Background(), extractionRequest()) + if err != nil { + t.Fatal(err) + } + if len(result.Value.Events) != 1 { + t.Fatalf("events = %#v, want one", result.Value.Events) + } + event := result.Value.Events[0] + if event.Quantity != nil || event.From != "" || event.To != "" { + t.Fatalf("nullable response fields mapped to artifact values: %#v", event) + } +} + func TestExtractUsesSourceDocumentOrderForCandidates(t *testing.T) { client := &fakeItemEventsLLMClient{response: extractionResponse{Events: []itemEventResponse{ {Name: "Later", Kind: "discovered", SourceRefs: responseRefs(10, 10)}, diff --git a/internal/modules/dnd/extract/itemevents/schema_test.go b/internal/modules/dnd/extract/itemevents/schema_test.go index 18497d4..edf4645 100644 --- a/internal/modules/dnd/extract/itemevents/schema_test.go +++ b/internal/modules/dnd/extract/itemevents/schema_test.go @@ -17,10 +17,16 @@ func TestResponseSchemaIsStrictlyStructuralAndPrivate(t *testing.T) { if schema.Key != ResponseSchemaKey || schema.ID != ResponseSchemaID || schema.Name != ResponseSchemaName || schema.Version != SchemaVersion || !strings.HasPrefix(schema.SHA256, "sha256:") || !json.Valid(schema.JSONSchema) { t.Fatalf("schema = %#v", schema) } - valid := map[string]any{"events": []any{map[string]any{ - "name": "", "kind": "unsupported", "quantity": 0, "from": "party", "to": "Party", - "source_refs": []any{map[string]any{"start_segment": 0, "end_segment": -1}}, - }}} + valid := map[string]any{"events": []any{ + map[string]any{ + "name": "", "kind": "unsupported", "quantity": 0, "from": "party", "to": "Party", + "source_refs": []any{map[string]any{"start_segment": 0, "end_segment": -1}}, + }, + map[string]any{ + "name": "Hidden Cache", "kind": "discovered", "quantity": nil, "from": nil, "to": nil, + "source_refs": []any{map[string]any{"start_segment": 1, "end_segment": 1}}, + }, + }} content, err := json.Marshal(valid) if err != nil { t.Fatal(err) @@ -33,10 +39,11 @@ func TestResponseSchemaIsStrictlyStructuralAndPrivate(t *testing.T) { value map[string]any }{ {"missing events", map[string]any{}}, - {"missing event name", map[string]any{"events": []any{map[string]any{"kind": "acquired", "source_refs": []any{}}}}}, - {"unknown event field", map[string]any{"events": []any{map[string]any{"name": "Ring", "kind": "acquired", "source_refs": []any{}, "extra": true}}}}, - {"unknown reference field", map[string]any{"events": []any{map[string]any{"name": "Ring", "kind": "acquired", "source_refs": []any{map[string]any{"start_segment": 1, "end_segment": 1, "extra": true}}}}}}, - {"noninteger range", map[string]any{"events": []any{map[string]any{"name": "Ring", "kind": "acquired", "source_refs": []any{map[string]any{"start_segment": 1.5, "end_segment": 1}}}}}}, + {"missing event name", map[string]any{"events": []any{withoutField(responseEvent(), "name")}}}, + {"missing nullable field", map[string]any{"events": []any{withoutField(responseEvent(), "quantity")}}}, + {"unknown event field", map[string]any{"events": []any{withField(responseEvent(), "extra", true)}}}, + {"unknown reference field", map[string]any{"events": []any{withField(responseEvent(), "source_refs", []any{map[string]any{"start_segment": 1, "end_segment": 1, "extra": true}})}}}, + {"noninteger range", map[string]any{"events": []any{withField(responseEvent(), "source_refs", []any{map[string]any{"start_segment": 1.5, "end_segment": 1}})}}}, } { t.Run(test.name, func(t *testing.T) { content, err := json.Marshal(test.value) @@ -59,6 +66,22 @@ func TestResponseSchemaIsStrictlyStructuralAndPrivate(t *testing.T) { } } +func responseEvent() map[string]any { + return map[string]any{ + "name": "Ring", "kind": "acquired", "quantity": nil, "from": nil, "to": "party", "source_refs": []any{}, + } +} + +func withoutField(value map[string]any, name string) map[string]any { + delete(value, name) + return value +} + +func withField(value map[string]any, name string, fieldValue any) map[string]any { + value[name] = fieldValue + return value +} + func validateJSONSchema(instanceContent, schemaContent []byte) error { instance, err := jsonschema.UnmarshalJSON(bytes.NewReader(instanceContent)) if err != nil { diff --git a/internal/modules/dnd/register/schema_compatibility_test.go b/internal/modules/dnd/register/schema_compatibility_test.go new file mode 100644 index 0000000..cee6107 --- /dev/null +++ b/internal/modules/dnd/register/schema_compatibility_test.go @@ -0,0 +1,85 @@ +package register + +import ( + "encoding/json" + "io/fs" + "sort" + "strconv" + "strings" + "testing" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/llm" +) + +func TestRegisteredResponseSchemasRequireEveryObjectProperty(t *testing.T) { + assets := llm.NewAssetRegistry() + if err := Register(completeRegistries(), assets); err != nil { + t.Fatal(err) + } + schemaFS, err := assets.SchemaFS() + if err != nil { + t.Fatal(err) + } + + count := 0 + err = fs.WalkDir(schemaFS, ".", func(path string, entry fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.IsDir() || !strings.HasSuffix(path, ".json") { + return nil + } + content, err := fs.ReadFile(schemaFS, path) + if err != nil { + return err + } + var schema any + if err := json.Unmarshal(content, &schema); err != nil { + t.Errorf("decode response schema %q: %v", path, err) + return nil + } + count++ + assertRequiredObjectProperties(t, path, "$", schema) + return nil + }) + if err != nil { + t.Fatal(err) + } + if count == 0 { + t.Fatal("no registered response schemas found") + } +} + +func assertRequiredObjectProperties(t *testing.T, schemaPath, nodePath string, value any) { + t.Helper() + switch node := value.(type) { + case map[string]any: + if properties, ok := node["properties"].(map[string]any); ok { + required := make(map[string]struct{}) + if values, ok := node["required"].([]any); ok { + for _, value := range values { + if name, ok := value.(string); ok { + required[name] = struct{}{} + } + } + } + propertyNames := make([]string, 0, len(properties)) + for name := range properties { + propertyNames = append(propertyNames, name) + } + sort.Strings(propertyNames) + for _, name := range propertyNames { + if _, ok := required[name]; !ok { + t.Errorf("response schema %q object %s declares property %q without requiring it", schemaPath, nodePath, name) + } + } + } + for name, child := range node { + assertRequiredObjectProperties(t, schemaPath, nodePath+"."+name, child) + } + case []any: + for index, child := range node { + assertRequiredObjectProperties(t, schemaPath, nodePath+"["+strconv.Itoa(index)+"]", child) + } + } +}