Make item event responses compatible with strict schemas

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
+currency denomination. Set `quantity` to the explicitly stated integer, or to
-states it.
+`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
+Return both `from` and `to` for every event, using `null` when a holder does not
-`lost` and `consumed`, provide only `from`; and for `transferred`, provide both
+apply. For `discovered`, set both holders to `null`. For `acquired`, set `from`
-`from` and `to`. Use `party` only for collective or unresolved party possession,
+to `null` and provide `to`; for `lost` and `consumed`, provide `from` and set
-never for either side of a transfer. Do not emit a transfer for a gift, sale, or
+`to` to `null`; and for `transferred`, provide both holders. Use `party` only
-payment outside the party.
+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

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"},
+          "quantity": {"type": ["integer", "null"]},
-          "from": {"type": "string"},
+          "from": {"type": ["string", "null"]},
-          "to": {"type": "string"},
+          "to": {"type": ["string", "null"]},
           "source_refs": {
             "type": "array",
             "items": {

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)},

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{
+	valid := map[string]any{"events": []any{
-		"name": "", "kind": "unsupported", "quantity": 0, "from": "party", "to": "Party",
+		map[string]any{
-		"source_refs": []any{map[string]any{"start_segment": 0, "end_segment": -1}},
+			"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{}}}}},
+		{"missing event name", map[string]any{"events": []any{withoutField(responseEvent(), "name")}}},
-		{"unknown event field", map[string]any{"events": []any{map[string]any{"name": "Ring", "kind": "acquired", "source_refs": []any{}, "extra": true}}}},
+		{"missing nullable field", map[string]any{"events": []any{withoutField(responseEvent(), "quantity")}}},
-		{"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}}}}}},
+		{"unknown event field", map[string]any{"events": []any{withField(responseEvent(), "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}}}}}},
+		{"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 {

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)
+		}
+	}
+}