Harden diagnostic handling and warning presentation
This commit is contained in:
@@ -126,6 +126,9 @@ func (candidate ModelCandidate) Validate() error {
|
||||
}
|
||||
|
||||
func ValidateValidationResult(result ValidationResult) error {
|
||||
if err := ValidateProducerDiagnostics(result.Diagnostics); err != nil {
|
||||
return fmt.Errorf("validation diagnostics: %w", err)
|
||||
}
|
||||
if !result.Approved && result.ReasonCode == "" {
|
||||
return errors.New("validation rejection reason code must not be empty")
|
||||
}
|
||||
|
||||
@@ -75,6 +75,9 @@ func TestCorrectionContractsRejectInvalidContent(t *testing.T) {
|
||||
{"oversized correction guidance", func() error {
|
||||
return ValidateValidationResult(ValidationResult{ReasonCode: "invalid", CorrectionGuidance: tooLongValidationGuidance})
|
||||
}},
|
||||
{"invalid diagnostics", func() error {
|
||||
return ValidateValidationResult(ValidationResult{Approved: true, Diagnostics: []ProducerDiagnostic{{}}})
|
||||
}},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
if err := test.call(); err == nil {
|
||||
|
||||
@@ -102,6 +102,16 @@ type DiagnosticCollection struct {
|
||||
UnrepresentedOccurrenceCount int `json:"unrepresented_occurrence_count"`
|
||||
}
|
||||
|
||||
// DiagnosticProjection is the validated warning/non-warning view used by
|
||||
// durable and presentation boundaries. Occurrence totals are checked before
|
||||
// they leave the framework contract.
|
||||
type DiagnosticProjection struct {
|
||||
Warnings []DiagnosticGroup
|
||||
Diagnostics []DiagnosticGroup
|
||||
WarningOccurrenceCount int
|
||||
DiagnosticOccurrenceCount int
|
||||
}
|
||||
|
||||
// Validate checks a producer-local diagnostic against the public safety and
|
||||
// classification contract.
|
||||
func (diagnostic ProducerDiagnostic) Validate() error {
|
||||
@@ -192,6 +202,41 @@ func (collection DiagnosticCollection) Validate() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// ProjectDiagnosticCollection validates, partitions, and totals one finalized
|
||||
// collection. Unrepresented occurrences belong to the non-warning projection.
|
||||
func ProjectDiagnosticCollection(collection DiagnosticCollection) (DiagnosticProjection, error) {
|
||||
if err := collection.Validate(); err != nil {
|
||||
return DiagnosticProjection{}, err
|
||||
}
|
||||
projection := DiagnosticProjection{
|
||||
Warnings: make([]DiagnosticGroup, 0),
|
||||
Diagnostics: make([]DiagnosticGroup, 0),
|
||||
}
|
||||
for _, group := range collection.Groups {
|
||||
if group.Disposition == DiagnosticDispositionWarning {
|
||||
count, err := addDiagnosticOccurrences(projection.WarningOccurrenceCount, group.OccurrenceCount)
|
||||
if err != nil {
|
||||
return DiagnosticProjection{}, fmt.Errorf("warning occurrences: %w", err)
|
||||
}
|
||||
projection.WarningOccurrenceCount = count
|
||||
projection.Warnings = append(projection.Warnings, cloneDiagnosticGroup(group))
|
||||
continue
|
||||
}
|
||||
count, err := addDiagnosticOccurrences(projection.DiagnosticOccurrenceCount, group.OccurrenceCount)
|
||||
if err != nil {
|
||||
return DiagnosticProjection{}, fmt.Errorf("diagnostic occurrences: %w", err)
|
||||
}
|
||||
projection.DiagnosticOccurrenceCount = count
|
||||
projection.Diagnostics = append(projection.Diagnostics, cloneDiagnosticGroup(group))
|
||||
}
|
||||
count, err := addDiagnosticOccurrences(projection.DiagnosticOccurrenceCount, collection.UnrepresentedOccurrenceCount)
|
||||
if err != nil {
|
||||
return DiagnosticProjection{}, fmt.Errorf("diagnostic occurrences: %w", err)
|
||||
}
|
||||
projection.DiagnosticOccurrenceCount = count
|
||||
return projection, nil
|
||||
}
|
||||
|
||||
// CloneProducerDiagnostics returns independent diagnostic slice ownership.
|
||||
func CloneProducerDiagnostics(diagnostics []ProducerDiagnostic) []ProducerDiagnostic {
|
||||
if len(diagnostics) == 0 {
|
||||
@@ -305,6 +350,13 @@ func cloneDiagnosticGroup(group DiagnosticGroup) DiagnosticGroup {
|
||||
return group
|
||||
}
|
||||
|
||||
func addDiagnosticOccurrences(current, incoming int) (int, error) {
|
||||
if incoming > int(^uint(0)>>1)-current {
|
||||
return 0, errors.New("occurrence count overflow")
|
||||
}
|
||||
return current + incoming, nil
|
||||
}
|
||||
|
||||
func cloneDiagnosticSamples(samples []DiagnosticSample) []DiagnosticSample {
|
||||
if len(samples) == 0 {
|
||||
return nil
|
||||
|
||||
@@ -146,6 +146,47 @@ func TestCloneDiagnosticCollectionOwnsGroupsAndChunkIndex(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestProjectDiagnosticCollectionPartitionsAndChecksTotals(t *testing.T) {
|
||||
warning := validDiagnosticGroup(DiagnosticDispositionWarning, DiagnosticCategoryFallback, "fallback", 2)
|
||||
diagnostic := validDiagnosticGroup(DiagnosticDispositionAdvisory, DiagnosticCategoryDataQuality, "quality", 3)
|
||||
collection := DiagnosticCollection{
|
||||
Groups: []DiagnosticGroup{warning, diagnostic},
|
||||
Truncated: true,
|
||||
UnrepresentedOccurrenceCount: 4,
|
||||
}
|
||||
projection, err := ProjectDiagnosticCollection(collection)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(projection.Warnings) != 1 || len(projection.Diagnostics) != 1 || projection.WarningOccurrenceCount != 2 || projection.DiagnosticOccurrenceCount != 7 {
|
||||
t.Fatalf("projection = %#v, want partitioned exact totals", projection)
|
||||
}
|
||||
collection.Groups[0].Samples[0].Message = "mutated"
|
||||
if projection.Warnings[0].Samples[0].Message != "message" {
|
||||
t.Fatal("projection retained caller-owned sample storage")
|
||||
}
|
||||
|
||||
overflow := DiagnosticCollection{Groups: []DiagnosticGroup{
|
||||
validDiagnosticGroup(DiagnosticDispositionWarning, DiagnosticCategoryFallback, "first", int(^uint(0)>>1)),
|
||||
validDiagnosticGroup(DiagnosticDispositionWarning, DiagnosticCategoryFallback, "second", 1),
|
||||
}}
|
||||
if _, err := ProjectDiagnosticCollection(overflow); err == nil {
|
||||
t.Fatal("ProjectDiagnosticCollection() overflow error = nil")
|
||||
}
|
||||
}
|
||||
|
||||
func validDiagnosticGroup(disposition DiagnosticDisposition, category DiagnosticCategory, reason string, occurrences int) DiagnosticGroup {
|
||||
return DiagnosticGroup{
|
||||
Disposition: disposition,
|
||||
Category: category,
|
||||
ReasonCode: reason,
|
||||
Origin: DiagnosticOrigin{Stage: DiagnosticOriginStageNormalize, StepID: "step", LaneID: "lane", ModuleKey: "module"},
|
||||
OccurrenceCount: occurrences,
|
||||
Samples: []DiagnosticSample{{Scope: "scope", Message: "message"}},
|
||||
OmittedSampleCount: occurrences - 1,
|
||||
}
|
||||
}
|
||||
|
||||
func validProducerDiagnostic() ProducerDiagnostic {
|
||||
return ProducerDiagnostic{
|
||||
Disposition: DiagnosticDispositionWarning,
|
||||
|
||||
@@ -17,9 +17,10 @@ const (
|
||||
type Aggregator struct {
|
||||
groups []contracts.DiagnosticGroup
|
||||
indices map[groupKey]int
|
||||
omitted map[groupKey]struct{}
|
||||
warningGroups int
|
||||
nonWarningGroups int
|
||||
warningOccurrences int
|
||||
nonWarningOccurrences int
|
||||
unrepresentedOccurrences int
|
||||
}
|
||||
|
||||
@@ -32,31 +33,62 @@ func (aggregator *Aggregator) Add(group contracts.DiagnosticGroup) error {
|
||||
}
|
||||
if aggregator.indices == nil {
|
||||
aggregator.indices = make(map[groupKey]int)
|
||||
aggregator.omitted = make(map[groupKey]struct{})
|
||||
}
|
||||
key := groupKeyFromGroup(group)
|
||||
if index, exists := aggregator.indices[key]; exists {
|
||||
return aggregator.merge(index, group)
|
||||
}
|
||||
if _, exists := aggregator.omitted[key]; exists {
|
||||
return aggregator.addUnrepresented(group.OccurrenceCount)
|
||||
if err := aggregator.checkOccurrenceTotal(group.Disposition, group.OccurrenceCount); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := aggregator.merge(index, group); err != nil {
|
||||
return err
|
||||
}
|
||||
aggregator.addOccurrenceTotal(group.Disposition, group.OccurrenceCount)
|
||||
return nil
|
||||
}
|
||||
if group.Disposition == contracts.DiagnosticDispositionWarning {
|
||||
if aggregator.warningGroups >= MaxWarningGroups {
|
||||
return errors.New("diagnostic warning groups exceed maximum count")
|
||||
}
|
||||
aggregator.warningGroups++
|
||||
} else if aggregator.nonWarningGroups >= MaxNonWarningGroups {
|
||||
aggregator.omitted[key] = struct{}{}
|
||||
if err := aggregator.checkOccurrenceTotal(group.Disposition, group.OccurrenceCount); err != nil {
|
||||
return err
|
||||
}
|
||||
aggregator.addOccurrenceTotal(group.Disposition, group.OccurrenceCount)
|
||||
return aggregator.addUnrepresented(group.OccurrenceCount)
|
||||
}
|
||||
if err := aggregator.checkOccurrenceTotal(group.Disposition, group.OccurrenceCount); err != nil {
|
||||
return err
|
||||
}
|
||||
if group.Disposition == contracts.DiagnosticDispositionWarning {
|
||||
aggregator.warningGroups++
|
||||
} else {
|
||||
aggregator.nonWarningGroups++
|
||||
}
|
||||
aggregator.addOccurrenceTotal(group.Disposition, group.OccurrenceCount)
|
||||
aggregator.indices[key] = len(aggregator.groups)
|
||||
aggregator.groups = append(aggregator.groups, contracts.CloneDiagnosticCollection(contracts.DiagnosticCollection{Groups: []contracts.DiagnosticGroup{group}}).Groups[0])
|
||||
return nil
|
||||
}
|
||||
|
||||
func (aggregator *Aggregator) checkOccurrenceTotal(disposition contracts.DiagnosticDisposition, count int) error {
|
||||
current := aggregator.nonWarningOccurrences
|
||||
if disposition == contracts.DiagnosticDispositionWarning {
|
||||
current = aggregator.warningOccurrences
|
||||
}
|
||||
if count > maximumInt()-current {
|
||||
return errors.New("diagnostic occurrence count overflow")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (aggregator *Aggregator) addOccurrenceTotal(disposition contracts.DiagnosticDisposition, count int) {
|
||||
if disposition == contracts.DiagnosticDispositionWarning {
|
||||
aggregator.warningOccurrences += count
|
||||
return
|
||||
}
|
||||
aggregator.nonWarningOccurrences += count
|
||||
}
|
||||
|
||||
// Collection returns an independently owned grouped result in first-occurrence
|
||||
// order.
|
||||
func (aggregator *Aggregator) Collection() contracts.DiagnosticCollection {
|
||||
|
||||
@@ -46,7 +46,7 @@ func TestAggregatorEnforcesWarningBoundAndTruncatesOnlyNonWarnings(t *testing.T)
|
||||
}
|
||||
|
||||
nonWarnings := Aggregator{}
|
||||
for index := 0; index < MaxNonWarningGroups+1; index++ {
|
||||
for index := 0; index < MaxNonWarningGroups+3; index++ {
|
||||
group := groupForAggregation("scope", "message", contracts.DiagnosticDispositionAdvisory, contracts.DiagnosticCategoryDataQuality, "module")
|
||||
group.ReasonCode = "advisory-" + string(rune('a'+index))
|
||||
if err := nonWarnings.Add(group); err != nil {
|
||||
@@ -54,8 +54,26 @@ func TestAggregatorEnforcesWarningBoundAndTruncatesOnlyNonWarnings(t *testing.T)
|
||||
}
|
||||
}
|
||||
collection := nonWarnings.Collection()
|
||||
if len(collection.Groups) != MaxNonWarningGroups || !collection.Truncated || collection.UnrepresentedOccurrenceCount != 1 {
|
||||
t.Fatalf("collection = %#v, want bounded non-warning groups and one unrepresented occurrence", collection)
|
||||
if len(collection.Groups) != MaxNonWarningGroups || !collection.Truncated || collection.UnrepresentedOccurrenceCount != 3 {
|
||||
t.Fatalf("collection = %#v, want bounded non-warning groups and three unrepresented occurrences", collection)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAggregatorRejectsOccurrenceTotalOverflow(t *testing.T) {
|
||||
aggregator := Aggregator{}
|
||||
first := groupForAggregation("first", "first", contracts.DiagnosticDispositionAdvisory, contracts.DiagnosticCategoryDataQuality, "first")
|
||||
first.OccurrenceCount = maximumInt()
|
||||
first.OmittedSampleCount = maximumInt() - 1
|
||||
if err := aggregator.Add(first); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
second := groupForAggregation("second", "second", contracts.DiagnosticDispositionAdvisory, contracts.DiagnosticCategoryDataQuality, "second")
|
||||
if err := aggregator.Add(second); err == nil {
|
||||
t.Fatal("Add() occurrence total overflow error = nil")
|
||||
}
|
||||
collection := aggregator.Collection()
|
||||
if len(collection.Groups) != 1 || collection.Groups[0].OccurrenceCount != maximumInt() {
|
||||
t.Fatalf("collection changed after rejected overflow = %#v", collection)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -193,6 +193,10 @@ func runProducerAttempts(ctx context.Context, config producerAttemptConfig, prod
|
||||
}
|
||||
return failedProducerAttempt(provenance), fmt.Errorf("producer failed after %d attempt(s): %w", number, err)
|
||||
}
|
||||
if err := validateProducerAttemptDiagnostics(output); err != nil {
|
||||
provenance = append(provenance, producerAttemptProvenance{Number: number, Kind: kind, Outcome: producerAttemptFailed})
|
||||
return failedProducerAttempt(provenance), err
|
||||
}
|
||||
|
||||
output, err = output.clone()
|
||||
if err != nil {
|
||||
@@ -265,6 +269,18 @@ func runProducerAttempts(ctx context.Context, config producerAttemptConfig, prod
|
||||
return failedProducerAttempt(provenance), errors.New("producer attempt budget was not exhausted deterministically")
|
||||
}
|
||||
|
||||
func validateProducerAttemptDiagnostics(output producerAttemptOutput) error {
|
||||
if err := contracts.ValidateProducerDiagnostics(output.Diagnostics); err != nil {
|
||||
return fmt.Errorf("producer returned invalid diagnostics: %w", err)
|
||||
}
|
||||
if output.Retry != nil {
|
||||
if err := contracts.ValidateProducerDiagnostics(output.Retry.FallbackDiagnostics); err != nil {
|
||||
return fmt.Errorf("producer returned invalid retry fallback diagnostics: %w", err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func isImmediateProducerFailure(err error) bool {
|
||||
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
|
||||
return true
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"gitea.maximumdirect.net/eric/notarius/internal/framework/contracts"
|
||||
@@ -247,6 +248,45 @@ func TestRunProducerAttemptsUsesModuleRetryBudgetAndFallback(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestRunProducerAttemptsRejectsInvalidDiagnosticsWithoutRetry(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
output producerAttemptOutput
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "producer diagnostics",
|
||||
output: producerAttemptOutput{Diagnostics: []contracts.ProducerDiagnostic{{}}},
|
||||
want: "producer returned invalid diagnostics",
|
||||
},
|
||||
{
|
||||
name: "retry fallback diagnostics",
|
||||
output: producerAttemptOutput{Retry: &producerRetryDirective{FallbackDiagnostics: []contracts.ProducerDiagnostic{{}}}},
|
||||
want: "producer returned invalid retry fallback diagnostics",
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
producerCalls := 0
|
||||
validatorCalls := 0
|
||||
terminal, err := runProducerAttempts(context.Background(), producerAttemptConfig{Retries: 3, Policy: DefaultValidationPolicy()}, func(context.Context, producerAttemptRequest) (producerAttemptOutput, error) {
|
||||
producerCalls++
|
||||
return test.output, nil
|
||||
}, func(context.Context, producerAttemptOutput) (validationReport, error) {
|
||||
validatorCalls++
|
||||
return validationReport{}, nil
|
||||
})
|
||||
if err == nil || !strings.Contains(err.Error(), test.want) {
|
||||
t.Fatalf("runProducerAttempts() error = %v, want %q", err, test.want)
|
||||
}
|
||||
if terminal.Action != producerTerminalFailed || producerCalls != 1 || validatorCalls != 0 {
|
||||
t.Fatalf("terminal = %#v, producer calls = %d, validator calls = %d; want immediate framework failure", terminal, producerCalls, validatorCalls)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunProducerAttemptsRejectionWinsOverValidatorFailure(t *testing.T) {
|
||||
firstDiagnostics := []contracts.ProducerDiagnostic{producerDiagnostic("discarded", "discarded warning")}
|
||||
secondDiagnostics := []contracts.ProducerDiagnostic{producerDiagnostic("accepted", "accepted warning")}
|
||||
|
||||
@@ -101,7 +101,7 @@ func (r *Runner) runChunkPlan(ctx context.Context, input RunInput, doc *source.S
|
||||
return result, failure
|
||||
}
|
||||
// A cache hit is not model material. Its rejection is discarded and
|
||||
// generation begins with the ordinary initial request below. Warnings
|
||||
// generation begins with the ordinary initial request below. Diagnostics
|
||||
// from this discarded candidate are intentionally not promoted.
|
||||
}
|
||||
result.lookup = ChunkPlanDecision{Status: ChunkPlanInvalid, Reason: chunkPlanLookupReason(ChunkPlanInvalid)}
|
||||
|
||||
@@ -21,7 +21,7 @@ const (
|
||||
const correctionRequestIntroduction = "The previous response failed semantic validation. Return one complete corrected replacement response, not a patch, explanation, or commentary.\n\nCorrect all of the following:\n"
|
||||
|
||||
// validationRecord captures the settled result of one configured validator.
|
||||
// Its fields remain private so reports cannot expose mutable warning storage.
|
||||
// Its fields remain private so reports cannot expose mutable diagnostic storage.
|
||||
type validationRecord struct {
|
||||
validatorName string
|
||||
outcome validationOutcome
|
||||
@@ -188,6 +188,9 @@ func executeValidationChain(ctx context.Context, chain preparedValidatorChain, i
|
||||
report.records = append(report.records, validationRecord{validatorName: binding.Module, outcome: validationSkipped, attemptCount: attempt, reasonCode: invocation.reason, message: invocation.message})
|
||||
break
|
||||
}
|
||||
if err := contracts.ValidateProducerDiagnostics(invocation.result.Diagnostics); err != nil {
|
||||
return validationReport{}, fatalValidationError(fmt.Errorf("validator %q returned invalid diagnostics: %w", binding.Module, err))
|
||||
}
|
||||
if err := contracts.ValidateValidationResult(invocation.result); err != nil {
|
||||
if attempt == attemptLimit {
|
||||
report.records = append(report.records, validationRecord{validatorName: binding.Module, outcome: validationFailed, attemptCount: attempt, message: "validator returned an invalid result", failure: err})
|
||||
|
||||
@@ -196,6 +196,19 @@ func TestExecuteValidationChainReturnsFrameworkAndCancellationErrors(t *testing.
|
||||
}
|
||||
}
|
||||
|
||||
func TestExecuteValidationChainRejectsInvalidDiagnosticsWithoutRetry(t *testing.T) {
|
||||
chain := validationChain(validationSpec("validator", contracts.ExecutionClassLLMBacked, 2))
|
||||
calls := 0
|
||||
_, err := executeValidationChain(context.Background(), chain, func(context.Context, preparedValidator, int) (validationInvocation, error) {
|
||||
calls++
|
||||
return validationInvocation{result: contracts.ValidationResult{Approved: true, Diagnostics: []contracts.ProducerDiagnostic{{}}}}, nil
|
||||
})
|
||||
var frameworkErr validationFrameworkError
|
||||
if !errors.As(err, &frameworkErr) || calls != 1 || !strings.Contains(err.Error(), "validator \"validator\" returned invalid diagnostics") {
|
||||
t.Fatalf("error = %v calls = %d, want one fatal invalid-diagnostics result", err, calls)
|
||||
}
|
||||
}
|
||||
|
||||
type validationStep struct {
|
||||
result contracts.ValidationResult
|
||||
invocation validationInvocation
|
||||
|
||||
@@ -70,7 +70,7 @@ type ApplicationPolicy[T any] struct {
|
||||
}
|
||||
|
||||
// GroupProvenance identifies the complete input contribution of one plan
|
||||
// group without prescribing domain warning or retry policy.
|
||||
// group without prescribing domain diagnostic or retry policy.
|
||||
type GroupProvenance struct {
|
||||
memberPositions []int
|
||||
canonicalPosition int
|
||||
@@ -108,7 +108,7 @@ func (event AppliedGroup) Provenance() GroupProvenance {
|
||||
return cloneGroupProvenance(event.provenance)
|
||||
}
|
||||
|
||||
// RejectedGroup records a typed guard decision while leaving warning and retry
|
||||
// RejectedGroup records a typed guard decision while leaving diagnostic and retry
|
||||
// construction to the consuming domain.
|
||||
type RejectedGroup struct {
|
||||
category RejectionCategory
|
||||
|
||||
@@ -32,7 +32,7 @@ const (
|
||||
)
|
||||
|
||||
// Issue identifies an unsafe proposal category at its original response group
|
||||
// index without prescribing caller warning text.
|
||||
// index without prescribing caller diagnostic text.
|
||||
type Issue struct {
|
||||
GroupIndex int
|
||||
Category IssueCategory
|
||||
|
||||
Reference in New Issue
Block a user