From f208dbe9544859bd5825f3bc21f3c7f1e5b77311 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 29 Aug 2026 11:39:02 +0000 Subject: [PATCH] Make combat scene validation more reliable --- .../combat-semantics/prompts/instructions.md | 17 +++---- .../combat-semantics/prompts/prompt.yaml | 7 --- .../combat-semantics/prompts/proposed-kind.md | 3 -- .../dnd_scene_combat_semantics_llm.v1.json | 6 +-- docs/config.md | 7 ++- docs/internal/dnd.md | 42 +++++++++------- docs/internal/pipeline.md | 2 + docs/roadmap/dnd-validation-reliability.md | 2 +- .../pipeline/runner_attempt_debug_test.go | 6 ++- .../pipeline/runner_terminal_debug_test.go | 29 +++++++---- internal/framework/pipeline/runner_typed.go | 7 ++- .../combat_semantics/prompt_assets.go | 1 - .../combat_semantics/prompt_assets_test.go | 14 +++--- .../combat_semantics/schema_test.go | 22 ++++---- .../testdata/evaluation_cases.json | 16 +++--- .../combat_semantics/validator.go | 21 ++++---- .../combat_semantics/validator_test.go | 50 +++++++++---------- .../integration/dnd_npc_grounded_test.go | 30 +++++------ 18 files changed, 146 insertions(+), 136 deletions(-) delete mode 100644 assets/dnd/scene-descriptions/validate/combat-semantics/prompts/proposed-kind.md diff --git a/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/instructions.md b/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/instructions.md index 4d28fa2b..566b7400 100644 --- a/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/instructions.md +++ b/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/instructions.md @@ -1,11 +1,8 @@ -Review only whether the proposed scene kind correctly represents substantive -active combat in the supplied transcript chunk under the shared combat policy. -Do not judge the scene title, summary, non-combat subtype, scene boundary, or -any other aspect of a scene description. +Classify only whether substantive active combat occurs in the supplied +transcript chunk under the shared combat policy. Do not judge the scene title, +summary, non-combat subtype, scene boundary, or any other aspect of a scene +description. -Return `approved` only when the proposed kind's combat status is supported; -approval does not endorse any other part of the scene description. Return -`combat_should_be_added` when a proposed non-combat kind omits substantive -active combat. Return `combat_should_be_removed` when a proposed `combat` kind -is unsupported. Give a concise, transcript-grounded explanation for every -verdict. +Return `combat` when the chunk contains substantive active combat and +`non_combat` otherwise. Give a concise, transcript-grounded explanation for +the classification. diff --git a/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/prompt.yaml b/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/prompt.yaml index 16f8d0ac..c5e25385 100644 --- a/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/prompt.yaml +++ b/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/prompt.yaml @@ -5,9 +5,6 @@ inputs: - name: transcript required: true content_type: application/json - - name: proposed_kind - required: true - content_type: text/plain messages: - role: system content_file: ./sharedassets/common-dnd-system.md @@ -15,10 +12,6 @@ messages: content_file: ./sharedassets/common-dnd-scene-combat-policy.md - role: user content_file: ./instructions.md - - role: user - content_file: ./proposed-kind.md - cache_control: - type: ephemeral - role: user content_file: ./sharedassets/common-dnd-transcript-chunk.md cache_control: diff --git a/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/proposed-kind.md b/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/proposed-kind.md deleted file mode 100644 index 79aa3487..00000000 --- a/assets/dnd/scene-descriptions/validate/combat-semantics/prompts/proposed-kind.md +++ /dev/null @@ -1,3 +0,0 @@ -The proposed scene kind is: - -{{ input "proposed_kind" }} diff --git a/assets/dnd/scene-descriptions/validate/combat-semantics/schemas/dnd_scene_combat_semantics_llm.v1.json b/assets/dnd/scene-descriptions/validate/combat-semantics/schemas/dnd_scene_combat_semantics_llm.v1.json index 4581efa3..93d5bbd8 100644 --- a/assets/dnd/scene-descriptions/validate/combat-semantics/schemas/dnd_scene_combat_semantics_llm.v1.json +++ b/assets/dnd/scene-descriptions/validate/combat-semantics/schemas/dnd_scene_combat_semantics_llm.v1.json @@ -3,11 +3,11 @@ "$id": "notarius.dnd.scene_descriptions.combat_semantics.llm", "type": "object", "additionalProperties": false, - "required": ["verdict", "explanation"], + "required": ["classification", "explanation"], "properties": { - "verdict": { + "classification": { "type": "string", - "enum": ["approved", "combat_should_be_added", "combat_should_be_removed"] + "enum": ["combat", "non_combat"] }, "explanation": { "type": "string", diff --git a/docs/config.md b/docs/config.md index ba01c2b3..0ef98631 100644 --- a/docs/config.md +++ b/docs/config.md @@ -521,11 +521,16 @@ extract: - extract/dnd/scene-descriptions/source_refs - generic/valid_json_schema - extract/dnd/scene-descriptions/source_relatedness - - extract/dnd/scene-descriptions/combat_semantics + - module: extract/dnd/scene-descriptions/combat_semantics + retries: 1 ~~~ An override replaces, rather than extends, the default chain. See [Module Bindings And Validators](#module-bindings-and-validators) for binding, profile, repair, retry, and failure-policy rules. +The validator retry shown above permits one additional execution against the +same scene candidate when the LLM-backed validator itself fails; it is separate +from both the extractor's producer retry and PromptKit structural repair. + When no override is configured, production D&D bindings use the following ordered chains. Each row lists extract then normalize; spell chains are the same at both stages. diff --git a/docs/internal/dnd.md b/docs/internal/dnd.md index a3dfd8c6..c4b9fcdf 100644 --- a/docs/internal/dnd.md +++ b/docs/internal/dnd.md @@ -153,28 +153,32 @@ is documented in The optional `extract/dnd/scene-descriptions/combat_semantics` validator is the D&D family's LLM-backed review of only combat versus non-combat classification. -It selects the shared combat-policy prompt fragment, receives the proposed kind -and current chunk, and maps its verdict deterministically into producer -guidance. It does not assess titles, summaries, non-combat subtype, or scene -boundaries; deferred boundary-coherence review remains separate. It is opt-in; +It selects the shared combat-policy prompt fragment and asks the model to +classify the current chunk independently as `combat` or `non_combat` without +receiving the proposed scene kind. Deterministic code compares that +classification with the proposed kind and either approves it or produces the +appropriate correction guidance. This keeps every schema-valid classification +interpretable and avoids anchoring the reviewer on the producer's answer. It +does not assess titles, summaries, non-combat subtype, or scene boundaries; +deferred boundary-coherence review remains separate. It is opt-in; [Configuration](../config.md) owns selection and retry/failure behavior. ### Combat-semantics provider evaluation The human-reviewed corpus at `internal/modules/dnd/validate/scenedescriptions/combat_semantics/testdata/evaluation_cases.json` -owns the proposed kind, expected verdict, and reviewer rationale for each -synthetic case. Its package test validates the fixture contract only. Provider -evaluation remains an explicit maintainer operation and must not be added to -the default offline test suite. +owns the proposed kind, expected combat classification, and reviewer rationale +for each synthetic case. Its package test validates the fixture contract only. +Provider evaluation remains an explicit maintainer operation and must not be +added to the default offline test suite. Use the following protocol before proposing default-chain inclusion: 1. Record the Notarius commit, prompt and schema fingerprints, provider, model, profile settings, reasoning effort, structural-repair setting, number of repetitions, and evaluation date before collecting results. Do not revise - expected verdicts merely to agree with provider output; a substantive corpus - correction requires independent human review. + expected classifications merely to agree with provider output; a substantive + corpus correction requires independent human review. 2. Exercise the production validator construction and prompt assets from an explicitly invoked, disposable evaluation driver or test in the validator package. For each corpus case, construct transcript source units from the @@ -186,12 +190,14 @@ Use the following protocol before proposing default-chain inclusion: validator at extract stage through the production registry and scheduled LLM client. Do not commit provider credentials, generated source material, or an always-on live test. -3. Compare the resulting approval or reason code with `expected_verdict`. +3. Compare the model classification with `expected_classification`, then verify + that its deterministic comparison with `proposed_kind` yields the expected + approval or rejection direction. Record an unexpected approval of an expected rejection as a false acceptance, an unexpected rejection of an expected approval as a false - rejection, the opposite rejection direction separately, and any validator - execution failure separately from semantic accuracy. Retain per-case results - so repeated trials and systematic failure modes remain visible. + rejection, and any validator execution failure separately from semantic + accuracy. Retain per-case results so repeated trials and systematic failure + modes remain visible. 4. Evaluate producer correction separately with representative complete scene-description runs configured as shown in [Configuration](../config.md#production-validator-keys-and-default-chains). @@ -204,10 +210,10 @@ Use the following protocol before proposing default-chain inclusion: from the debug attempt records. Compare these values with an otherwise identical run whose scene-description chain omits the semantic validator. -The default-chain review must consider false acceptance, false rejection, -wrong-direction rejection, execution failure, producer-correction success, -added calls, latency, and token use together. A structurally successful -provider run alone is not evidence that the validator should become a default. +The default-chain review must consider classification error, false acceptance, +false rejection, execution failure, producer-correction success, added calls, +latency, and token use together. A structurally successful provider run alone +is not evidence that the validator should become a default. Every producer-correctable D&D rejection describes all currently detectable corrections in transcript-grounded domain terms, using contextual names, diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index 3d353dd9..6b53a00b 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -231,6 +231,8 @@ Debug recording is attempt-scoped and application-owned. A failure to persist required debug data is a framework error. State roots, persistence, reason-code meanings, resume, and cleanup are intentionally owned by [Run State Internals](state.md) and [Operations](../operations.md). +Extract-validator trace scopes include the current chunk ordinal so concurrent +chunks cannot overwrite one another's validator attempts or LLM artifacts. ## Invariants To Preserve diff --git a/docs/roadmap/dnd-validation-reliability.md b/docs/roadmap/dnd-validation-reliability.md index 8d5e23a8..668d510f 100644 --- a/docs/roadmap/dnd-validation-reliability.md +++ b/docs/roadmap/dnd-validation-reliability.md @@ -110,7 +110,7 @@ correct its result: repair deterministically derived ordering, identity, or normalization state. If such a rejection can reach a feedback-capable producer, guidance must be expressed only in terms of the source candidate that producer controls. -- LLM-backed semantic validators may continue using their verdict explanation +- LLM-backed semantic validators may continue using their model explanation when it is bounded and semantically meaningful. Introduce a small D&D-shared diagnostic utility only for demonstrated common diff --git a/internal/framework/pipeline/runner_attempt_debug_test.go b/internal/framework/pipeline/runner_attempt_debug_test.go index 3580feec..cd3ac0c8 100644 --- a/internal/framework/pipeline/runner_attempt_debug_test.go +++ b/internal/framework/pipeline/runner_attempt_debug_test.go @@ -107,8 +107,12 @@ func (attemptDebugLLM) CompleteStructured(_ context.Context, request contracts.S } func preparedAttemptDebugPipeline(t *testing.T) *PreparedPipeline { + return preparedAttemptDebugPipelineWithChunks(t, 1) +} + +func preparedAttemptDebugPipelineWithChunks(t *testing.T, chunkCount int) *PreparedPipeline { t.Helper() - prepared := preparedConcurrentPipeline(t, 1) + prepared := preparedConcurrentPipeline(t, chunkCount) prepared.Steps[0].lanes = prepared.Steps[0].lanes[:1] prepared.resolved.Steps[0].ArtifactLanes = prepared.resolved.Steps[0].ArtifactLanes[:1] prepared.Steps[0].ArtifactLanes = prepared.Steps[0].ArtifactLanes[:1] diff --git a/internal/framework/pipeline/runner_terminal_debug_test.go b/internal/framework/pipeline/runner_terminal_debug_test.go index ff284739..44882392 100644 --- a/internal/framework/pipeline/runner_terminal_debug_test.go +++ b/internal/framework/pipeline/runner_terminal_debug_test.go @@ -3,6 +3,7 @@ package pipeline import ( "context" "errors" + "fmt" "reflect" "sort" "strconv" @@ -427,7 +428,7 @@ func TestRunnerDoesNotRetryAfterTerminalAttemptWriteFailure(t *testing.T) { } func TestRunnerKeepsExtractModuleAndValidatorLLMCallsIsolated(t *testing.T) { - prepared := preparedAttemptDebugPipeline(t) + prepared := preparedAttemptDebugPipelineWithChunks(t, 2) debug := newCapturedDebugRecorder() client := WithDebugLLMRecording(attemptDebugLLM{}, debug) installExtractOperation(prepared, 0, func(ctx context.Context, request contracts.TypedExtractionRequest) (erasedTypedResult, error) { @@ -449,15 +450,23 @@ func TestRunnerKeepsExtractModuleAndValidatorLLMCallsIsolated(t *testing.T) { if _, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Debug: debug}); err != nil { t.Fatalf("Run() error = %v, want nil", err) } - module := debug.envelope(t, "extract/notes/chunk-000001/attempt-01.json") - validator := debug.envelope(t, "validate/extract/notes/typed%2Fextract-notes/01-llm-check-attempt-01.json") - if len(module.LLMCalls) != 1 || !strings.Contains(module.LLMCalls[0].ResponsePath, "extract/notes/chunk-000001/attempt-01/") { - t.Fatalf("module LLM calls = %#v, want extract module call only", module.LLMCalls) + validatorResponses := make(map[string]struct{}) + for chunkNumber := 1; chunkNumber <= 2; chunkNumber++ { + chunkPath := fmt.Sprintf("chunk-%06d", chunkNumber) + module := debug.envelope(t, "extract/notes/"+chunkPath+"/attempt-01.json") + validator := debug.envelope(t, "validate/extract/notes/typed%2Fextract-notes/"+chunkPath+"/01-llm-check-attempt-01.json") + if len(module.LLMCalls) != 1 || !strings.Contains(module.LLMCalls[0].ResponsePath, "extract/notes/"+chunkPath+"/attempt-01/") { + t.Fatalf("module LLM calls for %s = %#v, want extract module call only", chunkPath, module.LLMCalls) + } + if len(validator.LLMCalls) != 1 || !strings.Contains(validator.LLMCalls[0].ResponsePath, "validate/extract/notes/typed%2Fextract-notes/"+chunkPath+"/") { + t.Fatalf("validator LLM calls for %s = %#v, want chunk-scoped validator call only", chunkPath, validator.LLMCalls) + } + if module.LLMCalls[0].CallID == validator.LLMCalls[0].CallID { + t.Fatalf("module and validator attempts for %s share LLM call %#v", chunkPath, module.LLMCalls) + } + validatorResponses[validator.LLMCalls[0].ResponsePath] = struct{}{} } - if len(validator.LLMCalls) != 1 || !strings.Contains(validator.LLMCalls[0].ResponsePath, "validate/extract/notes/") { - t.Fatalf("validator LLM calls = %#v, want validator call only", validator.LLMCalls) - } - if module.LLMCalls[0].CallID == validator.LLMCalls[0].CallID { - t.Fatalf("module and validator attempts share LLM call %#v", module.LLMCalls) + if len(validatorResponses) != 2 { + t.Fatalf("validator response paths = %#v, want one distinct path per chunk", validatorResponses) } } diff --git a/internal/framework/pipeline/runner_typed.go b/internal/framework/pipeline/runner_typed.go index 72679347..f790809f 100644 --- a/internal/framework/pipeline/runner_typed.go +++ b/internal/framework/pipeline/runner_typed.go @@ -596,7 +596,12 @@ func (r *Runner) validateTypedReport(ctx context.Context, codec artifactCodecEnt var result contracts.ValidationResult var err error started := time.Now().UTC() - attemptPath := validatorAttemptPath(path.Join("validate", fileio.EncodePathComponent(string(target.stage)), fileio.EncodePathComponent(target.laneID), fileio.EncodePathComponent(target.moduleKey), fmt.Sprintf("%02d-%s-attempt-%02d", item.position, fileio.EncodePathComponent(binding.Module), attempt)), validatorAttempt) + pathParts := []string{"validate", fileio.EncodePathComponent(string(target.stage)), fileio.EncodePathComponent(target.laneID), fileio.EncodePathComponent(target.moduleKey)} + if target.chunk != nil { + pathParts = append(pathParts, fmt.Sprintf("chunk-%06d", target.chunk.Index+1)) + } + pathParts = append(pathParts, fmt.Sprintf("%02d-%s-attempt-%02d", item.position, fileio.EncodePathComponent(binding.Module), attempt)) + attemptPath := validatorAttemptPath(path.Join(pathParts...), validatorAttempt) validatorCtx, llmScope := withIsolatedDebugLLMScope(validatorCtx, attemptPath) requestTarget := target requestTarget.sourceInput = target.sourceInput.Clone() diff --git a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/prompt_assets.go b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/prompt_assets.go index cedbe782..e1a239e0 100644 --- a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/prompt_assets.go +++ b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/prompt_assets.go @@ -18,7 +18,6 @@ var promptAssetManifest = shared.PromptAssetManifest{ ModuleFiles: []promptfs.ModulePromptFile{ {Name: "prompt.yaml", Path: "prompts/prompt.yaml"}, {Name: "instructions.md", Path: "prompts/instructions.md"}, - {Name: "proposed-kind.md", Path: "prompts/proposed-kind.md"}, }, SharedFiles: []string{ "common-dnd-system.md", diff --git a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/prompt_assets_test.go b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/prompt_assets_test.go index 514a2fb7..81572764 100644 --- a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/prompt_assets_test.go +++ b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/prompt_assets_test.go @@ -17,8 +17,7 @@ func TestRegisterPromptAssetsPreparesCombatSemanticsPrompt(t *testing.T) { } engine := newCombatSemanticsPromptEngine(t, registry) inputs := map[string]promptkit.ArtifactRef{ - "proposed_kind": promptkit.Inline("narrative"), - "transcript": promptkit.InlineWithURI("file:///session.json", `{"sentinel":"combat-semantics-transcript"}`), + "transcript": promptkit.InlineWithURI("file:///session.json", `{"sentinel":"combat-semantics-transcript"}`), } prepared, err := engine.Prepare(context.Background(), promptkit.RunRequest{ PromptID: PromptID, PromptVersion: SchemaVersion, ProfileID: "combat-semantics-test-profile", Inputs: inputs, @@ -29,19 +28,18 @@ func TestRegisterPromptAssetsPreparesCombatSemanticsPrompt(t *testing.T) { if prepared.PromptID != PromptID || prepared.OutputContract.SchemaPath != "dnd_scene_combat_semantics_llm.v1.json" { t.Fatalf("prepared prompt = %#v, want combat-semantics prompt identity and schema wiring", prepared) } - if len(prepared.Messages) != 5 { - t.Fatalf("prepared message count = %d, want 5", len(prepared.Messages)) + if len(prepared.Messages) != 4 { + t.Fatalf("prepared message count = %d, want 4", len(prepared.Messages)) } for index, message := range prepared.Messages { - wantEphemeral := index == 3 || index == 4 + wantEphemeral := index == 3 gotEphemeral := message.CacheControl != nil && message.CacheControl.Type == promptkit.CacheControlEphemeral if gotEphemeral != wantEphemeral { t.Fatalf("message %d cache control = %#v, want ephemeral=%t", index, message.CacheControl, wantEphemeral) } } assertRenderedExactlyOnce(t, prepared.Messages, "substantive active combat materially organizes the", 1) - assertRenderedExactlyOnce(t, prepared.Messages, "narrative", 3) - assertRenderedExactlyOnce(t, prepared.Messages, "combat-semantics-transcript", 4) + assertRenderedExactlyOnce(t, prepared.Messages, "combat-semantics-transcript", 3) for name := range inputs { missing := make(map[string]promptkit.ArtifactRef, len(inputs)-1) for inputName, input := range inputs { @@ -62,7 +60,7 @@ func TestPromptAssetMetadataIsContentSafe(t *testing.T) { if err != nil || !strings.HasPrefix(hash, "sha256:") { t.Fatalf("promptAssetMetadata() = %q, %v; want hash", hash, err) } - if strings.Contains(hash, "combat_should_be_added") { + if strings.Contains(hash, "non_combat") { t.Fatalf("prompt asset metadata leaked raw prompt content: %q", hash) } } diff --git a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/schema_test.go b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/schema_test.go index b79f0e97..522bd3ec 100644 --- a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/schema_test.go +++ b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/schema_test.go @@ -17,23 +17,23 @@ func TestLoadResponseSchemaUsesStrictCombatSemanticsContract(t *testing.T) { if schema.Key != ResponseSchemaKey || schema.ID != ResponseSchemaID || schema.Version != SchemaVersion || schema.Name != ResponseSchemaName || !strings.HasPrefix(schema.SHA256, "sha256:") || !json.Valid(schema.JSONSchema) { t.Fatalf("schema = %#v, want private combat-semantics schema identity", schema) } - for _, verdict := range []string{"approved", "combat_should_be_added", "combat_should_be_removed"} { - if err := validateCombatSemanticsSchema(map[string]any{"verdict": verdict, "explanation": "The chunk contains sustained attack exchanges."}, schema.JSONSchema); err != nil { - t.Fatalf("valid %q verdict rejected: %v", verdict, err) + for _, classification := range []string{"combat", "non_combat"} { + if err := validateCombatSemanticsSchema(map[string]any{"classification": classification, "explanation": "The chunk contains sustained attack exchanges."}, schema.JSONSchema); err != nil { + t.Fatalf("valid %q classification rejected: %v", classification, err) } } for _, test := range []struct { name string response map[string]any }{ - {name: "missing explanation", response: map[string]any{"verdict": "approved"}}, - {name: "missing verdict", response: map[string]any{"explanation": "The proposed kind is supported."}}, - {name: "unknown property", response: map[string]any{"verdict": "approved", "explanation": "The proposed kind is supported.", "confidence": 1}}, - {name: "wrong verdict type", response: map[string]any{"verdict": 1, "explanation": "The proposed kind is supported."}}, - {name: "wrong explanation type", response: map[string]any{"verdict": "approved", "explanation": 1}}, - {name: "unsupported verdict", response: map[string]any{"verdict": "uncertain", "explanation": "The proposed kind is supported."}}, - {name: "blank explanation", response: map[string]any{"verdict": "approved", "explanation": ""}}, - {name: "oversized explanation", response: map[string]any{"verdict": "approved", "explanation": strings.Repeat("a", 513)}}, + {name: "missing explanation", response: map[string]any{"classification": "combat"}}, + {name: "missing classification", response: map[string]any{"explanation": "The chunk contains active combat."}}, + {name: "unknown property", response: map[string]any{"classification": "combat", "explanation": "The chunk contains active combat.", "confidence": 1}}, + {name: "wrong classification type", response: map[string]any{"classification": 1, "explanation": "The chunk contains active combat."}}, + {name: "wrong explanation type", response: map[string]any{"classification": "combat", "explanation": 1}}, + {name: "unsupported classification", response: map[string]any{"classification": "uncertain", "explanation": "The classification is uncertain."}}, + {name: "blank explanation", response: map[string]any{"classification": "combat", "explanation": ""}}, + {name: "oversized explanation", response: map[string]any{"classification": "combat", "explanation": strings.Repeat("a", 513)}}, } { t.Run(test.name, func(t *testing.T) { if err := validateCombatSemanticsSchema(test.response, schema.JSONSchema); err == nil { diff --git a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/testdata/evaluation_cases.json b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/testdata/evaluation_cases.json index 3b95d0e9..718ca649 100644 --- a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/testdata/evaluation_cases.json +++ b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/testdata/evaluation_cases.json @@ -1,10 +1,10 @@ [ - {"name":"active encounter","transcript_units":[{"id":1,"text":"Roll initiative; the goblins attack."},{"id":2,"text":"The ranger hits and deals damage."}],"proposed_kind":"combat","expected_verdict":"approved","reviewer_rationale":"Initiative and hostile actions organize the chunk."}, - {"name":"combat after setup","transcript_units":[{"id":1,"text":"They open the crypt door."},{"id":2,"text":"Skeletons attack and turns begin."}],"proposed_kind":"narrative","expected_verdict":"combat_should_be_added","reviewer_rationale":"Brief setup does not displace substantive active combat."}, - {"name":"combat aftermath","transcript_units":[{"id":1,"text":"The last enemy falls."},{"id":2,"text":"They search bodies and heal."}],"proposed_kind":"combat","expected_verdict":"combat_should_be_removed","reviewer_rationale":"Looting and healing after a completed fight are not active combat."}, - {"name":"multi phase encounter","transcript_units":[{"id":1,"text":"The dragon attacks."},{"id":2,"text":"After a rules clarification, its next turn begins."}],"proposed_kind":"combat","expected_verdict":"approved","reviewer_rationale":"A brief rules interruption does not end the encounter."}, - {"name":"planning","transcript_units":[{"id":1,"text":"They plan how to ambush the guard."}],"proposed_kind":"combat","expected_verdict":"combat_should_be_removed","reviewer_rationale":"Planning a possible fight is not active encounter play."}, - {"name":"hostile dialogue","transcript_units":[{"id":1,"text":"The captain threatens them and they argue."}],"proposed_kind":"combat","expected_verdict":"combat_should_be_removed","reviewer_rationale":"Threats and hostile dialogue alone are insufficient."}, - {"name":"recap recollection","transcript_units":[{"id":1,"text":"They recap last session's battle with the lich."}],"proposed_kind":"combat","expected_verdict":"combat_should_be_removed","reviewer_rationale":"Recounting earlier combat is not current active combat."}, - {"name":"rules discussion","transcript_units":[{"id":1,"text":"The table discusses how concentration works."}],"proposed_kind":"meta","expected_verdict":"approved","reviewer_rationale":"Sustained out-of-character rules discussion has no active encounter."} + {"name":"active encounter","transcript_units":[{"id":1,"text":"Roll initiative; the goblins attack."},{"id":2,"text":"The ranger hits and deals damage."}],"proposed_kind":"combat","expected_classification":"combat","reviewer_rationale":"Initiative and hostile actions organize the chunk."}, + {"name":"combat after setup","transcript_units":[{"id":1,"text":"They open the crypt door."},{"id":2,"text":"Skeletons attack and turns begin."}],"proposed_kind":"narrative","expected_classification":"combat","reviewer_rationale":"Brief setup does not displace substantive active combat."}, + {"name":"combat aftermath","transcript_units":[{"id":1,"text":"The last enemy falls."},{"id":2,"text":"They search bodies and heal."}],"proposed_kind":"combat","expected_classification":"non_combat","reviewer_rationale":"Looting and healing after a completed fight are not active combat."}, + {"name":"multi phase encounter","transcript_units":[{"id":1,"text":"The dragon attacks."},{"id":2,"text":"After a rules clarification, its next turn begins."}],"proposed_kind":"combat","expected_classification":"combat","reviewer_rationale":"A brief rules interruption does not end the encounter."}, + {"name":"planning","transcript_units":[{"id":1,"text":"They plan how to ambush the guard."}],"proposed_kind":"combat","expected_classification":"non_combat","reviewer_rationale":"Planning a possible fight is not active encounter play."}, + {"name":"hostile dialogue","transcript_units":[{"id":1,"text":"The captain threatens them and they argue."}],"proposed_kind":"combat","expected_classification":"non_combat","reviewer_rationale":"Threats and hostile dialogue alone are insufficient."}, + {"name":"recap recollection","transcript_units":[{"id":1,"text":"They recap last session's battle with the lich."}],"proposed_kind":"combat","expected_classification":"non_combat","reviewer_rationale":"Recounting earlier combat is not current active combat."}, + {"name":"rules discussion","transcript_units":[{"id":1,"text":"The table discusses how concentration works."}],"proposed_kind":"meta","expected_classification":"non_combat","reviewer_rationale":"Sustained out-of-character rules discussion has no active encounter."} ] diff --git a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator.go b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator.go index 80d3587c..832b8d09 100644 --- a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator.go +++ b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator.go @@ -25,8 +25,8 @@ const ( type Options struct{} type completionResponse struct { - Verdict string `json:"verdict"` - Explanation string `json:"explanation"` + Classification string `json:"classification"` + Explanation string `json:"explanation"` } type Validator struct { @@ -109,8 +109,7 @@ func (v *Validator) Validate(ctx context.Context, req contracts.TypedValidationR SessionID: req.SessionID, StructuredOutputRepairAttempts: req.StructuredOutputRepairAttempts, Inputs: contracts.LLMInputSet{ - "transcript": shared.TranscriptPromptMaterial(sourceInput), - "proposed_kind": contracts.NewLLMInputMaterial("proposed_kind", "text/plain", []byte(scene.Kind), "", ""), + "transcript": shared.TranscriptPromptMaterial(sourceInput), }, }, &response) if err != nil { @@ -160,12 +159,10 @@ func interpretResponse(response completionResponse, proposedKind dnd.SceneKind) if err != nil { return contracts.ValidationResult{}, err } - switch response.Verdict { - case "approved": - return contracts.ValidationResult{Approved: true}, nil - case "combat_should_be_added": + switch response.Classification { + case "combat": if proposedKind == dnd.SceneKindCombat { - return contracts.ValidationResult{}, validatorErrorf("combat_should_be_added verdict is inconsistent with proposed combat kind") + return contracts.ValidationResult{Approved: true}, nil } return contracts.ValidationResult{ Approved: false, @@ -173,9 +170,9 @@ func interpretResponse(response completionResponse, proposedKind dnd.SceneKind) Message: "The current chunk contains substantive active combat that is not classified as combat.", CorrectionGuidance: "Return kind: combat for this scene. " + explanation, }, nil - case "combat_should_be_removed": + case "non_combat": if proposedKind != dnd.SceneKindCombat { - return contracts.ValidationResult{}, validatorErrorf("combat_should_be_removed verdict is inconsistent with proposed non-combat kind") + return contracts.ValidationResult{Approved: true}, nil } return contracts.ValidationResult{ Approved: false, @@ -184,7 +181,7 @@ func interpretResponse(response completionResponse, proposedKind dnd.SceneKind) CorrectionGuidance: "Choose the appropriate narrative, recap, or meta kind for this scene. " + explanation, }, nil default: - return contracts.ValidationResult{}, validatorErrorf("unsupported combat-semantics verdict %q", response.Verdict) + return contracts.ValidationResult{}, validatorErrorf("unsupported combat-semantics classification %q", response.Classification) } } diff --git a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator_test.go b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator_test.go index 91dd456d..5956dacb 100644 --- a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator_test.go +++ b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator_test.go @@ -28,16 +28,14 @@ func TestValidatorInterpretsCombatSemanticsResponses(t *testing.T) { correction string wantError string }{ - {name: "approve non-combat", kind: dnd.SceneKindNarrative, response: completionResponse{Verdict: "approved", Explanation: "No active encounter occurs."}, approved: true}, - {name: "approve combat", kind: dnd.SceneKindCombat, response: completionResponse{Verdict: "approved", Explanation: "Initiative and attacks organize the chunk."}, approved: true}, - {name: "add combat", kind: dnd.SceneKindNarrative, response: completionResponse{Verdict: "combat_should_be_added", Explanation: "The combatants exchange attacks and damage."}, reasonCode: ReasonCodeActiveCombatNotClassified, guidance: "Return kind: combat"}, - {name: "remove combat", kind: dnd.SceneKindCombat, response: completionResponse{Verdict: "combat_should_be_removed", Explanation: "The group only plans for a possible fight."}, reasonCode: ReasonCodeCombatClassificationUnsupported, guidance: "narrative, recap, or meta"}, - {name: "inconsistent added", kind: dnd.SceneKindCombat, response: completionResponse{Verdict: "combat_should_be_added", Explanation: "The combatants exchange attacks."}, wantError: "inconsistent"}, - {name: "inconsistent removed", kind: dnd.SceneKindNarrative, response: completionResponse{Verdict: "combat_should_be_removed", Explanation: "The group only plans."}, wantError: "inconsistent"}, - {name: "unknown verdict", kind: dnd.SceneKindNarrative, response: completionResponse{Verdict: "uncertain", Explanation: "The group only plans."}, wantError: "unsupported"}, - {name: "blank explanation", kind: dnd.SceneKindNarrative, response: completionResponse{Verdict: "approved", Explanation: " "}, wantError: "blank"}, - {name: "trim explanation", kind: dnd.SceneKindNarrative, response: completionResponse{Verdict: "combat_should_be_added", Explanation: " The combatants exchange attacks.\n"}, reasonCode: ReasonCodeActiveCombatNotClassified, correction: "Return kind: combat for this scene. The combatants exchange attacks."}, - {name: "oversized explanation", kind: dnd.SceneKindNarrative, response: completionResponse{Verdict: "approved", Explanation: strings.Repeat("界", 513)}, wantError: "exceeds"}, + {name: "approve non-combat", kind: dnd.SceneKindNarrative, response: completionResponse{Classification: "non_combat", Explanation: "No active encounter occurs."}, approved: true}, + {name: "approve combat", kind: dnd.SceneKindCombat, response: completionResponse{Classification: "combat", Explanation: "Initiative and attacks organize the chunk."}, approved: true}, + {name: "add combat", kind: dnd.SceneKindNarrative, response: completionResponse{Classification: "combat", Explanation: "The combatants exchange attacks and damage."}, reasonCode: ReasonCodeActiveCombatNotClassified, guidance: "Return kind: combat"}, + {name: "remove combat", kind: dnd.SceneKindCombat, response: completionResponse{Classification: "non_combat", Explanation: "The group only plans for a possible fight."}, reasonCode: ReasonCodeCombatClassificationUnsupported, guidance: "narrative, recap, or meta"}, + {name: "unknown classification", kind: dnd.SceneKindNarrative, response: completionResponse{Classification: "uncertain", Explanation: "The group only plans."}, wantError: "unsupported"}, + {name: "blank explanation", kind: dnd.SceneKindNarrative, response: completionResponse{Classification: "non_combat", Explanation: " "}, wantError: "blank"}, + {name: "trim explanation", kind: dnd.SceneKindNarrative, response: completionResponse{Classification: "combat", Explanation: " The combatants exchange attacks.\n"}, reasonCode: ReasonCodeActiveCombatNotClassified, correction: "Return kind: combat for this scene. The combatants exchange attacks."}, + {name: "oversized explanation", kind: dnd.SceneKindNarrative, response: completionResponse{Classification: "non_combat", Explanation: strings.Repeat("界", 513)}, wantError: "exceeds"}, } { t.Run(test.name, func(t *testing.T) { request.Value.Scenes[0].Kind = test.kind @@ -66,14 +64,14 @@ func TestValidatorInterpretsCombatSemanticsResponses(t *testing.T) { } } -func TestValidatorUsesOnlyChunkTranscriptAndProposedKind(t *testing.T) { +func TestValidatorUsesOnlyChunkTranscript(t *testing.T) { request := validRequest(dnd.SceneKindNarrative) repairAttempts := 2 request.LLMProfile = "validator-profile" request.SessionID = "validator-session" request.StructuredOutputRepairAttempts = &repairAttempts request.References = contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{"players": {Items: []contracts.ReferenceItem{{Content: []byte("hidden reference")}}}}} - client := &fakeCombatSemanticsClient{response: completionResponse{Verdict: "approved", Explanation: "No active encounter occurs."}} + client := &fakeCombatSemanticsClient{response: completionResponse{Classification: "non_combat", Explanation: "No active encounter occurs."}} validator := newValidator(t, client) if result, err := validator.Validate(context.Background(), request); err != nil || !result.Approved { t.Fatalf("Validate() = %#v, %v", result, err) @@ -85,7 +83,7 @@ func TestValidatorUsesOnlyChunkTranscriptAndProposedKind(t *testing.T) { if completed.StageName != Key || completed.PromptID != PromptID || completed.PromptVersion != SchemaVersion || completed.ProfileID != request.LLMProfile || completed.SessionID != request.SessionID || completed.StructuredOutputRepairAttempts == request.StructuredOutputRepairAttempts || *completed.StructuredOutputRepairAttempts != repairAttempts { t.Fatalf("completion request = %#v", completed) } - if completed.Correction != nil || !reflect.DeepEqual(sortedInputNames(completed.Inputs), []string{"proposed_kind", "transcript"}) || string(completed.Inputs["proposed_kind"].Content) != "narrative" || !reflect.DeepEqual(completed.Inputs["transcript"].Content, request.Chunk.Content) { + if completed.Correction != nil || !reflect.DeepEqual(sortedInputNames(completed.Inputs), []string{"transcript"}) || !reflect.DeepEqual(completed.Inputs["transcript"].Content, request.Chunk.Content) { t.Fatalf("completion inputs = %#v", completed.Inputs) } for _, forbidden := range []string{request.Value.Scenes[0].ID, request.Value.Scenes[0].Title, request.Value.Scenes[0].Summary, "hidden reference"} { @@ -132,7 +130,7 @@ func TestValidatorRejectsInvalidRequestsAndCompletionFailures(t *testing.T) { if test.mutate != nil { test.mutate(&request) } - client := &fakeCombatSemanticsClient{response: completionResponse{Verdict: "approved", Explanation: "No active encounter occurs."}} + client := &fakeCombatSemanticsClient{response: completionResponse{Classification: "non_combat", Explanation: "No active encounter occurs."}} _, err := newValidator(t, client).Validate(test.ctx, request) if err == nil || !strings.Contains(err.Error(), test.wantErr) || len(client.requests) != 0 { t.Fatalf("Validate() error = %v, calls = %d; want %q and no calls", err, len(client.requests), test.wantErr) @@ -157,7 +155,7 @@ func TestValidatorConstructionOptionsAndMetadata(t *testing.T) { t.Fatalf("validator contract = %#v / %#v", validator, Spec()) } metadata, err := json.Marshal(validator.ManifestMetadata()) - if err != nil || !strings.Contains(string(metadata), ResponseSchemaID) || strings.Contains(string(metadata), "combat_should_be_added") { + if err != nil || !strings.Contains(string(metadata), ResponseSchemaID) || strings.Contains(string(metadata), "non_combat") { t.Fatalf("manifest metadata = %s, %v", metadata, err) } fingerprints := validator.CheckpointFingerprints() @@ -173,7 +171,7 @@ func TestValidatorConstructionOptionsAndMetadata(t *testing.T) { } } -func TestEvaluationCasesAreStrictAndCoverVerdicts(t *testing.T) { +func TestEvaluationCasesAreStrictAndCoverClassifications(t *testing.T) { content, err := os.ReadFile("testdata/evaluation_cases.json") if err != nil { t.Fatal(err) @@ -184,9 +182,9 @@ func TestEvaluationCasesAreStrictAndCoverVerdicts(t *testing.T) { ID int `json:"id"` Text string `json:"text"` } `json:"transcript_units"` - ProposedKind string `json:"proposed_kind"` - ExpectedVerdict string `json:"expected_verdict"` - ReviewerRationale string `json:"reviewer_rationale"` + ProposedKind string `json:"proposed_kind"` + ExpectedClassification string `json:"expected_classification"` + ReviewerRationale string `json:"reviewer_rationale"` } decoder := json.NewDecoder(strings.NewReader(string(content))) decoder.DisallowUnknownFields() @@ -196,7 +194,7 @@ func TestEvaluationCasesAreStrictAndCoverVerdicts(t *testing.T) { if err := decoder.Decode(&struct{}{}); !errors.Is(err, io.EOF) { t.Fatalf("decode trailing evaluation data: %v, want EOF", err) } - seenNames, seenVerdicts := map[string]bool{}, map[string]bool{} + seenNames, seenClassifications := map[string]bool{}, map[string]bool{} for _, value := range cases { if strings.TrimSpace(value.Name) == "" || seenNames[value.Name] || len(value.TranscriptUnits) == 0 || strings.TrimSpace(value.ProposedKind) == "" || strings.TrimSpace(value.ReviewerRationale) == "" { t.Fatalf("invalid evaluation case: %#v", value) @@ -207,11 +205,11 @@ func TestEvaluationCasesAreStrictAndCoverVerdicts(t *testing.T) { default: t.Fatalf("unsupported proposed kind %q", value.ProposedKind) } - switch value.ExpectedVerdict { - case "approved", "combat_should_be_added", "combat_should_be_removed": - seenVerdicts[value.ExpectedVerdict] = true + switch value.ExpectedClassification { + case "combat", "non_combat": + seenClassifications[value.ExpectedClassification] = true default: - t.Fatalf("unsupported verdict %q", value.ExpectedVerdict) + t.Fatalf("unsupported classification %q", value.ExpectedClassification) } for _, unit := range value.TranscriptUnits { if unit.ID <= 0 || strings.TrimSpace(unit.Text) == "" { @@ -219,8 +217,8 @@ func TestEvaluationCasesAreStrictAndCoverVerdicts(t *testing.T) { } } } - if len(seenVerdicts) != 3 { - t.Fatalf("evaluation verdicts = %#v, want all classes", seenVerdicts) + if len(seenClassifications) != 2 { + t.Fatalf("evaluation classifications = %#v, want both classes", seenClassifications) } } diff --git a/internal/modules/integration/dnd_npc_grounded_test.go b/internal/modules/integration/dnd_npc_grounded_test.go index 62b0a0bb..dacfef39 100644 --- a/internal/modules/integration/dnd_npc_grounded_test.go +++ b/internal/modules/integration/dnd_npc_grounded_test.go @@ -276,7 +276,7 @@ func TestSceneCombatSemanticsCorrectionRetriesAndRevalidates(t *testing.T) { profile.Steps[0].Artifacts["scene-descriptions"] = sceneLane configValue.Pipelines["dnd-npc-grounded"] = profile - client := &groundedDNDLLMClient{sceneKinds: []dnd.SceneKind{dnd.SceneKindNarrative, dnd.SceneKindCombat}, combatSemanticsVerdicts: []string{"combat_should_be_added", "approved"}} + client := &groundedDNDLLMClient{sceneKinds: []dnd.SceneKind{dnd.SceneKindNarrative, dnd.SceneKindCombat}, combatSemanticsClassifications: []string{"combat", "combat"}} output := runGroundedPipeline(t, configValue, registries, client, nil) if len(output.Rejected) != 0 { t.Fatalf("rejected = %#v, want corrected acceptance", output.Rejected) @@ -573,16 +573,16 @@ func (loader *generatedReferenceCheckpointLoader) extractDependencies(laneID str } type groundedDNDLLMClient struct { - mu sync.Mutex - requests []contracts.StructuredCompletionRequest - sceneKind dnd.SceneKind - sceneTitle string - firstUnitID int - thirdUnitID int - sceneKinds []dnd.SceneKind - combatSemanticsVerdicts []string - sceneCalls int - combatSemanticsCalls int + mu sync.Mutex + requests []contracts.StructuredCompletionRequest + sceneKind dnd.SceneKind + sceneTitle string + firstUnitID int + thirdUnitID int + sceneKinds []dnd.SceneKind + combatSemanticsClassifications []string + sceneCalls int + combatSemanticsCalls int } func (client *groundedDNDLLMClient) CompleteStructured(ctx context.Context, request contracts.StructuredCompletionRequest, out any) (contracts.StructuredCompletionResponse, error) { @@ -629,12 +629,12 @@ func (client *groundedDNDLLMClient) CompleteStructured(ctx context.Context, requ } payload = map[string]any{"kind": kind, "title": title, "summary": "The party faces an active encounter."} case combatsemantics.PromptID: - verdict := "approved" - if client.combatSemanticsCalls < len(client.combatSemanticsVerdicts) { - verdict = client.combatSemanticsVerdicts[client.combatSemanticsCalls] + classification := "combat" + if client.combatSemanticsCalls < len(client.combatSemanticsClassifications) { + classification = client.combatSemanticsClassifications[client.combatSemanticsCalls] } client.combatSemanticsCalls++ - payload = map[string]any{"verdict": verdict, "explanation": "The transcript shows an active encounter with hostile action."} + payload = map[string]any{"classification": classification, "explanation": "The transcript shows an active encounter with hostile action."} case spells.PromptID: payload = map[string]any{"spell_casts": []any{map[string]any{ "caster": "Mira Thorn", "spell": "Cure Wounds",