diff --git a/docs/cli.md b/docs/cli.md index 442deea..a4d95c9 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -47,7 +47,8 @@ Flags: invocation. - `--llm-profile id`: override every effective LLM-capable pipeline module binding to use one Scriptorium profile ID. Validator-specific profiles are - not overridden. + not overridden. Configured LLM-backed validators with explicit profiles are + validated against the configured Scriptorium profile source. - `--session-id id`: pass a stable prompt session identifier through LLM-backed module calls. - `--reference selector=path`: bind a reference path to a chunk, extractor, @@ -208,9 +209,31 @@ The production CLI currently registers these module keys: - normalize: `noop` - output: `json` +## Implemented Production Validators + +The production CLI currently registers these validator keys: + +- `generic/always_accept` +- `generic/always_reject` +- `generic/valid_json` +- `generic/valid_json_schema` +- `extract/dnd/spells/shape` +- `extract/dnd/spells/source_refs` +- `extract/dnd/spells/source_relatedness` + +The production default chain for the `dnd/spells` extractor is: + +1. `generic/valid_json` +2. `generic/valid_json_schema` +3. `extract/dnd/spells/shape` +4. `extract/dnd/spells/source_refs` +5. `extract/dnd/spells/source_relatedness` + Validator chain overrides are configured on `chunk`, lane `extract`, lane -`merge`, and lane `normalize` bindings. Validator keys are resolved against the -registered validator catalog. +`merge`, and lane `normalize` bindings. Omitted overrides use production +defaults, `validators: []` disables validation for that binding, and non-empty +lists replace the default chain in configured order. Validator keys are resolved +against the registered validator catalog. For YAML structure, Scriptorium profile sources, environment overrides, and module binding syntax, see [Configuration](config.md). diff --git a/docs/config.md b/docs/config.md index 8cc8dda..4efacf2 100644 --- a/docs/config.md +++ b/docs/config.md @@ -258,6 +258,10 @@ binding to use one Scriptorium profile ID: chunk, every selected lane extract, merge, and normalize binding. It does not override validator-specific `llm_profile` values. +Configured LLM-backed validators with explicit `llm_profile` values are +validated against the configured Scriptorium profile source. Deterministic +production validators do not call the LLM and must not set `llm_profile`. + ## Implemented Production Modules | Slot | Key | Notes | @@ -270,6 +274,32 @@ merge, and normalize binding. It does not override validator-specific | normalize | `noop` | Passes merged raw outputs through unchanged. | | output | `json` | Produces JSON output files for normalized `application/json` lanes. | +## Implemented Production Validators + +| Key | Execution | Notes | +| --- | --- | --- | +| `generic/always_accept` | deterministic | Accepts returned module output. | +| `generic/always_reject` | deterministic | Rejects returned module output with reason `always_reject`. | +| `generic/valid_json` | deterministic | Rejects payloads that are not syntactically valid JSON. | +| `generic/valid_json_schema` | deterministic | Rejects invalid JSON or JSON that does not conform to the module response schema. | +| `extract/dnd/spells/shape` | deterministic | Rejects malformed D&D spell-cast JSON payloads. | +| `extract/dnd/spells/source_refs` | deterministic | Rejects missing or invalid D&D spell source references. | +| `extract/dnd/spells/source_relatedness` | deterministic | Emits warnings when a spell name is not found near its cited source text. | + +The production default chain for the `dnd/spells` extractor is: + +```yaml +validators: + - generic/valid_json + - generic/valid_json_schema + - extract/dnd/spells/shape + - extract/dnd/spells/source_refs + - extract/dnd/spells/source_relatedness +``` + +No other production module currently has a default validator chain. Empty +chains approve output by default. + The `generic` chunker accepts: - `max_units`: positive integer, default `50`; @@ -326,6 +356,10 @@ Pipeline resolution additionally checks: - required module keys are present; - module keys are registered for the expected slot; - module capability requirements are satisfied; +- non-empty validator overrides reference registered validator keys; +- deterministic validators do not set `llm_profile`; +- LLM-backed validators with explicit `llm_profile` values reference configured + Scriptorium profile IDs; - bound reference slots are declared by selected chunk, extractor, merger, or normalizer targets; - required reference slots are bound for selected targets. diff --git a/docs/integrations/json-output.md b/docs/integrations/json-output.md index d446f3f..7a99f04 100644 --- a/docs/integrations/json-output.md +++ b/docs/integrations/json-output.md @@ -87,7 +87,28 @@ sanitizing the lane ID: "stage": "extract", "lane_id": "spells", "module_key": "dnd/spells", - "validators": [] + "validators": [ + { + "key": "generic/valid_json", + "execution_class": "deterministic" + }, + { + "key": "generic/valid_json_schema", + "execution_class": "deterministic" + }, + { + "key": "extract/dnd/spells/shape", + "execution_class": "deterministic" + }, + { + "key": "extract/dnd/spells/source_refs", + "execution_class": "deterministic" + }, + { + "key": "extract/dnd/spells/source_relatedness", + "execution_class": "deterministic" + } + ] } ], "validation_status": "approved", @@ -114,7 +135,8 @@ references. `validator_chains` records the resolved validator chain for each validation point. Entries include stage, lane ID when applicable, module key, and validators with key and execution class. Empty chains are recorded with an empty -`validators` array. +`validators` array, including chains resolved from explicit empty config +overrides. `normalized_outputs` summarizes each normalized lane output without embedding payload bytes. Entries include lane ID, normalizer module key, source ID, media diff --git a/docs/internal/llm.md b/docs/internal/llm.md index 76bb709..e359bc5 100644 --- a/docs/internal/llm.md +++ b/docs/internal/llm.md @@ -19,9 +19,10 @@ structured output. The response also carries the raw structured output bytes returned by the runtime so modules can preserve raw payloads in pipeline stage outputs. -Modules that call the LLM own their prompts, schemas, prompt IDs, validators, -and domain-specific interpretation. Provider adapters should not contain -domain-specific prompt logic. +Modules that call the LLM own their prompts, schemas, prompt IDs, and +domain-specific interpretation. Validator packages own approve/reject policy, +and central catalog mappings decide which validators run by default. Provider +adapters should not contain domain-specific prompt logic. Prompt input materials carry source or reference bytes with optional origin metadata. The Scriptorium-backed runtime receives them as named artifacts rather @@ -43,9 +44,11 @@ The runtime records the actual selected Scriptorium profile, provider, and model used during execution. Manifest population does not rely on a precomputed profile ID before pipeline execution. -Explicit profile validation and `--llm-profile` overrides apply to LLM-capable -pipeline stages: chunk, extract, merge, and normalize. Input, output, and -validator bindings are not part of the current production LLM profile scope. +Explicit profile validation applies to LLM-capable pipeline stages: chunk, +extract, merge, normalize, and LLM-backed validators with explicit +`llm_profile` values. Input, output, and deterministic validators do not call +the LLM. The `--llm-profile` run flag overrides effective chunk, extract, merge, +and normalize bindings; it does not override validator-specific profiles. ## Scriptorium Adapter diff --git a/docs/internal/modules.md b/docs/internal/modules.md index 71ad88c..73e975e 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -5,6 +5,9 @@ contract from `internal/framework/contracts`, exposes a `ModuleSpec`, and registers itself with the matching pipeline registry. The CLI production catalog currently registers only the modules listed here. +Validator implementations live under `internal/validators` and are registered +separately from modules. Production default validator chains are central CLI +catalog policy; module packages do not own their default validation chains. ## Contract Pattern @@ -181,6 +184,12 @@ metadata under `artifact_lanes[].metadata.extractor`. Durable raw output details belong in the [D&D spell raw output contract](../integrations/dnd-spell-artifacts.md). +The production catalog validates `dnd/spells` raw extract output with generic +JSON validators followed by D&D spell validators under +`internal/validators/extract/dnd/spells`. The extractor itself remains +responsible for prompt, schema, and raw output production rather than +approve/reject policy. + The `dnd/scenes` chunker and `dnd/spells` extractor declare optional `players`, `party`, and `glossary` reference slots accepting UTF-8 plain text, Markdown, YAML, or JSON. They also accept `roster` as a deprecated compatibility alias for diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index 6b2f549..fc6ef95 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -75,7 +75,8 @@ prompt execution metadata. `pipeline.Registries` holds concrete constructors for execution. A `pipeline.ModuleCatalog` exposes module specs for config validation and -resolution. +resolution. The catalog also exposes validator specs and central default +validator-chain mappings without constructing modules or validators. Every production module registers a `ModuleSpec` with: @@ -91,6 +92,14 @@ instances. Input, validate, and output specs must not declare reference slots. Capability checks prevent incompatible pipeline composition before a run starts. +Every production validator registers a `ValidatorSpec` with: + +- `Key`: validator key used in config and manifests; +- `ExecutionClass`: `deterministic` or `llm_backed`. + +Default validator chains are keyed by workflow stage and module key. Production +currently registers a default chain for `extract` module `dnd/spells` only. + ## Runner Input And Output `pipeline.RunInput` carries: @@ -118,8 +127,8 @@ The runner: 2. builds the input adapter and parses the raw input into a source document; 3. validates the source document; 4. builds the chunker and produces source chunks, retrying when configured; -5. validates source chunks against framework invariants and any registered raw - chunk validators; +5. validates source chunks against framework invariants and the resolved chunk + validator chain; 6. runs each selected artifact lane in sorted resolved order; 7. builds the output encoder and validates logical output file names. 8. passes accepted normalized raw outputs, rejected output records, warnings, @@ -181,25 +190,38 @@ Within an artifact lane, the runner: ## Validators -The current runner handoff is raw-output based. Extractors, mergers, and +The runner handoff is raw-output based. Chunkers, extractors, mergers, and normalizers do not advertise validator chains through their module interfaces. -Runner-side raw validation chains receive the raw module output plus stage, -lane, module, source, chunk, schema, session, reference, LLM client/profile, -binding option, and run metadata context. Merge validators also receive the -ordered extract outputs used by the merge, and normalize validators receive the -accepted merge output. Empty raw validation chains approve output by default. -Response-schema provenance may include in-memory JSON schema bytes for -validators. Those bytes are omitted from manifests, diagnostics, and encoded -output files. +Resolved validation chains receive the raw module output plus stage, lane, +module, source, chunk, schema, session, reference, LLM client/profile, binding +option, and run metadata context. Chunk validators receive the chunk result +collection, merge validators receive the ordered extract outputs used by the +merge, and normalize validators receive the accepted merge output. Empty chains +approve output by default. Response-schema provenance may include in-memory JSON +schema bytes for validators. Those bytes are omitted from manifests, +diagnostics, and encoded output files. Resolved validator chains come from central default mappings unless a stage-local config override is set on `chunk`, lane `extract`, lane `merge`, or lane `normalize`. Explicit empty overrides are valid and are recorded as empty -chains in manifests. +chains in manifests. Explicit non-empty overrides replace the default chain and +preserve configured order. + +The production default chain for `extract` module `dnd/spells` is: + +1. `generic/valid_json` +2. `generic/valid_json_schema` +3. `extract/dnd/spells/shape` +4. `extract/dnd/spells/source_refs` +5. `extract/dnd/spells/source_relatedness` + +No other production module currently has a default validator chain. Validator rejection is a non-fatal run outcome: the rejected output is recorded in `RunOutput.Rejected` and does not pass to the next stage. Validator execution errors are framework-level errors and retry according to the relevant binding. +Warning-only validators return approved results with warnings; those warnings +are promoted only from successful attempts whose outputs are used. ## Warnings And Failures diff --git a/docs/policy/architecture.md b/docs/policy/architecture.md index dfbfb32..0e7cb83 100644 --- a/docs/policy/architecture.md +++ b/docs/policy/architecture.md @@ -82,8 +82,9 @@ format-specific validation rules. They should not own extraction-domain decisions. Extract modules own artifact semantics, prompt usage, structured response schema -selection, validator defaults, and domain-specific interpretation. They should -depend on framework contracts and core source/artifact types, not concrete input +selection, and domain-specific interpretation. They should depend on framework +contracts and core source/artifact types, not concrete input module packages. +Production validation defaults are central catalog policy, not behavior owned by module packages. Merge modules combine extracted candidates. Normalize modules reconcile merged @@ -99,14 +100,19 @@ warnings. Validators should be independently testable and composable. -Deterministic validators should run before LLM-backed validators when both are -present. Validator decision semantics should be explicit: each candidate -artifact evaluated by a validator should receive exactly one decision from that -validator. +Validators evaluate immutable module outputs returned by `chunk`, `extract`, +`merge`, and `normalize` stages. Validator decision semantics should be +explicit: each validator call approves, rejects, or approves with warnings for +the whole module output it receives. Validator rejection records rejected raw +output; validator execution errors are framework errors. -LLM-backed review belongs in module-owned validator chains, not in an implicit -global review phase. Extract and normalize modules may both use deterministic -and LLM-backed validators. +Default validator chains belong in central production catalog mappings keyed by +stage and module key. Pipeline configuration may override those mappings at the +stage-local module binding. Empty chains are valid and approve by default. + +Deterministic validators should run before LLM-backed validators in production +defaults when both are present. Configured validator order is authoritative and +must not be silently reordered. Shared validator runtime mechanics belong in framework code. Concrete validator behavior belongs in module or validator implementation packages. diff --git a/docs/roadmap/future.md b/docs/roadmap/future.md index be78b4f..9909d19 100644 --- a/docs/roadmap/future.md +++ b/docs/roadmap/future.md @@ -20,7 +20,8 @@ future work only. if references become large enough to require preprocessing. - Cross-lane entity normalization. - Cross-chunk semantic deduplication. -- Configurable validator chains with production validator modules. +- Additional validator packages and production default chains for future + modules. - Parallel execution where it preserves deterministic manifests and diagnostics. - Additional output encoders. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 94ace03..e2d2b40 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,486 +1,16 @@ -# Validation Refactor Implementation Plan +# Validation Refactor Implementation -This plan implements the target state described in -[validation.md](validation.md). Follow the stages in order. Do not skip the -focused tests for a stage before moving to the next one. +The validation refactor described by this temporary implementation plan has +been completed. -## Stage 1: Replace Legacy Validator Contracts With One Raw Output Contract +Current behavior is documented in: -Goal: make the raw module-output validation boundary the only framework -validator contract. +- [Configuration](../config.md) +- [CLI Reference](../cli.md) +- [Pipeline Internals](../internal/pipeline.md) +- [Modules](../internal/modules.md) +- [JSON Output](../integrations/json-output.md) +- [Troubleshooting](../troubleshooting.md) -Tasks: - -- In `internal/framework/contracts`, replace the legacy candidate-oriented - validator contract with a module-output contract. -- Remove these legacy types after updating callers: - - `ValidationDecision`; - - candidate-oriented `ValidationRequest`; - - candidate-oriented `ValidationResult`; - - legacy `Validator` behavior that works over `artifacts.ArtifactCandidate`. -- Remove `RawValidator`, `RawValidationRequest`, and `RawValidationResult`. - The new `Validator` contract covers the current raw validation use case - directly. -- Define execution class metadata: - - ```go - type ExecutionClass string - - const ( - ExecutionClassDeterministic ExecutionClass = "deterministic" - ExecutionClassLLMBacked ExecutionClass = "llm_backed" - ) - ``` - -- Define the new request shape around immutable module output. It should carry: - - `Stage` as a string-compatible stage identifier; - - `LaneID`; - - `ModuleKey`; - - `Source` and `SourceID`; - - `SourceInput`; - - `SessionID`; - - `References`; - - `LLMClient`; - - `LLMProfile`; - - `Options`; - - `Metadata`; - - `Schema`; - - `Payload`; - - chunk provenance for extract validation; - - `Chunks` for chunk validation; - - ordered `ExtractOutputs` for merge validation; - - `MergeOutput` for normalize validation. -- Define the result shape as one decision over the whole module output: - - ```go - type ValidationResult struct { - Approved bool - ReasonCode string - Message string - DiagnosticArtifactPath string - Warnings []Warning - } - ``` - -- Define `Validator` as: - - ```go - type Validator interface { - Name() string - ExecutionClass() ExecutionClass - Validate(ctx context.Context, req ValidationRequest) (ValidationResult, error) - } - ``` - -- Update tests under `internal/framework/contracts` so fake validators exercise - the new contract and no longer reference artifact candidates. -- Keep `artifacts.ArtifactCandidate` only if other non-validation code still - needs it. If it becomes unused after later stages, remove it in Stage 6. - -Design requirements: - -- Validators must not mutate request payloads, source documents, chunks, or - upstream outputs. -- Validator execution errors are framework errors. Validator semantic rejection - is represented by `ValidationResult{Approved:false}`. -- Empty validation chains still approve output by default. - -Focused tests: - -```sh -go test ./internal/framework/contracts -go test ./internal/framework/validate -``` - -## Stage 2: Add Validator Specs, Chain Mappings, And Manifest Provenance - -Goal: make validators centrally registered and make default chains explicit, -reviewable, and auditable. - -Tasks: - -- Replace the current `pipeline.ValidatorRegistry` internals so it registers - validator specs, not generic module specs. -- Add a `pipeline.ValidatorSpec` with at least: - - `Key`; - - `ExecutionClass`. -- Keep validator construction lazy through `ValidatorRegistry.Build(key)`. -- Add `ValidatorRegistry.Spec(key)` and sorted registered-spec accessors for - catalog and manifest use. -- Remove `pipeline.RawValidationRegistry` after replacing its callers with the - new chain registry. The new chain registry should be keyed by: - - stage; - - module key. -- Add a production/default mapping type such as: - - ```go - type ValidatorChainMapping struct { - Stage ModuleStage - Module string - Validators []ModuleBinding - } - ``` - -- Add a chain registry with these behaviors: - - duplicate stage/module mappings are rejected; - - unknown stages are rejected; - - empty default chains may be represented by absence of a mapping; - - lookup returns a defensive copy. -- Extend `pipeline.ModuleCatalog` and `pipeline.Registries` to carry the chain - mapping registry alongside module and validator registries. -- Add manifest types under `internal/core/artifacts` for resolved validator - chain provenance. Use a top-level manifest field so chunk, extract, merge, and - normalize chains can all be represented: - - ```go - type ValidatorChainManifest struct { - Stage string `json:"stage"` - LaneID string `json:"lane_id,omitempty"` - ModuleKey string `json:"module_key"` - Validators []ValidatorManifest `json:"validators"` - } - - type ValidatorManifest struct { - Key string `json:"key"` - ExecutionClass string `json:"execution_class"` - } - ``` - -- Remove `ArtifactLaneManifest.Validators` and update affected tests and JSON - output documentation to use top-level validator-chain provenance instead. - -Design requirements: - -- The production chain mapping is central catalog policy, not module-owned - behavior. -- A resolved run manifest must record the exact chain used for each validation - point, including explicit empty chains. - -Focused tests: - -```sh -go test ./internal/framework/pipeline -go test ./internal/core/artifacts -``` - -## Stage 3: Implement Stage-Scoped Config Overrides - -Goal: allow pipeline config to override default validator mappings for each -validatable stage while preserving unset versus explicit empty semantics. - -Tasks: - -- Add stage-local validator override syntax to module bindings for only: - - `chunk`; - - artifact lane `extract`; - - artifact lane `merge`; - - artifact lane `normalize`. -- Do not revive artifact-lane-level `validators` as an active chain. Keep that - field rejected with an error that directs users to - `extract.validators`, `merge.validators`, or `normalize.validators`. -- Update file-config parsing so the implementation can distinguish: - - validators omitted; - - `validators: []`; - - `validators: [ ... ]`. -- Represent this in pipeline profiles with this explicit override type: - - ```go - type ValidatorOverride struct { - Set bool - Validators []ModuleBinding - } - ``` - -- Validate configured validators during config validation: - - validator module key must be non-empty; - - `references` are not supported on validator bindings; - - nested validator overrides inside validator bindings are not supported; - - retries are not supported on validator bindings in this pass; - - explicit `llm_profile` is allowed only for LLM-backed validators and is - rejected for deterministic validators during resolution. -- During pipeline resolution: - - unset override uses the central default chain; - - explicit empty override resolves to an empty chain and approves by default; - - explicit non-empty override resolves exactly the configured validators in - configured order; - - every referenced validator key must exist in the validator registry; - - configured order must not be silently reordered. -- Update `--llm-profile` behavior: - - continue overriding chunk, extract, merge, and normalize module bindings; - - do not override validator bindings unless validator-specific profile - override behavior is explicitly added in a later roadmap. -- Update Scriptorium explicit profile validation so it includes LLM-backed - validators with explicit `llm_profile` values, and ignores deterministic - validators. - -Design requirements: - -- Validator compatibility with a module or stage is not enforced during config - resolution. -- Empty resolved chains are valid and should be recorded in manifest - provenance. -- Config validation must fail before runtime if a non-empty override references - an unknown validator. - -Focused tests: - -```sh -go test ./internal/core/config -go test ./internal/framework/pipeline -go test ./internal/cli -``` - -## Stage 4: Wire Validator Execution Into The Runner - -Goal: run resolved validator chains at chunk, extract, merge, and normalize -validation points. - -Tasks: - -- Replace runner calls to the old raw validation registry with calls to the - resolved validator chain for the current validation point. -- Build validators from `ValidatorRegistry` at runtime in resolved order. -- For each validator call, populate the new `contracts.ValidationRequest` with: - - immutable module output payload; - - stage/module/lane/source/chunk provenance; - - schema metadata and in-memory schema content when available; - - source input material; - - session ID; - - resolved references for the target stage; - - LLM client/profile/options/metadata for the validator; - - upstream outputs needed by merge and normalize validators. -- Preserve current retry behavior: - - semantic rejection returns a `RejectedOutput` and may be retried according - to the module binding's retry setting; - - validator execution errors are framework errors and are retried under the - same module binding retry setting; - - final rejection is recorded and does not pass downstream. -- Preserve current warning behavior: - - warnings from accepted attempts are promoted; - - warnings from discarded retry attempts are not promoted; - - warning-only validators return `Approved:true` with warnings. -- Continue enforcing framework-level chunk invariants in the runner before - chunk validators run. -- Populate manifest validator-chain provenance after resolution and before - output encoding. -- Keep rejected output manifest entries compatible with current `rejected.json` - and `manifest.json` shapes where practical. - -Design requirements: - -- Validators are read-only. Do not let a validator return rewritten payloads or - replacement stage outputs. -- If a validator chain is empty, approve without building validators. -- If a validator is mapped to an unsuitable output shape, the validator should - return a clear execution error or an explicit documented approval. - -Focused tests: - -```sh -go test ./internal/framework/pipeline -go test ./internal/cli -``` - -## Stage 5: Add Generic Validators - -Goal: provide reusable validators needed for incremental module development and -production structural checks. - -Tasks: - -- Add `internal/validators/generic/always_accept`. - - Key: `generic/always_accept`. - - Execution class: deterministic. - - Always returns approved. -- Add `internal/validators/generic/always_reject`. - - Key: `generic/always_reject`. - - Execution class: deterministic. - - Always returns rejected with stable reason code `always_reject`. -- Add `internal/validators/generic/valid_json`. - - Key: `generic/valid_json`. - - Execution class: deterministic. - - Accepts syntactically valid JSON. - - Rejects invalid JSON with stable reason code `invalid_json`. -- Add `internal/validators/generic/valid_json_schema`. - - Key: `generic/valid_json_schema`. - - Execution class: deterministic. - - Uses the module output's in-memory JSON schema content. - - Rejects invalid JSON with `invalid_json`. - - Rejects schema non-conformance with `json_schema_invalid`. - - Returns a validator execution error when no schema content is available. -- Make `github.com/santhosh-tekuri/jsonschema/v6` a direct dependency for - schema validation. -- Extend `contracts.ResponseSchema` with an in-memory-only field: - - ```go - JSONSchema []byte `json:"-"` - ``` - - Module outputs should carry schema bytes to validators without serializing raw - schema content into manifests, diagnostics, or default output files. -- Update D&D scene and D&D spell modules to populate in-memory schema content on - their output schema values. -- Do not add a shared validator test helper package in this pass. Keep tests - local unless a later cleanup finds substantial duplication. - -Design requirements: - -- Generic validators must not depend on concrete production modules. -- Tests must not assert exact embedded production prompt text. -- Raw schema content must not be emitted in diagnostics or manifests. - -Focused tests: - -```sh -go test ./internal/validators/generic/... -go test ./internal/modules/chunk/dnd/scenes -go test ./internal/modules/extract/dnd/spells -go test ./internal/framework/pipeline -``` - -## Stage 6: Migrate D&D Spell Validators - -Goal: move D&D spell validation behavior out of the extractor package and into -raw-output validators. - -Tasks: - -- Add `internal/validators/extract/dnd/spells/shape`. - - Key: `extract/dnd/spells/shape`. - - Execution class: deterministic. - - Parses raw spell JSON and rejects malformed spell-cast payloads or missing - required fields. -- Add `internal/validators/extract/dnd/spells/source_refs`. - - Key: `extract/dnd/spells/source_refs`. - - Execution class: deterministic. - - Parses raw spell JSON and rejects missing or invalid source references. - - Use `internal/core/source.ValidateRef` for source reference validation. -- Add `internal/validators/extract/dnd/spells/source_relatedness`. - - Key: `extract/dnd/spells/source_relatedness`. - - Execution class: deterministic. - - Warning-only validator. - - Approves output and emits `spell_not_near_source` warnings when a spell name - is not found in the cited source text. -- Keep D&D-specific validator parsing local to validator packages or a validator - helper package. Do not import concrete extractor packages from validators. -- Remove `internal/modules/extract/dnd/spells/validator.go` and its - candidate-oriented tests after equivalent validator-package tests exist. -- Remove now-unused legacy candidate validation helpers and artifact candidate - types if they have no remaining callers. - -Design requirements: - -- The spell extractor should remain responsible for prompt/schema/provenance - and raw output production, not accept/reject policy. -- `source_relatedness` must not reject output. It should only warn. -- The default D&D spell chain must preserve the intended order: - `generic/valid_json`, `generic/valid_json_schema`, - `extract/dnd/spells/shape`, `extract/dnd/spells/source_refs`, - `extract/dnd/spells/source_relatedness`. - -Focused tests: - -```sh -go test ./internal/validators/extract/dnd/spells/... -go test ./internal/modules/extract/dnd/spells -go test ./internal/framework/pipeline -``` - -## Stage 7: Register Production Validators And Defaults - -Goal: make production validation policy explicit and active. - -Tasks: - -- Register all generic validators in `internal/cli/catalog.go`. -- Register all D&D spell validators in `internal/cli/catalog.go`. -- Register default production chain mappings centrally near production module - registration. -- Add the default chain for `extract` module `dnd/spells`: - - `generic/valid_json`; - - `generic/valid_json_schema`; - - `extract/dnd/spells/shape`; - - `extract/dnd/spells/source_refs`; - - `extract/dnd/spells/source_relatedness`. -- Do not add default chains for other modules unless the stage has a meaningful - validator for that output shape. -- Update catalog construction tests so available modules, available validators, - and default mappings can be reviewed together. -- Add CLI tests proving: - - default D&D spell validators run; - - explicit empty override disables the default chain; - - explicit non-empty override replaces the default chain; - - configured validator order is preserved; - - unknown configured validator keys fail during validation/resolution; - - deterministic validators reject explicit `llm_profile`; - - LLM-backed validator profile validation checks explicit profile IDs. - -Design requirements: - -- Production defaults must be reviewable without constructing concrete modules. -- Empty default chains approve by default. -- The manifest records resolved chains for default chains, empty overrides, and - explicit override chains. - -Focused tests: - -```sh -go test ./internal/cli -go test ./internal/core/config -go test ./internal/framework/pipeline -``` - -## Stage 8: Documentation, Examples, And Cleanup - -Goal: make implemented validation behavior canonical outside roadmap docs and -remove stale roadmap instructions. - -Tasks: - -- Update `docs/policy/architecture.md` so validation policy reflects central - mappings and validator packages rather than module-owned validator chains. -- Update `docs/config.md` to document stage-local validator overrides: - - unset uses defaults; - - explicit empty disables validators; - - explicit non-empty replaces defaults in configured order. -- Update `docs/cli.md` to list production validators and describe validation - profile behavior. -- Update `docs/internal/pipeline.md` with the implemented validator contract, - chain mapping, retry behavior, and manifest provenance. -- Update `docs/internal/modules.md` to remove stale claims that modules own - validator defaults. -- Update `docs/integrations/json-output.md` for any manifest shape changes. -- Update `docs/troubleshooting.md` with validator-chain debugging guidance. -- Leave maintained example configs unchanged unless a validator override example - is added intentionally with corresponding CLI/config tests. Keep examples - secret-free and loadable. -- Replace `docs/roadmap/implementation.md` with a concise completed note after - all stages are implemented. -- Update `docs/roadmap/validation.md` so it no longer describes completed work - as future work. Keep only deferred validation work, if any remains. - -Focused documentation checks: - -```sh -rg -n "reserved for future configurable validator chains|configured validators are not supported|candidate-oriented|RawValidationRegistry" docs internal -``` - -Expected result after implementation: no stale current-behavior documentation -describes configured validators as rejected, and no production code path depends -on the legacy candidate-oriented validator contract. - -## Full Validation - -Run after every stage that changes shared contracts, and at the end: - -```sh -go test ./... -go vet ./... -go build ./cmd/notarius -``` - -Also run focused packages added by this plan: - -```sh -go test ./internal/validators/... -``` +Remaining future validation ideas, if any, belong in +[Validation Roadmap](validation.md). diff --git a/docs/roadmap/validation.md b/docs/roadmap/validation.md index 8c27355..b62063a 100644 --- a/docs/roadmap/validation.md +++ b/docs/roadmap/validation.md @@ -1,459 +1,31 @@ -# Validation System Refactor - -This roadmap defines the target state for making validation a first-class, -composable pipeline concern. - -Current pipeline behavior is raw-output based. The runner validates `chunk`, -`extract`, `merge`, and `normalize` outputs through `contracts.Validator` -chains resolved from `pipeline.ValidatorChainRegistry`. Empty chains approve by -default, validator rejection records a rejected raw output, and rejected output -does not pass to the next stage. Production currently registers no validators, -and non-empty pipeline-configured validator lists are rejected until -stage-scoped override syntax is implemented. - -The desired end state is that validator implementations, validator -registration, and default module-to-validator mappings are explicit, reviewable, -and independent of concrete module packages. - -## Goals - -- Move artifact and module-output validation behavior out of `internal/modules` - and into `internal/validators`. -- Keep the raw module-output `contracts.Validator`, `ValidationRequest`, and - `ValidationResult` path as the single framework validator contract. -- Keep each validator in its own package. -- Mirror the stage and domain shape of `internal/modules` where a validator is - module-specific. -- Support deterministic and LLM-backed validators through the same framework - contract. -- Allow validators to be mapped to modules at any pipeline stage that returns - module output for validation: `chunk`, `extract`, `merge`, or `normalize`. -- Make default production module-to-validator mappings centralized and - human-readable. -- Allow pipeline configuration to override default mappings for advanced use. -- Preserve the distinction between an unset validator override and an explicit - empty validator override. -- Treat an empty validator set as valid and equivalent to approval. -- Preserve the rule that module output passes forward unless a validator rejects - it. -- Make successfully returned module output the explicit validation boundary: - questions about output syntax, media type, schema conformance, and domain - acceptability should be answered by validators. - -## Non-Goals - -- Do not create a general workflow engine or arbitrary validation DAG. -- Do not revive the legacy artifact-candidate validation model as the primary - runner path. -- Do not enforce validator compatibility with a module or stage in this pass. -- Do not move ordinary runtime invariant checks into validator packages. -- Do not require every module to have validators. -- Do not require LLM-backed validators for modules that can be checked - deterministically. -- Do not silently reorder configured validator chains unless that behavior is - introduced deliberately and documented as part of the validator contract. - -## Validation Boundary - -Validation packages should own approve/reject/warning evaluation of successfully -returned module outputs. This means logic that decides whether a chunk result or -raw extract, merge, or normalize payload should continue through the pipeline -belongs in `internal/validators`. - -The boundary is: - -- no module output was returned: execution failed, and the pipeline should report - a module or runtime error; -- module output was returned: the validator chain decides whether that output is - acceptable, and an empty validator chain approves it. - -Scriptorium and provider errors are execution failures rather than validator -rejections. This includes provider timeouts, authentication failures, -transport/runtime failures, Scriptorium structured-output retry exhaustion, and -malformed responses that Scriptorium rejects before returning module output. - -Other validation-like checks should remain with their owning packages: - -- input parsing and source-format validation stay in input modules; -- source document and source reference invariants stay in `internal/core/source`; -- generic framework chunk invariants that make extraction possible stay in the - runner, such as non-empty chunk content, valid unit ranges, and canonical - source-unit ordering; -- config validation stays in `internal/core/config`; -- registry, profile, and pipeline consistency checks stay in framework and CLI - code; -- response schema loading stays in module asset code; -- Scriptorium runtime errors stay in LLM runtime code. - -Domain validators may call reusable core helpers such as `source.ValidateRef`, -but the module-output approval or rejection decision should be made by a -validator. - -Validators should answer module-output questions such as: - -- is returned content syntactically valid JSON; -- does returned JSON conform to the module's declared schema; -- does the returned media type match the module or pipeline policy; -- are required domain fields present and non-empty; -- are source references valid and appropriately grounded; -- does domain-specific output satisfy the configured policy. - -## Audita Patterns To Adapt - -The validator architecture should adapt useful patterns from -[`audita`](https://gitea.maximumdirect.net/eric/audita) without copying its -narrower transcript-correction shape directly. - -Useful patterns: - -- concrete validators live under `internal/validators`; -- shared validator runtime mechanics live under a framework package; -- built-in validator keys are stable and centrally registered; -- built-in chains are centrally reviewable; -- validators carry execution-class metadata; -- deterministic and LLM-backed validators implement one contract; -- LLM-backed validator runtime can share batching, diagnostics, structured - response handling, and malformed-response policy; -- reports and manifests can classify validator decisions by execution class. - -Important Notarius differences: - -- mappings must be keyed by stage and module key, not module key alone; -- mappings should be owned by the central production catalog, not resolved inside - concrete module constructors; -- configured mapping order should be authoritative unless the config explicitly - opts into a different ordering policy; -- validators must support chunk, extract, merge, and normalize outputs rather - than only one proposal shape. - -## Validator Package Layout - -Concrete validators should live under `internal/validators`. Module-specific -validators should mirror the module tree and use one package per validator: - -```text -internal/validators/extract/dnd/spells/shape -internal/validators/extract/dnd/spells/source_refs -internal/validators/extract/dnd/spells/source_relatedness -``` - -D&D spell validation policy belongs under -`internal/validators/extract/dnd/spells`, split by concern rather than bundled -inside the extractor module. - -Generic validators may live under stage-specific generic paths when they operate -on a particular stage output shape: - -```text -internal/validators/chunk/generic/... -internal/validators/extract/generic/... -internal/validators/merge/generic/... -internal/validators/normalize/generic/... -``` - -Truly stage-independent validators may live under `internal/validators/generic` -once there is a real shared validator that justifies that location. Generic JSON -syntax and JSON schema validators are likely candidates for -`internal/validators/generic/valid_json` and -`internal/validators/generic/valid_json_schema`. - -Each validator package should expose: - -- a stable validator key; -- execution-class metadata; -- a constructor; -- a validator spec suitable for registration; -- a `Register` function; -- focused tests for decisions, warnings, errors, and diagnostics behavior. - -Reusable validator runtime mechanics should live in framework code, such as -`internal/framework/validators`, not in concrete validator packages. This package -can own shared helpers for decision cardinality, approval/rejection construction, -LLM validator batching, validator diagnostics, and Scriptorium request plumbing. - -The concrete validator packages should own policy: what they inspect, what they -approve or reject, what warning reason codes they emit, and how they interpret -domain-specific data. - -## Validator Design Policy - -Validators should follow a small-tool model: each validator should do one thing -well. If a validator both rejects output and emits unrelated warnings, split -those concerns into separate validators so production mappings can include, -exclude, and order them independently. - -Validators are read-only. A validator must not mutate pipeline state, rewrite -module output, materialize raw output into typed stage output, or enrich the -`ModuleOutput` passed to later validators. A validator returns an -accept/reject verdict for the output it evaluates, plus any warnings or -diagnostic references. Any conversion from raw module output into a downstream -representation is a separate materialization concern and must not be hidden -inside a validator. - -The initial generic validator set should include: - -- `generic/always_accept`: accepts returned module output unchanged. This is - functionally equivalent to a no-op validator and is primarily useful for tests, - demonstrations, and explicit pass-through configurations. -- `generic/always_reject`: rejects returned module output without inspecting it. - This is primarily useful for tests and for proving rejection plumbing, - manifests, and diagnostics. -- `generic/valid_json`: inspects raw returned module output and accepts only - syntactically valid JSON. -- `generic/valid_json_schema`: compares raw returned JSON with the module's - configured response schema and accepts only schema-conformant output. - -The exact keys may be adjusted during implementation to match local naming -conventions, but the validator set should preserve these four behaviors. - -For the current D&D spell behavior, the target split is: - -- `generic/valid_json`: rejects returned module output that is not syntactically - valid JSON. -- `generic/valid_json_schema`: rejects returned JSON that does not conform to - the configured response schema. -- `extract/dnd/spells/shape`: rejects malformed spell-cast payloads and missing - required spell fields. -- `extract/dnd/spells/source_refs`: rejects missing or invalid source - references. -- `extract/dnd/spells/source_relatedness`: warning-only validator that reports - when a spell name is not found in the cited source text. - -Production default mappings should generally list deterministic validators -before LLM-backed validators. This keeps cheap structural failures from consuming -model calls and keeps diagnostics easier to interpret. Pipeline-configured order -should still be authoritative; if a user explicitly lists an LLM-backed -validator before a deterministic validator, the framework should honor that -order rather than silently reshuffling it. - -## Execution Classes - -Validator specs should declare an execution class: - -```go -type ExecutionClass string - -const ( - ExecutionClassDeterministic ExecutionClass = "deterministic" - ExecutionClassLLMBacked ExecutionClass = "llm_backed" -) -``` - -Execution class should be metadata on the validator spec or registered -definition, not an ad hoc convention inferred from package paths. It should be -used for: - -- human-readable catalog and manifest reporting; -- diagnostics and timing summaries; -- operational policy such as concurrency budgeting for LLM-backed validators; -- default mapping review, where deterministic validators should usually appear - before LLM-backed validators. - -Execution class should not by itself imply compatibility with a stage or module. - -## Validator Contract - -The validator framework should support validation of outputs from `chunk`, -`extract`, `merge`, and `normalize` stages. The contract should be generalized -enough for stage-specific validators to inspect the output they care about while -ignoring irrelevant fields. - -The current `contracts.ValidationRequest` carries stage, lane, module, source, -source and chunk provenance, response schema metadata, raw payload, and run -metadata. The final contract should continue evolving from that raw-output -shape. - -Additional fields needed for the full validator system include: - -- source input material when a validator needs to compare module output to the - original source payload; -- session ID; -- resolved references for the validated target; -- LLM client and profile for LLM-backed validators; -- validator options; -- chunk output collections when validating a chunk module; -- ordered upstream output envelopes when validating merge or normalize behavior. - -A shared module-output envelope should represent the validation boundary. -Validators may inspect raw returned content and any already-existing typed stage -envelope, such as `SourceChunk` values for chunk validation, but they must not -modify it. - -Conceptually: - -```go -type ModuleOutput struct { - Stage pipeline.ModuleStage - ModuleKey string - LaneID string - - RawContent []byte - MediaType string - ResponseSchema contracts.ResponseSchema - - SourceID string - ChunkID string - ChunkIndex int - Chunks []contracts.SourceChunk - Warnings []contracts.Warning -} -``` - -The final implementation does not need to use this exact shape, but it should -preserve the boundary: returned raw module output enters validation as immutable -module output. Validators may parse raw bytes internally to decide approve, -reject, or warn, but parsing inside a validator must not create or replace the -payload passed to later stages. - -The result should continue to express validator identity, warnings, and explicit -decisions. For output collections, the implementation should define an explicit -decision shape rather than silently mutating lists. Validator decisions reject or -approve output; validators do not rewrite output. - -An empty validator list is always valid. With no validators, the framework should -pass module output forward unchanged and treat the output as approved for that -validation point. If the approved output cannot be consumed by a later stage -because its media type or envelope shape is unsuitable, that failure should be -reported at the downstream boundary that requires a different shape, not as an -implicit pre-validation rejection. - -## Module Development Workflow - -The validation system should make iterative module development easier. Once -pipeline overrides are implemented, a module author should be able to start with -an explicit empty validator mapping and inspect returned raw LLM output without -first satisfying JSON syntax, schema, media-type, or domain validators. - -A typical development path should be: - -1. Configure an empty validator set for the module and inspect raw returned - output. -2. Add `generic/valid_json` and adjust prompts until the model reliably returns - syntactically valid JSON. -3. Add `generic/valid_json_schema` and iterate on prompt/schema alignment. -4. Add media-type or schema validators appropriate to the module's intended - output format. -5. Add domain-specific validators one at a time until production policy is - represented explicitly in the chain. - -This workflow is a central reason for making output validation explicit and -composable rather than hiding schema, shape, or domain checks inside module -implementation code. - -## Central Production Mappings - -Production defaults should be defined in a central, human-readable location near -the production module and validator registries. The mapping should be keyed by -stage and module key, not only by module key, so future modules can share keys -only when stage context makes their ownership unambiguous. - -Conceptually: - -```go -{ - Stage: pipeline.StageExtract, - Module: "dnd/spells", - Validators: []pipeline.ModuleBinding{ - pipeline.Binding("generic/valid_json"), - pipeline.Binding("generic/valid_json_schema"), - pipeline.Binding("extract/dnd/spells/shape"), - pipeline.Binding("extract/dnd/spells/source_refs"), - pipeline.Binding("extract/dnd/spells/source_relatedness"), - }, -} -``` - -The production catalog should expose three related surfaces together: - -- available modules; -- available validators; -- default module-to-validator mappings. - -This makes production validation policy reviewable without constructing concrete -modules or searching inside module implementation packages. - -The validator registry should expose registered validator specs without building -validators, including key and execution class. Building a validator should still -be available for runtime execution. - -The mapping surface should preserve the current useful behavior of stage/module -lookup and empty-chain approval while adding validator specs, execution-class -metadata, production registration, config override integration, and manifest -reporting of the resolved chain. - -## Pipeline Overrides - -Pipeline configuration should be able to override the central default mapping -for a module binding. Current configuration validation rejects non-empty -validator lists, and the current config/profile structs do not preserve whether -an empty list was explicitly configured or simply omitted. The target config -model must preserve that distinction. - -Override semantics should distinguish three states: - -- unset validators: use the central production/default mapping; -- explicit empty validators: run no validators and pass output forward; -- explicit non-empty validators: run exactly the configured validators in the - configured order. - -This keeps the happy path concise while preserving advanced control for -experimentation, debugging, and custom deployments. - -The run manifest should record the resolved validator chain for each validation -point so completed runs remain auditable after defaults or configuration change. -Each manifest entry should include at least validator key and execution class, -and should preserve the resolved order actually used for the run. - -## LLM-Backed Validator Runtime - -LLM-backed validators should use the same validator contract as deterministic -validators. Shared framework runtime should provide common support for: - -- Scriptorium request construction; -- validator prompt and schema provenance; -- diagnostics redaction; -- optional batching or context-window controls when validator inputs are large; -- mapping successful LLM validator responses into validator decisions and - warnings; -- consistent handling of Scriptorium/runtime errors. - -Scriptorium errors raised during validator execution should be treated as -validator execution errors unless a specific validator deliberately converts a -successful response into reject/warn decisions. This keeps provider/runtime -failure distinct from a validator's semantic rejection of module output. - -## Stage Coverage - -Validators should be composable across all LLM-eligible stages: - -- `chunk`: validators can evaluate chunk boundaries, coverage, overlap, metadata, - or module-specific chunk quality. -- `extract`: validators can evaluate raw extracted output, source references, - payload shape, evidence quality, media type, or domain constraints. -- `merge`: validators can evaluate merged output, cross-chunk consistency, - deduplication results, media type, or domain-specific reconciliation. -- `normalize`: validators can evaluate normalized output, final shape, - post-processing results, media type, or domain-specific policy. - -The framework should not require compatibility declarations in this pass. A -validator mapped to an unsuitable output shape should return a clear error, or -approve unchanged only when that is explicitly the validator's documented -behavior. - -## Documentation Impact - -Current-behavior docs and policy should describe the implemented validation -system once the refactor is complete: - -- `docs/policy/architecture.md` should describe centralized validator mappings - rather than module-owned validator chains. -- `docs/internal/modules.md` should remove claims that concrete modules own - validator defaults. -- Internal validation docs should describe validator package ownership, mapping - precedence, empty-chain approval behavior, and LLM-backed validator support. -- User/config docs should replace the current "configured validators are - reserved and rejected" language with the implemented pipeline override - contract. - -Roadmap docs should not remain the canonical description of implemented -validation behavior after the refactor is complete. +# Validation Roadmap + +The first-class raw-output validation system is implemented. Current behavior is +documented outside roadmap files, especially in [Configuration](../config.md), +[CLI Reference](../cli.md), [Pipeline Internals](../internal/pipeline.md), and +[JSON Output](../integrations/json-output.md). + +Implemented behavior includes: + +- a single raw module-output validator contract for chunk, extract, merge, and + normalize outputs; +- validator specs with deterministic and LLM-backed execution classes; +- central production validator registration and default chain mappings; +- stage-local config overrides that distinguish omitted, explicit empty, and + explicit non-empty validator chains; +- manifest provenance for resolved validator chains; +- generic JSON validators and D&D spell validators under `internal/validators`; +- production defaults for the `dnd/spells` extractor. + +## Future Work + +- Add production LLM-backed validators when there is a concrete review policy + that benefits from model judgment. +- Add validator diagnostics and timing summaries if operators need more detail + than `manifest.json`, `rejected.json`, and `warnings.json` provide. +- Add media-type validators for non-JSON module outputs when such modules are + introduced. +- Add compatibility metadata only if real deployments need config-time + enforcement that a validator is suitable for a specific stage or module. +- Add batching or context-window controls for LLM-backed validators if validator + inputs become large enough to require them. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index ea0374e..d90f596 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -118,9 +118,10 @@ Symptoms include: Fix: -- Confirm the selected chunker, extractor, merger, or normalizer declares the slot. The - implemented `dnd/scenes` chunker and `dnd/spells` extractor declare optional - `roster` and `glossary` slots. +- Confirm the selected chunker, extractor, merger, or normalizer declares the + slot. The implemented `dnd/scenes` chunker and `dnd/spells` extractor declare + optional `players`, `party`, and `glossary` slots, plus `roster` as a + deprecated compatibility alias for `party`. - Use a specific selector when more than one selected target declares the same slot: `chunk.context=./context.txt`, `spells.extract.context=./extract-context.txt`, @@ -317,11 +318,31 @@ Explanation and fixes: pass to the next pipeline stage. - Check `rejected.json` for the stage, lane, module, chunk, validator, reason, message, and attempt count. +- Check `manifest.json` `validator_chains` to see the exact resolved validators + and order used for the rejected validation point. The production `dnd/spells` + extractor runs JSON syntax, JSON schema, D&D spell shape, source-reference, + and source-relatedness validators by default. +- If the configured chain is not what you expected, inspect the selected + binding in config. Omitted `validators` uses production defaults, + `validators: []` disables validators for that binding, and a non-empty list + replaces the default chain in configured order. +- Run `notarius config validate --pipeline ` to catch unknown validator keys + and invalid validator `llm_profile` usage before running the pipeline. - Increase a module binding's `retries` only when re-running the same module input can reasonably produce an acceptable output. - If rejection is deterministic, fix the source input, module configuration, or validation policy rather than adding retries. +Common production D&D spell validator reasons: + +- `invalid_json`: the raw output is not valid JSON. +- `json_schema_invalid`: the raw JSON does not match the spell response schema. +- `invalid_spell_shape`: required spell-cast fields are missing or malformed. +- `invalid_source_refs`: source references are missing or do not point to valid + source units. +- `spell_not_near_source`: warning-only; the spell name was not found near the + cited source text. + ## Retry Exhaustion Symptoms include: