fix(dash): let the invalid-config banner clear itself once the config is fixed

dash/overview.go

@@ -3,6 +3,7 @@ package dash
 import (
 	"database/sql"
 	"fmt"
+	"time"
 
 	"golang.org/x/sync/errgroup"
 )
@@ -49,6 +50,16 @@ type WaterfallStep struct {
 // Overview is the payload behind the dashboard's headline. Every percentage here
 // is derived at read time from stored absolutes, so a rate change never rewrites
 // history and a filter change never needs a rebuild.
+// invalidConfigRecentWindow is how recently a request must have run with a failed configuration
+// for the dashboard to still call it a live problem.
+//
+// An hour, because the thing being detected is a stored config that cannot build: while it lasts
+// EVERY request for that account is affected, so on any account with traffic the signal appears
+// within seconds and keeps appearing. An hour is therefore long enough that a quiet account still
+// trips it, and short enough that the banner clears on its own once someone fixes the config —
+// without anyone having to know that clearing it is a thing that needs doing.
+const invalidConfigRecentWindow = time.Hour
+
 type Overview struct {
 	Since    int64 `json:"since"`
 	Until    int64 `json:"until"`
@@ -65,6 +76,21 @@ type Overview struct {
 	// zero before anyone noticed from a log line rather than from here. Nonzero here means the
 	// account's Settings page needs attention now, not "check the logs".
 	InvalidConfigRequests int64 `json:"invalid_config_requests"`
+	// InvalidConfigRecent is the same count over the last invalidConfigRecentWindow, scoped to
+	// the account but NOT to the filter's time range or any of its facets. It exists because
+	// InvalidConfigRequests alone cannot answer the question the banner above the headline asks,
+	// which is "is compaction broken RIGHT NOW".
+	//
+	// The dashboard's default view has no time filter at all, so a window-scoped count keeps
+	// reporting an incident forever after it is over: this deployment carried 1,752 such requests
+	// from a single afternoon (a config key removed with no migration, since fixed), and the page
+	// went on telling every viewer to "open Settings and fix the configuration" for days, about a
+	// configuration that was already correct. An alarm that cannot switch itself off is one people
+	// learn to scroll past, which costs exactly what it was built to buy.
+	//
+	// Facets are deliberately ignored, not just the time range: filtering to one model must not be
+	// able to hide a live breakage that happens to be showing up on another.
+	InvalidConfigRecent int64 `json:"invalid_config_recent"`
 
 	TokensBefore int64 `json:"tokens_before"`
 	TokensAfter  int64 `json:"tokens_after"`
@@ -680,6 +706,7 @@ func (d *DB) Overview(f Filter) (*Overview, error) {
 		accountingM, cacheMissM, uncompressedM map[string]int64
 		p95cg, p95up                           float64
 		invalidConfigRequests                  int64
+		invalidConfigRecent                    int64
 	)
 	var g errgroup.Group
 	// See InvalidConfigRequests' own comment: preset is set to "invalid" by
@@ -691,6 +718,20 @@ func (d *DB) Overview(f Filter) (*Overview, error) {
 		return d.sql.QueryRowContext(d.readCtx(), `SELECT COUNT(*) FROM requests r
 			WHERE `+cond+` AND r.preset = 'invalid'`, args...).Scan(&invalidConfigRequests)
 	})
+	// The same fact over a fixed recent window, which is what the banner is actually about. Built
+	// here rather than by copying f and overriding Since, so that it demonstrably carries the
+	// TENANT scope and nothing else: a facet leaking in could hide a live breakage, and dropping
+	// the tenant term would show one account another's problem.
+	g.Go(func() error {
+		rcond := "r.keepalive = 0 AND r.ts >= ? AND r.preset = 'invalid'"
+		rargs := []any{time.Now().Add(-invalidConfigRecentWindow).UnixMilli()}
+		if !f.TenantAll {
+			rcond += " AND r.tenant_id = ?"
+			rargs = append(rargs, f.Tenant)
+		}
+		return d.sql.QueryRowContext(d.readCtx(),
+			`SELECT COUNT(*) FROM requests r WHERE `+rcond, rargs...).Scan(&invalidConfigRecent)
+	})
 	// The replay ceiling's raw form, corrected below by the inflation query once both are in —
 	// see the comment above `splitMoved` usage earlier in this function for the ceiling's own
 	// derivation, and see the correlated-vs-window-function tradeoff explained where this query
@@ -906,6 +947,7 @@ func (d *DB) Overview(f Filter) (*Overview, error) {
 	o.Uncompressed = uncompressedM
 	o.CGLatencyMsP95, o.UpstreamMsP95 = p95cg, p95up
 	o.InvalidConfigRequests = invalidConfigRequests
+	o.InvalidConfigRecent = invalidConfigRecent
 
 	o.SafetyCost.FrozenTokens = o.FrozenTokens
 	o.SafetyCost.RestoredTokens = o.ExpandTokens

dash/store_test.go

@@ -489,6 +489,52 @@ func TestOverviewCountsInvalidConfigRequests(t *testing.T) {
 	}
 }
 
+// A resolved config incident must stop raising the alarm, which is what InvalidConfigRecent is for.
+//
+// The dashboard's banner is present-tense — "open Settings and fix the configuration" — but it was
+// driven by a count over the filter's window, and the default view has NO window. So on this
+// deployment 1,752 invalid-config requests from a single afternoon kept the banner up for days
+// after the config was fixed, telling every viewer to go and repair something already correct. An
+// alarm that cannot switch itself off is one people learn to ignore.
+func TestInvalidConfigRecentIgnoresResolvedHistory(t *testing.T) {
+	db := openTestDB(t)
+	now := time.Now()
+	// A burst of breakage well in the past, and nothing wrong since.
+	old := mkEvent(now.Add(-48*time.Hour).UnixMilli(), "s-old", "m", 100, 100)
+	old.Preset = "invalid"
+	fine := mkEvent(now.Add(-time.Minute).UnixMilli(), "s-now", "m", 100, 90)
+	fine.Preset = "codesmart"
+	if err := db.insertBatch([]*Event{old, fine}); err != nil {
+		t.Fatal(err)
+	}
+	o, err := db.Overview(Filter{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if o.InvalidConfigRequests != 1 {
+		t.Fatalf("invalid_config_requests = %d, want 1 (the historical fact must still be reported)",
+			o.InvalidConfigRequests)
+	}
+	if o.InvalidConfigRecent != 0 {
+		t.Errorf("invalid_config_recent = %d, want 0: a config fixed two days ago is not a live "+
+			"problem, and the banner keyed on this must clear itself", o.InvalidConfigRecent)
+	}
+
+	// ...and a breakage happening NOW must still raise it, or the fix has removed the alarm.
+	live := mkEvent(now.Add(-2*time.Minute).UnixMilli(), "s-live", "m", 100, 100)
+	live.Preset = "invalid"
+	if err := db.insertBatch([]*Event{live}); err != nil {
+		t.Fatal(err)
+	}
+	if o, err = db.Overview(Filter{}); err != nil {
+		t.Fatal(err)
+	}
+	if o.InvalidConfigRecent != 1 {
+		t.Errorf("invalid_config_recent = %d, want 1: a configuration failing to build right now is "+
+			"exactly what this banner exists to surface", o.InvalidConfigRecent)
+	}
+}
+
 // TestNewInputRatioNeverDividesSavingsByThemselves is the guard the issue calls
 // non-negotiable: with no provider usage data the denominator would be `saved`
 // alone and the ratio would read ~100%. It must read n/a.

dash/ui/app.js

@@ -1477,10 +1477,16 @@ function renderTiles(o) {
   // proxy's own log. Shown above the headline, not folded into Diagnostics: the number it
   // reports is "your bill was bigger than it needed to be, right now", which is exactly what
   // this page exists to prevent from going unnoticed.
-  if (o.invalid_config_requests > 0) {
+  // Gated on the RECENT count, not the window's total, and that distinction is the whole point.
+  // The default view has no time filter, so a total keeps reporting an incident forever after it
+  // is fixed: this deployment carried 1,752 such requests from one afternoon and then told every
+  // viewer for days to go and fix a configuration that was already correct. The banner asks a
+  // present-tense question, so it reads a present-tense number and clears itself once someone
+  // fixes the config — see InvalidConfigRecent in dash/overview.go.
+  if (o.invalid_config_recent > 0) {
     host.appendChild(el('div', { class: 'banner bad', 'data-testid': 'invalid-config' },
-      el('div', {}, el('strong', {}, num(o.invalid_config_requests) + ' request'
-        + (o.invalid_config_requests === 1 ? '' : 's') + ' ran with NO compaction. '),
+      el('div', {}, el('strong', {}, num(o.invalid_config_recent) + ' request'
+        + (o.invalid_config_recent === 1 ? '' : 's') + ' ran with NO compaction in the last hour. '),
         'This account’s stored configuration failed to build, so every one of them was ' +
         'forwarded as-is instead — traffic kept working, but none of it was compacted. ' +
         'Open Settings, fix the configuration, and save it again.')));