From 480680b25708bec78148afd7aeedb34e6183ab7b Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 27 Aug 2026 16:04:32 +0000 Subject: [PATCH] Classify remaining D&D producer diagnostics --- docs/roadmap/implementation.md | 2 +- .../dnd/extract/combatturns/extractor.go | 10 ++- .../dnd/extract/combatturns/extractor_test.go | 8 +- .../dnd/extract/enemyevents/extractor.go | 10 ++- .../dnd/extract/enemyevents/extractor_test.go | 8 +- .../dnd/normalize/combatturns/normalizer.go | 10 +-- .../normalize/combatturns/normalizer_test.go | 35 +++----- .../dnd/normalize/enemyevents/normalizer.go | 9 +- .../normalize/enemyevents/normalizer_test.go | 18 ++-- .../normalize/itemoccurrences/normalizer.go | 9 +- .../itemoccurrences/normalizer_test.go | 10 +-- .../locationoccurrences/normalizer.go | 9 +- .../locationoccurrences/normalizer_test.go | 22 ++--- .../normalize/npcoccurrences/normalizer.go | 10 ++- .../npcoccurrences/normalizer_test.go | 21 +++-- .../normalize/scenedescriptions/normalizer.go | 20 ++--- .../scenedescriptions/normalizer_test.go | 25 ++---- .../dnd/normalize/spells/fixture_test.go | 6 +- .../dnd/normalize/spells/normalizer.go | 10 +-- .../dnd/normalize/spells/normalizer_test.go | 87 ++++++++----------- .../dnd/shared/diagnostics/diagnostics.go | 27 ++++++ .../shared/diagnostics/diagnostics_test.go | 18 ++++ 22 files changed, 202 insertions(+), 182 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 38398a2c..b8e76f37 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -444,7 +444,7 @@ old warning slice lengths or omission prose. This stage is appropriately sized for one high-reasoning `gpt-5.6-terra` prompt. -## Stage 7 — Migrate Remaining D&D Producers +## Stage 7 — Migrate Remaining D&D Producers ✅ ### Goal diff --git a/internal/modules/dnd/extract/combatturns/extractor.go b/internal/modules/dnd/extract/combatturns/extractor.go index aa690316..9b878a2b 100644 --- a/internal/modules/dnd/extract/combatturns/extractor.go +++ b/internal/modules/dnd/extract/combatturns/extractor.go @@ -190,10 +190,12 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe case sceneregistry.MatchMissing, sceneregistry.MatchMismatched: return contracts.TypedExtractionResult[dnd.CombatTurnList]{ Value: dnd.CombatTurnList{CombatTurns: []dnd.CombatTurn{}}, - Warnings: []contracts.Warning{{ - Scope: SceneDescriptionReferenceSlot, - ReasonCode: "scene_classification_unavailable", - Message: "No exact scene classification was available; combat extraction was skipped.", + Diagnostics: []contracts.ProducerDiagnostic{{ + Disposition: contracts.DiagnosticDispositionWarning, + Category: contracts.DiagnosticCategoryDegradation, + ReasonCode: "scene_classification_unavailable", + OccurrenceCount: 1, + Samples: []contracts.DiagnosticSample{{Scope: SceneDescriptionReferenceSlot, Message: "No exact scene classification was available; combat extraction was skipped."}}, }}, }, nil default: diff --git a/internal/modules/dnd/extract/combatturns/extractor_test.go b/internal/modules/dnd/extract/combatturns/extractor_test.go index f1fa4c4c..0fbc23e3 100644 --- a/internal/modules/dnd/extract/combatturns/extractor_test.go +++ b/internal/modules/dnd/extract/combatturns/extractor_test.go @@ -310,11 +310,11 @@ func TestExtractAppliesSceneEligibilityBeforePromptConstruction(t *testing.T) { t.Fatal("CombatTurns = nil, want accepted non-nil empty list") } if test.wantWarning != "" { - if len(result.Warnings) != 1 || result.Warnings[0].Scope != SceneDescriptionReferenceSlot || result.Warnings[0].ReasonCode != test.wantWarning { - t.Fatalf("warnings = %#v", result.Warnings) + if len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].Samples[0].Scope != SceneDescriptionReferenceSlot || result.Diagnostics[0].ReasonCode != test.wantWarning || result.Diagnostics[0].Disposition != contracts.DiagnosticDispositionWarning || result.Diagnostics[0].Category != contracts.DiagnosticCategoryDegradation { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } - } else if len(result.Warnings) != 0 { - t.Fatalf("warnings = %#v, want none", result.Warnings) + } else if len(result.Warnings) != 0 || len(result.Diagnostics) != 0 { + t.Fatalf("diagnostics = %#v, want none", result.Diagnostics) } }) } diff --git a/internal/modules/dnd/extract/enemyevents/extractor.go b/internal/modules/dnd/extract/enemyevents/extractor.go index 83851d4f..ba5eb354 100644 --- a/internal/modules/dnd/extract/enemyevents/extractor.go +++ b/internal/modules/dnd/extract/enemyevents/extractor.go @@ -171,10 +171,12 @@ func emptyResult() contracts.TypedExtractionResult[dnd.EnemyEventList] { func unavailableSceneResult() contracts.TypedExtractionResult[dnd.EnemyEventList] { result := emptyResult() - result.Warnings = []contracts.Warning{{ - Scope: SceneDescriptionReferenceSlot, - ReasonCode: "scene_classification_unavailable", - Message: "No exact scene classification was available; enemy-event extraction was skipped.", + result.Diagnostics = []contracts.ProducerDiagnostic{{ + Disposition: contracts.DiagnosticDispositionWarning, + Category: contracts.DiagnosticCategoryDegradation, + ReasonCode: "scene_classification_unavailable", + OccurrenceCount: 1, + Samples: []contracts.DiagnosticSample{{Scope: SceneDescriptionReferenceSlot, Message: "No exact scene classification was available; enemy-event extraction was skipped."}}, }} return result } diff --git a/internal/modules/dnd/extract/enemyevents/extractor_test.go b/internal/modules/dnd/extract/enemyevents/extractor_test.go index 10db980f..d105f9b5 100644 --- a/internal/modules/dnd/extract/enemyevents/extractor_test.go +++ b/internal/modules/dnd/extract/enemyevents/extractor_test.go @@ -98,11 +98,11 @@ func TestExtractSkipsModelForIneligibleScenes(t *testing.T) { t.Fatalf("result = %#v, calls = %d", result, len(client.requests)) } if test.wantWarning { - if len(result.Warnings) != 1 || result.Warnings[0].ReasonCode != "scene_classification_unavailable" || result.Warnings[0].Scope != SceneDescriptionReferenceSlot { - t.Fatalf("warnings = %#v", result.Warnings) + if len(result.Warnings) != 0 || len(result.Diagnostics) != 1 || result.Diagnostics[0].ReasonCode != "scene_classification_unavailable" || result.Diagnostics[0].Disposition != contracts.DiagnosticDispositionWarning || result.Diagnostics[0].Category != contracts.DiagnosticCategoryDegradation || result.Diagnostics[0].Samples[0].Scope != SceneDescriptionReferenceSlot { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } - } else if len(result.Warnings) != 0 { - t.Fatalf("warnings = %#v", result.Warnings) + } else if len(result.Warnings) != 0 || len(result.Diagnostics) != 0 { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } }) } diff --git a/internal/modules/dnd/normalize/combatturns/normalizer.go b/internal/modules/dnd/normalize/combatturns/normalizer.go index 82885472..779ba031 100644 --- a/internal/modules/dnd/normalize/combatturns/normalizer.go +++ b/internal/modules/dnd/normalize/combatturns/normalizer.go @@ -27,7 +27,6 @@ const ( ReasonCodeSourceRefsNormalized = "source_references_normalized" ReasonCodeTurnsReordered = "combat_turns_reordered" ReasonCodeDuplicateCollapsed = "duplicate_combat_turn_collapsed" - ReasonCodeWarningsOmitted = "combat_turn_normalization_warnings_omitted" ) const ( @@ -114,10 +113,11 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize index := source.NewDocumentIndex(req.Source) order := shared.NewSourceRefOrderFromIndex(index) value, warnings := normalizeList(req.MergeOutput.Value, index, order, npcRegistry) - return contracts.TypedNormalizeResult[dnd.CombatTurnList]{ - Value: value, - Warnings: diagnostics.LimitWarnings(warnings, "combat_turns", ReasonCodeWarningsOmitted), - }, nil + diagnosticGroups, err := diagnostics.NormalizationDiagnostics(warnings) + if err != nil { + return contracts.TypedNormalizeResult[dnd.CombatTurnList]{}, normalizerErrorf("collect diagnostics: %w", err) + } + return contracts.TypedNormalizeResult[dnd.CombatTurnList]{Value: value, Diagnostics: diagnosticGroups}, nil } type normalizedRecord struct { diff --git a/internal/modules/dnd/normalize/combatturns/normalizer_test.go b/internal/modules/dnd/normalize/combatturns/normalizer_test.go index 54ad607a..342d8fb3 100644 --- a/internal/modules/dnd/normalize/combatturns/normalizer_test.go +++ b/internal/modules/dnd/normalize/combatturns/normalizer_test.go @@ -47,8 +47,8 @@ func TestNormalizeCanonicalizesFieldsAndRegistryIdentities(t *testing.T) { t.Fatalf("normalized refs = %#v, want %#v", result.Value.CombatTurns[0].SourceRefs, wantRefs) } for _, reason := range []string{ReasonCodeActorCanonicalized, ReasonCodeSourceRefsNormalized} { - if !hasWarningReason(result.Warnings, reason) { - t.Fatalf("warnings = %#v, missing reason %q", result.Warnings, reason) + if !hasDiagnosticReason(result.Diagnostics, reason) { + t.Fatalf("diagnostics = %#v, missing reason %q", result.Diagnostics, reason) } } if !reflect.DeepEqual(input.CombatTurns[0], original) { @@ -82,15 +82,8 @@ func TestNormalizeLimitsWarningsWithoutChangingCombatTurnValues(t *testing.T) { if len(result.Value.CombatTurns) != len(turns) || result.Value.CombatTurns[0].Actor != "Aria" { t.Fatalf("normalized turns = %#v, want canonicalized values", result.Value.CombatTurns) } - 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 != ReasonCodeActorCanonicalized { - t.Fatalf("first warning = %#v, want first input warning", first) - } - summary := result.Warnings[len(result.Warnings)-1] - if summary.Scope != "combat_turns" || summary.ReasonCode != ReasonCodeWarningsOmitted || summary.Message != "2 additional warning(s) omitted" { - t.Fatalf("warning summary = %#v", summary) + if len(result.Warnings) != 0 || len(result.Diagnostics) == 0 || result.Diagnostics[0].Disposition != contracts.DiagnosticDispositionObservation || result.Diagnostics[0].OccurrenceCount != len(turns) || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } } @@ -147,12 +140,12 @@ func TestNormalizeOrdersBySourcePositionAndCollapsesExactDuplicates(t *testing.T if result.Value.CombatTurns[0].SourceRefs[0].StartUnitID != 50 || result.Value.CombatTurns[1].SourceRefs[0].StartUnitID != 90 || result.Value.CombatTurns[2].Actor != "Unknown" { t.Fatalf("normalized order/value = %#v, want chronology then invalid evidence", result.Value.CombatTurns) } - if !hasWarningReason(result.Warnings, ReasonCodeTurnsReordered) || !hasWarningReason(result.Warnings, ReasonCodeDuplicateCollapsed) { - t.Fatalf("warnings = %#v, want reorder and duplicate warnings", result.Warnings) + if !hasDiagnosticReason(result.Diagnostics, ReasonCodeTurnsReordered) || !hasDiagnosticReason(result.Diagnostics, ReasonCodeDuplicateCollapsed) { + t.Fatalf("diagnostics = %#v, want reorder and duplicate observations", result.Diagnostics) } - for _, warning := range result.Warnings { - if warning.ReasonCode == ReasonCodeDuplicateCollapsed && warning.Scope != "combat_turns[1]" { - t.Fatalf("duplicate warning = %#v, want retained input scope combat_turns[1]", warning) + for _, diagnostic := range result.Diagnostics { + if diagnostic.ReasonCode == ReasonCodeDuplicateCollapsed && diagnostic.Samples[0].Scope != "combat_turns[1]" { + t.Fatalf("duplicate diagnostic = %#v, want retained input scope combat_turns[1]", diagnostic) } } } @@ -202,8 +195,8 @@ func TestNormalizePreservesStableOrderForEqualEvidencePositions(t *testing.T) { if got := []string{result.Value.CombatTurns[0].Actor, result.Value.CombatTurns[1].Actor}; !reflect.DeepEqual(got, []string{"Aria", "Borin"}) { t.Fatalf("equal-position order = %#v, want stable input order", got) } - if hasWarningReason(result.Warnings, ReasonCodeTurnsReordered) { - t.Fatalf("warnings = %#v, equal-position stable sort should not warn", result.Warnings) + if hasDiagnosticReason(result.Diagnostics, ReasonCodeTurnsReordered) { + t.Fatalf("diagnostics = %#v, equal-position stable sort should not report reordering", result.Diagnostics) } } @@ -361,9 +354,9 @@ func npcReferences(t *testing.T) contracts.ReferenceSet { }} } -func hasWarningReason(warnings []contracts.Warning, reason string) bool { - for _, warning := range warnings { - if warning.ReasonCode == reason { +func hasDiagnosticReason(diagnostics []contracts.ProducerDiagnostic, reason string) bool { + for _, diagnostic := range diagnostics { + if diagnostic.ReasonCode == reason && diagnostic.Disposition == contracts.DiagnosticDispositionObservation { return true } } diff --git a/internal/modules/dnd/normalize/enemyevents/normalizer.go b/internal/modules/dnd/normalize/enemyevents/normalizer.go index 4662e555..e60a4165 100644 --- a/internal/modules/dnd/normalize/enemyevents/normalizer.go +++ b/internal/modules/dnd/normalize/enemyevents/normalizer.go @@ -25,7 +25,6 @@ const ( ReasonCodeSourceRefsNormalized = "source_references_normalized" ReasonCodeEventsReordered = "enemy_events_reordered" ReasonCodeDuplicateCollapsed = "duplicate_enemy_event_collapsed" - ReasonCodeWarningsOmitted = "enemy_event_normalization_warnings_omitted" ) const ( @@ -106,7 +105,11 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize index := source.NewDocumentIndex(req.Source) order := shared.NewSourceRefOrderFromIndex(index) value, warnings := normalizeList(req.MergeOutput.Value, order, registry) - return contracts.TypedNormalizeResult[dnd.EnemyEventList]{Value: value, Warnings: warnings}, nil + diagnosticGroups, err := diagnostics.NormalizationDiagnostics(warnings) + if err != nil { + return contracts.TypedNormalizeResult[dnd.EnemyEventList]{}, normalizerErrorf("collect diagnostics: %w", err) + } + return contracts.TypedNormalizeResult[dnd.EnemyEventList]{Value: value, Diagnostics: diagnosticGroups}, nil } type normalizedRecord struct { @@ -163,7 +166,7 @@ func normalizeList(input dnd.EnemyEventList, order shared.SourceRefOrder, regist output, duplicateWarnings := collapseDuplicates(records) warnings = append(warnings, duplicateWarnings...) - return dnd.EnemyEventList{Events: output}, diagnostics.LimitWarnings(warnings, "enemy_events", ReasonCodeWarningsOmitted) + return dnd.EnemyEventList{Events: output}, warnings } func normalizeEvent(input dnd.EnemyEvent, order shared.SourceRefOrder, registry *npcregistry.Registry) (dnd.EnemyEvent, *nameCanonicalization, bool) { diff --git a/internal/modules/dnd/normalize/enemyevents/normalizer_test.go b/internal/modules/dnd/normalize/enemyevents/normalizer_test.go index a20141ff..6974cacd 100644 --- a/internal/modules/dnd/normalize/enemyevents/normalizer_test.go +++ b/internal/modules/dnd/normalize/enemyevents/normalizer_test.go @@ -38,8 +38,8 @@ func TestNormalizeCanonicalizesSubjectsEvidenceOrderAndDuplicates(t *testing.T) t.Fatalf("normalized events = %#v", got) } for _, reason := range []string{ReasonCodeNameCanonicalized, ReasonCodeSourceRefsNormalized, ReasonCodeEventsReordered, ReasonCodeDuplicateCollapsed} { - if !hasWarning(result.Warnings, reason) { - t.Fatalf("warnings = %#v, missing %q", result.Warnings, reason) + if !hasDiagnostic(result.Diagnostics, reason) { + t.Fatalf("diagnostics = %#v, missing %q", result.Diagnostics, reason) } } if !reflect.DeepEqual(input.Events[0].SourceRefs, originalRefs) { @@ -93,7 +93,7 @@ func TestNormalizePreservesDistinctEvidenceWhenEncodedKeysCoincide(t *testing.T) }} result, err := newNormalizer(t, npcReferences(t)).Normalize(context.Background(), normalizeRequest(document, input, contracts.ReferenceSet{})) - if err != nil || len(result.Value.Events) != 2 || hasWarning(result.Warnings, ReasonCodeDuplicateCollapsed) { + if err != nil || len(result.Value.Events) != 2 || hasDiagnostic(result.Diagnostics, ReasonCodeDuplicateCollapsed) { t.Fatalf("Normalize() = %#v, %v; want distinct evidence observations retained", result, err) } } @@ -112,7 +112,7 @@ func TestNormalizeIsIdempotentAndPreservesEmptyRepresentation(t *testing.T) { t.Fatal(err) } second, err := normalizer.Normalize(context.Background(), normalizeRequest(testDocument(), first.Value, contracts.ReferenceSet{})) - if err != nil || !reflect.DeepEqual(second.Value, first.Value) || len(second.Warnings) != 0 { + if err != nil || !reflect.DeepEqual(second.Value, first.Value) || len(second.Diagnostics) != 0 { t.Fatalf("second normalization = %#v, %v", second, err) } } @@ -170,8 +170,8 @@ func TestNormalizerContractAndWarningBound(t *testing.T) { input.Events[index] = dnd.EnemyEvent{Name: " ÁRIA ", Kind: dnd.EnemyEventKindEngaged, SourceRefs: []source.SourceRef{{SourceID: document.ID, StartUnitID: count - index, EndUnitID: count - index}}} } result, err := normalizer.Normalize(context.Background(), normalizeRequest(document, input, contracts.ReferenceSet{})) - if err != nil || len(result.Warnings) != diagnostics.MaxWarnings || result.Warnings[len(result.Warnings)-1].ReasonCode != ReasonCodeWarningsOmitted { - t.Fatalf("warnings = %#v, %v", result.Warnings, err) + if err != nil || len(result.Warnings) != 0 || len(result.Diagnostics) == 0 { + t.Fatalf("diagnostics = %#v, %v", result.Diagnostics, err) } } @@ -212,9 +212,9 @@ func npcReferences(t *testing.T) contracts.ReferenceSet { }}} } -func hasWarning(warnings []contracts.Warning, reason string) bool { - for _, warning := range warnings { - if warning.ReasonCode == reason { +func hasDiagnostic(diagnostics []contracts.ProducerDiagnostic, reason string) bool { + for _, diagnostic := range diagnostics { + if diagnostic.ReasonCode == reason && diagnostic.Disposition == contracts.DiagnosticDispositionObservation { return true } } diff --git a/internal/modules/dnd/normalize/itemoccurrences/normalizer.go b/internal/modules/dnd/normalize/itemoccurrences/normalizer.go index 76f0d8cb..c8423a0f 100644 --- a/internal/modules/dnd/normalize/itemoccurrences/normalizer.go +++ b/internal/modules/dnd/normalize/itemoccurrences/normalizer.go @@ -26,7 +26,6 @@ const ( ReasonCodeSourceRefsNormalized = "source_references_normalized" ReasonCodeOccurrencesReordered = "item_occurrences_reordered" ReasonCodeDuplicateCollapsed = "duplicate_item_occurrence_collapsed" - ReasonCodeWarningsOmitted = "item_occurrence_normalization_warnings_omitted" ) const ( @@ -108,7 +107,11 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize } index := source.NewDocumentIndex(req.Source) value, warnings := normalizeList(req.MergeOutput.Value, index, shared.NewSourceRefOrderFromIndex(index), registry) - return contracts.TypedNormalizeResult[dnd.ItemOccurrenceList]{Value: value, Warnings: warnings}, nil + diagnosticGroups, err := diagnostics.NormalizationDiagnostics(warnings, ReasonCodeUnknownItemID) + if err != nil { + return contracts.TypedNormalizeResult[dnd.ItemOccurrenceList]{}, normalizerErrorf("collect diagnostics: %w", err) + } + return contracts.TypedNormalizeResult[dnd.ItemOccurrenceList]{Value: value, Diagnostics: diagnosticGroups}, nil } type normalizedRecord struct { @@ -169,7 +172,7 @@ func normalizeList(input dnd.ItemOccurrenceList, index source.DocumentIndex, ord output, duplicateWarnings := collapseDuplicates(records, index) warnings = append(warnings, duplicateWarnings...) - return dnd.ItemOccurrenceList{Occurrences: output}, diagnostics.LimitWarnings(warnings, "item_occurrences", ReasonCodeWarningsOmitted) + return dnd.ItemOccurrenceList{Occurrences: output}, warnings } func normalizeOccurrence(input dnd.ItemOccurrence, order shared.SourceRefOrder, registry *itemregistry.Registry) (dnd.ItemOccurrence, []string, bool, bool) { diff --git a/internal/modules/dnd/normalize/itemoccurrences/normalizer_test.go b/internal/modules/dnd/normalize/itemoccurrences/normalizer_test.go index ff732cd4..910866c7 100644 --- a/internal/modules/dnd/normalize/itemoccurrences/normalizer_test.go +++ b/internal/modules/dnd/normalize/itemoccurrences/normalizer_test.go @@ -40,8 +40,8 @@ func TestNormalizeCanonicalizesRegistryNameAndRetainsUnknownValues(t *testing.T) if result.Value.Occurrences[1].Name != "Torch" || result.Value.Occurrences[0].Name != "Unknown" { t.Fatalf("occurrences = %#v", result.Value.Occurrences) } - if !hasWarning(result.Warnings, ReasonCodeNameCanonicalized) || !hasWarning(result.Warnings, ReasonCodeUnknownItemID) { - t.Fatalf("warnings = %#v", result.Warnings) + if !hasDiagnostic(result.Diagnostics, ReasonCodeNameCanonicalized, contracts.DiagnosticDispositionObservation) || !hasDiagnostic(result.Diagnostics, ReasonCodeUnknownItemID, contracts.DiagnosticDispositionAdvisory) { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } } @@ -63,9 +63,9 @@ func TestNormalizeRequiresRegistryAndRegistersSlot(t *testing.T) { } } -func hasWarning(warnings []contracts.Warning, reason string) bool { - for _, warning := range warnings { - if warning.ReasonCode == reason { +func hasDiagnostic(diagnostics []contracts.ProducerDiagnostic, reason string, disposition contracts.DiagnosticDisposition) bool { + for _, diagnostic := range diagnostics { + if diagnostic.ReasonCode == reason && diagnostic.Disposition == disposition { return true } } diff --git a/internal/modules/dnd/normalize/locationoccurrences/normalizer.go b/internal/modules/dnd/normalize/locationoccurrences/normalizer.go index e6ffa250..a57d8ce5 100644 --- a/internal/modules/dnd/normalize/locationoccurrences/normalizer.go +++ b/internal/modules/dnd/normalize/locationoccurrences/normalizer.go @@ -27,7 +27,6 @@ const ( ReasonCodeSourceRefsNormalized = "source_references_normalized" ReasonCodeOccurrencesReordered = "location_occurrences_reordered" ReasonCodeDuplicateCollapsed = "duplicate_location_occurrence_collapsed" - ReasonCodeWarningsOmitted = "location_occurrence_normalization_warnings_omitted" ) const ( @@ -109,7 +108,11 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize } index := source.NewDocumentIndex(req.Source) value, warnings := normalizeList(req.MergeOutput.Value, index, shared.NewSourceRefOrderFromIndex(index), registry) - return contracts.TypedNormalizeResult[dnd.LocationOccurrenceList]{Value: value, Warnings: warnings}, nil + diagnosticGroups, err := diagnostics.NormalizationDiagnostics(warnings, ReasonCodeUnknownLocationID) + if err != nil { + return contracts.TypedNormalizeResult[dnd.LocationOccurrenceList]{}, normalizerErrorf("collect diagnostics: %w", err) + } + return contracts.TypedNormalizeResult[dnd.LocationOccurrenceList]{Value: value, Diagnostics: diagnosticGroups}, nil } type normalizedRecord struct { @@ -152,7 +155,7 @@ func normalizeList(input dnd.LocationOccurrenceList, documentIndex source.Docume } output, duplicateWarnings := collapseDuplicates(records, documentIndex) warnings = append(warnings, duplicateWarnings...) - return dnd.LocationOccurrenceList{Occurrences: output}, diagnostics.LimitWarnings(warnings, "location_occurrences", ReasonCodeWarningsOmitted) + return dnd.LocationOccurrenceList{Occurrences: output}, warnings } func normalizeOccurrence(input dnd.LocationOccurrence, order shared.SourceRefOrder, registry *locationregistry.Registry) (dnd.LocationOccurrence, *nameCanonicalization, bool, bool) { diff --git a/internal/modules/dnd/normalize/locationoccurrences/normalizer_test.go b/internal/modules/dnd/normalize/locationoccurrences/normalizer_test.go index c0e5f202..4c3c9688 100644 --- a/internal/modules/dnd/normalize/locationoccurrences/normalizer_test.go +++ b/internal/modules/dnd/normalize/locationoccurrences/normalizer_test.go @@ -32,11 +32,11 @@ func TestNormalizeCanonicalizesNamesByIDAndClonesInputs(t *testing.T) { if occurrence.Name != locations.Locations[1].Name || !reflect.DeepEqual(occurrence.SourceRefs, []source.SourceRef{{SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}, {SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}}) { t.Fatalf("normalized occurrence = %#v", occurrence) } - if !hasWarning(result.Warnings, ReasonCodeNameCanonicalized) || !hasWarning(result.Warnings, ReasonCodeSourceRefsNormalized) || !reflect.DeepEqual(input, before) { - t.Fatalf("warnings/input = %#v/%#v", result.Warnings, input) + if !hasDiagnostic(result.Diagnostics, ReasonCodeNameCanonicalized, contracts.DiagnosticDispositionObservation) || !hasDiagnostic(result.Diagnostics, ReasonCodeSourceRefsNormalized, contracts.DiagnosticDispositionObservation) || !reflect.DeepEqual(input, before) { + t.Fatalf("diagnostics/input = %#v/%#v", result.Diagnostics, input) } second, err := normalizer.Normalize(context.Background(), normalizeRequest(result.Value, doc, contracts.ReferenceSet{})) - if err != nil || !reflect.DeepEqual(second.Value, result.Value) || len(second.Warnings) != 0 { + if err != nil || !reflect.DeepEqual(second.Value, result.Value) || len(second.Diagnostics) != 0 { t.Fatalf("second normalization = %#v, %v", second, err) } result.Value.Occurrences[0].SourceRefs[0].StartUnitID = 999 @@ -71,8 +71,8 @@ func TestNormalizeKeepsSameNamedIDsAndDistinctEvidence(t *testing.T) { if len(got) != 7 || got[0].Kind != dnd.LocationOccurrenceKindVisited || got[1].Kind != dnd.LocationOccurrenceKindPlanned || got[2].Kind != dnd.LocationOccurrenceKindRecalled || got[3].Kind != dnd.LocationOccurrenceKindMentioned || got[3].SourceRefs[0].StartUnitID != 50 || got[4].SourceRefs[0].StartUnitID != 10 || got[5].LocationID != second.ID || got[6].SourceRefs[0].StartUnitID != 999 { t.Fatalf("canonical occurrences = %#v", got) } - if !hasWarning(result.Warnings, ReasonCodeDuplicateCollapsed) || !hasWarning(result.Warnings, ReasonCodeOccurrencesReordered) { - t.Fatalf("warnings = %#v", result.Warnings) + if !hasDiagnostic(result.Diagnostics, ReasonCodeDuplicateCollapsed, contracts.DiagnosticDispositionObservation) || !hasDiagnostic(result.Diagnostics, ReasonCodeOccurrencesReordered, contracts.DiagnosticDispositionObservation) { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } } @@ -81,7 +81,7 @@ func TestNormalizePreservesUnknownIDsAndMalformedOperationRegistry(t *testing.T) locations := registryLocations("The Mill") unknown := dnd.LocationOccurrence{LocationID: "location:sha256:unknown", Name: "The Mill", Kind: "unexpected", SourceRefs: []source.SourceRef{{SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}}} result, err := newNormalizer(t, registryReferences(t, locations)).Normalize(context.Background(), normalizeRequest(dnd.LocationOccurrenceList{Occurrences: []dnd.LocationOccurrence{unknown}}, doc, contracts.ReferenceSet{})) - if err != nil || !reflect.DeepEqual(result.Value.Occurrences[0], unknown) || !hasWarning(result.Warnings, ReasonCodeUnknownLocationID) { + if err != nil || !reflect.DeepEqual(result.Value.Occurrences[0], unknown) || !hasDiagnostic(result.Diagnostics, ReasonCodeUnknownLocationID, contracts.DiagnosticDispositionAdvisory) { t.Fatalf("unknown normalization = %#v, %v", result, err) } malformed := contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{LocationRegistryReferenceSlot: { @@ -134,8 +134,8 @@ func TestNormalizerContractsRequiredRegistryAndWarningBounds(t *testing.T) { input.Occurrences[index] = dnd.LocationOccurrence{LocationID: location.ID, Name: "not canonical", Kind: dnd.LocationOccurrenceKindMentioned, SourceRefs: []source.SourceRef{{SourceID: doc.ID, StartUnitID: count - index, EndUnitID: count - index}}} } bounded, err := newNormalizer(t, registryReferences(t, dnd.LocationRegistry{Locations: []dnd.Location{location}})).Normalize(context.Background(), normalizeRequest(input, doc, contracts.ReferenceSet{})) - if err != nil || len(bounded.Warnings) != diagnostics.MaxWarnings || bounded.Warnings[len(bounded.Warnings)-1].ReasonCode != ReasonCodeWarningsOmitted { - t.Fatalf("bounded warnings = %#v, %v", bounded.Warnings, err) + if err != nil || len(bounded.Warnings) != 0 || len(bounded.Diagnostics) == 0 { + t.Fatalf("bounded diagnostics = %#v, %v", bounded.Diagnostics, err) } } @@ -204,9 +204,9 @@ func cloneList(input dnd.LocationOccurrenceList) dnd.LocationOccurrenceList { return output } -func hasWarning(warnings []contracts.Warning, code string) bool { - for _, warning := range warnings { - if warning.ReasonCode == code { +func hasDiagnostic(diagnostics []contracts.ProducerDiagnostic, code string, disposition contracts.DiagnosticDisposition) bool { + for _, diagnostic := range diagnostics { + if diagnostic.ReasonCode == code && diagnostic.Disposition == disposition { return true } } diff --git a/internal/modules/dnd/normalize/npcoccurrences/normalizer.go b/internal/modules/dnd/normalize/npcoccurrences/normalizer.go index 80a5efc7..d57c73ec 100644 --- a/internal/modules/dnd/normalize/npcoccurrences/normalizer.go +++ b/internal/modules/dnd/normalize/npcoccurrences/normalizer.go @@ -24,7 +24,6 @@ const ( ReasonCodeSourceRefsNormalized = "source_references_normalized" ReasonCodeOccurrencesReordered = "npc_occurrences_reordered" ReasonCodeDuplicateCollapsed = "duplicate_npc_occurrence_collapsed" - ReasonCodeWarningsOmitted = "npc_occurrence_normalization_warnings_omitted" ) const ( @@ -113,7 +112,11 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize if err != nil { return contracts.TypedNormalizeResult[dnd.NPCOccurrenceList]{}, normalizerErrorf("validate NPC registry pairs: %w", err) } - return contracts.TypedNormalizeResult[dnd.NPCOccurrenceList]{Value: value, Warnings: warnings}, nil + diagnosticGroups, err := diagnostics.NormalizationDiagnostics(warnings) + if err != nil { + return contracts.TypedNormalizeResult[dnd.NPCOccurrenceList]{}, normalizerErrorf("collect diagnostics: %w", err) + } + return contracts.TypedNormalizeResult[dnd.NPCOccurrenceList]{Value: value, Diagnostics: diagnosticGroups}, nil } type normalizedRecord struct { @@ -160,8 +163,7 @@ func normalizeList(input dnd.NPCOccurrenceList, documentIndex source.DocumentInd output, duplicateWarnings := collapseDuplicates(records, documentIndex) warnings = append(warnings, duplicateWarnings...) - return dnd.NPCOccurrenceList{Occurrences: output}, - diagnostics.LimitWarnings(warnings, "npc_occurrences", ReasonCodeWarningsOmitted), nil + return dnd.NPCOccurrenceList{Occurrences: output}, warnings, nil } func normalizeOccurrence(input dnd.NPCOccurrence, order shared.SourceRefOrder, registry *npcregistry.Registry) (dnd.NPCOccurrence, bool, error) { diff --git a/internal/modules/dnd/normalize/npcoccurrences/normalizer_test.go b/internal/modules/dnd/normalize/npcoccurrences/normalizer_test.go index 771d79f1..6bfcd0d9 100644 --- a/internal/modules/dnd/normalize/npcoccurrences/normalizer_test.go +++ b/internal/modules/dnd/normalize/npcoccurrences/normalizer_test.go @@ -33,14 +33,14 @@ func TestNormalizeValidatesPairsAndClones(t *testing.T) { if got.NPCID != identity.DeriveID("Ária") || got.Name != "Ária" || !reflect.DeepEqual(got.SourceRefs, []source.SourceRef{{SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}, {SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}}) { t.Fatalf("normalized occurrence = %#v", got) } - if !hasWarning(result.Warnings, ReasonCodeSourceRefsNormalized) { - t.Fatalf("warnings = %#v", result.Warnings) + if !hasDiagnostic(result.Diagnostics, ReasonCodeSourceRefsNormalized) { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } if !reflect.DeepEqual(input.Occurrences[0].SourceRefs, original) { t.Fatalf("Normalize() mutated input: %#v", input) } second, err := normalizer.Normalize(context.Background(), contracts.TypedNormalizeRequest[dnd.NPCOccurrenceList]{Source: doc, MergeOutput: contracts.MergeArtifact[dnd.NPCOccurrenceList]{Value: result.Value}}) - if err != nil || !reflect.DeepEqual(second.Value, result.Value) || len(second.Warnings) != 0 { + if err != nil || !reflect.DeepEqual(second.Value, result.Value) || len(second.Diagnostics) != 0 { t.Fatalf("second normalization = %#v, %v; want idempotent output without warnings", second, err) } result.Value.Occurrences[0].SourceRefs[0].StartUnitID = 999 @@ -116,8 +116,8 @@ func TestNormalizeOrdersAndCollapsesExactDuplicatesOnly(t *testing.T) { if got[0].Kind != dnd.NPCOccurrenceKindCombatAlly || got[0].SourceRefs[0].StartUnitID != 50 || got[1].SourceRefs[0].StartUnitID != 50 || got[2].SourceRefs[0].StartUnitID != 10 || got[3].SourceRefs[0].StartUnitID != 90 || got[4].SourceRefs[0].StartUnitID != 999 { t.Fatalf("canonical order = %#v", got) } - if !hasWarning(result.Warnings, ReasonCodeOccurrencesReordered) || !hasWarning(result.Warnings, ReasonCodeDuplicateCollapsed) { - t.Fatalf("warnings = %#v", result.Warnings) + if !hasDiagnostic(result.Diagnostics, ReasonCodeOccurrencesReordered) || !hasDiagnostic(result.Diagnostics, ReasonCodeDuplicateCollapsed) { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } } @@ -163,9 +163,8 @@ func TestNormalizeBoundsWarnings(t *testing.T) { if err != nil { t.Fatal(err) } - if len(result.Warnings) != diagnostics.MaxWarnings || - result.Warnings[len(result.Warnings)-1].ReasonCode != ReasonCodeWarningsOmitted { - t.Fatalf("warnings = %#v", result.Warnings) + if len(result.Warnings) != 0 || len(result.Diagnostics) == 0 || result.Diagnostics[0].Disposition != contracts.DiagnosticDispositionObservation { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } } @@ -193,9 +192,9 @@ func npcReferences(t *testing.T) contracts.ReferenceSet { }}} } -func hasWarning(warnings []contracts.Warning, reason string) bool { - for _, warning := range warnings { - if warning.ReasonCode == reason { +func hasDiagnostic(diagnostics []contracts.ProducerDiagnostic, reason string) bool { + for _, diagnostic := range diagnostics { + if diagnostic.ReasonCode == reason && diagnostic.Disposition == contracts.DiagnosticDispositionObservation { return true } } diff --git a/internal/modules/dnd/normalize/scenedescriptions/normalizer.go b/internal/modules/dnd/normalize/scenedescriptions/normalizer.go index 1383f2bf..f02a4152 100644 --- a/internal/modules/dnd/normalize/scenedescriptions/normalizer.go +++ b/internal/modules/dnd/normalize/scenedescriptions/normalizer.go @@ -16,12 +16,11 @@ import ( ) const ( - Key = "dnd/scene-descriptions" - normalizerPolicy = "dnd.scene_descriptions.normalizer.v1" - ReasonCodeProseNormalized = "scene_description_prose_normalized" - ReasonCodeOrderNormalized = "scene_description_order_normalized" - ReasonCodeDuplicateCollapsed = "scene_description_duplicate_collapsed" - ReasonCodeNormalizationWarningsOmitted = "scene_description_normalization_warnings_omitted" + Key = "dnd/scene-descriptions" + normalizerPolicy = "dnd.scene_descriptions.normalizer.v1" + ReasonCodeProseNormalized = "scene_description_prose_normalized" + ReasonCodeOrderNormalized = "scene_description_order_normalized" + ReasonCodeDuplicateCollapsed = "scene_description_duplicate_collapsed" ) var requiredCapabilities = []string{"merged"} @@ -68,10 +67,11 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize if err != nil { return contracts.TypedNormalizeResult[dnd.SceneDescriptionList]{}, normalizerErrorf("normalize scenes: %w", err) } - return contracts.TypedNormalizeResult[dnd.SceneDescriptionList]{ - Value: value, - Warnings: diagnostics.LimitWarnings(warnings, "scenes", ReasonCodeNormalizationWarningsOmitted), - }, nil + diagnosticGroups, err := diagnostics.NormalizationDiagnostics(warnings) + if err != nil { + return contracts.TypedNormalizeResult[dnd.SceneDescriptionList]{}, normalizerErrorf("collect diagnostics: %w", err) + } + return contracts.TypedNormalizeResult[dnd.SceneDescriptionList]{Value: value, Diagnostics: diagnosticGroups}, nil } type normalizedScene struct { diff --git a/internal/modules/dnd/normalize/scenedescriptions/normalizer_test.go b/internal/modules/dnd/normalize/scenedescriptions/normalizer_test.go index 2be92f66..36c61142 100644 --- a/internal/modules/dnd/normalize/scenedescriptions/normalizer_test.go +++ b/internal/modules/dnd/normalize/scenedescriptions/normalizer_test.go @@ -35,14 +35,8 @@ func TestNormalizeTrimsOrdersDeduplicatesAndOwnsOutput(t *testing.T) { if !reflect.DeepEqual(result.Value.Scenes, want) { t.Fatalf("scenes = %#v, want %#v", result.Value.Scenes, want) } - if got := result.Warnings; len(got) != 7 || !reflect.DeepEqual( - []string{got[0].ReasonCode, got[1].ReasonCode, got[2].ReasonCode, got[3].ReasonCode, got[4].ReasonCode, got[5].ReasonCode, got[6].ReasonCode}, - []string{ReasonCodeProseNormalized, ReasonCodeProseNormalized, ReasonCodeProseNormalized, ReasonCodeProseNormalized, ReasonCodeOrderNormalized, ReasonCodeOrderNormalized, ReasonCodeDuplicateCollapsed}, - ) || !reflect.DeepEqual( - []string{got[0].Scope, got[1].Scope, got[2].Scope, got[3].Scope, got[4].Scope, got[5].Scope, got[6].Scope}, - []string{"scenes[0]", "scenes[1]", "scenes[2]", "scenes[3]", "scenes[2]", "scenes[0]", "scenes[3]"}, - ) { - t.Fatalf("warnings = %#v, want prose, order, then duplicate mutation diagnostics", got) + if len(result.Warnings) != 0 || len(result.Diagnostics) != 3 || result.Diagnostics[0].ReasonCode != ReasonCodeProseNormalized || result.Diagnostics[0].OccurrenceCount != 4 || result.Diagnostics[1].ReasonCode != ReasonCodeOrderNormalized || result.Diagnostics[1].OccurrenceCount != 2 || result.Diagnostics[2].ReasonCode != ReasonCodeDuplicateCollapsed || result.Diagnostics[2].OccurrenceCount != 1 { + t.Fatalf("diagnostics = %#v, want grouped prose, order, and duplicate observations", result.Diagnostics) } if !reflect.DeepEqual(input, before) { t.Fatalf("Normalize() mutated input: %#v", input) @@ -55,8 +49,8 @@ func TestNormalizeTrimsOrdersDeduplicatesAndOwnsOutput(t *testing.T) { if err != nil { t.Fatalf("second Normalize() error = %v", err) } - if !reflect.DeepEqual(second.Value.Scenes, want) || len(second.Warnings) != 0 { - t.Fatalf("second normalization = %#v, want unchanged value without warnings", second) + if !reflect.DeepEqual(second.Value.Scenes, want) || len(second.Diagnostics) != 0 { + t.Fatalf("second normalization = %#v, want unchanged value without diagnostics", second) } } @@ -76,15 +70,8 @@ func TestNormalizeLimitsCombinedSceneMutationWarnings(t *testing.T) { if len(result.Value.Scenes) != count || result.Value.Scenes[0].SourceRef.StartUnitID != 1 { t.Fatalf("normalized scenes = %#v, want unchanged canonical values", result.Value.Scenes) } - 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 != "scenes[0]" || first.ReasonCode != ReasonCodeProseNormalized { - t.Fatalf("first warning = %#v, want first prose warning", first) - } - summary := result.Warnings[len(result.Warnings)-1] - if summary.Scope != "scenes" || summary.ReasonCode != ReasonCodeNormalizationWarningsOmitted || summary.Message != "5 additional warning(s) omitted" { - t.Fatalf("warning summary = %#v", summary) + if len(result.Warnings) != 0 || len(result.Diagnostics) != 2 || result.Diagnostics[0].ReasonCode != ReasonCodeProseNormalized || result.Diagnostics[0].Disposition != contracts.DiagnosticDispositionObservation || result.Diagnostics[0].OccurrenceCount != count || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples || result.Diagnostics[1].ReasonCode != ReasonCodeOrderNormalized || result.Diagnostics[1].OccurrenceCount != count { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } } diff --git a/internal/modules/dnd/normalize/spells/fixture_test.go b/internal/modules/dnd/normalize/spells/fixture_test.go index 6162add7..742eda6c 100644 --- a/internal/modules/dnd/normalize/spells/fixture_test.go +++ b/internal/modules/dnd/normalize/spells/fixture_test.go @@ -44,9 +44,9 @@ func TestNormalizeAcceptedFixtures(t *testing.T) { t.Fatalf("normalized value = %#v, want %#v", result.Value, fixture.Output) } - gotReasonCodes := make([]string, 0, len(result.Warnings)) - for _, warning := range result.Warnings { - gotReasonCodes = append(gotReasonCodes, warning.ReasonCode) + gotReasonCodes := make([]string, 0, len(result.Diagnostics)) + for _, diagnostic := range result.Diagnostics { + gotReasonCodes = append(gotReasonCodes, diagnostic.ReasonCode) } if !reflect.DeepEqual(gotReasonCodes, fixture.WarningReasonCodes) { t.Fatalf("warning reason codes = %#v, want %#v", gotReasonCodes, fixture.WarningReasonCodes) diff --git a/internal/modules/dnd/normalize/spells/normalizer.go b/internal/modules/dnd/normalize/spells/normalizer.go index 9f5e0df2..dd775517 100644 --- a/internal/modules/dnd/normalize/spells/normalizer.go +++ b/internal/modules/dnd/normalize/spells/normalizer.go @@ -27,7 +27,6 @@ const ( ReasonCodeSpellNameUnresolved = "spell_name_unresolved" ReasonCodeSourceReferencesNormalized = "source_references_normalized" ReasonCodeDuplicateSpellCastCollapsed = "duplicate_spell_cast_collapsed" - ReasonCodeWarningsOmitted = "spell_normalization_warnings_omitted" ) var requiredCapabilities = []string{"merged"} @@ -104,10 +103,11 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize value, warnings := normalizeSpellList(req.MergeOutput.Value, n.effectiveCatalog, order) value, duplicateWarnings := collapseDuplicateSpellCasts(value, index, n.effectiveCatalog) warnings = append(warnings, duplicateWarnings...) - return contracts.TypedNormalizeResult[dnd.SpellList]{ - Value: value, - Warnings: diagnostics.LimitWarnings(warnings, "spell_casts", ReasonCodeWarningsOmitted), - }, nil + diagnosticGroups, err := diagnostics.NormalizationDiagnostics(warnings, ReasonCodeSpellNameUnresolved) + if err != nil { + return contracts.TypedNormalizeResult[dnd.SpellList]{}, normalizerErrorf("collect diagnostics: %w", err) + } + return contracts.TypedNormalizeResult[dnd.SpellList]{Value: value, Diagnostics: diagnosticGroups}, nil } func normalizeSpellList(input dnd.SpellList, catalog spellcatalog.EffectiveCatalog, order shared.SourceRefOrder) (dnd.SpellList, []contracts.Warning) { diff --git a/internal/modules/dnd/normalize/spells/normalizer_test.go b/internal/modules/dnd/normalize/spells/normalizer_test.go index cd0858c5..b7a98d80 100644 --- a/internal/modules/dnd/normalize/spells/normalizer_test.go +++ b/internal/modules/dnd/normalize/spells/normalizer_test.go @@ -149,17 +149,10 @@ func TestNormalizeCanonicalizesNamesAndReportsUnresolvedNames(t *testing.T) { t.Fatalf("spell[%d] = %q, want %q", index, result.Value.SpellCasts[index].Spell, want) } } - if len(result.Warnings) != 4 { - t.Fatalf("warnings = %#v, want four name warnings", result.Warnings) - } - if result.Warnings[0].ReasonCode != ReasonCodeSpellNameCanonicalized || result.Warnings[0].Scope != "spell_casts[0]" || !strings.Contains(result.Warnings[0].Message, "input index 0") { - t.Fatalf("first warning = %#v, want canonicalization warning", result.Warnings[0]) - } - if result.Warnings[1].ReasonCode != ReasonCodeSpellNameCanonicalized || result.Warnings[2].ReasonCode != ReasonCodeSpellNameCanonicalized { - t.Fatalf("catalog spelling warnings = %#v", result.Warnings[1:3]) - } - if result.Warnings[3].ReasonCode != ReasonCodeSpellNameUnresolved || result.Warnings[3].Scope != "spell_casts[3]" || strings.Contains(result.Warnings[3].Message, "Mystery\nSpell") || !strings.Contains(result.Warnings[3].Message, `Mystery\nSpell\tName`) { - t.Fatalf("unresolved warning = %#v, want quoted control characters", result.Warnings[3]) + canonicalized := diagnosticByReason(result.Diagnostics, ReasonCodeSpellNameCanonicalized) + unresolved := diagnosticByReason(result.Diagnostics, ReasonCodeSpellNameUnresolved) + if len(result.Warnings) != 0 || canonicalized == nil || canonicalized.Disposition != contracts.DiagnosticDispositionObservation || canonicalized.OccurrenceCount != 3 || unresolved == nil || unresolved.Disposition != contracts.DiagnosticDispositionAdvisory || unresolved.OccurrenceCount != 1 || unresolved.Samples[0].Scope != "spell_casts[3]" || strings.Contains(unresolved.Samples[0].Message, "Mystery\nSpell") || !strings.Contains(unresolved.Samples[0].Message, `Mystery\nSpell\tName`) { + t.Fatalf("diagnostics = %#v, want grouped canonicalization and unresolved-quality diagnostics", result.Diagnostics) } } @@ -175,15 +168,8 @@ func TestNormalizeLimitsWarningsWithoutChangingSpellValues(t *testing.T) { if !reflect.DeepEqual(result.Value, input) { t.Fatalf("normalized value = %#v, want unresolved spell values preserved", result.Value) } - 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 != "spell_casts[0]" || first.ReasonCode != ReasonCodeSpellNameUnresolved { - t.Fatalf("first warning = %#v, want first input warning", first) - } - summary := result.Warnings[len(result.Warnings)-1] - if summary.Scope != "spell_casts" || summary.ReasonCode != ReasonCodeWarningsOmitted || 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].ReasonCode != ReasonCodeSpellNameUnresolved || result.Diagnostics[0].Disposition != contracts.DiagnosticDispositionAdvisory || result.Diagnostics[0].OccurrenceCount != len(input.SpellCasts) || len(result.Diagnostics[0].Samples) != contracts.MaxDiagnosticSamples { + t.Fatalf("diagnostics = %#v", result.Diagnostics) } } @@ -198,11 +184,9 @@ func TestNormalizeBoundsUnicodeNamesAndQuotesCanonicalReplacement(t *testing.T) if err != nil { t.Fatalf("Normalize() error = %v, want nil", err) } - if len(result.Warnings) != 1 || result.Warnings[0].ReasonCode != ReasonCodeSpellNameCanonicalized { - t.Fatalf("warnings = %#v, want one canonicalization warning", result.Warnings) - } - if !utf8.ValidString(result.Warnings[0].Message) || !strings.Contains(result.Warnings[0].Message, "…") || strings.Contains(result.Warnings[0].Message, longName) { - t.Fatalf("warning = %q, want valid bounded Unicode diagnostic", result.Warnings[0].Message) + diagnostic := diagnosticByReason(result.Diagnostics, ReasonCodeSpellNameCanonicalized) + if diagnostic == nil || !utf8.ValidString(diagnostic.Samples[0].Message) || !strings.Contains(diagnostic.Samples[0].Message, "…") || strings.Contains(diagnostic.Samples[0].Message, longName) { + t.Fatalf("diagnostic = %#v, want valid bounded Unicode diagnostic", diagnostic) } if got := result.Value.SpellCasts[0].Spell; got != longName { t.Fatalf("canonical value = %q, want full catalog name", got) @@ -234,8 +218,9 @@ func TestNormalizeSortsAndDeduplicatesExactSourceReferences(t *testing.T) { if !reflect.DeepEqual(result.Value.SpellCasts[0].SourceRefs, wantRefs) { t.Fatalf("source refs = %#v, want %#v", result.Value.SpellCasts[0].SourceRefs, wantRefs) } - if len(result.Warnings) != 1 || result.Warnings[0].ReasonCode != ReasonCodeSourceReferencesNormalized || !strings.Contains(result.Warnings[0].Message, "original count 6") || !strings.Contains(result.Warnings[0].Message, "final count 5") || !strings.Contains(result.Warnings[0].Message, "order changed true") || !strings.Contains(result.Warnings[0].Message, "duplicates removed 1") { - t.Fatalf("warnings = %#v, want source normalization warning", result.Warnings) + diagnostic := diagnosticByReason(result.Diagnostics, ReasonCodeSourceReferencesNormalized) + if diagnostic == nil || !strings.Contains(diagnostic.Samples[0].Message, "original count 6") || !strings.Contains(diagnostic.Samples[0].Message, "final count 5") || !strings.Contains(diagnostic.Samples[0].Message, "order changed true") || !strings.Contains(diagnostic.Samples[0].Message, "duplicates removed 1") { + t.Fatalf("diagnostics = %#v, want source normalization observation", result.Diagnostics) } } @@ -251,10 +236,11 @@ func TestNormalizeReportsDuplicateRemovalWithoutOrderChange(t *testing.T) { if err != nil { t.Fatalf("Normalize() error = %v, want nil", err) } - if len(result.Warnings) != 1 || result.Warnings[0].ReasonCode != ReasonCodeSourceReferencesNormalized { - t.Fatalf("warnings = %#v, want one source normalization warning", result.Warnings) + diagnostic := diagnosticByReason(result.Diagnostics, ReasonCodeSourceReferencesNormalized) + if diagnostic == nil { + t.Fatalf("diagnostics = %#v, want one source normalization observation", result.Diagnostics) } - message := result.Warnings[0].Message + message := diagnostic.Samples[0].Message if !strings.Contains(message, "order changed false") || !strings.Contains(message, "duplicates removed 1") { t.Fatalf("warning = %q, want duplicate-only repair without order change", message) } @@ -361,26 +347,11 @@ func TestNormalizeCollapsesDuplicateGroupsAfterCanonicalization(t *testing.T) { t.Fatalf("Unicode caster output = %q, want first occurrence text unchanged", got) } - if len(result.Warnings) != 6 { - t.Fatalf("warnings = %#v, want per-cast warnings followed by two group warnings", result.Warnings) - } - if result.Warnings[0].ReasonCode != ReasonCodeSpellNameCanonicalized || result.Warnings[0].Scope != "spell_casts[0]" { - t.Fatalf("warning[0] = %#v, want input name warning", result.Warnings[0]) - } - if result.Warnings[1].ReasonCode != ReasonCodeSourceReferencesNormalized || result.Warnings[1].Scope != "spell_casts[0]" { - t.Fatalf("warning[1] = %#v, want input source warning", result.Warnings[1]) - } - if result.Warnings[2].ReasonCode != ReasonCodeSpellNameCanonicalized || result.Warnings[2].Scope != "spell_casts[3]" { - t.Fatalf("warning[2] = %#v, want removed occurrence warning", result.Warnings[2]) - } - if result.Warnings[3].ReasonCode != ReasonCodeSpellNameCanonicalized || result.Warnings[3].Scope != "spell_casts[4]" { - t.Fatalf("warning[3] = %#v, want removed occurrence warning", result.Warnings[3]) - } - if result.Warnings[4].ReasonCode != ReasonCodeDuplicateSpellCastCollapsed || result.Warnings[4].Scope != "spell_casts[0]" || !strings.Contains(result.Warnings[4].Message, "retained input index 0") || !strings.Contains(result.Warnings[4].Message, "removed input indices [3]") { - t.Fatalf("warning[4] = %#v, want first duplicate group warning", result.Warnings[4]) - } - if result.Warnings[5].ReasonCode != ReasonCodeDuplicateSpellCastCollapsed || result.Warnings[5].Scope != "spell_casts[2]" || !strings.Contains(result.Warnings[5].Message, "removed input indices [4, 5]") { - t.Fatalf("warning[5] = %#v, want second duplicate group warning", result.Warnings[5]) + canonicalized := diagnosticByReason(result.Diagnostics, ReasonCodeSpellNameCanonicalized) + refsNormalized := diagnosticByReason(result.Diagnostics, ReasonCodeSourceReferencesNormalized) + collapsed := diagnosticByReason(result.Diagnostics, ReasonCodeDuplicateSpellCastCollapsed) + if len(result.Warnings) != 0 || canonicalized == nil || canonicalized.OccurrenceCount != 3 || refsNormalized == nil || refsNormalized.OccurrenceCount != 1 || collapsed == nil || collapsed.OccurrenceCount != 2 || !strings.Contains(collapsed.Samples[0].Message, "retained input index 0") { + t.Fatalf("diagnostics = %#v, want grouped normalization observations", result.Diagnostics) } } @@ -451,10 +422,11 @@ func TestNormalizeBoundsDuplicateWarningIndices(t *testing.T) { if err != nil { t.Fatalf("Normalize() error = %v, want nil", err) } - if len(result.Value.SpellCasts) != 1 || len(result.Warnings) != 1 || result.Warnings[0].ReasonCode != ReasonCodeDuplicateSpellCastCollapsed { - t.Fatalf("result = %#v, warnings = %#v, want one retained cast and one bounded warning", result.Value, result.Warnings) + diagnostic := diagnosticByReason(result.Diagnostics, ReasonCodeDuplicateSpellCastCollapsed) + if len(result.Value.SpellCasts) != 1 || diagnostic == nil || diagnostic.OccurrenceCount != 1 { + t.Fatalf("result = %#v, diagnostics = %#v, want one retained cast and one grouped observation", result.Value, result.Diagnostics) } - message := result.Warnings[0].Message + message := diagnostic.Samples[0].Message if !strings.Contains(message, "removed input indices [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]") || strings.Contains(message, ", 21]") || !strings.Contains(message, "1 additional removed input indices omitted") { t.Fatalf("warning message = %q, want 20 displayed indices and exact omitted count", message) } @@ -533,6 +505,15 @@ func sourceDocument(unitCount int) *source.SourceDocument { return &source.SourceDocument{ID: "source", Units: units} } +func diagnosticByReason(diagnostics []contracts.ProducerDiagnostic, reason string) *contracts.ProducerDiagnostic { + for index := range diagnostics { + if diagnostics[index].ReasonCode == reason { + return &diagnostics[index] + } + } + return nil +} + func overlayReference() contracts.ReferenceSet { return spellCatalogReference(`{"schema_version":"notarius.dnd.spell-catalog-overlay.v1","catalogs":[{"id":"campaign.example","ruleset":"dnd-5e-2014","source":{"title":"Private campaign source","version":"1","url":"file:///private-source.json","license":"private"},"spells":[{"name":"Aegis of Emberfall","aliases":["Emberfall Aegis"]}]}]}`) } diff --git a/internal/modules/dnd/shared/diagnostics/diagnostics.go b/internal/modules/dnd/shared/diagnostics/diagnostics.go index 39bae120..f5f089b0 100644 --- a/internal/modules/dnd/shared/diagnostics/diagnostics.go +++ b/internal/modules/dnd/shared/diagnostics/diagnostics.go @@ -47,6 +47,33 @@ func Collect(findings []contracts.Warning, disposition contracts.DiagnosticDispo return collector.Diagnostics(), nil } +// NormalizationDiagnostics classifies deterministic normalization findings as +// observations, except for explicitly identified unresolved-quality findings. +func NormalizationDiagnostics(findings []contracts.Warning, advisoryReasonCodes ...string) ([]contracts.ProducerDiagnostic, error) { + advisoryReasons := make(map[string]struct{}, len(advisoryReasonCodes)) + for _, reasonCode := range advisoryReasonCodes { + advisoryReasons[reasonCode] = struct{}{} + } + observations := make([]contracts.Warning, 0, len(findings)) + advisories := make([]contracts.Warning, 0) + for _, finding := range findings { + if _, advisory := advisoryReasons[finding.ReasonCode]; advisory { + advisories = append(advisories, finding) + continue + } + observations = append(observations, finding) + } + diagnostics, err := Collect(observations, contracts.DiagnosticDispositionObservation, contracts.DiagnosticCategoryNormalization) + if err != nil { + return nil, err + } + qualityDiagnostics, err := Collect(advisories, contracts.DiagnosticDispositionAdvisory, contracts.DiagnosticCategoryDataQuality) + if err != nil { + return nil, err + } + return append(diagnostics, qualityDiagnostics...), 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 a2c38c6d..42c800e5 100644 --- a/internal/modules/dnd/shared/diagnostics/diagnostics_test.go +++ b/internal/modules/dnd/shared/diagnostics/diagnostics_test.go @@ -92,3 +92,21 @@ func TestCollectGroupsNormalizationFindingsWithBoundedSamples(t *testing.T) { t.Fatalf("diagnostic = %#v", diagnostic) } } + +func TestNormalizationDiagnosticsCombinesQualityAndCleanupWithoutWarnings(t *testing.T) { + groups, err := NormalizationDiagnostics([]contracts.Warning{ + {Scope: "spell_casts[0]", ReasonCode: "spell_name_canonicalized", Message: "canonicalized spell name"}, + {Scope: "spell_casts[1]", ReasonCode: "spell_name_unresolved", Message: "spell was not in the catalog"}, + }, "spell_name_unresolved") + if err != nil { + t.Fatal(err) + } + if len(groups) != 2 || groups[0].Disposition != contracts.DiagnosticDispositionObservation || groups[1].Disposition != contracts.DiagnosticDispositionAdvisory { + t.Fatalf("groups = %#v, want cleanup observation and quality advisory", groups) + } + for _, group := range groups { + if group.Disposition == contracts.DiagnosticDispositionWarning { + t.Fatalf("groups = %#v, want zero warning groups", groups) + } + } +}