[ENHANCEMENT] Notification Pipeline Architecture: Category Renderer Visitor Pattern & Control-Plane Event Separation

[jcscottiii]

Description

As discovered during the resolution of the All Features (all) saved search and notification error visibility bug (#2621), our notification and query evaluation architecture has several structural opportunities for long-term hardening and refactoring.

1. System Global Saved Searches CI Harness (TestSystemGlobalSearches_ExecutionValidity)

Currently, built-in global searches (all, top-css-interop, top-html-interop, baseline-2026, etc.) are seeded via raw SQL inside infra/storage/spanner/migrations/000032.sql. Because SQL strings aren't checked by Go compiler tools, grammar changes or parser updates can break built-in queries without compile-time failure.

We should introduce an automated table-driven CI harness in spanneradapters/system_global_saved_searches_test.go that iterates through all SystemGlobalSavedSearches and asserts that FetchFeatures(ctx, query) succeeds with zero UserError:

func TestSystemGlobalSearches_ExecutionValidity(t *testing.T) {
	for _, search := range AllSystemGlobalSearches {
		t.Run(search.ID, func(t *testing.T) {
			result, err := differClient.FetchFeatures(ctx, search.Query)
			if err != nil {
				t.Fatalf("FetchFeatures(%q) unexpected error: %v", search.ID, err)
			}
			if result.UserError != nil {
				t.Errorf("Built-in search %q failed evaluation with UserError: %v", search.ID, result.UserError)
			}
		})
	}
}

2. Universal UI Presentation & Categorization Core (BaseSummaryVisitor across all channels)

In #2621 (PR 1), our short-term fix focused strictly on resolving the All Features April outage and ensuring query errors are visible across shouldNotifyV1 and rssVisitor without refactoring existing presentation code.

As part of this long-term enhancement, we will introduce workertypes.BaseSummaryVisitor and CategorizedSummary (lib/workertypes/summary_categorizer.go & summary_categorizer_test.go — ~361 lines of code and tests) to centralize highlight trigger filtering (FilterHighlights), category grouping (Added, Removed, Changed, Moved, Split, Deleted), and uniform evaluation of QueryErrors & ResolvedQueryErrors.

We will refactor all three presentation and delivery pipelines to embed and use BaseSummaryVisitor:

• Email (workers/email/pkg/digest/renderer.go): Embed BaseSummaryVisitor inside templateDataGenerator, completely replacing its custom categorizeHighlights and routeHighlightToCategory loops, and wire resolved_query_error_banner HTML components so subscribers receive explicit ✅ Query Recovered banners upon error resolution.
• Slack Webhook (workers/webhook/pkg/webhook/slack.go): Embed BaseSummaryVisitor inside slackPayloadBuilder, replacing its processHighlight loop, and wire appendResolvedQueryErrors JSON block sections.
• RSS (backend/pkg/httpserver/rss_visitor.go): Embed BaseSummaryVisitor inside rssVisitor, replacing its loop logic and standardizing XML category item generation.

In alignment with issue #2602 (shared summary presentation core), this ensures that every renderer across Email, Slack, and RSS shares identical categorization math, trigger evaluation, and visual error/recovery reporting without worker-specific duplication.

3. Strong Category Visitor Interface & Private Encapsulation (workertypes.CategorizedSummaryVisitor)

At the schema level, workertypes.SummaryVisitor currently forces renderers to implement VisitV1(summary EventSummary). Once inside VisitV1, each channel manually inspects slices because Highlights, QueryErrors, and ResolvedQueryErrors are public exported fields on EventSummary.

A. Private Encapsulation (Enforcing Visitor Compliance by Compiler)

To prevent future channel renderers (or ad-hoc worker scripts) from bypassing the visitor method and doing manual, error-prone for _, h := range summary.Highlights loops (which can bypass FilterHighlights triggers or drop error alerts), we should encapsulate and unexport (make private) the underlying slice fields on EventSummary (using an unexported inner struct or custom UnmarshalJSON):

type EventSummary struct {
	SchemaVersion  VersionEventSummary `json:"schemaVersion"`
	SnapshotOrigin SnapshotOrigin      `json:"snapshotOrigin"`
	Text           string              `json:"text"`
	Categories     SummaryCategories   `json:"categories"`
	Truncated      bool                `json:"truncated"`
	privateRaw     eventSummaryRaw     // Unexported: hides raw Highlights and QueryErrors from external packages
}

By making privateRaw unexported, the Go compiler literally forces all external packages (workers/email, workers/webhook, backend/pkg/httpserver) to use summary.Accept(visitor) or ParseEventSummary(bytes, visitor) to access highlight and error data.

B. Typed Visitor Interface

We will pair this private encapsulation with an explicit typed visitor interface:

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
}

This guarantees complete compile-time safety across our entire notification pipeline: no renderer can bypass trigger filtering, and if a new highlight category (e.g., SummaryHighlightTypeMerged) is ever added, every output channel (Email, Slack, RSS) will fail compilation until explicit rendering for that category is implemented.

4. Separate Control-Plane vs. Data-Plane Event Schemas

Query syntax errors (query_grammar_invalid) and cyclic references (saved_search_cycle_detected) are control-plane system health failures, whereas FeatureDiffEvent / EventSummary represent data-plane web feature transitions.

We should separate operational query health into its own event schema (QueryHealthAlertEvent) published to a dedicated alerting topic/workflow with explicit backoff, deduplication, and auto-resolution notifications when a broken query is fixed.