Plan combat scene validation

docs/roadmap/combat-scene-validation.md

@@ -0,0 +1,303 @@
+# D&D Combat Scene Semantic Validation
+
+## Purpose
+
+Add an optional LLM-backed validator for `dnd/scene-descriptions` that checks
+whether the proposed scene kind correctly represents substantive active combat
+in the current transcript chunk. The validator will improve the reliability of
+the existing downstream rule that combat turns and enemy events run only for
+scenes classified as `combat`, without moving scene classification into the
+chunker or broadening the durable scene-description contract.
+
+This roadmap defines the intended final state and the code and documentation
+changes required to reach it.
+
+## Current State
+
+- `dnd/scenes` produces a complete, gap-free scene plan containing source
+  ranges. It does not classify those ranges.
+- `dnd/scene-descriptions` processes one accepted chunk at a time and produces
+  exactly one scene with a deterministic chunk ID and source range plus the
+  model-supplied `kind`, `title`, and `summary`.
+- The supported scene kinds are `combat`, `narrative`, `recap`, and `meta`.
+- The production extract chain validates shape, source identity, durable JSON
+  schema, and advisory lexical relatedness. None of those checks can decide
+  whether active combat was classified correctly.
+- Combat-turn and enemy-event extraction already require an exact matching
+  scene description whose kind is `combat`. A missing, ambiguous, or
+  non-combat classification does not run those LLM extractors.
+- The validation framework already supports typed LLM-backed validators,
+  PromptKit structured-output repair, bounded validator execution retries,
+  aggregated semantic correction guidance, and configurable terminal handling
+  of validator rejection or execution failure.
+- There are currently no production LLM-backed validators. The new module will
+  establish the first concrete D&D implementation of that existing framework
+  capability.
+
+## User Intent And Policy
+
+The validator exists to catch both consequential classification errors:
+
+1. a scene containing substantive active combat is assigned a non-combat kind;
+   and
+2. a scene without substantive active combat is assigned `combat`.
+
+“Substantive active combat” means that an in-session encounter is materially
+organized around participants taking or resolving hostile actions, such as
+initiative or turn exchanges, attacks, combat spells, damage, saves, movement,
+or similarly sustained conflict. The following do not establish active combat
+by themselves:
+
+- planning or preparing for a possible fight;
+- threats, hostile dialogue, or a tense confrontation;
+- immediate aftermath, looting, healing, or discussion of a completed fight;
+- a recap or in-world recollection of earlier combat; or
+- out-of-character rules discussion without active encounter play.
+
+A chunk with substantive active combat remains combat when it also contains
+brief setup, rules clarification, interruption, phase transition, or immediate
+aftermath. The validator judges whether the supplied chunk's proposed combat
+status is supported; it does not redesign the scene boundary or review the
+quality of the title and summary.
+
+The model must receive only semantically useful material: the transcript chunk,
+the proposed scene kind, and the combat decision policy. Transcript unit IDs
+may remain visible as part of the ordinary source presentation, but the model
+must not be asked to reproduce them or the chunk ID, source range, hashes,
+validator keys, reason codes, or any other opaque application identifier.
+Application code owns all mapping, provenance, and correction metadata
+deterministically.
+
+## Target End State
+
+### Validator Module
+
+Add a typed validator package at
+`internal/modules/dnd/validate/scenedescriptions/combat_semantics` with the
+registered key
+`extract/dnd/scene-descriptions/combat_semantics`. The module will:
+
+- register for `dnd.SceneDescriptionListKind`;
+- declare `contracts.ExecutionClassLLMBacked`;
+- accept no module-specific options and reject unknown options;
+- require an extract-stage request containing one valid scene and the current
+  transcript chunk;
+- call the run-scoped scheduled `StructuredLLMClient` supplied through
+  `pipeline.BuildRequest`;
+- forward the resolved validator `llm_profile`, session ID, and structured
+  output repair override without inventing a separate concurrency or provider
+  path;
+- expose prompt/schema/policy fingerprints through the existing manifest and
+  checkpoint metadata conventions; and
+- return an ordinary `contracts.ValidationResult` whose semantic meaning is
+  fully owned by the module while terminal disposition remains framework and
+  pipeline policy.
+
+The module is intentionally extract-stage-specific. If it is configured for a
+merged or normalized value, receives no current chunk, receives anything other
+than exactly one scene, or cannot establish the required input invariants, it
+must return a contextual execution error rather than make a semantic judgment
+against incomplete or ambiguous material.
+
+### LLM Decision Contract
+
+The private LLM response will contain exactly two required fields:
+
+- `verdict`, one of `approved`, `combat_should_be_added`, or
+  `combat_should_be_removed`; and
+- `explanation`, a concise, transcript-grounded explanation of the decision.
+
+The JSON schema must reject unknown fields and require every declared field.
+It must contain no optional properties and no `uniqueItems` keyword. The prompt
+must force a best judgment and must not request a confidence score or an
+“uncertain” verdict; model-estimated uncertainty is a quality signal, not a
+process warning.
+
+The adapter will validate verdict consistency deterministically:
+
+- `approved` accepts either a supported combat classification or a supported
+  non-combat classification;
+- `combat_should_be_added` is valid only when the proposed kind is not
+  `combat`; and
+- `combat_should_be_removed` is valid only when the proposed kind is
+  `combat`.
+
+An inconsistent verdict, blank or over-limit explanation, malformed final
+response, missing prompt input, or completion error is a validator execution
+failure. PromptKit may repair structurally invalid output within its configured
+budget, and the framework may re-execute the validator within its distinct
+validator retry budget. The validator will not create a recursive semantic
+validation or feedback loop for its own response.
+
+On semantic mismatch, application code will assign stable internal reason
+codes and build bounded, actionable correction guidance. The guidance will
+state whether the corrected scene must be `combat` or must use the appropriate
+non-combat kind and will include the model's safe, bounded evidence explanation.
+It will not expose validator keys or reason codes to the producer. The existing
+producer retry mechanism will append that guidance to the exact defective
+scene-description response and request one complete replacement.
+
+### Prompt Assets
+
+Store the validator's LLM-facing assets under
+`assets/dnd/scene-descriptions/validate/combat-semantics/`, using the existing
+content-only root assets package and module-owned prompt registration pattern.
+The asset set will include a consistently named `prompt.yaml`, one concise
+module instruction file, and one private v1 response schema.
+
+The prompt declaration will use `dnd.scene_descriptions.validate_combat` as
+its prompt ID and `dnd-extraction` as its default profile. Stable shared and
+module policy messages will precede variable inputs so PromptKit backends can
+reuse identical prefixes. The prompt will reuse the shared D&D system and
+chunk-transcript assets rather than copying their contents. A small local input
+asset will present only the proposed scene kind; the complete scene artifact,
+its deterministic ID, and its source reference will not be rendered.
+
+Factor the combat/non-combat decision policy currently embedded in the
+scene-description extractor instructions into one `common-dnd-` shared asset
+selected by both the extractor and validator manifests. Keep the non-combat
+`narrative`, `recap`, and `meta` selection rules local to the extractor. The
+shared combat policy must be byte-identical for both consumers; do not create
+two near-duplicate definitions. The manifests, asset hashes, and prompt-cache
+tests must make the resulting ownership and message ordering explicit without
+using brittle exact-length assertions.
+
+### Registration And Configuration
+
+Register the validator builder and its prompt/schema assets through the D&D
+registrar. Catalog inspection must report its typed artifact kind and
+`llm_backed` execution class.
+
+The validator will be selectable in an extract validator override, but it will
+not be added to the production default chain by this feature. An operator who
+opts in must place it after the existing deterministic shape, source-reference,
+and durable-schema checks so malformed candidates are rejected before a paid
+LLM call. It should be last in the chain after advisory relatedness unless
+evaluation demonstrates a concrete reason to change that order.
+
+The validator uses the ordinary profile precedence and validator binding
+fields already documented by Notarius. It introduces no new configuration
+field, retry budget, failure policy, scheduler, or environment variable.
+Validator rejection and execution failure continue to follow the configured
+stage validation policy. Under the application defaults, exhausted semantic
+rejection fails the run, while exhausted validator execution failure may
+advance with an actionable process warning and incomplete-validation
+provenance.
+
+### Default-Chain Promotion Gate
+
+The final state for this roadmap is a production-quality, documented,
+operator-selectable validator that remains opt-in. It must not enter the
+registered production default chain until provider-backed evaluation shows
+that its accuracy, retry value, latency, and token cost justify enabling an
+additional model call for every scene-description chunk.
+
+Promotion, if later selected, requires a separate deliberate change to the
+default chain and maintained configuration expectations. It does not require a
+new architecture decision so long as the validator remains at the
+scene-description extract stage.
+
+## Required Code Changes
+
+The implementation must make the following coherent changes:
+
+- add the new typed validator, strict option decoder, LLM response model,
+  response interpretation, error wrapping, fingerprints, and registration;
+- add and register its embedded prompt, instruction, input, and response-schema
+  assets;
+- factor the combat classification policy into a shared prompt fragment if
+  doing so is necessary to keep the extractor and validator language exactly
+  identical;
+- update D&D registrar tests, prompt-asset registration tests, prompt-cache
+  ordering tests, and catalog/spec assertions for the new selectable key;
+- add focused validator tests with a fake structured LLM client for approval,
+  both rejection directions, inconsistent verdicts, malformed or failed
+  completions, input preconditions, profile/session/repair forwarding, bounded
+  explanations, correction guidance, and immutable candidate handling;
+- add an integration test proving that a semantic rejection from this
+  validator supplies useful correction guidance to a retry-capable
+  `dnd/scene-descriptions` producer and that the corrected candidate is
+  revalidated before acceptance;
+- retain and extend downstream contract coverage proving that combat-turn and
+  enemy-event extraction still runs only for an exact accepted `combat` scene;
+  and
+- update current configuration and D&D internal documentation to list the key,
+  describe its opt-in status and recommended chain position, explain its
+  process-failure behavior, and distinguish it from a future chunk-boundary
+  validator.
+
+No durable artifact schema or integration contract needs a version change.
+The validator produces validation provenance and correction behavior, not a
+new published artifact.
+
+## Evaluation Requirements
+
+Maintain a small human-reviewed evaluation set that exercises at least:
+
+- ordinary active combat and initiative-like exchanges;
+- combat preceded by setup or followed by immediate aftermath;
+- multi-phase encounters and brief rules interruptions;
+- planning, threats, hostile dialogue, and tense confrontations without active
+  combat;
+- aftermath, looting, and healing after combat has ended;
+- prior-session recap and in-world recollection of combat; and
+- sustained out-of-character combat rules discussion.
+
+Default automated tests must remain deterministic, offline, and provider-free.
+They should test prompt inputs, response interpretation, retry integration, and
+failure policy with fakes rather than asserting exact natural-language output.
+Provider-backed evaluation is an explicit manual or opt-in task. Record false
+acceptance, false rejection, successful producer correction, added calls,
+latency, and token use before proposing default-chain promotion.
+
+## Documentation And Architecture Record
+
+Update `docs/config.md` as the canonical owner of the selectable validator key,
+binding behavior, and chain placement. Update `docs/internal/dnd.md` as the
+canonical owner of the D&D-specific decision policy, prompt ownership, and
+module boundary. Link rather than duplicate the generic retry, diagnostic, and
+failure-policy contracts owned by their existing architecture and operator
+documentation.
+
+No new ADR is required for this scope. It applies the accepted generic
+validation and feedback-aware retry architecture without changing stage
+ownership or a durable contract. Create or supersede an ADR only if later work
+moves classification into `dnd/scenes`, adds annotations to the chunk plan, or
+otherwise transfers ownership across pipeline stages.
+
+## Out Of Scope
+
+- changing scene boundaries or deciding whether one combat was fragmented
+  across multiple chunks;
+- adding an LLM-backed chunk-plan validator;
+- moving scene kind into chunk-plan annotations;
+- changing the scene-description, combat-turn, or enemy-event durable schemas;
+- replacing the deterministic combat-only downstream extraction gate;
+- validating title or summary quality with this module;
+- adding confidence scores or surfacing model uncertainty as warnings;
+- adding a validator-specific concurrency or retry subsystem; and
+- enabling the validator in the production default chain before evaluation.
+
+## Acceptance Criteria
+
+- The new LLM-backed validator is registered for scene-description artifacts
+  and can be selected only through the existing validator binding mechanism.
+- It reviews exactly one extract-stage scene against the corresponding
+  transcript chunk and detects both missing and unsupported `combat` tags.
+- Its response contract and candidate presentation do not ask the model to
+  reproduce transcript unit IDs, chunk IDs, source ranges, hashes, validator
+  keys, or reason codes.
+- Its strict structured schema has only required fields, rejects unknown
+  fields, and avoids unsupported JSON Schema keywords.
+- Rejections produce bounded, transcript-grounded corrective guidance that the
+  existing scene-description producer retry can use.
+- Validator contract or execution failures remain distinct from semantic
+  rejection and follow existing configured policy.
+- The existing deterministic validator chain and downstream exact-combat gate
+  remain intact.
+- Prompt definitions share identical combat policy content rather than
+  maintaining near-duplicate instructions.
+- Automated coverage is deterministic and provider-free; an explicit
+  human-reviewed evaluation protocol exists for later default-chain review.
+- Current documentation describes implemented selection and behavior without
+  claiming that the validator is enabled by default.