Cleanup and complete the validator refactor
This commit is contained in:
@@ -162,8 +162,8 @@ chunk metadata. Extractors and downstream stages therefore see canonical source
|
||||
units, while `SourceChunk.Metadata` remains the supported place for
|
||||
chunker-owned context.
|
||||
|
||||
If chunk validation rejects a chunk after configured retries, the runner records
|
||||
a rejected raw output and skips downstream lane execution. Framework-level
|
||||
If chunk validation rejects the chunk result after configured retries, the runner
|
||||
records a rejected raw output and skips downstream lane execution. Framework-level
|
||||
chunking or validation errors that remain after configured retries fail the run.
|
||||
|
||||
The framework does not require complete source-unit coverage and does not reject
|
||||
|
||||
@@ -22,6 +22,16 @@ future work only.
|
||||
- Cross-chunk semantic deduplication.
|
||||
- Additional validator packages and production default chains for future
|
||||
modules.
|
||||
- Production LLM-backed validators when there is a concrete review policy that
|
||||
benefits from model judgment.
|
||||
- Validator diagnostics and timing summaries if operators need more detail than
|
||||
`manifest.json`, `rejected.json`, and `warnings.json` provide.
|
||||
- Media-type validators for non-JSON module outputs when such modules are
|
||||
introduced.
|
||||
- Validator compatibility metadata if real deployments need config-time
|
||||
enforcement that a validator is suitable for a specific stage or module.
|
||||
- Batching or context-window controls for LLM-backed validators if validator
|
||||
inputs become large enough to require them.
|
||||
- Parallel execution where it preserves deterministic manifests and diagnostics.
|
||||
- Additional output encoders.
|
||||
|
||||
|
||||
@@ -1,16 +0,0 @@
|
||||
# Validation Refactor Implementation
|
||||
|
||||
The validation refactor described by this temporary implementation plan has
|
||||
been completed.
|
||||
|
||||
Current behavior is documented in:
|
||||
|
||||
- [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)
|
||||
|
||||
Remaining future validation ideas, if any, belong in
|
||||
[Validation Roadmap](validation.md).
|
||||
@@ -1,31 +0,0 @@
|
||||
# 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.
|
||||
@@ -127,12 +127,12 @@ func (r *Runner) Run(ctx context.Context, input RunInput) (output RunOutput, err
|
||||
if err != nil {
|
||||
return false, nil, fmt.Errorf("validate chunks from chunker %q: %w", chunker.Key(), err)
|
||||
}
|
||||
rejection, err := r.validateChunksRaw(ctx, doc, chunker.Key(), chunks, sourceInput, sessionID, input.Pipeline.ChunkReferences.ReferenceSet, input.LLMClient, input.Metadata, input.Pipeline.ValidatorChains, attempt)
|
||||
validationWarnings, rejection, err := r.validateChunksRaw(ctx, doc, chunker.Key(), chunks, sourceInput, sessionID, input.Pipeline.ChunkReferences.ReferenceSet, input.LLMClient, input.Metadata, input.Pipeline.ValidatorChains, attempt)
|
||||
if err != nil || rejection != nil {
|
||||
return false, rejection, err
|
||||
}
|
||||
canonicalChunks = chunks
|
||||
chunkWarnings = cloneWarnings(chunkResult.Warnings)
|
||||
chunkWarnings = append(cloneWarnings(chunkResult.Warnings), validationWarnings...)
|
||||
return true, nil, nil
|
||||
})
|
||||
if err != nil {
|
||||
@@ -455,39 +455,21 @@ func runWithRetry(ctx context.Context, retries int, run func(attempt int) (bool,
|
||||
return false, lastRejection, nil
|
||||
}
|
||||
|
||||
func (r *Runner) validateChunksRaw(ctx context.Context, doc *source.SourceDocument, moduleKey string, chunks []contracts.SourceChunk, sourceInput contracts.LLMInputMaterial, sessionID string, references contracts.ReferenceSet, llmClient contracts.StructuredLLMClient, metadata map[string]any, chains []ResolvedValidatorChain, attempt int) (*contracts.RejectedOutput, error) {
|
||||
for index := range chunks {
|
||||
chunk := chunks[index]
|
||||
_, rejection, err := r.validateRaw(ctx, rawValidationTarget{
|
||||
stage: StageChunk,
|
||||
moduleKey: moduleKey,
|
||||
source: doc,
|
||||
sourceID: doc.ID,
|
||||
sourceInput: sourceInput.Clone(),
|
||||
sessionID: sessionID,
|
||||
references: references,
|
||||
llmClient: llmClient,
|
||||
chunkID: chunk.ID,
|
||||
chunkIndex: chunk.Index,
|
||||
chunk: &chunk,
|
||||
chunks: chunks,
|
||||
payload: contracts.RawPayload{
|
||||
Content: append([]byte(nil), chunk.Content...),
|
||||
MediaType: chunk.MediaType,
|
||||
Metadata: cloneMetadata(chunk.Metadata),
|
||||
},
|
||||
metadata: metadata,
|
||||
chains: chains,
|
||||
attempt: attempt,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if rejection != nil {
|
||||
return rejection, nil
|
||||
}
|
||||
}
|
||||
return nil, nil
|
||||
func (r *Runner) validateChunksRaw(ctx context.Context, doc *source.SourceDocument, moduleKey string, chunks []contracts.SourceChunk, sourceInput contracts.LLMInputMaterial, sessionID string, references contracts.ReferenceSet, llmClient contracts.StructuredLLMClient, metadata map[string]any, chains []ResolvedValidatorChain, attempt int) ([]contracts.Warning, *contracts.RejectedOutput, error) {
|
||||
return r.validateRaw(ctx, rawValidationTarget{
|
||||
stage: StageChunk,
|
||||
moduleKey: moduleKey,
|
||||
source: doc,
|
||||
sourceID: doc.ID,
|
||||
sourceInput: sourceInput.Clone(),
|
||||
sessionID: sessionID,
|
||||
references: references,
|
||||
llmClient: llmClient,
|
||||
chunks: chunks,
|
||||
metadata: metadata,
|
||||
chains: chains,
|
||||
attempt: attempt,
|
||||
})
|
||||
}
|
||||
|
||||
func (r *Runner) validateRaw(ctx context.Context, target rawValidationTarget) ([]contracts.Warning, *contracts.RejectedOutput, error) {
|
||||
|
||||
@@ -801,8 +801,8 @@ func TestRunPassesValidationRequestContextToValidators(t *testing.T) {
|
||||
t.Fatalf("Run() error = %v, want nil", err)
|
||||
}
|
||||
|
||||
if len(chunkValidator.requests) != 2 {
|
||||
t.Fatalf("chunk validator requests = %d, want one per chunk", len(chunkValidator.requests))
|
||||
if len(chunkValidator.requests) != 1 {
|
||||
t.Fatalf("chunk validator requests = %d, want one collection request", len(chunkValidator.requests))
|
||||
}
|
||||
chunkReq := chunkValidator.requests[0]
|
||||
if chunkReq.Stage != string(StageChunk) || chunkReq.ModuleKey != "chunk" || chunkReq.SourceID != "source-1" || chunkReq.SessionID != "session-123" {
|
||||
@@ -811,8 +811,12 @@ func TestRunPassesValidationRequestContextToValidators(t *testing.T) {
|
||||
if chunkReq.LLMClient == nil || string(chunkReq.SourceInput.Content) != string(rawInput) {
|
||||
t.Fatalf("chunk validation source/client = %#v, want full source input and LLM client", chunkReq.SourceInput)
|
||||
}
|
||||
if chunkReq.Chunk == nil || chunkReq.Chunk.ID != "chunk-0" || len(chunkReq.Chunks) != 2 || string(chunkReq.Payload.Content) != string(chunkReq.Chunk.Content) {
|
||||
t.Fatalf("chunk validation chunk fields = %#v chunks=%#v payload=%s, want chunk payload and all chunks", chunkReq.Chunk, chunkReq.Chunks, chunkReq.Payload.Content)
|
||||
if chunkReq.Chunk != nil || chunkReq.ChunkID != "" || len(chunkReq.Chunks) != 2 || chunkReq.Chunks[0].ID != "chunk-0" || chunkReq.Chunks[1].ID != "chunk-1" {
|
||||
t.Fatalf("chunk validation chunk fields = chunk=%#v chunk_id=%q chunks=%#v, want whole chunk collection", chunkReq.Chunk, chunkReq.ChunkID, chunkReq.Chunks)
|
||||
}
|
||||
chunkReq.Chunks[0].Content[0] = 'X'
|
||||
if got := string(modules.chunker.chunks[0].Content); got == string(chunkReq.Chunks[0].Content) {
|
||||
t.Fatalf("chunk validation chunks alias module output content = %q", got)
|
||||
}
|
||||
if item := chunkReq.References.Slots["scene_guide"].Items[0]; string(item.Content) != "chunk reference text" {
|
||||
t.Fatalf("chunk validation references = %#v, want chunk references", chunkReq.References)
|
||||
@@ -860,7 +864,6 @@ func TestRunPassesValidationRequestContextToValidators(t *testing.T) {
|
||||
t.Fatalf("normalize validation references = %#v, want normalize references", normalizeReq.References)
|
||||
}
|
||||
|
||||
chunkReq.Payload.Content[0] = 'X'
|
||||
chunkReq.Chunks[0].Content[0] = 'Y'
|
||||
if got := string(modules.chunker.chunks[0].Content); got != `{"units":[{"id":1,"kind":"unit","text":"Source unit."}]}` {
|
||||
t.Fatalf("validator request mutated original chunk content: %q", got)
|
||||
@@ -1211,6 +1214,27 @@ func TestRunCollectsStageWarnings(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunCollectsChunkValidatorWarnings(t *testing.T) {
|
||||
modules := defaultRunnerModules()
|
||||
modules.chunker.chunks = []contracts.SourceChunk{sourceChunkWithID("chunk-0", 0)}
|
||||
validator := &runnerChainValidator{
|
||||
name: "chain-chunk",
|
||||
warnings: []contracts.Warning{{ReasonCode: "chunk-validator-warning", Message: "chunk validator warning"}},
|
||||
}
|
||||
modules.validators[validator.name] = validator
|
||||
pipeline := resolvedPipeline()
|
||||
setResolvedValidatorChain(t, &pipeline, StageChunk, "", "chunk", resolvedValidatorForTest(validator))
|
||||
|
||||
output, err := New(newRunnerRegistries(t, modules)).Run(context.Background(), RunInput{Pipeline: pipeline})
|
||||
if err != nil {
|
||||
t.Fatalf("Run() error = %v, want nil", err)
|
||||
}
|
||||
|
||||
if got := warningReasons(output.Warnings); !reflect.DeepEqual(got, []string{"chunk-validator-warning"}) {
|
||||
t.Fatalf("warning reasons = %#v, want chunk validator warning", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunOutputEncoderReceivesManifestAndRawOutputs(t *testing.T) {
|
||||
modules := defaultRunnerModules()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user