diff --git a/.agent/skills/webstatus-backend/SKILL.md b/.agent/skills/webstatus-backend/SKILL.md index e5c131282..e13385132 100644 --- a/.agent/skills/webstatus-backend/SKILL.md +++ b/.agent/skills/webstatus-backend/SKILL.md @@ -61,6 +61,7 @@ We use a Hexagonal-style **Adapter Pattern** to decouple application logic from - **DO** define `noAuth:` under `components.securitySchemes` in `openapi.yaml` whenever any operation specifies `noAuth: []` in its `security:` requirements. In `oapi-codegen v2.7+`, `ContextKey` types (`noAuthContextKey`) are generated strictly from `components.securitySchemes`; omitting `noAuth` causes `undefined: noAuthContextKey` build failures. - **DO** pass pointers (`*string`) when populating optional OpenAPI response header fields (like `Location` in `301` responses), as `oapi-codegen v2.7+` models optional response headers as pointer types. - **DO** type strict middleware closures using `backend.StrictHandlerFunc` and `backend.StrictMiddlewareFunc` directly from the generated package rather than importing from `github.com/oapi-codegen/runtime/strictmiddleware/nethttp`. +- **DO** use modern Go 1.26+ `new(expr)` built-in syntax (e.g., `new("my-string")` or `new(42)`) when creating pointers to values or literals. **DON'T** introduce custom pointer helper functions (e.g., `stringPtr`, `intPtr`, or generic `ptr(...)`). ## Testing & Linting diff --git a/.agent/skills/webstatus-workers/references/how-to-add-a-worker.md b/.agent/skills/webstatus-workers/references/how-to-add-a-worker.md index b35847aeb..0ddb09f3f 100644 --- a/.agent/skills/webstatus-workers/references/how-to-add-a-worker.md +++ b/.agent/skills/webstatus-workers/references/how-to-add-a-worker.md @@ -17,7 +17,15 @@ Push workers actively send notifications to users based on user-configured subsc 2. **The New Worker (`workers//`)**: - Create a new directory under `workers/`. - The worker must subscribe to its dedicated Pub/Sub topic (e.g., `webhook-delivery-sub-id`). - - It should consume the job payload, format the payload appropriately (e.g., into a JSON webhook payload), and perform the network request. + - **Formatting & Rendering (`CategorizedSummaryVisitor`)**: + All notification renderers (Email, Webhooks/Slack, RSS, Discord, Web Push, etc.) MUST implement `workertypes.CategorizedSummaryVisitor` and receive summaries via `summary.Accept(visitor, triggers)`. This ensures core category filtering and baseline promotion logic are handled centrally by `lib/workertypes/summary_categorizer.go`. + - **Testing Standard (100% Test Parity Requirement)**: + New renderer packages MUST implement the 5-part symmetrical unit testing blueprint to match existing renderers (`slack_test.go`, `renderer_test.go`, `rss_visitor_test.go`): + 1. `Test_FeatureCategories` (table-driven for `Added`, `Removed`, `Changed`, `Moved`, `Split`, `Deleted`) + 2. `Test_QueryErrors_RenderMessage` (table-driven for all 4 `SummaryQueryErrorCode` enums) + 3. `Test_TriggerFiltering` (verifying highlight filtering by subscriber triggers) + 4. `Test_NilPointerGuards` (verifying zero panics when handling nil pointers on optional diff structs like `Moved` or `Split`) + 5. `Test_Golden` (output regression tests using `.golden` files and `cmp.Diff`) - **State Management:** It must use a `ChannelStateManager` to record delivery successes or failures back to Spanner. - **Error Handling:** Permanent errors (e.g., 404 Not Found on a webhook URL) should be ACKed and marked as a permanent failure in the DB. Transient errors (e.g., 500 Internal Server Error) should be NACKed via `errors.Join(event.ErrTransientFailure, err)` to trigger a Pub/Sub retry. diff --git a/lib/workertypes/comparables/types_test.go b/lib/workertypes/comparables/types_test.go index 158022ea8..3d31f19e0 100644 --- a/lib/workertypes/comparables/types_test.go +++ b/lib/workertypes/comparables/types_test.go @@ -4,14 +4,11 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - package comparables import ( diff --git a/lib/workertypes/summary_categorizer.go b/lib/workertypes/summary_categorizer.go new file mode 100644 index 000000000..145f062b7 --- /dev/null +++ b/lib/workertypes/summary_categorizer.go @@ -0,0 +1,214 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package workertypes + +// CategorizedSummary contains pre-grouped highlights and error slices +// resulting from BaseSummaryVisitor categorization. +type CategorizedSummary struct { + Text string + Truncated bool + QueryErrors []SummaryQueryError + ResolvedQueryErrors []SummaryQueryError + Added []SummaryHighlight + Removed []SummaryHighlight + Changed []SummaryHighlight + Moved []SummaryHighlight + Split []SummaryHighlight + Deleted []SummaryHighlight +} + +// NewEmptyCategorizedSummary returns a zero-initialized CategorizedSummary +// satisfying exhaustruct requirements. +func NewEmptyCategorizedSummary() CategorizedSummary { + return CategorizedSummary{ + Text: "", + Truncated: false, + QueryErrors: nil, + ResolvedQueryErrors: nil, + Added: nil, + Removed: nil, + Changed: nil, + Moved: nil, + Split: nil, + Deleted: nil, + } +} + +// CategorizedSummaryVisitor defines the strongly-typed contract for consuming +// categorized summary elements. Delivery channels (e.g. RSS, Email, Webhook, Slack) +// implement this interface to receive filtered and promoted categories via double-dispatch. +// +// Documenting & Enforcing a Successfully Tested Renderer: +// Standard delivery channel visitors MUST satisfy the following contract invariants and testing standards: +// +// Runtime Implementation Invariants: +// 1. Nil & Empty Slice Safety: All Visit* methods MUST safely handle nil or empty ([]T{}) +// slices without panicking or dereferencing nil pointers. +// 2. Error Propagation: Rendering failures (e.g., template execution errors) MUST be returned +// as non-nil errors to allow BaseSummaryVisitor.Dispatch() to fail fast. +// 3. State Isolation: Visitor instances must maintain state isolation across separate dispatch passes. +// +// 5-Part Unit Testing Blueprint (Symmetrical Test Parity Standard): +// Package unit tests for new delivery channels MUST implement the 5 symmetrical test suites: +// 1. Test_FeatureCategories: Table-driven tests covering all 6 categories +// (Added, Removed, Changed, Moved, Split, Deleted). +// 2. Test_QueryErrors_RenderMessage: Table-driven tests covering all SummaryQueryErrorCode enums. +// 3. Test_TriggerFiltering: Verifying highlight filtering by subscriber triggers. +// 4. Test_NilPointerGuards: Verifying zero panics when handling optional diff structs (Moved/Split = nil). +// 5. Test_Golden: Output regression testing using .golden snapshot files and cmp.Diff. +type CategorizedSummaryVisitor interface { + VisitQueryErrors(errors []SummaryQueryError) error + VisitResolvedQueryErrors(errors []SummaryQueryError) error + VisitAddedFeatures(features []SummaryHighlight) error + VisitRemovedFeatures(features []SummaryHighlight) error + VisitChangedFeatures(features []SummaryHighlight) error + VisitMovedFeatures(features []SummaryHighlight) error + VisitSplitFeatures(features []SummaryHighlight) error + VisitDeletedFeatures(features []SummaryHighlight) error +} + +// BaseSummaryVisitor implements SummaryVisitor and centralizes highlight filtering, +// category grouping, promotion logic, and double-dispatching. +// BaseSummaryVisitor is stateful and is NOT safe for concurrent use across goroutines. +// Create a new instance per EventSummary categorization pass. +type BaseSummaryVisitor struct { + triggers []JobTrigger + target CategorizedSummaryVisitor + Summary CategorizedSummary +} + +// newBaseSummaryVisitor constructs a new BaseSummaryVisitor with the specified triggers +// and optional CategorizedSummaryVisitor dispatch target. +func newBaseSummaryVisitor(triggers []JobTrigger, target CategorizedSummaryVisitor) *BaseSummaryVisitor { + return &BaseSummaryVisitor{ + triggers: triggers, + target: target, + Summary: NewEmptyCategorizedSummary(), + } +} + +// VisitV1 processes an EventSummary, filters highlights by triggers, categorizes them, +// applies promotion rules, and dispatches to the target visitor if provided. +func (v *BaseSummaryVisitor) VisitV1(s EventSummary) error { + v.Summary = NewEmptyCategorizedSummary() + v.Summary.Text = s.Text + v.Summary.Truncated = s.Truncated + v.Summary.QueryErrors = s.QueryErrors + v.Summary.ResolvedQueryErrors = s.ResolvedQueryErrors + + filtered := FilterHighlights(s.Highlights, v.triggers) + for _, h := range filtered { + v.routeHighlight(h) + } + + if v.target != nil { + return v.dispatch() + } + + return nil +} + +// Dispatch executes double-dispatch on the provided CategorizedSummaryVisitor target. +func (v *BaseSummaryVisitor) Dispatch(target CategorizedSummaryVisitor) error { + if target == nil { + return nil + } + v.target = target + + return v.dispatch() +} + +// routeHighlight assigns a highlight to its respective category in Summary. +// Note: Highlights with Type "Removed" represent features no longer matching query criteria. +// If a Removed highlight also contains active baseline or browser implementation updates +// (h.BaselineChange != nil || len(h.BrowserChanges) > 0), it is promoted to Changed so +// UI renderers display the detailed browser status change rather than a pure query removal. +func (v *BaseSummaryVisitor) routeHighlight(h SummaryHighlight) { + switch h.Type { + case SummaryHighlightTypeAdded: + v.Summary.Added = append(v.Summary.Added, h) + case SummaryHighlightTypeRemoved: + if h.BaselineChange != nil || len(h.BrowserChanges) > 0 { + v.Summary.Changed = append(v.Summary.Changed, h) + } else { + v.Summary.Removed = append(v.Summary.Removed, h) + } + case SummaryHighlightTypeChanged: + v.Summary.Changed = append(v.Summary.Changed, h) + case SummaryHighlightTypeMoved: + v.Summary.Moved = append(v.Summary.Moved, h) + case SummaryHighlightTypeSplit: + v.Summary.Split = append(v.Summary.Split, h) + case SummaryHighlightTypeDeleted: + v.Summary.Deleted = append(v.Summary.Deleted, h) + } +} + +func (v *BaseSummaryVisitor) dispatch() error { + if len(v.Summary.QueryErrors) > 0 { + if err := v.target.VisitQueryErrors(v.Summary.QueryErrors); err != nil { + return err + } + } + if len(v.Summary.ResolvedQueryErrors) > 0 { + if err := v.target.VisitResolvedQueryErrors(v.Summary.ResolvedQueryErrors); err != nil { + return err + } + } + if len(v.Summary.Added) > 0 { + if err := v.target.VisitAddedFeatures(v.Summary.Added); err != nil { + return err + } + } + if len(v.Summary.Removed) > 0 { + if err := v.target.VisitRemovedFeatures(v.Summary.Removed); err != nil { + return err + } + } + if len(v.Summary.Changed) > 0 { + if err := v.target.VisitChangedFeatures(v.Summary.Changed); err != nil { + return err + } + } + if len(v.Summary.Moved) > 0 { + if err := v.target.VisitMovedFeatures(v.Summary.Moved); err != nil { + return err + } + } + if len(v.Summary.Split) > 0 { + if err := v.target.VisitSplitFeatures(v.Summary.Split); err != nil { + return err + } + } + if len(v.Summary.Deleted) > 0 { + if err := v.target.VisitDeletedFeatures(v.Summary.Deleted); err != nil { + return err + } + } + + return nil +} + +// HasContent returns true if any query errors or categorized highlights are present. +func (v *BaseSummaryVisitor) HasContent() bool { + return len(v.Summary.QueryErrors) > 0 || + len(v.Summary.ResolvedQueryErrors) > 0 || + len(v.Summary.Added) > 0 || + len(v.Summary.Removed) > 0 || + len(v.Summary.Changed) > 0 || + len(v.Summary.Moved) > 0 || + len(v.Summary.Split) > 0 || + len(v.Summary.Deleted) > 0 +} diff --git a/lib/workertypes/summary_categorizer_test.go b/lib/workertypes/summary_categorizer_test.go new file mode 100644 index 000000000..ec4363f50 --- /dev/null +++ b/lib/workertypes/summary_categorizer_test.go @@ -0,0 +1,285 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package workertypes + +import ( + "errors" + "testing" +) + +type mockCategorizedVisitor struct { + queryErrorsVisited int + resolvedQueryErrorsVisited int + addedVisited int + removedVisited int + changedVisited int + movedVisited int + splitVisited int + deletedVisited int + errToReturn error +} + +func newMockCategorizedVisitor() *mockCategorizedVisitor { + return &mockCategorizedVisitor{ + queryErrorsVisited: 0, + resolvedQueryErrorsVisited: 0, + addedVisited: 0, + removedVisited: 0, + changedVisited: 0, + movedVisited: 0, + splitVisited: 0, + deletedVisited: 0, + errToReturn: nil, + } +} + +func (m *mockCategorizedVisitor) VisitQueryErrors(_ []SummaryQueryError) error { + m.queryErrorsVisited++ + + return m.errToReturn +} + +func (m *mockCategorizedVisitor) VisitResolvedQueryErrors(_ []SummaryQueryError) error { + m.resolvedQueryErrorsVisited++ + + return m.errToReturn +} + +func (m *mockCategorizedVisitor) VisitAddedFeatures(_ []SummaryHighlight) error { + m.addedVisited++ + + return m.errToReturn +} + +func (m *mockCategorizedVisitor) VisitRemovedFeatures(_ []SummaryHighlight) error { + m.removedVisited++ + + return m.errToReturn +} + +func (m *mockCategorizedVisitor) VisitChangedFeatures(_ []SummaryHighlight) error { + m.changedVisited++ + + return m.errToReturn +} + +func (m *mockCategorizedVisitor) VisitMovedFeatures(_ []SummaryHighlight) error { + m.movedVisited++ + + return m.errToReturn +} + +func (m *mockCategorizedVisitor) VisitSplitFeatures(_ []SummaryHighlight) error { + m.splitVisited++ + + return m.errToReturn +} + +func (m *mockCategorizedVisitor) VisitDeletedFeatures(_ []SummaryHighlight) error { + m.deletedVisited++ + + return m.errToReturn +} + +func TestBaseSummaryVisitor_CategorizationAndPromotion(t *testing.T) { + summary := NewEmptyEventSummary() + summary.Text = "Production Check" + summary.QueryErrors = []SummaryQueryError{ + {Code: SummaryQueryErrorCodeInvalidQuery}, + } + summary.ResolvedQueryErrors = []SummaryQueryError{ + {Code: SummaryQueryErrorCodeQueryGrammar}, + } + summary.Highlights = []SummaryHighlight{ + { + Type: SummaryHighlightTypeAdded, + FeatureID: "f-1", + FeatureName: "Feature One", + Docs: nil, + NameChange: nil, + BaselineChange: &Change[BaselineValue]{ + From: BaselineValue{Status: BaselineStatusLimited, LowDate: nil, HighDate: nil}, + To: BaselineValue{Status: BaselineStatusWidely, LowDate: nil, HighDate: nil}, + }, + BrowserChanges: nil, + Moved: nil, + Split: nil, + }, + { + Type: SummaryHighlightTypeRemoved, + FeatureID: "f-2", + FeatureName: "Feature Two (Promoted)", + Docs: nil, + NameChange: nil, + BaselineChange: &Change[BaselineValue]{ + From: BaselineValue{Status: BaselineStatusWidely, LowDate: nil, HighDate: nil}, + To: BaselineValue{Status: BaselineStatusLimited, LowDate: nil, HighDate: nil}, + }, + BrowserChanges: nil, + Moved: nil, + Split: nil, + }, + { + Type: SummaryHighlightTypeRemoved, + FeatureID: "f-3", + FeatureName: "Feature Three (Unchanged Removed)", + Docs: nil, + NameChange: nil, + BaselineChange: nil, + BrowserChanges: nil, + Moved: nil, + Split: nil, + }, + } + + triggers := []JobTrigger{FeaturePromotedToWidely, FeatureRegressedToLimited} + mock := newMockCategorizedVisitor() + visitor := newBaseSummaryVisitor(triggers, mock) + + if err := visitor.VisitV1(summary); err != nil { + t.Fatalf("VisitV1 unexpected error: %v", err) + } + + if !visitor.HasContent() { + t.Error("expected HasContent() to be true") + } + + if len(visitor.Summary.Added) != 1 || visitor.Summary.Added[0].FeatureID != "f-1" { + t.Errorf("expected 1 Added feature f-1, got %v", visitor.Summary.Added) + } + if len(visitor.Summary.Changed) != 1 || visitor.Summary.Changed[0].FeatureID != "f-2" { + t.Errorf("expected 1 Changed feature f-2 via promotion, got %v", visitor.Summary.Changed) + } + if len(visitor.Summary.Removed) != 0 { + t.Errorf("expected 0 Removed features (f-3 filtered out by triggers), got %v", visitor.Summary.Removed) + } + + if mock.queryErrorsVisited != 1 { + t.Errorf("expected query errors to be visited once, got %d", mock.queryErrorsVisited) + } + if mock.resolvedQueryErrorsVisited != 1 { + t.Errorf("expected resolved query errors to be visited once, got %d", mock.resolvedQueryErrorsVisited) + } + if mock.addedVisited != 1 { + t.Errorf("expected added features to be visited once, got %d", mock.addedVisited) + } + if mock.changedVisited != 1 { + t.Errorf("expected changed features to be visited once, got %d", mock.changedVisited) + } +} + +func TestBaseSummaryVisitor_ErrorDispatchPropagation(t *testing.T) { + summary := NewEmptyEventSummary() + summary.QueryErrors = []SummaryQueryError{ + {Code: SummaryQueryErrorCodeInvalidQuery}, + } + + mock := newMockCategorizedVisitor() + expectedErr := errors.New("dispatch failure") + mock.errToReturn = expectedErr + + visitor := newBaseSummaryVisitor(nil, mock) + err := visitor.VisitV1(summary) + if !errors.Is(err, expectedErr) { + t.Errorf("expected error %v, got %v", expectedErr, err) + } +} + +func TestBaseSummaryVisitor_HasContent_Empty(t *testing.T) { + summary := NewEmptyEventSummary() + visitor := newBaseSummaryVisitor(nil, nil) + if err := visitor.VisitV1(summary); err != nil { + t.Fatalf("VisitV1 unexpected error: %v", err) + } + if visitor.HasContent() { + t.Error("expected HasContent() to be false on empty summary") + } +} + +func newTestHighlight(typ SummaryHighlightType, id, name string) SummaryHighlight { + return SummaryHighlight{ + Type: typ, + FeatureID: id, + FeatureName: name, + Docs: nil, + NameChange: nil, + BaselineChange: nil, + BrowserChanges: nil, + Moved: nil, + Split: nil, + } +} + +func TestBaseSummaryVisitor_AllCategoryRoutes(t *testing.T) { + summary := NewEmptyEventSummary() + summary.AddHighlight(newTestHighlight(SummaryHighlightTypeMoved, "f-m", "Moved")) + summary.AddHighlight(newTestHighlight(SummaryHighlightTypeSplit, "f-s", "Split")) + summary.AddHighlight(newTestHighlight(SummaryHighlightTypeDeleted, "f-d", "Deleted")) + + mock := newMockCategorizedVisitor() + visitor := newBaseSummaryVisitor(nil, mock) + + if err := visitor.VisitV1(summary); err != nil { + t.Fatalf("VisitV1 unexpected error: %v", err) + } + + if len(visitor.Summary.Moved) != 1 || visitor.Summary.Moved[0].FeatureID != "f-m" { + t.Errorf("Moved routing failed: %v", visitor.Summary.Moved) + } + if len(visitor.Summary.Split) != 1 || visitor.Summary.Split[0].FeatureID != "f-s" { + t.Errorf("Split routing failed: %v", visitor.Summary.Split) + } + if len(visitor.Summary.Deleted) != 1 || visitor.Summary.Deleted[0].FeatureID != "f-d" { + t.Errorf("Deleted routing failed: %v", visitor.Summary.Deleted) + } + + if mock.movedVisited != 1 || mock.splitVisited != 1 || mock.deletedVisited != 1 { + t.Errorf("Dispatch failed for Moved/Split/Deleted: moved=%d, split=%d, deleted=%d", + mock.movedVisited, mock.splitVisited, mock.deletedVisited) + } +} + +func TestBaseSummaryVisitor_AllHighlightsFilteredOut(t *testing.T) { + summary := NewEmptyEventSummary() + summary.AddHighlight(newTestHighlight(SummaryHighlightTypeAdded, "f-1", "Unmatched Feature")) + + triggers := []JobTrigger{FeaturePromotedToWidely} + + mock1 := newMockCategorizedVisitor() + v1 := newBaseSummaryVisitor(triggers, mock1) + if err := v1.VisitV1(summary); err != nil { + t.Fatalf("VisitV1 unexpected error: %v", err) + } + if v1.HasContent() { + t.Error("expected HasContent() to be false when all highlights are filtered and no errors exist") + } + if mock1.addedVisited != 0 { + t.Errorf("expected 0 addedVisited when filtered out, got %d", mock1.addedVisited) + } + + summary.SetQueryErrors([]SummaryQueryError{{Code: SummaryQueryErrorCodeInvalidQuery}}) + mock2 := newMockCategorizedVisitor() + v2 := newBaseSummaryVisitor(triggers, mock2) + if err := v2.VisitV1(summary); err != nil { + t.Fatalf("VisitV1 unexpected error: %v", err) + } + if !v2.HasContent() { + t.Error("expected HasContent() to be true when QueryErrors exist even if highlights are filtered out") + } + if mock2.queryErrorsVisited != 1 || mock2.addedVisited != 0 { + t.Errorf("expected queryErrorsVisited=1 and addedVisited=0, got qe=%d, added=%d", + mock2.queryErrorsVisited, mock2.addedVisited) + } +} diff --git a/lib/workertypes/types.go b/lib/workertypes/types.go index 4cb77fd8b..32285668e 100644 --- a/lib/workertypes/types.go +++ b/lib/workertypes/types.go @@ -173,6 +173,57 @@ type EventSummary struct { Highlights []SummaryHighlight `json:"highlights"` } +// Categorize filters and categorizes the summary highlights against the provided triggers. +// It returns a BaseSummaryVisitor containing the categorized summary and any processing errors. +func (s *EventSummary) Categorize(triggers []JobTrigger) (*BaseSummaryVisitor, error) { + base := newBaseSummaryVisitor(triggers, nil) + if err := base.VisitV1(*s); err != nil { + return nil, err + } + + return base, nil +} + +// Accept filters highlights against triggers and executes double-dispatch via BaseSummaryVisitor. +func (s *EventSummary) Accept(v CategorizedSummaryVisitor, triggers []JobTrigger) error { + base := newBaseSummaryVisitor(triggers, v) + + return base.VisitV1(*s) +} + +// ExtractUniqueFeatureIDs returns deduplicated feature IDs for highlights matching the given triggers. +func (s *EventSummary) ExtractUniqueFeatureIDs(triggers []JobTrigger) []string { + filtered := FilterHighlights(s.Highlights, triggers) + if len(filtered) == 0 { + return nil + } + idMap := make(map[string]struct{}, len(filtered)) + ids := make([]string, 0, len(filtered)) + for _, h := range filtered { + if _, exists := idMap[h.FeatureID]; !exists { + idMap[h.FeatureID] = struct{}{} + ids = append(ids, h.FeatureID) + } + } + + return slices.Clip(ids) +} + +// AddHighlight adds a highlight to the summary. +func (s *EventSummary) AddHighlight(h SummaryHighlight) { + s.Highlights = append(s.Highlights, h) +} + +// SetQueryErrors sets the active query errors on the summary. +func (s *EventSummary) SetQueryErrors(errs []SummaryQueryError) { + s.QueryErrors = errs +} + +// SetResolvedQueryErrors sets the resolved query errors on the summary. +func (s *EventSummary) SetResolvedQueryErrors(errs []SummaryQueryError) { + s.ResolvedQueryErrors = errs +} + func NewEmptySummaryCategories() SummaryCategories { return SummaryCategories{ Updated: 0, @@ -319,8 +370,14 @@ type SummaryHighlight struct { NameChange *Change[string] `json:"name_change,omitempty"` BaselineChange *Change[BaselineValue] `json:"baseline_change,omitempty"` BrowserChanges map[BrowserName]*Change[BrowserValue] `json:"browser_changes,omitempty"` - Moved *Change[FeatureRef] `json:"moved,omitempty"` - Split *SplitChange `json:"split,omitempty"` + + // Moved details feature rename/location changes. May be nil if the highlight + // type is not SummaryHighlightTypeMoved or if unmarshaled from historical/partial event payloads. + Moved *Change[FeatureRef] `json:"moved,omitempty"` + + // Split details feature split changes into sub-features. May be nil if the highlight + // type is not SummaryHighlightTypeSplit or if unmarshaled from historical/partial event payloads. + Split *SplitChange `json:"split,omitempty"` } // SummaryVisitor defines the contract for consuming immutable Event Summaries. diff --git a/lib/workertypes/types_test.go b/lib/workertypes/types_test.go index bdf123e93..e67eb16cc 100644 --- a/lib/workertypes/types_test.go +++ b/lib/workertypes/types_test.go @@ -57,6 +57,7 @@ func TestParseEventSummary(t *testing.T) { SchemaVersion: "v1", SnapshotOrigin: "", Text: "Hello", + Truncated: false, Categories: SummaryCategories{ QueryChanged: 0, Added: 0, @@ -69,7 +70,6 @@ func TestParseEventSummary(t *testing.T) { UpdatedRename: 0, UpdatedBaseline: 0, }, - Truncated: false, Highlights: nil, QueryErrors: nil, ResolvedQueryErrors: nil, @@ -126,6 +126,7 @@ func TestParseEventSummary(t *testing.T) { func TestGenerateJSONSummaryFeatureDiffV1(t *testing.T) { newlyDate := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) browserImplDate := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) + _ = browserImplDate tests := []struct { name string diff v1.FeatureDiff @@ -783,10 +784,84 @@ func TestGenerateJSONSummary_QueryErrorsAndResolvedQueryErrors(t *testing.T) { t.Fatalf("json.Unmarshal failed: %v", err) } - if len(summary.QueryErrors) != 1 || summary.QueryErrors[0].Code != SummaryQueryErrorCodeSavedSearchNotFound { + if len(summary.QueryErrors) != 1 || + summary.QueryErrors[0].Code != SummaryQueryErrorCodeSavedSearchNotFound { t.Errorf("QueryErrors = %+v, want SavedSearchNotFound", summary.QueryErrors) } - if len(summary.ResolvedQueryErrors) != 1 || summary.ResolvedQueryErrors[0].Code != SummaryQueryErrorCodeQueryGrammar { + if len(summary.ResolvedQueryErrors) != 1 || + summary.ResolvedQueryErrors[0].Code != SummaryQueryErrorCodeQueryGrammar { t.Errorf("ResolvedQueryErrors = %+v, want QueryGrammar", summary.ResolvedQueryErrors) } } + +func TestEventSummary_EncapsulationAndMutators(t *testing.T) { + summary := NewEmptyEventSummary() + summary.Text = "Mutated summary" + summary.AddHighlight(newTestHighlight(SummaryHighlightTypeAdded, "f1", "F1")) + summary.SetQueryErrors([]SummaryQueryError{{Code: SummaryQueryErrorCodeInvalidQuery}}) + summary.SetResolvedQueryErrors([]SummaryQueryError{{Code: SummaryQueryErrorCodeQueryGrammar}}) + + data, err := json.Marshal(summary) + if err != nil { + t.Fatalf("json.Marshal failed: %v", err) + } + + var parsed EventSummary + if err := json.Unmarshal(data, &parsed); err != nil { + t.Fatalf("json.Unmarshal failed: %v", err) + } + + if len(parsed.Highlights) != 1 || parsed.Highlights[0].FeatureID != "f1" { + t.Errorf("Highlights mismatch after round-trip: %v", parsed.Highlights) + } + if len(parsed.QueryErrors) != 1 || + parsed.QueryErrors[0].Code != SummaryQueryErrorCodeInvalidQuery { + t.Errorf("QueryErrors mismatch after round-trip: %v", parsed.QueryErrors) + } + if len(parsed.ResolvedQueryErrors) != 1 || + parsed.ResolvedQueryErrors[0].Code != SummaryQueryErrorCodeQueryGrammar { + t.Errorf("ResolvedQueryErrors mismatch after round-trip: %v", parsed.ResolvedQueryErrors) + } +} + +func TestParseEventSummary_NonEmptyRawDispatch(t *testing.T) { + rawJSON := []byte(`{ + "schemaVersion": "v1", + "text": "Full payload", + "queryErrors": [{"code": "saved_search_not_found"}], + "resolvedQueryErrors": [{"code": "query_grammar_invalid"}], + "highlights": [ + {"type": "Added", "feature_id": "f-100", "feature_name": "Grid"} + ] + }`) + + mock := newMockCategorizedVisitor() + visitor := newBaseSummaryVisitor(nil, mock) + + if err := ParseEventSummary(rawJSON, visitor); err != nil { + t.Fatalf("ParseEventSummary unexpected error: %v", err) + } + + if mock.queryErrorsVisited != 1 { + t.Errorf("expected queryErrorsVisited = 1, got %d", mock.queryErrorsVisited) + } + if mock.resolvedQueryErrorsVisited != 1 { + t.Errorf("expected resolvedQueryErrorsVisited = 1, got %d", mock.resolvedQueryErrorsVisited) + } + if mock.addedVisited != 1 { + t.Errorf("expected addedVisited = 1, got %d", mock.addedVisited) + } +} + +func TestEventSummary_AcceptContract(t *testing.T) { + summary := NewEmptyEventSummary() + summary.AddHighlight(newTestHighlight(SummaryHighlightTypeAdded, "f-1", "F1")) + + mock := newMockCategorizedVisitor() + if err := summary.Accept(mock, nil); err != nil { + t.Fatalf("summary.Accept unexpected error: %v", err) + } + if mock.addedVisited != 1 { + t.Errorf("summary.Accept did not dispatch to visitor, addedVisited = %d", mock.addedVisited) + } +}