Update planning roadmap and add a staged imnplementation plan for the validator registry
This commit is contained in:
486
docs/roadmap/implementation.md
Normal file
486
docs/roadmap/implementation.md
Normal file
@@ -0,0 +1,486 @@
|
||||
# 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/...
|
||||
```
|
||||
Reference in New Issue
Block a user