diff --git a/docs/config.md b/docs/config.md index 032473b5..ba01c2b3 100644 --- a/docs/config.md +++ b/docs/config.md @@ -505,6 +505,9 @@ Available validator keys are: | Item registry | **extract/dnd/item-registry/shape**, **extract/dnd/item-registry/source_refs**, **extract/dnd/item-registry/source_relatedness**, **normalize/dnd/item-registry/identity** | | NPC occurrences | **extract/dnd/npc-occurrences/shape**, **extract/dnd/npc-occurrences/registry**, **extract/dnd/npc-occurrences/source_refs**, **extract/dnd/npc-occurrences/source_relatedness**, **normalize/dnd/npc-occurrences/invariants** | | Scene descriptions | **extract/dnd/scene-descriptions/shape**, **extract/dnd/scene-descriptions/source_refs**, **extract/dnd/scene-descriptions/source_relatedness**, **extract/dnd/scene-descriptions/combat_semantics** (LLM-backed, opt-in), **normalize/dnd/scene-descriptions/invariants** | +| Enemy events | **extract/dnd/enemy-events/shape**, **extract/dnd/enemy-events/engagements**, **extract/dnd/enemy-events/source_refs**, **extract/dnd/enemy-events/source_relatedness**, **normalize/dnd/enemy-events/invariants** | +| Location registry | **extract/dnd/location-registry/shape**, **extract/dnd/location-registry/source_refs**, **extract/dnd/location-registry/source_relatedness**, **normalize/dnd/location-registry/identity** | +| Location occurrences | **extract/dnd/location-occurrences/shape**, **extract/dnd/location-occurrences/registry**, **extract/dnd/location-occurrences/source_refs**, **extract/dnd/location-occurrences/source_relatedness**, **normalize/dnd/location-occurrences/invariants** | `extract/dnd/scene-descriptions/combat_semantics` is not in a production default chain. To opt in, replace the scene extractor validator chain with the current ordered chain plus the semantic validator last, and set a positive producer retry budget if a rejection should request a corrected scene: @@ -522,9 +525,6 @@ extract: ~~~ 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. -| Enemy events | **extract/dnd/enemy-events/shape**, **extract/dnd/enemy-events/engagements**, **extract/dnd/enemy-events/source_refs**, **extract/dnd/enemy-events/source_relatedness**, **normalize/dnd/enemy-events/invariants** | -| Location registry | **extract/dnd/location-registry/shape**, **extract/dnd/location-registry/source_refs**, **extract/dnd/location-registry/source_relatedness**, **normalize/dnd/location-registry/identity** | -| Location occurrences | **extract/dnd/location-occurrences/shape**, **extract/dnd/location-occurrences/registry**, **extract/dnd/location-occurrences/source_refs**, **extract/dnd/location-occurrences/source_relatedness**, **normalize/dnd/location-occurrences/invariants** | When no override is configured, production D&D bindings use the following ordered chains. Each row lists extract then normalize; spell chains are the diff --git a/docs/internal/dnd.md b/docs/internal/dnd.md index abf88f8e..5d3e7910 100644 --- a/docs/internal/dnd.md +++ b/docs/internal/dnd.md @@ -142,10 +142,55 @@ guidance. 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. -Before proposing default-chain inclusion, run a deliberate provider evaluation -against the synthetic corpus and record false acceptance, false rejection, -producer-correction success, added calls, latency, and token use. Default tests -remain offline and do not measure provider quality. +### 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. + +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. +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 + listed IDs and text, assign matching per-unit source references, and use + `source.MaterializeChunkPlan` with one range spanning those units. Construct + exactly one scene whose ID and source range match that chunk and whose kind + is the case's `proposed_kind`; title and summary may use fixed placeholders + because the validator neither receives nor evaluates them. Invoke the typed + 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`. + 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. +4. Evaluate producer correction separately with representative complete + scene-description runs configured as shown in + [Configuration](../config.md#production-validator-keys-and-default-chains). + For every initial semantic rejection, record whether the next producer + attempt returns the requested combat status and is approved. Do not count a + PromptKit structural repair as a producer-correction attempt. +5. Run the correction evaluation with debug capture enabled and without reused + extraction checkpoints. Record added validator and producer calls, elapsed + latency, and cumulative prompt, completion, cached, and total token usage + 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. Every D&D rejection describes the correction in transcript-grounded domain terms, using contextual names, artifact fields, and source segment ranges when diff --git a/docs/roadmap/combat-scene-validation.md b/docs/roadmap/combat-scene-validation.md deleted file mode 100644 index 7c616adf..00000000 --- a/docs/roadmap/combat-scene-validation.md +++ /dev/null @@ -1,303 +0,0 @@ -# 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. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index 4bc2ad4d..00000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,502 +0,0 @@ -# Implementation Plan - -## Objective - -Implement [D&D Combat Scene Semantic Validation](combat-scene-validation.md): -add the first production D&D LLM-backed validator as an optional extract-stage -validator for `dnd/scene-descriptions`, while preserving the existing chunk -plan, durable artifacts, downstream combat gate, maintained examples, and -production default validator chains. - -This plan is ordered and decision-complete. Each numbered stage is scoped for -one gpt-5.6-terra implementation prompt. Complete and validate each stage -before beginning the next one. - -## Decisions Shared By All Stages - -- Scene classification remains owned by the per-chunk - `dnd/scene-descriptions` extractor. Do not annotate `dnd/scenes` plans or add - a chunk-stage validator. -- Add the typed validator package - `internal/modules/dnd/validate/scenedescriptions/combat_semantics` with key - `extract/dnd/scene-descriptions/combat_semantics`, registered for - `dnd.SceneDescriptionListKind` with execution class `llm_backed`. -- The validator is selectable through the existing validator override but is - absent from all production default chains. It reviews only combat versus - non-combat, not title, summary, non-combat subtype, or boundary quality. -- Reuse the injected scheduled `StructuredLLMClient`, existing profile - precedence, PromptKit structural-repair budget, validator execution retries, - producer semantic retries, and validation failure policy. Add no parallel - provider, scheduler, retry, or configuration mechanism. -- Use prompt ID `dnd.scene_descriptions.validate_combat`, default profile - `dnd-extraction`, and private prompt/schema version `v1`. -- The LLM response has exactly two required fields: `verdict` and - `explanation`. Verdict is one of `approved`, `combat_should_be_added`, or - `combat_should_be_removed`. Reject unknown fields, optional properties, and - `uniqueItems`. Require a trimmed, nonblank explanation of at most 512 Unicode - code points in both schema and deterministic interpretation. -- Map rejections to application-owned reason codes - `scene_active_combat_not_classified` and - `scene_combat_classification_unsupported`. Codes, validator keys, opaque IDs, - and source coordinates are never model instructions. -- Transcript unit IDs may remain in ordinary source presentation, but the - model must not reproduce them. Candidate presentation contains only the - proposed kind; application code owns IDs, ranges, mapping, and provenance. -- PromptKit may repair structural validator output. A final malformed, - inconsistent, blank, or oversized validator response is an execution - failure, not a semantic rejection. Do not recursively validate or provide - semantic feedback to the validator's own LLM call. -- Rejection guidance tells the producer either to return `kind: combat` or to - choose the appropriate `narrative`, `recap`, or `meta` kind, and includes the - bounded transcript-grounded explanation. It contains no internal code. -- Keep durable schemas at their current versions and retain the exact `combat` - gates in combat-turn and enemy-event extraction. -- Default tests are deterministic, offline, and credential-free. Use fakes at - the structured LLM boundary. Do not add exact prompt-length, token-count, or - complete prose snapshot tests. -- No ADR is required. This feature applies the architecture and ADRs 0012, - 0014, and 0015 without changing stage ownership or a durable contract. - -## Stage 1: Extract The Shared Combat Policy - -✅ Complete - -### Goal - -Give the existing extractor and future validator one byte-identical owner for -the combat decision policy without changing the extractor contract or the -common extraction prompt prefix. - -### Work - -1. Add `assets/dnd/shared/prompts/common-dnd-scene-combat-policy.md` and move - into it the current combat definition, mixed-chunk precedence, and explicit - non-examples: planning, threats, hostile dialogue, aftermath, recollection, - and rules discussion without active encounter play. -2. Remove those rules from the local scene-description instructions. Retain - the exactly-one-scene task, all four supported kind names, the - `narrative`/`recap`/`meta` definitions and precedence, and title/summary - policy. The rendered prompt must remain complete and non-repetitive. -3. Add the shared file to the scene extractor's `PromptAssetManifest` and - `prompt.yaml`. Preserve the extraction-wide prefix through the cached - transcript. Place the combat-policy message after the transcript and before - local instructions so other extraction lanes retain an identical prefix. -4. Update prompt asset and composition tests to prove the shared policy appears - once in the correct relative position. Use semantic sentinels rather than a - full prompt snapshot or exact size assertion. -5. Do not change the prompt ID/version, response schema, extractor logic, - artifact, or validator chains. - -### Inspect - -- `assets/dnd/shared/prompts/` -- `assets/dnd/scene-descriptions/prompts/` -- `internal/modules/dnd/extract/scenedescriptions/prompt_assets.go` -- `internal/modules/dnd/extract/scenedescriptions/prompt_assets_test.go` -- `internal/modules/dnd/register/prompt_cache_test.go` - -### Acceptance And Validation - -- One embedded shared asset owns the combat policy. -- The scene prompt still prepares with complete classification instructions. -- All extraction prompts retain the same prefix through the transcript. -- No brittle change-detector test is added. - -Run: - -```sh -go test ./internal/modules/dnd/extract/scenedescriptions -go test ./internal/modules/dnd/register -git diff --check -``` - -This stage is small enough for one implementation prompt. - -## Stage 2: Add Validator Prompt And Schema Assets - -✅ Complete - -### Goal - -Create and locally verify the private PromptKit contract before implementing -semantic logic or global registration. - -### Work - -1. Create - `assets/dnd/scene-descriptions/validate/combat-semantics/` with: - - - `prompts/prompt.yaml`; - - `prompts/instructions.md`; - - `prompts/proposed-kind.md`; and - - `schemas/dnd_scene_combat_semantics_llm.v1.json`. - -2. Define required inputs `transcript` (`application/json`) and - `proposed_kind` (`text/plain`). Order messages as system, shared combat - policy, stable local validator instructions, variable proposed kind, and - final variable shared chunk transcript. Mark only the two variable messages - ephemeral. Do not select campaign references or the identity fragment. -3. Define the verdict semantics precisely. `approved` approves only the - proposed combat status, not the whole scene description. -4. Define a strict object schema with `additionalProperties: false`, both - fields in `required`, the three-value verdict enum, and explanation - `minLength: 1` and `maxLength: 512`. Include no confidence, ID, range, - diagnostics, or correction field. -5. Add package-local asset/schema plumbing in `combat_semantics`, following - existing D&D manifests. Use: - - - schema key `dnd_scene_combat_semantics_llm`; - - schema ID `notarius.dnd.scene_descriptions.combat_semantics.llm`; and - - schema name `notarius_dnd_scene_combat_semantics_llm_v1`. - - Expose `RegisterPromptAssets`, cached asset metadata, and mutation-safe - schema loading. Do not register them at the D&D composition root yet. -6. Add focused tests that prepare the embedded prompt, require both inputs, - accept all valid verdicts, and reject missing, extra, mistyped, blank, and - oversized fields. Metadata tests must prove hashes are available without - exposing raw assets. - -### Inspect - -- existing scene-description extractor asset and schema code -- analogous D&D semantic-reconciliation prompt assets -- `internal/framework/llm` -- `internal/framework/promptfs` - -### Acceptance And Validation - -- The package-local embedded prompt and schema prepare offline. -- All schema fields are required and no `uniqueItems` appears. -- The prompt contains only the policy, proposed kind, and transcript needed for - this decision and selects rather than copies the shared policy. -- No global registration changes occur yet. - -Run: - -```sh -go test ./internal/modules/dnd/validate/scenedescriptions/combat_semantics -go test ./internal/modules/dnd/extract/scenedescriptions -git diff --check -``` - -This stage is small enough for one implementation prompt. - -## Stage 3: Implement The Typed Validator - -✅ Complete - -### Goal - -Implement package-local validator construction, preconditions, structured -completion, verdict interpretation, corrections, fingerprints, and behavioral -tests. - -### Work - -1. Define the module key, two reason codes, zero-field strict `Options`, private - response type, and validator holding only the injected client and immutable - prompt/schema/policy metadata. Construction fails contextually for a nil LLM - dependency. -2. Register through a typed builder consuming `request.Dependencies.LLM`. - Retain no unrelated references or mutable request maps. -3. Before calling the model, require extract stage, non-nil source and chunk, - valid chunk-scoped source input, exactly one scene satisfying the existing - extract shape contract, and scene ID/range equal to the current chunk. - Violations are execution errors, not approval or skip. -4. Call `CompleteStructured` once per execution with validator key as stage - name; the selected prompt/version; request profile, session ID, and repair - pointer; cloned chunk source input as `transcript`; and only the scene kind - string as `proposed_kind`. Do not forward references, scene prose, - identifiers, ranges, reason codes, or semantic correction. -5. Interpret output deterministically: - - - trim and enforce the 512-code-point explanation bound; - - approve `approved` for either proposed combat status; - - accept `combat_should_be_added` only for a non-combat proposal; - - accept `combat_should_be_removed` only for a combat proposal; and - - treat every unknown or inconsistent result as execution failure. - -6. Build bounded `ValidationResult` messages and semantic correction guidance. - Approved results carry no rejection message or quality diagnostic. -7. Implement `Name`, `ExecutionClass`, `Spec`, manifest metadata, checkpoint - fingerprints, and strict option validation. A validator spec does not - declare a producer correction protocol. -8. Add consolidated table-driven tests with a recording fake LLM for approvals, - both rejection directions, inconsistent results, explanation bounds, input - preconditions, completion error/cancellation, request-field forwarding, - model-visible input exclusions, option rejection, and metadata ownership. - -### Inspect - -- `internal/framework/contracts/{contracts,typed_pipeline}.go` -- `internal/framework/pipeline/{construction,validator_registry}.go` -- `internal/modules/dnd/validate/scenedescriptions/{shape,source_refs}` -- existing LLM-backed D&D extractor and normalizer packages - -### Acceptance And Validation - -- The package is usable without provider-specific types or global state. -- It cannot judge a wrong-stage, aggregate, malformed, or wrong-chunk value. -- Semantic rejection and execution failure remain distinct. -- Guidance is meaningful and contains no opaque identifiers or internal codes. -- Consequential contracts are tested offline through the package boundary. - -Run: - -```sh -go test ./internal/modules/dnd/validate/scenedescriptions/... -go vet ./internal/modules/dnd/validate/scenedescriptions/... -git diff --check -``` - -This stage is small enough for one implementation prompt. - -## Stage 4: Register And Resolve The Optional Validator - -✅ Complete - -### Goal - -Expose the module through production D&D composition and prove explicit -configuration works without changing defaults. - -### Work - -1. Register the validator builder in - `internal/modules/dnd/register/validators.go` and its assets in - `internal/modules/dnd/register/modules.go`, preserving deterministic - registration and contextual errors. -2. Extend registrar/catalog tests to prove the typed kind, execution class, - production prompt/schema availability, and absence from both scene default - chains. -3. Add one focused resolution/preparation case whose scene extract override - repeats the current default validators in order and appends the semantic - validator last. Prove validator `llm_profile`, - `structured_output_repair_attempts`, and `retries` reach the prepared - LLM-backed validator through existing machinery. -4. Add a negative case for selecting this typed validator against an - incompatible artifact kind if existing typed-registry tests do not already - exercise the concrete production key. Rely on generic tests for all other - validator-binding rules. -5. Do not alter the minimal or complete D&D example configurations. - -### Inspect - -- `internal/modules/dnd/register/{validators,modules,chains}.go` -- `internal/modules/dnd/register/register_test.go` -- `internal/modules/dnd/register/prompt_cache_test.go` -- typed validator resolution/preparation tests under - `internal/framework/pipeline` - -### Acceptance And Validation - -- Production registration exposes the validator and prompt. -- An explicit compatible override resolves before execution. -- Default chain membership/order and maintained examples are unchanged. -- Incompatible selection still fails during resolution or preparation. - -Run: - -```sh -go test ./internal/modules/dnd/register -go test ./internal/framework/pipeline -go test ./internal/cli -git diff --check -``` - -This stage is small enough for one implementation prompt. - -## Stage 5: Prove Feedback-Aware Producer Correction - -✅ Complete - -### Goal - -Prove the assembled D&D path can reject a well-formed wrong scene kind, guide -the scene producer, and accept a corrected replacement without leaking -internal identifiers. - -### Work - -1. Add one focused test under `internal/modules/integration` (or extend the - narrowest assembled scene test) with a stateful fake structured client that - distinguishes scene producer and semantic validator prompt IDs. -2. Configure the current deterministic scene extract chain followed by the - semantic validator, with a positive producer retry budget and no unnecessary - validator execution retries. -3. Use a chunk containing clear active combat. Return a valid non-combat scene - first, `combat_should_be_added` from the validator, a corrected combat scene - after feedback, and approval on revalidation. -4. Assert outcomes rather than private choreography: - - - the accepted normalized scene is `combat`; - - superseded attempts leave no rejection or process warning; - - the producer receives the exact latest defective model candidate and one - semantic correction request; - - guidance directs `kind: combat` and includes the bounded explanation; - - guidance excludes validator key, reason code, chunk ID, and source range; - and - - the corrected candidate traverses the complete validator chain again. - -5. Keep unsupported-combat verdict mapping owned by Stage 3 package tests - unless adding it to this integration test is materially cheaper than a - duplicate assembled setup. -6. Rely on existing framework tests for aggregate guidance, exhaustion, - `warn_continue`, validator retries, and checkpoint ineligibility. Change - generic framework code only if this test exposes a real defect, and report - the scope expansion explicitly. - -### Inspect - -- `internal/modules/integration/` -- framework extract-correction and semantic-correction tests -- `internal/cli/assembled_spell_pipeline_contract_test.go` -- `internal/modules/dnd/extract/scenedescriptions/` - -### Acceptance And Validation - -- One deterministic assembled test proves D&D semantic correction and - revalidation. -- Only the latest defective response and semantic guidance reach the producer. -- No internal code or deterministic identifier reaches correction text. -- Existing framework budgets and policies remain the only orchestration path. - -Run: - -```sh -go test ./internal/modules/integration -go test ./internal/framework/pipeline -go test ./internal/modules/dnd/extract/scenedescriptions -git diff --check -``` - -This stage is small enough for one implementation prompt. - -## Stage 6: Add Evaluation Material And Documentation - -✅ Complete - -### Goal - -Provide accurate opt-in configuration guidance and a bounded human-reviewed -corpus for evaluation before any future default-chain proposal. - -### Work - -1. Add a synthetic, non-sensitive corpus at - `internal/modules/dnd/validate/scenedescriptions/combat_semantics/testdata/evaluation_cases.json`. - Each case has a human-readable unique name, transcript units, proposed kind, - expected verdict, and reviewer rationale. Cover active combat, setup and - aftermath around combat, multi-phase combat, rules interruption, planning, - threats, hostile dialogue, aftermath without combat, recap/recollection, - and sustained out-of-character discussion. -2. Add one cheap fixture-contract test for strict decoding, unique/nonblank - names, nonblank input, supported proposed kinds/verdicts, and representation - of all verdict classes. Do not assert the exact fixture count or treat fake - responses as a measurement of model quality. -3. Document a manual or explicitly opt-in provider evaluation procedure in - `docs/internal/dnd.md`. Require recording false acceptance, false rejection, - producer correction success, added calls, latency, and token use before - default-chain promotion. Do not add live calls to the default suite. -4. Update `docs/config.md`, the canonical configuration owner, to list the key, - state its LLM-backed opt-in status, and show the smallest illustrative - override that repeats the existing scene extract chain and appends the - semantic validator. Emphasize that an override replaces the chain and that - a positive producer retry budget is required if rejection should trigger a - corrected scene-description attempt. Link to the existing binding table - instead of repeating profile, structural-repair, producer-retry, or - validator-retry rules. -5. Update `docs/internal/dnd.md` to describe the module boundary, prompt-policy - ownership, verdict mapping, deterministic correction mapping, and difference - from deferred boundary-coherence review. Link to configuration and generic - retry/failure owners rather than duplicating volatile contracts. -6. Update `docs/internal/overview.md` or `docs/internal/modules.md` only if its - implemented inventory would otherwise become false. Do not add an ADR, a - third example, or a durable integration contract change. - -### Inspect - -- `docs/{config,internal/dnd,internal/modules,internal/overview}.md` -- all files under `docs/policy/` -- the two maintained D&D example configurations - -### Acceptance And Validation - -- The corpus is bounded, synthetic, strict, and covers the intended cases - without masquerading as a provider quality test. -- Configuration documents one correct opt-in path and preserves replacement - semantics. -- Canonical documentation owners describe only behavior implemented in Stages - 1–5, with no volatile duplication or default-enabled claim. -- All new relative Markdown links resolve manually; the repository currently - has no automated documentation link checker. - -Run: - -```sh -go test ./internal/modules/dnd/validate/scenedescriptions/combat_semantics -go test ./internal/modules/dnd/register ./internal/framework/pipeline ./internal/cli -git diff --check -``` - -This stage is small enough for one implementation prompt. - -## Stage 7: Final Verification And Scope Audit - -✅ Complete - -### Goal - -Verify the feature as one coherent change and leave it ready for a separate -post-implementation review. - -### Work - -1. Compare the complete diff with the feature roadmap and every stage above. - Implement or report every unmet acceptance criterion; do not silently defer - required work. -2. Confirm by inspection that the validator is registered but absent from - default chains/examples; both consumers select the shared combat policy; no - duplicate policy remains; its schema has all fields required and no - `uniqueItems`; model-facing content requests no opaque IDs or internal - codes; it uses the scheduled client and existing budgets; and downstream - exact-combat gates are unchanged. -3. Format changed Go files. Remove only redundant tests, stale comments, unused - helpers, or roadmap-inconsistent documentation found during this review; do - not perform unrelated refactoring. -4. Run the full validation suite. If an environmental limitation blocks a - command, run unaffected checks and record the exact limitation rather than - weakening tests. -5. Leave both roadmap documents in place for the post-implementation audit. Do - not promote the validator or retire the roadmaps. - -### Acceptance And Validation - -- Focused and repository-wide checks pass. -- No durable schema, chunk plan, default chain, example, or downstream gate - changed outside the roadmap. -- The worktree contains no binary, credential, private transcript, ignored - debris, or unrelated modification. -- Documentation paths and links resolve, and the repository is ready for - completeness and code-quality review. - -Run: - -```sh -gofmt -w -go test ./... -go test -race ./... -go vet ./... -go build ./cmd/notarius -git diff --check -git status --short --untracked-files=all -``` - -This stage is small enough for one implementation prompt. - -## Open Questions - -None. The feature roadmap and this plan fix the stage, scope, schema, prompt -ownership, reason-code mapping, configuration status, failure semantics, -evaluation gate, and documentation ownership. Default-chain promotion and -chunk-boundary semantic review are explicitly outside this plan. diff --git a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator.go b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator.go index 947031a4..80d3587c 100644 --- a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator.go +++ b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator.go @@ -196,13 +196,10 @@ func validateExplanation(value string) (string, error) { if trimmed == "" { return "", validatorErrorf("validator explanation must not be blank") } - if trimmed != value { - return "", validatorErrorf("validator explanation must be trimmed") - } - if utf8.RuneCountInString(value) > maximumExplanationRunes { + if utf8.RuneCountInString(trimmed) > maximumExplanationRunes { return "", validatorErrorf("validator explanation exceeds %d Unicode code points", maximumExplanationRunes) } - return value, nil + return trimmed, nil } func Spec() pipeline.ValidatorSpec { 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 4fabdd8e..91dd456d 100644 --- a/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator_test.go +++ b/internal/modules/dnd/validate/scenedescriptions/combat_semantics/validator_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "io" "os" "reflect" "strings" @@ -24,6 +25,7 @@ func TestValidatorInterpretsCombatSemanticsResponses(t *testing.T) { approved bool reasonCode string guidance string + correction string wantError string }{ {name: "approve non-combat", kind: dnd.SceneKindNarrative, response: completionResponse{Verdict: "approved", Explanation: "No active encounter occurs."}, approved: true}, @@ -34,7 +36,7 @@ func TestValidatorInterpretsCombatSemanticsResponses(t *testing.T) { {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: "untrimmed explanation", kind: dnd.SceneKindNarrative, response: completionResponse{Verdict: "approved", Explanation: " evidence"}, wantError: "trimmed"}, + {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"}, } { t.Run(test.name, func(t *testing.T) { @@ -51,6 +53,9 @@ func TestValidatorInterpretsCombatSemanticsResponses(t *testing.T) { if err != nil || result.Approved != test.approved || result.ReasonCode != test.reasonCode || !strings.Contains(result.CorrectionGuidance, test.guidance) { t.Fatalf("Validate() = %#v, %v", result, err) } + if test.correction != "" && result.CorrectionGuidance != test.correction { + t.Fatalf("CorrectionGuidance = %q, want %q", result.CorrectionGuidance, test.correction) + } if !result.Approved && (strings.Contains(result.CorrectionGuidance, Key) || strings.Contains(result.CorrectionGuidance, result.ReasonCode) || strings.Contains(result.CorrectionGuidance, request.Chunk.ID)) { t.Fatalf("correction guidance leaked opaque application detail: %q", result.CorrectionGuidance) } @@ -188,6 +193,9 @@ func TestEvaluationCasesAreStrictAndCoverVerdicts(t *testing.T) { if err := decoder.Decode(&cases); err != nil { t.Fatalf("decode evaluation cases: %v", err) } + 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{} for _, value := range cases { if strings.TrimSpace(value.Name) == "" || seenNames[value.Name] || len(value.TranscriptUnits) == 0 || strings.TrimSpace(value.ProposedKind) == "" || strings.TrimSpace(value.ReviewerRationale) == "" {