487 lines
17 KiB
Markdown
487 lines
17 KiB
Markdown
# Validation Refactor Implementation Plan
|
|
|
|
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.
|
|
|
|
## Stage 1: Replace Legacy Validator Contracts With One Raw Output Contract
|
|
|
|
Goal: make the raw module-output validation boundary the only framework
|
|
validator contract.
|
|
|
|
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/...
|
|
```
|