diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 62cca12b..5cc9dfdf 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -363,7 +363,7 @@ replay, and current-validator diagnostics on a reused chunk plan. This stage is appropriately sized for one `gpt-5.6-terra` prompt. -## Stage 5 — Migrate All D&D Source-Relatedness Validators +## Stage 5 ✅ — Migrate All D&D Source-Relatedness Validators ### Goal diff --git a/internal/cli/spell_catalog_retry_contract_test.go b/internal/cli/spell_catalog_retry_contract_test.go index bda25b92..d3934ee2 100644 --- a/internal/cli/spell_catalog_retry_contract_test.go +++ b/internal/cli/spell_catalog_retry_contract_test.go @@ -108,8 +108,16 @@ func TestProductionSpellCatalogValidationRetries(t *testing.T) { if len(value.SpellCasts) != 1 || value.SpellCasts[0].Spell != tt.wantSpell { t.Fatalf("normalized spell list = %#v, want accepted overlay spell", value) } - if len(output.Warnings) != 2 || output.Warnings[0].ReasonCode != tt.wantWarningCode || output.Warnings[1].ReasonCode != tt.wantWarningCode { - t.Fatalf("warnings = %#v, want accepted-attempt warnings from extract and normalize validation", output.Warnings) + if len(output.Warnings) != 1 || output.Warnings[0].ReasonCode != tt.wantWarningCode { + t.Fatalf("warnings = %#v, want the terminal extract compatibility projection", output.Warnings) + } + if len(output.Diagnostics.Groups) != 2 { + t.Fatalf("diagnostics = %#v, want extract and normalize data-quality advisories", output.Diagnostics) + } + for _, diagnostic := range output.Diagnostics.Groups { + if diagnostic.Disposition != contracts.DiagnosticDispositionAdvisory || diagnostic.ReasonCode != tt.wantWarningCode { + t.Fatalf("diagnostics = %#v, want only data-quality advisories", output.Diagnostics) + } } }) } diff --git a/internal/modules/dnd/shared/diagnostics/diagnostics.go b/internal/modules/dnd/shared/diagnostics/diagnostics.go index 4bc81a9e..bbfe0313 100644 --- a/internal/modules/dnd/shared/diagnostics/diagnostics.go +++ b/internal/modules/dnd/shared/diagnostics/diagnostics.go @@ -8,6 +8,7 @@ import ( "strings" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" + frameworkdiagnostics "gitea.maximumdirect.net/eric/notarius/internal/framework/diagnostics" ) const ( @@ -17,6 +18,25 @@ const ( MaxMessageBytes = 4096 ) +// DataQualityResult converts accepted source-quality findings into bounded, +// locally grouped advisories. These findings do not indicate process +// degradation. +func DataQualityResult(findings []contracts.Warning) (contracts.ValidationResult, error) { + collector := frameworkdiagnostics.NewCollector() + for _, finding := range findings { + if err := collector.Add(contracts.ProducerDiagnostic{ + Disposition: contracts.DiagnosticDispositionAdvisory, + Category: contracts.DiagnosticCategoryDataQuality, + ReasonCode: finding.ReasonCode, + OccurrenceCount: 1, + Samples: []contracts.DiagnosticSample{{Scope: finding.Scope, Message: finding.Message}}, + }); err != nil { + return contracts.ValidationResult{}, fmt.Errorf("collect data-quality diagnostic: %w", err) + } + } + return contracts.ValidationResult{Approved: true, Diagnostics: collector.Diagnostics()}, nil +} + func Truncate(value string) string { runes := []rune(value) if len(runes) <= MaxDisplayedRunes { diff --git a/internal/modules/dnd/shared/diagnostics/diagnostics_test.go b/internal/modules/dnd/shared/diagnostics/diagnostics_test.go index fdecfa76..17e99650 100644 --- a/internal/modules/dnd/shared/diagnostics/diagnostics_test.go +++ b/internal/modules/dnd/shared/diagnostics/diagnostics_test.go @@ -51,3 +51,21 @@ func TestLimitWarningsBoundsOutputAndReportsOmissions(t *testing.T) { t.Fatal("LimitWarnings() mutated its input") } } + +func TestDataQualityResultGroupsFindingsWithoutWarnings(t *testing.T) { + result, err := DataQualityResult([]contracts.Warning{ + {Scope: "records[0]", ReasonCode: "not_near_source", Message: "first"}, + {Scope: "records[0]", ReasonCode: "not_near_source", Message: "first"}, + {Scope: "records[1]", ReasonCode: "not_near_source", Message: "second"}, + }) + if err != nil { + t.Fatal(err) + } + if !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 { + t.Fatalf("result = %#v", result) + } + diagnostic := result.Diagnostics[0] + if diagnostic.Disposition != contracts.DiagnosticDispositionAdvisory || diagnostic.Category != contracts.DiagnosticCategoryDataQuality || diagnostic.OccurrenceCount != 3 || diagnostic.OmittedSampleCount != 1 || len(diagnostic.Samples) != 2 { + t.Fatalf("diagnostic = %#v", diagnostic) + } +} diff --git a/internal/modules/dnd/validate/combatturns/source_relatedness/validator.go b/internal/modules/dnd/validate/combatturns/source_relatedness/validator.go index ad5803bb..3e56f3fb 100644 --- a/internal/modules/dnd/validate/combatturns/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/combatturns/source_relatedness/validator.go @@ -13,10 +13,9 @@ import ( ) const ( - Key = "extract/dnd/combat-turns/source_relatedness" - WarningReasonCode = "combat_turn_not_near_source" - OmittedReasonCode = "combat_turn_relatedness_warnings_omitted" - policy = "dnd.combat_turns.validator.source_relatedness.v2" + Key = "extract/dnd/combat-turns/source_relatedness" + ReasonCode = "combat_turn_not_near_source" + policy = "dnd.combat_turns.validator.source_relatedness.v2" ) type Options struct{} @@ -59,16 +58,13 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq } warnings = append(warnings, contracts.Warning{ Scope: fmt.Sprintf("combat_turns[%d]", turnIndex), - ReasonCode: WarningReasonCode, + ReasonCode: ReasonCode, Message: diagnostics.Aggregate("combat turn not near source", []string{ fmt.Sprintf("actor %s was not found in cited source text", diagnostics.Quote(turn.Actor)), }), }) } - return contracts.ValidationResult{ - Approved: true, - Warnings: diagnostics.LimitWarnings(warnings, "combat_turns", OmittedReasonCode), - }, nil + return diagnostics.DataQualityResult(warnings) } func actorAppearsInCitedText(citedText string, actor string) bool { return shared.ContainsTokenSequence(citedText, actor) diff --git a/internal/modules/dnd/validate/combatturns/source_relatedness/validator_test.go b/internal/modules/dnd/validate/combatturns/source_relatedness/validator_test.go index 30f0847a..f6562121 100644 --- a/internal/modules/dnd/validate/combatturns/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/combatturns/source_relatedness/validator_test.go @@ -10,7 +10,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) func TestValidatorUsesDocumentOrderAndUnicodeComparisonForActor(t *testing.T) { @@ -26,8 +25,8 @@ func TestValidatorUsesDocumentOrderAndUnicodeComparisonForActor(t *testing.T) { {ID: 2, Kind: "message", Text: "O’rin attacks the goblin."}, }} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.CombatTurnList]{Source: doc, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { - t.Fatalf("Validate() = %#v, %v; want Unicode-related approval without target warning", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { + t.Fatalf("Validate() = %#v, %v; want Unicode-related approval without a finding", result, err) } } @@ -37,17 +36,18 @@ func TestValidatorWarnsOncePerTurnForUnrelatedActor(t *testing.T) { SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}, {SourceID: "session", StartUnitID: 1, EndUnitID: 1}}, }}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.CombatTurnList]{Source: relatednessDocument(), Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 1 { - t.Fatalf("Validate() = %#v, %v; want one warning for the turn", result, err) + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 { + t.Fatalf("Validate() = %#v, %v; want one advisory for the turn", result, err) } - warning := result.Warnings[0] - if warning.Scope != "combat_turns[0]" || warning.ReasonCode != WarningReasonCode || !strings.Contains(warning.Message, `Missing\nName`) || strings.Contains(warning.Message, "Missing\nName") || !utf8.ValidString(warning.Message) { - t.Fatalf("warning = %#v, want one safely quoted bounded warning", warning) + diagnostic := result.Diagnostics[0] + sample := diagnostic.Samples[0] + if diagnostic.Disposition != contracts.DiagnosticDispositionAdvisory || diagnostic.Category != contracts.DiagnosticCategoryDataQuality || diagnostic.ReasonCode != ReasonCode || sample.Scope != "combat_turns[0]" || !strings.Contains(sample.Message, `Missing\nName`) || strings.Contains(sample.Message, "Missing\nName") || !utf8.ValidString(sample.Message) { + t.Fatalf("diagnostic = %#v, want one safely quoted advisory", diagnostic) } } func TestValidatorLimitsUnrelatedActorWarnings(t *testing.T) { - turns := make([]dnd.CombatTurn, diagnostics.MaxWarnings+1) + turns := make([]dnd.CombatTurn, contracts.MaxDiagnosticSamples+2) for index := range turns { turns[index] = dnd.CombatTurn{ Actor: "Missing Actor", @@ -62,15 +62,8 @@ func TestValidatorLimitsUnrelatedActorWarnings(t *testing.T) { if err != nil || !result.Approved { t.Fatalf("Validate() = %#v, %v", result, err) } - if len(result.Warnings) != diagnostics.MaxWarnings { - t.Fatalf("warning count = %d, want %d", len(result.Warnings), diagnostics.MaxWarnings) - } - if first := result.Warnings[0]; first.Scope != "combat_turns[0]" || first.ReasonCode != WarningReasonCode { - t.Fatalf("first warning = %#v, want first turn warning", first) - } - summary := result.Warnings[len(result.Warnings)-1] - if summary.Scope != "combat_turns" || summary.ReasonCode != OmittedReasonCode || summary.Message != "2 additional warning(s) omitted" { - t.Fatalf("warning summary = %#v", summary) + if len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].OccurrenceCount != len(turns) || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples || result.Diagnostics[0].OmittedSampleCount != len(turns)-contracts.MaxDiagnosticSamples { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } } @@ -81,22 +74,22 @@ func TestValidatorDoesNotMatchShortActorSubstring(t *testing.T) { }}} doc := &source.SourceDocument{ID: "session", Kind: "transcript", Format: "application/json", Digest: "sha256:session", Units: []source.SourceUnit{{ID: 1, Kind: "message", Text: "The cart attacks."}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.CombatTurnList]{Source: doc, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 1 || !strings.Contains(result.Warnings[0].Message, "actor") { - t.Fatalf("Validate() = %#v, %v; want short-actor boundary warning", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 1 || !strings.Contains(result.Diagnostics[0].Samples[0].Message, "actor") { + t.Fatalf("Validate() = %#v, %v; want short-actor boundary advisory", result, err) } } func TestValidatorDefersMalformedShapeAndInvalidRanges(t *testing.T) { invalidShape := dnd.CombatTurnList{CombatTurns: []dnd.CombatTurn{{Actor: "Aria"}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.CombatTurnList]{Source: relatednessDocument(), Value: invalidShape}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { - t.Fatalf("shape deferral = %#v, %v; want approval without warning", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { + t.Fatalf("shape deferral = %#v, %v; want approval without finding", result, err) } invalidRange := validCombatTurnList() invalidRange.CombatTurns[0].SourceRefs[0].StartUnitID = 99 result, err = New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.CombatTurnList]{Source: relatednessDocument(), Value: invalidRange}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { - t.Fatalf("invalid-range deferral = %#v, %v; want approval without warning", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { + t.Fatalf("invalid-range deferral = %#v, %v; want approval without finding", result, err) } } @@ -105,8 +98,8 @@ func TestValidatorIgnoresReferenceMaterialAndRegistersPolicy(t *testing.T) { references := contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{"glossary": {Items: []contracts.ReferenceItem{{Content: []byte("Aria attacks")}}}}} doc := &source.SourceDocument{ID: "session", Kind: "transcript", Format: "application/json", Digest: "sha256:session", Units: []source.SourceUnit{{ID: 1, Kind: "message", Text: "The party waits."}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.CombatTurnList]{Source: doc, References: references, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 1 { - t.Fatalf("reference-only relatedness = %#v, %v; want warning from transcript-only evidence", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 1 { + t.Fatalf("reference-only relatedness = %#v, %v; want advisory from transcript-only evidence", result, err) } if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Name != "policy" || got[0].Value != "dnd.combat_turns.validator.source_relatedness.v2" { t.Fatalf("CheckpointFingerprints() = %#v, want relatedness policy", got) diff --git a/internal/modules/dnd/validate/correction_contract_test.go b/internal/modules/dnd/validate/correction_contract_test.go index 64331cf1..4054a51f 100644 --- a/internal/modules/dnd/validate/correction_contract_test.go +++ b/internal/modules/dnd/validate/correction_contract_test.go @@ -32,10 +32,12 @@ func TestRepresentativeProductionValidatorResultsSatisfyCorrectionContract(t *te unknownNPCID := "npc:sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" tests := []struct { - name string - validate func() (contracts.ValidationResult, error) - wantApproved bool - wantWarningCount int + name string + validate func() (contracts.ValidationResult, error) + wantApproved bool + wantWarningCount int + wantDiagnosticCount int + wantDisposition contracts.DiagnosticDisposition }{ { name: "shape rejection", @@ -106,8 +108,9 @@ func TestRepresentativeProductionValidatorResultsSatisfyCorrectionContract(t *te }}}, }) }, - wantApproved: true, - wantWarningCount: 1, + wantApproved: true, + wantDiagnosticCount: 1, + wantDisposition: contracts.DiagnosticDispositionAdvisory, }, } @@ -123,6 +126,12 @@ func TestRepresentativeProductionValidatorResultsSatisfyCorrectionContract(t *te if len(result.Warnings) != test.wantWarningCount { t.Fatalf("Validate() warnings = %d, want %d: %#v", len(result.Warnings), test.wantWarningCount, result) } + if len(result.Diagnostics) != test.wantDiagnosticCount { + t.Fatalf("Validate() diagnostics = %d, want %d: %#v", len(result.Diagnostics), test.wantDiagnosticCount, result) + } + if test.wantDisposition != "" && result.Diagnostics[0].Disposition != test.wantDisposition { + t.Fatalf("Validate() diagnostic disposition = %q, want %q: %#v", result.Diagnostics[0].Disposition, test.wantDisposition, result) + } if err := contracts.ValidateValidationResult(result); err != nil { t.Fatalf("ValidateValidationResult() error = %v for %#v", err, result) } diff --git a/internal/modules/dnd/validate/enemyevents/source_relatedness/validator.go b/internal/modules/dnd/validate/enemyevents/source_relatedness/validator.go index 9eccbea9..08378e3f 100644 --- a/internal/modules/dnd/validate/enemyevents/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/enemyevents/source_relatedness/validator.go @@ -14,10 +14,9 @@ import ( ) const ( - Key = "extract/dnd/enemy-events/source_relatedness" - WarningReasonCode = "enemy_event_not_near_source" - OmittedReasonCode = "enemy_event_relatedness_warnings_omitted" - policy = "dnd.enemy_events.validator.source_relatedness.v1" + Key = "extract/dnd/enemy-events/source_relatedness" + ReasonCode = "enemy_event_not_near_source" + policy = "dnd.enemy_events.validator.source_relatedness.v1" ) type Options struct{} @@ -51,13 +50,13 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq } warnings = append(warnings, contracts.Warning{ Scope: fmt.Sprintf("events[%d]", eventIndex), - ReasonCode: WarningReasonCode, + ReasonCode: ReasonCode, Message: diagnostics.Aggregate("enemy event subject not near source", []string{ fmt.Sprintf("subject %s was not found in cited source text", diagnostics.Quote(event.Name)), }), }) } - return contracts.ValidationResult{Approved: true, Warnings: diagnostics.LimitWarnings(warnings, "enemy_events", OmittedReasonCode)}, nil + return diagnostics.DataQualityResult(warnings) } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/enemyevents/source_relatedness/validator_test.go b/internal/modules/dnd/validate/enemyevents/source_relatedness/validator_test.go index 2b97da37..a17c7a3b 100644 --- a/internal/modules/dnd/validate/enemyevents/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/enemyevents/source_relatedness/validator_test.go @@ -14,7 +14,7 @@ func TestValidatorUsesOnlyCitedTranscriptEvidence(t *testing.T) { value := validEventList() references := contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{"npc_registry": {Items: []contracts.ReferenceItem{{Content: []byte("Ashfang")}}}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.EnemyEventList]{Source: document("The party waits."), References: references, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 1 || result.Warnings[0].ReasonCode != WarningReasonCode { + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].ReasonCode != ReasonCode || result.Diagnostics[0].Disposition != contracts.DiagnosticDispositionAdvisory { t.Fatalf("Validate() = %#v, %v", result, err) } } @@ -23,7 +23,7 @@ func TestValidatorAcceptsUnicodeSubjectInCitedEvidence(t *testing.T) { value := validEventList() value.Events[0].Name = "O'Rin Thorn" result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.EnemyEventList]{Source: document("o’rin\u2003thorn flees."), Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("Validate() = %#v, %v", result, err) } } @@ -31,7 +31,7 @@ func TestValidatorAcceptsUnicodeSubjectInCitedEvidence(t *testing.T) { func TestValidatorDefersMalformedValuesAndBoundsWarnings(t *testing.T) { malformed := dnd.EnemyEventList{Events: []dnd.EnemyEvent{{Name: "Ashfang"}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.EnemyEventList]{Source: document("none"), Value: malformed}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("shape deferral = %#v, %v", result, err) } value := dnd.EnemyEventList{Events: make([]dnd.EnemyEvent, 30)} @@ -39,8 +39,8 @@ func TestValidatorDefersMalformedValuesAndBoundsWarnings(t *testing.T) { value.Events[index] = dnd.EnemyEvent{Name: "Missing", Kind: dnd.EnemyEventKindEngaged, SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}} } result, err = New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.EnemyEventList]{Source: document("none"), Value: value}) - if err != nil || !result.Approved || len(result.Warnings) == 0 || result.Warnings[len(result.Warnings)-1].ReasonCode != OmittedReasonCode { - t.Fatalf("bounded warnings = %#v, %v", result, err) + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].OccurrenceCount != len(value.Events) || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples { + t.Fatalf("bounded advisories = %#v, %v", result, err) } registry := pipeline.NewValidatorRegistry() if err := Register(registry); err != nil { diff --git a/internal/modules/dnd/validate/itemoccurrences/source_relatedness/validator.go b/internal/modules/dnd/validate/itemoccurrences/source_relatedness/validator.go index 75681d03..b097cbb3 100644 --- a/internal/modules/dnd/validate/itemoccurrences/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/itemoccurrences/source_relatedness/validator.go @@ -1,4 +1,4 @@ -// Package sourcerelatedness warns when item-occurrence evidence does not mention its item. +// Package sourcerelatedness reports advisory findings when item-occurrence evidence does not mention its item. package sourcerelatedness import ( @@ -14,10 +14,9 @@ import ( ) const ( - Key = "extract/dnd/item-occurrences/source_relatedness" - WarningReasonCode = "item_occurrence_source_unrelated" - OmittedReasonCode = "item_occurrence_relatedness_warnings_omitted" - policy = "dnd.item_occurrences.source_relatedness.v1" + Key = "extract/dnd/item-occurrences/source_relatedness" + ReasonCode = "item_occurrence_not_near_source" + policy = "dnd.item_occurrences.source_relatedness.v1" ) type Options struct{} @@ -54,14 +53,11 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq } warnings = append(warnings, contracts.Warning{ Scope: fmt.Sprintf("occurrences[%d]", index), - ReasonCode: WarningReasonCode, + ReasonCode: ReasonCode, Message: fmt.Sprintf("item occurrence name %s was not found in cited source text", diagnostics.Quote(occurrence.Name)), }) } - return contracts.ValidationResult{ - Approved: true, - Warnings: diagnostics.LimitWarnings(warnings, "item_occurrences", OmittedReasonCode), - }, nil + return diagnostics.DataQualityResult(warnings) } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/itemoccurrences/source_relatedness/validator_test.go b/internal/modules/dnd/validate/itemoccurrences/source_relatedness/validator_test.go index cd749d63..be856778 100644 --- a/internal/modules/dnd/validate/itemoccurrences/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/itemoccurrences/source_relatedness/validator_test.go @@ -10,7 +10,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) func TestValidatorMatchesTokenSequencesAcrossRanges(t *testing.T) { @@ -26,11 +25,11 @@ func TestValidatorMatchesTokenSequencesAcrossRanges(t *testing.T) { "glossary": {Items: []contracts.ReferenceItem{{Content: []byte("missing-item")}}}, }} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.ItemOccurrenceList]{Source: doc, References: references, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 1 { + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 { t.Fatalf("Validate() = %#v, %v", result, err) } - if warning := result.Warnings[0]; warning.Scope != "occurrences[1]" || warning.ReasonCode != WarningReasonCode || !strings.Contains(warning.Message, "missing-item") { - t.Fatalf("warning = %#v", warning) + if diagnostic := result.Diagnostics[0]; diagnostic.Samples[0].Scope != "occurrences[1]" || diagnostic.ReasonCode != ReasonCode || !strings.Contains(diagnostic.Samples[0].Message, "missing-item") { + t.Fatalf("diagnostic = %#v", diagnostic) } } @@ -41,14 +40,14 @@ func TestValidatorDefersMalformedAndUnreadableEvidence(t *testing.T) { {Occurrences: []dnd.ItemOccurrence{{ItemID: "item", Name: "potion", Kind: dnd.ItemOccurrenceKindDiscovered, SourceRefs: []source.SourceRef{{SourceID: doc.ID, StartUnitID: 99, EndUnitID: 99}}}}}, } { result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.ItemOccurrenceList]{Source: doc, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("deferred result = %#v, %v", result, err) } } } func TestValidatorBoundsWarningsAndRegistersPolicy(t *testing.T) { - count := diagnostics.MaxWarnings + 5 + count := contracts.MaxDiagnosticSamples + 5 occurrences := make([]dnd.ItemOccurrence, count) for index := range occurrences { occurrences[index] = dnd.ItemOccurrence{ItemID: "item", Name: "missing item", Kind: dnd.ItemOccurrenceKindDiscovered, SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}} @@ -57,7 +56,7 @@ func TestValidatorBoundsWarningsAndRegistersPolicy(t *testing.T) { Source: &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1, Text: "nothing useful"}}}, Value: dnd.ItemOccurrenceList{Occurrences: occurrences}, }) - if err != nil || !result.Approved || len(result.Warnings) != diagnostics.MaxWarnings || result.Warnings[len(result.Warnings)-1].ReasonCode != OmittedReasonCode { + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].OccurrenceCount != count || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples { t.Fatalf("Validate() = %#v, %v", result, err) } if got := New(Options{}).CheckpointFingerprints(); !reflect.DeepEqual(got, []pipeline.CheckpointFingerprint{{Name: "policy", Value: policy}}) { diff --git a/internal/modules/dnd/validate/itemregistry/source_relatedness/validator.go b/internal/modules/dnd/validate/itemregistry/source_relatedness/validator.go index ce6adf3a..0a2ec6ef 100644 --- a/internal/modules/dnd/validate/itemregistry/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/itemregistry/source_relatedness/validator.go @@ -1,4 +1,4 @@ -// Package sourcerelatedness warns when cited source text does not mention an item. +// Package sourcerelatedness reports advisory findings when cited source text does not mention an item. package sourcerelatedness import ( @@ -14,10 +14,9 @@ import ( ) const ( - Key = "extract/dnd/item-registry/source_relatedness" - WarningReasonCode = "item_not_near_source" - OmittedReasonCode = "item_relatedness_warnings_omitted" - policy = "dnd.item_registry.validator.source_relatedness.v1" + Key = "extract/dnd/item-registry/source_relatedness" + ReasonCode = "item_not_near_source" + policy = "dnd.item_registry.validator.source_relatedness.v1" ) type Options struct{} @@ -57,11 +56,11 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq continue } warnings = append(warnings, contracts.Warning{ - Scope: fmt.Sprintf("items[%d]", itemIndex), ReasonCode: WarningReasonCode, + Scope: fmt.Sprintf("items[%d]", itemIndex), ReasonCode: ReasonCode, Message: fmt.Sprintf("Item %s was not found in cited source text", diagnostics.Quote(item.Name)), }) } - return contracts.ValidationResult{Approved: true, Warnings: diagnostics.LimitWarnings(warnings, "items", OmittedReasonCode)}, nil + return diagnostics.DataQualityResult(warnings) } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/itemregistry/source_relatedness/validator_test.go b/internal/modules/dnd/validate/itemregistry/source_relatedness/validator_test.go index 147d839d..d816a266 100644 --- a/internal/modules/dnd/validate/itemregistry/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/itemregistry/source_relatedness/validator_test.go @@ -9,32 +9,31 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) func TestValidatorUsesOnlyCitedTranscriptText(t *testing.T) { value := dnd.ItemRegistry{Items: []dnd.Item{{ID: "item", Name: "Star Compass", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}}}} doc := &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1, Text: "They recover the star compass."}, {ID: 2, Text: "Unrelated text."}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.ItemRegistry]{Source: doc, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("cited match = %#v, %v; want approval without warnings", result, err) } value.Items[0].Name = "Glossary Relic" before := value result, err = New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.ItemRegistry]{Source: doc, References: contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{"glossary": {Items: []contracts.ReferenceItem{{Content: []byte("Glossary Relic")}}}}}, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 1 || result.Warnings[0].ReasonCode != WarningReasonCode || !reflect.DeepEqual(value, before) { + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].ReasonCode != ReasonCode || !reflect.DeepEqual(value, before) { t.Fatalf("reference-only match = %#v, %v; want advisory warning", result, err) } } func TestValidatorBoundsWarningsAndRegisters(t *testing.T) { - items := make([]dnd.Item, diagnostics.MaxWarnings+1) + items := make([]dnd.Item, contracts.MaxDiagnosticSamples+2) for index := range items { items[index] = dnd.Item{ID: "item", Name: "Missing", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}} } result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.ItemRegistry]{Source: &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1, Text: "Nothing here."}}}, Value: dnd.ItemRegistry{Items: items}}) - if err != nil || !result.Approved || len(result.Warnings) != diagnostics.MaxWarnings || result.Warnings[len(result.Warnings)-1].ReasonCode != OmittedReasonCode { - t.Fatalf("bounded warnings = %#v, %v", result, err) + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].OccurrenceCount != len(items) || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples { + t.Fatalf("bounded advisories = %#v, %v", result, err) } registry := pipeline.NewValidatorRegistry() if err := Register(registry); err != nil { diff --git a/internal/modules/dnd/validate/locationoccurrences/source_relatedness/validator.go b/internal/modules/dnd/validate/locationoccurrences/source_relatedness/validator.go index c1795388..240d5a0c 100644 --- a/internal/modules/dnd/validate/locationoccurrences/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/locationoccurrences/source_relatedness/validator.go @@ -1,4 +1,4 @@ -// Package sourcerelatedness warns when cited source text does not mention a location occurrence. +// Package sourcerelatedness reports advisory findings when cited source text does not mention a location occurrence. package sourcerelatedness import ( @@ -14,10 +14,9 @@ import ( ) const ( - Key = "extract/dnd/location-occurrences/source_relatedness" - WarningReasonCode = "location_occurrence_not_near_source" - OmittedReasonCode = "location_occurrence_relatedness_warnings_omitted" - policy = "dnd.location_occurrences.validator.source_relatedness.v1" + Key = "extract/dnd/location-occurrences/source_relatedness" + ReasonCode = "location_occurrence_not_near_source" + policy = "dnd.location_occurrences.validator.source_relatedness.v1" ) type Options struct{} @@ -56,9 +55,9 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq if shared.ContainsTokenSequence(citedTexts[index], occurrence.Name) { continue } - warnings = append(warnings, contracts.Warning{Scope: fmt.Sprintf("occurrences[%d]", index), ReasonCode: WarningReasonCode, Message: fmt.Sprintf("Location occurrence name %s was not found in cited source text", diagnostics.Quote(occurrence.Name))}) + warnings = append(warnings, contracts.Warning{Scope: fmt.Sprintf("occurrences[%d]", index), ReasonCode: ReasonCode, Message: fmt.Sprintf("Location occurrence name %s was not found in cited source text", diagnostics.Quote(occurrence.Name))}) } - return contracts.ValidationResult{Approved: true, Warnings: diagnostics.LimitWarnings(warnings, "location_occurrences", OmittedReasonCode)}, nil + return diagnostics.DataQualityResult(warnings) } func Spec() pipeline.ValidatorSpec { return pipeline.ValidatorSpec{Key: Key, ExecutionClass: contracts.ExecutionClassDeterministic} diff --git a/internal/modules/dnd/validate/locationoccurrences/source_relatedness/validator_test.go b/internal/modules/dnd/validate/locationoccurrences/source_relatedness/validator_test.go index 36965686..a2c478a2 100644 --- a/internal/modules/dnd/validate/locationoccurrences/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/locationoccurrences/source_relatedness/validator_test.go @@ -10,21 +10,20 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) func TestValidatorUsesOnlyCitedTranscriptEvidence(t *testing.T) { value := occurrenceList("O'Rin Gate") doc := &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1, Text: "The party enters o’rin gate."}, {ID: 2, Text: "No location here."}}} result, err := New(Options{}).Validate(context.Background(), request(doc, value, contracts.ReferenceSet{})) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("cited match = %#v, %v", result, err) } value = occurrenceList("Glossary Keep") before := cloneList(value) references := contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{"glossary": {Items: []contracts.ReferenceItem{{Content: []byte("Glossary Keep")}}}}} result, err = New(Options{}).Validate(context.Background(), request(doc, value, references)) - if err != nil || !result.Approved || len(result.Warnings) != 1 || result.Warnings[0].ReasonCode != WarningReasonCode || !strings.Contains(result.Warnings[0].Message, "Glossary Keep") || !reflect.DeepEqual(value, before) { + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].ReasonCode != ReasonCode || !strings.Contains(result.Diagnostics[0].Samples[0].Message, "Glossary Keep") || !reflect.DeepEqual(value, before) { t.Fatalf("reference-only match = %#v, %v", result, err) } } @@ -34,16 +33,16 @@ func TestValidatorDefersUnreadableEvidenceAndBoundsWarnings(t *testing.T) { invalid.Occurrences[0].SourceRefs[0].StartUnitID = 99 invalid.Occurrences[0].SourceRefs[0].EndUnitID = 99 result, err := New(Options{}).Validate(context.Background(), request(document(), invalid, contracts.ReferenceSet{})) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("unreadable evidence = %#v, %v", result, err) } - occurrences := make([]dnd.LocationOccurrence, diagnostics.MaxWarnings+1) + occurrences := make([]dnd.LocationOccurrence, contracts.MaxDiagnosticSamples+2) for index := range occurrences { occurrences[index] = occurrenceList("Missing").Occurrences[0] } result, err = New(Options{}).Validate(context.Background(), request(document(), dnd.LocationOccurrenceList{Occurrences: occurrences}, contracts.ReferenceSet{})) - if err != nil || !result.Approved || len(result.Warnings) != diagnostics.MaxWarnings || result.Warnings[len(result.Warnings)-1].ReasonCode != OmittedReasonCode { - t.Fatalf("bounded warnings = %#v, %v", result, err) + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].OccurrenceCount != len(occurrences) || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples { + t.Fatalf("bounded advisories = %#v, %v", result, err) } } diff --git a/internal/modules/dnd/validate/locationregistry/source_relatedness/validator.go b/internal/modules/dnd/validate/locationregistry/source_relatedness/validator.go index 291f0cb0..d6062f2e 100644 --- a/internal/modules/dnd/validate/locationregistry/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/locationregistry/source_relatedness/validator.go @@ -1,4 +1,4 @@ -// Package sourcerelatedness warns when cited source text does not mention a location. +// Package sourcerelatedness reports advisory findings when cited source text does not mention a location. package sourcerelatedness import ( @@ -14,10 +14,9 @@ import ( ) const ( - Key = "extract/dnd/location-registry/source_relatedness" - WarningReasonCode = "location_not_near_source" - OmittedReasonCode = "location_relatedness_warnings_omitted" - policy = "dnd.location_registry.validator.source_relatedness.v2" + Key = "extract/dnd/location-registry/source_relatedness" + ReasonCode = "location_not_near_source" + policy = "dnd.location_registry.validator.source_relatedness.v2" ) type Options struct{} @@ -57,11 +56,11 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq continue } warnings = append(warnings, contracts.Warning{ - Scope: fmt.Sprintf("locations[%d]", locationIndex), ReasonCode: WarningReasonCode, + Scope: fmt.Sprintf("locations[%d]", locationIndex), ReasonCode: ReasonCode, Message: fmt.Sprintf("Location %s was not found in cited source text", diagnostics.Quote(location.Name)), }) } - return contracts.ValidationResult{Approved: true, Warnings: diagnostics.LimitWarnings(warnings, "locations", OmittedReasonCode)}, nil + return diagnostics.DataQualityResult(warnings) } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/locationregistry/source_relatedness/validator_test.go b/internal/modules/dnd/validate/locationregistry/source_relatedness/validator_test.go index db5b21d5..c76c5bdb 100644 --- a/internal/modules/dnd/validate/locationregistry/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/locationregistry/source_relatedness/validator_test.go @@ -10,21 +10,20 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) func TestValidatorUsesOnlyCitedTranscriptText(t *testing.T) { value := dnd.LocationRegistry{Locations: []dnd.Location{{ID: "candidate", Name: "O'Rin's Gate", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}}}} doc := &source.SourceDocument{ID: "session", Kind: "transcript", Format: "application/json", Digest: "sha256:session", Units: []source.SourceUnit{{ID: 1, Kind: "message", Text: "The party enters o’rin’s gate."}, {ID: 2, Kind: "message", Text: "Unrelated location."}}} result, err := New(Options{}).Validate(context.Background(), request(doc, value, contracts.ReferenceSet{})) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("cited match = %#v, %v; want approval without warnings", result, err) } value.Locations[0].Name = "Glossary Keep" before := value references := contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{"glossary": {Items: []contracts.ReferenceItem{{Content: []byte("Glossary Keep")}}}}} result, err = New(Options{}).Validate(context.Background(), request(doc, value, references)) - if err != nil || !result.Approved || len(result.Warnings) != 1 || result.Warnings[0].ReasonCode != WarningReasonCode || !strings.Contains(result.Warnings[0].Message, "Glossary Keep") || !reflect.DeepEqual(value, before) { + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].ReasonCode != ReasonCode || !strings.Contains(result.Diagnostics[0].Samples[0].Message, "Glossary Keep") || !reflect.DeepEqual(value, before) { t.Fatalf("reference-only match = %#v, %v; want advisory warning", result, err) } } @@ -32,12 +31,12 @@ func TestValidatorUsesOnlyCitedTranscriptText(t *testing.T) { func TestValidatorDefersMalformedOrUnreadableCitations(t *testing.T) { invalid := dnd.LocationRegistry{Locations: []dnd.Location{{Name: "Missing"}}} result, err := New(Options{}).Validate(context.Background(), request(document(), invalid, contracts.ReferenceSet{})) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("shape deferral = %#v, %v", result, err) } value := dnd.LocationRegistry{Locations: []dnd.Location{{ID: "candidate", Name: "Missing", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 99, EndUnitID: 99}}}}} result, err = New(Options{}).Validate(context.Background(), request(document(), value, contracts.ReferenceSet{})) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("unreadable citation = %#v, %v", result, err) } } @@ -56,13 +55,13 @@ func TestValidatorRegisters(t *testing.T) { } func TestValidatorBoundsWarnings(t *testing.T) { - locations := make([]dnd.Location, diagnostics.MaxWarnings+1) + locations := make([]dnd.Location, contracts.MaxDiagnosticSamples+2) for index := range locations { locations[index] = dnd.Location{ID: "candidate", Name: "Missing", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}} } result, err := New(Options{}).Validate(context.Background(), request(document(), dnd.LocationRegistry{Locations: locations}, contracts.ReferenceSet{})) - if err != nil || !result.Approved || len(result.Warnings) != diagnostics.MaxWarnings || result.Warnings[len(result.Warnings)-1].ReasonCode != OmittedReasonCode { - t.Fatalf("bounded warnings = %#v, %v", result, err) + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].OccurrenceCount != len(locations) || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples { + t.Fatalf("bounded advisories = %#v, %v", result, err) } } diff --git a/internal/modules/dnd/validate/npcoccurrences/source_relatedness/validator.go b/internal/modules/dnd/validate/npcoccurrences/source_relatedness/validator.go index ab352982..af23bde8 100644 --- a/internal/modules/dnd/validate/npcoccurrences/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/npcoccurrences/source_relatedness/validator.go @@ -1,4 +1,4 @@ -// Package sourcerelatedness warns about NPC occurrence evidence unrelated to its NPC. +// Package sourcerelatedness reports advisory findings about NPC occurrence evidence unrelated to its NPC. package sourcerelatedness import ( @@ -14,10 +14,9 @@ import ( ) const ( - Key = "extract/dnd/npc-occurrences/source_relatedness" - WarningReasonCode = "npc_occurrence_not_near_source" - OmittedReasonCode = "npc_occurrence_relatedness_warnings_omitted" - policy = "dnd.npc_occurrences.validator.source_relatedness.v2" + Key = "extract/dnd/npc-occurrences/source_relatedness" + ReasonCode = "npc_occurrence_not_near_source" + policy = "dnd.npc_occurrences.validator.source_relatedness.v2" ) type Options struct{} @@ -58,14 +57,11 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq } warnings = append(warnings, contracts.Warning{ Scope: fmt.Sprintf("occurrences[%d]", index), - ReasonCode: WarningReasonCode, + ReasonCode: ReasonCode, Message: fmt.Sprintf("NPC occurrence name %s was not found in cited source text", diagnostics.Quote(occurrence.Name)), }) } - return contracts.ValidationResult{ - Approved: true, - Warnings: diagnostics.LimitWarnings(warnings, "npc_occurrences", OmittedReasonCode), - }, nil + return diagnostics.DataQualityResult(warnings) } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/npcoccurrences/source_relatedness/validator_test.go b/internal/modules/dnd/validate/npcoccurrences/source_relatedness/validator_test.go index 4f34d6b7..8851adab 100644 --- a/internal/modules/dnd/validate/npcoccurrences/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/npcoccurrences/source_relatedness/validator_test.go @@ -10,7 +10,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) func TestValidatorUsesOnlyCurrentTranscriptAndWarnsOncePerOccurrence(t *testing.T) { @@ -21,11 +20,11 @@ func TestValidatorUsesOnlyCurrentTranscriptAndWarnsOncePerOccurrence(t *testing. doc := &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1, Text: "O’Rin Thorn speaks."}, {ID: 2, Text: "The party waits."}}} references := contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{"glossary": {Items: []contracts.ReferenceItem{{Content: []byte("Missing NPC")}}}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCOccurrenceList]{Source: doc, References: references, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 1 { + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 { t.Fatalf("Validate() = %#v, %v", result, err) } - if warning := result.Warnings[0]; warning.Scope != "occurrences[1]" || warning.ReasonCode != WarningReasonCode || !strings.Contains(warning.Message, `Missing\nNPC`) { - t.Fatalf("warning = %#v", warning) + if diagnostic := result.Diagnostics[0]; diagnostic.Samples[0].Scope != "occurrences[1]" || diagnostic.ReasonCode != ReasonCode || !strings.Contains(diagnostic.Samples[0].Message, `Missing\nNPC`) { + t.Fatalf("diagnostic = %#v", diagnostic) } } @@ -35,7 +34,7 @@ func TestValidatorDefersMalformedShapeAndInvalidRanges(t *testing.T) { {Occurrences: []dnd.NPCOccurrence{{NPCID: "npc:test", Name: "Mira Thorn", Kind: dnd.NPCOccurrenceKindDialogue, SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 99, EndUnitID: 99}}}}}, } { result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCOccurrenceList]{Source: &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1}}}, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("deferral = %#v, %v", result, err) } } @@ -52,7 +51,7 @@ func TestValidatorDefersMalformedShapeAndInvalidRanges(t *testing.T) { } func TestValidatorBoundsWarnings(t *testing.T) { - count := diagnostics.MaxWarnings + 5 + count := contracts.MaxDiagnosticSamples + 5 occurrences := make([]dnd.NPCOccurrence, count) for index := range occurrences { occurrences[index] = dnd.NPCOccurrence{ @@ -69,8 +68,7 @@ func TestValidatorBoundsWarnings(t *testing.T) { if err != nil || !result.Approved { t.Fatalf("Validate() = %#v, %v", result, err) } - if len(result.Warnings) != diagnostics.MaxWarnings || - result.Warnings[len(result.Warnings)-1].ReasonCode != OmittedReasonCode { - t.Fatalf("warnings = %#v", result.Warnings) + if len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].OccurrenceCount != count || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } } diff --git a/internal/modules/dnd/validate/npcregistry/source_relatedness/validator.go b/internal/modules/dnd/validate/npcregistry/source_relatedness/validator.go index 40258cd2..8ef01264 100644 --- a/internal/modules/dnd/validate/npcregistry/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/npcregistry/source_relatedness/validator.go @@ -13,9 +13,9 @@ import ( ) const ( - Key = "extract/dnd/npc-registry/source_relatedness" - WarningReasonCode = "npc_not_near_source" - policy = "dnd.npc_registry.validator.source_relatedness.v2" + Key = "extract/dnd/npc-registry/source_relatedness" + ReasonCode = "npc_not_near_source" + policy = "dnd.npc_registry.validator.source_relatedness.v2" ) type Options struct{} @@ -56,11 +56,11 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq } warnings = append(warnings, contracts.Warning{ Scope: fmt.Sprintf("npcs[%d]", npcIndex), - ReasonCode: WarningReasonCode, + ReasonCode: ReasonCode, Message: fmt.Sprintf("NPC %s was not found in cited source text", diagnostics.Quote(npc.Name)), }) } - return contracts.ValidationResult{Approved: true, Warnings: warnings}, nil + return diagnostics.DataQualityResult(warnings) } func npcAppearsInCitedText(citedText string, npc dnd.NPC) bool { diff --git a/internal/modules/dnd/validate/npcregistry/source_relatedness/validator_test.go b/internal/modules/dnd/validate/npcregistry/source_relatedness/validator_test.go index 995731ac..aae0bd7a 100644 --- a/internal/modules/dnd/validate/npcregistry/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/npcregistry/source_relatedness/validator_test.go @@ -26,7 +26,7 @@ func TestValidatorMatchesCanonicalNamesWithUnicodeVariants(t *testing.T) { {ID: 2, Kind: "message", Text: "The greencloak watches."}, }} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCRegistry]{Source: doc, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("Validate() = %#v, %v; want canonical-name relatedness approval", result, err) } } @@ -36,12 +36,13 @@ func TestValidatorWarnsAtMostOncePerNPCForUnrelatedCitations(t *testing.T) { {ID: "one", Name: "Missing\nName", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}, {SourceID: "session", StartUnitID: 1, EndUnitID: 1}}}, }} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCRegistry]{Source: relatednessDocument(), Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 1 { - t.Fatalf("Validate() = %#v, %v; want one warning", result, err) + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 { + t.Fatalf("Validate() = %#v, %v; want one advisory", result, err) } - warning := result.Warnings[0] - if warning.Scope != "npcs[0]" || warning.ReasonCode != WarningReasonCode || !strings.Contains(warning.Message, `Missing\nName`) || strings.Contains(warning.Message, "Missing\nName") || !utf8.ValidString(warning.Message) { - t.Fatalf("warning = %#v, want safely quoted bounded warning", warning) + diagnostic := result.Diagnostics[0] + sample := diagnostic.Samples[0] + if sample.Scope != "npcs[0]" || diagnostic.ReasonCode != ReasonCode || !strings.Contains(sample.Message, `Missing\nName`) || strings.Contains(sample.Message, "Missing\nName") || !utf8.ValidString(sample.Message) { + t.Fatalf("diagnostic = %#v, want safely quoted advisory", diagnostic) } } @@ -51,21 +52,21 @@ func TestValidatorDoesNotMatchShortNameSubstring(t *testing.T) { }}} doc := &source.SourceDocument{ID: "session", Kind: "transcript", Format: "application/json", Digest: "sha256:session", Units: []source.SourceUnit{{ID: 1, Kind: "message", Text: "A cart rolls past."}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCRegistry]{Source: doc, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 1 { - t.Fatalf("Validate() = %#v, %v; want short-name boundary warning", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 1 { + t.Fatalf("Validate() = %#v, %v; want short-name boundary advisory", result, err) } } func TestValidatorDefersMalformedShapeAndInvalidRangesDoNotPanic(t *testing.T) { invalidShape := dnd.NPCRegistry{NPCs: []dnd.NPC{{Name: "Mira Thorn"}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCRegistry]{Source: relatednessDocument(), Value: invalidShape}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { - t.Fatalf("shape deferral = %#v, %v; want approval without warning", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { + t.Fatalf("shape deferral = %#v, %v; want approval without finding", result, err) } invalidRange := dnd.NPCRegistry{NPCs: []dnd.NPC{{ID: "one", Name: "Mira Thorn", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 99, EndUnitID: 99}}}}} result, err = New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCRegistry]{Source: relatednessDocument(), Value: invalidRange}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { - t.Fatalf("invalid-range relatedness = %#v, %v; want approval without warning", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { + t.Fatalf("invalid-range relatedness = %#v, %v; want approval without finding", result, err) } } @@ -73,8 +74,8 @@ func TestValidatorUsesOnlyTranscriptEvidenceAndRegistersPolicy(t *testing.T) { value := dnd.NPCRegistry{NPCs: []dnd.NPC{{ID: "one", Name: "Opaque NPC", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}}}} references := contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{"glossary": {Items: []contracts.ReferenceItem{{Content: []byte("Opaque NPC")}}}}} result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCRegistry]{Source: relatednessDocument(), References: references, Value: value}) - if err != nil || len(result.Warnings) != 1 { - t.Fatalf("reference-only relatedness = %#v, %v; want warning", result, err) + if err != nil || len(result.Diagnostics) != 1 { + t.Fatalf("reference-only relatedness = %#v, %v; want advisory", result, err) } if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Name != "policy" || got[0].Value != "dnd.npc_registry.validator.source_relatedness.v2" { t.Fatalf("CheckpointFingerprints() = %#v, want local policy", got) diff --git a/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator.go b/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator.go index ee40a4fd..5bd9ca40 100644 --- a/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator.go @@ -1,4 +1,4 @@ -// Package sourcerelatedness warns when scene prose has no lexical grounding. +// Package sourcerelatedness reports advisory findings when scene prose has no lexical grounding. package sourcerelatedness import ( @@ -16,10 +16,9 @@ import ( ) const ( - Key = "extract/dnd/scene-descriptions/source_relatedness" - WarningReasonCode = "scene_description_not_near_source" - OmittedReasonCode = "scene_description_relatedness_warnings_omitted" - policy = "dnd.scene_descriptions.validator.source_relatedness.v1" + Key = "extract/dnd/scene-descriptions/source_relatedness" + ReasonCode = "scene_description_not_near_source" + policy = "dnd.scene_descriptions.validator.source_relatedness.v1" ) var stopwords = map[string]struct{}{ @@ -66,10 +65,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq warnings = append(warnings, warning(index, "summary")) } } - return contracts.ValidationResult{ - Approved: true, - Warnings: diagnostics.LimitWarnings(warnings, "scenes", OmittedReasonCode), - }, nil + return diagnostics.DataQualityResult(warnings) } func tokenSet(value string) map[string]struct{} { @@ -105,7 +101,7 @@ func significant(token string) bool { func warning(index int, field string) contracts.Warning { return contracts.Warning{ Scope: fmt.Sprintf("scenes[%d].%s", index, field), - ReasonCode: WarningReasonCode, + ReasonCode: ReasonCode, Message: "scene description " + field + " has no significant token in cited source text", } } diff --git a/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator_test.go b/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator_test.go index 5e615c60..505beced 100644 --- a/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/scenedescriptions/source_relatedness/validator_test.go @@ -24,16 +24,16 @@ func TestValidatorWarnsIndependentlyForUngroundedTitleAndSummary(t *testing.T) { t.Fatalf("Validate() = %#v, %v", result, err) } wantScopes := []string{"scenes[1].title", "scenes[1].summary", "scenes[2].title", "scenes[2].summary"} - if len(result.Warnings) != len(wantScopes) { - t.Fatalf("warnings = %#v, want %d", result.Warnings, len(wantScopes)) + if len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].OccurrenceCount != len(wantScopes) { + t.Fatalf("diagnostics = %#v, want %d occurrences", result.Diagnostics, len(wantScopes)) } - for index, scope := range wantScopes { - item := result.Warnings[index] - if item.Scope != scope || item.ReasonCode != WarningReasonCode { - t.Fatalf("warning[%d] = %#v, want scope %q", index, item, scope) + for index, scope := range wantScopes[:contracts.MaxDiagnosticSamples] { + item := result.Diagnostics[0].Samples[index] + if item.Scope != scope || result.Diagnostics[0].ReasonCode != ReasonCode { + t.Fatalf("sample[%d] = %#v, want scope %q", index, item, scope) } if len(item.Message) > 4096 || strings.Contains(item.Message, doc.Units[0].Text) { - t.Fatalf("warning[%d] is not safely bounded: %#v", index, item) + t.Fatalf("sample[%d] is not safely bounded: %#v", index, item) } } } @@ -47,11 +47,11 @@ func TestValidatorUsesTranscriptOnlyAndDefersInvalidInputs(t *testing.T) { "glossary": {Items: []contracts.ReferenceItem{{Content: []byte("Greencloak: title")}}}, }}, }) - if err != nil || len(result.Warnings) != 2 { - t.Fatalf("Validate() = %#v, %v; want transcript-only warnings", result, err) + if err != nil || len(result.Diagnostics) != 1 || result.Diagnostics[0].OccurrenceCount != 2 { + t.Fatalf("Validate() = %#v, %v; want transcript-only advisories", result, err) } malformed, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.SceneDescriptionList]{Source: doc, Value: dnd.SceneDescriptionList{}}) - if err != nil || !malformed.Approved || len(malformed.Warnings) != 0 { + if err != nil || !malformed.Approved || len(malformed.Diagnostics) != 0 { t.Fatalf("malformed Validate() = %#v, %v; want deferral", malformed, err) } @@ -59,7 +59,7 @@ func TestValidatorUsesTranscriptOnlyAndDefersInvalidInputs(t *testing.T) { invalidScene.SourceRef.StartUnitID = 99 invalidSource := dnd.SceneDescriptionList{Scenes: []dnd.SceneDescription{invalidScene}} deferred, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.SceneDescriptionList]{Source: doc, Value: invalidSource}) - if err != nil || !deferred.Approved || len(deferred.Warnings) != 0 { + if err != nil || !deferred.Approved || len(deferred.Diagnostics) != 0 { t.Fatalf("invalid-source Validate() = %#v, %v; want deferral", deferred, err) } } diff --git a/internal/modules/dnd/validate/spells/source_relatedness/validator.go b/internal/modules/dnd/validate/spells/source_relatedness/validator.go index 96eded72..1533a15b 100644 --- a/internal/modules/dnd/validate/spells/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/spells/source_relatedness/validator.go @@ -9,13 +9,14 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" spellshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/spells/shape" ) const ( - Key = "extract/dnd/spells/source_relatedness" - WarningReasonCode = "spell_not_near_source" - policy = "dnd.spells.validator.source_relatedness.v1" + Key = "extract/dnd/spells/source_relatedness" + ReasonCode = "spell_not_near_source" + policy = "dnd.spells.validator.source_relatedness.v1" ) type Options struct{} @@ -51,10 +52,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq var warnings []contracts.Warning for spellIndex, spell := range req.Value.SpellCasts { if !spellAppearsInCitedText(citedTexts[spellIndex], spell) { - warnings = append(warnings, contracts.Warning{Scope: fmt.Sprintf("spell_casts[%d]", spellIndex), ReasonCode: WarningReasonCode, Message: fmt.Sprintf("spell %q was not found in cited source text", strings.TrimSpace(spell.Spell))}) + warnings = append(warnings, contracts.Warning{Scope: fmt.Sprintf("spell_casts[%d]", spellIndex), ReasonCode: ReasonCode, Message: fmt.Sprintf("spell %s was not found in cited source text", diagnostics.Quote(strings.TrimSpace(spell.Spell)))}) } } - return contracts.ValidationResult{Approved: true, Warnings: warnings}, nil + return diagnostics.DataQualityResult(warnings) } func spellAppearsInCitedText(citedText string, spell dnd.SpellCast) bool { name := strings.TrimSpace(spell.Spell) diff --git a/internal/modules/dnd/validate/spells/source_relatedness/validator_test.go b/internal/modules/dnd/validate/spells/source_relatedness/validator_test.go index b82a1cde..774e1e8a 100644 --- a/internal/modules/dnd/validate/spells/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/spells/source_relatedness/validator_test.go @@ -18,8 +18,8 @@ func TestValidatorApprovesWithoutWarningWhenSpellAppearsInCitedText(t *testing.T if !result.Approved { t.Fatalf("Approved = false, want true") } - if len(result.Warnings) != 0 { - t.Fatalf("Warnings = %#v, want none", result.Warnings) + if len(result.Diagnostics) != 0 { + t.Fatalf("Diagnostics = %#v, want none", result.Diagnostics) } } @@ -31,11 +31,11 @@ func TestValidatorWarnsWhenSpellDoesNotAppearInCitedText(t *testing.T) { if !result.Approved { t.Fatalf("Approved = false, want true") } - if len(result.Warnings) != 1 { - t.Fatalf("Warnings = %#v, want one warning", result.Warnings) + if len(result.Warnings) != 0 || len(result.Diagnostics) != 1 { + t.Fatalf("Diagnostics = %#v, want one advisory", result.Diagnostics) } - if result.Warnings[0].ReasonCode != WarningReasonCode { - t.Fatalf("ReasonCode = %q, want %q", result.Warnings[0].ReasonCode, WarningReasonCode) + if result.Diagnostics[0].ReasonCode != ReasonCode || result.Diagnostics[0].Disposition != contracts.DiagnosticDispositionAdvisory { + t.Fatalf("diagnostic = %#v", result.Diagnostics[0]) } } @@ -56,7 +56,7 @@ func TestValidatorMatchesCaseInsensitiveUnicodeMultiwordSpellAcrossCitations(t * }, } result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.SpellList]{Source: doc, Value: value}) - if err != nil || !result.Approved || len(result.Warnings) != 0 { + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { t.Fatalf("Validate() = %#v, %v; want normalized multiword spell approval", result, err) } } @@ -66,8 +66,27 @@ func TestValidatorDoesNotMatchShortSpellNameSubstring(t *testing.T) { ID: "session", Kind: "transcript", Format: "application/json", Digest: "sha256:session", Units: []source.SourceUnit{{ID: 1, Kind: "message", Text: "The party said nothing."}}, }, "Aid", 1)) - if err != nil || !result.Approved || len(result.Warnings) != 1 { - t.Fatalf("Validate() = %#v, %v; want short-name boundary warning", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 1 { + t.Fatalf("Validate() = %#v, %v; want short-name boundary advisory", result, err) + } +} + +func TestValidatorBoundsHighCardinalitySpellFindings(t *testing.T) { + count := contracts.MaxDiagnosticSamples + 4 + casts := make([]dnd.SpellCast, count) + for index := range casts { + casts[index] = dnd.SpellCast{Caster: "Aria", Spell: "Missing Spell", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}} + } + result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.SpellList]{ + Source: &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1, Text: "The party waits."}}}, + Value: dnd.SpellList{SpellCasts: casts}, + }) + if err != nil || !result.Approved || len(result.Warnings) != 0 || len(result.Diagnostics) != 1 { + t.Fatalf("Validate() = %#v, %v", result, err) + } + diagnostic := result.Diagnostics[0] + if diagnostic.OccurrenceCount != count || len(diagnostic.Samples) != contracts.MaxDiagnosticSamples || diagnostic.OmittedSampleCount != count-contracts.MaxDiagnosticSamples { + t.Fatalf("diagnostic = %#v", diagnostic) } } @@ -75,8 +94,8 @@ func TestValidatorIgnoresInvalidCitations(t *testing.T) { request := requestWithSpell(validDocument(), "Cure Wounds", 2) request.Value.SpellCasts[0].SourceRefs[0].StartUnitID = 99 result, err := New(Options{}).Validate(context.Background(), request) - if err != nil || !result.Approved || len(result.Warnings) != 0 { - t.Fatalf("Validate() = %#v, %v; want approval without relatedness warning", result, err) + if err != nil || !result.Approved || len(result.Diagnostics) != 0 { + t.Fatalf("Validate() = %#v, %v; want approval without relatedness advisory", result, err) } }