perf(dash): serve a stale cached read for up to ten minutes and show its real age

[OsherElhadad]

The dashboard's stale-while-revalidate cache could serve an old body for only 30 s past a 30 s TTL. The all-time stats read it protects takes about 6 s warm and 35 s on a cold process on a 3.6 GB database, so anyone who came back after a minute found nothing to serve and paid the whole query again: the "TTL shorter than the work" pattern.

Change

• dashCacheStale 30 s to 10 minutes. The single-flight refresh behind a stale answer is unchanged (one compute per key, however many readers).
• Every cached response carries an Age header (seconds since it was computed; 0 for a body computed for this request).
• The Overview reads it: Updated N ago is now the age of the data, not of the request, so the page never claims fresher figures than it shows. After a stale answer older than 45 s it looks once more after 10 s, so it settles on current figures without a click.

Measured (3.6 GB snapshot copy, stats?lean=1, all time; both servers on their own copy, same moment)

	before (main)	after
first read, cold process	6,854 ms	6,562 ms
read after 90 s (past the old 60 s cap)	9,937 ms, X-Cache: miss	18 ms, X-Cache: stale, Age: 99
read right after the background refresh	15 ms, hit	11 ms, Age: 7

The numbers are the same numbers; they can now be up to 10 minutes old, and the response says how old.

Tests

Stale body served at once with its age and exactly one refresh behind it; past the cap nothing stale is served (Age: 0, miss); the Overview's freshness uses the Age header. The existing single-flight and cap tests pass unchanged.

Cached reads that include user-editable state (review follow-up)

Audit of every cached route (statsCache, facetsCache, componentsCache, toolsCache, toolFilterCache, promptCache, spendCache, routeCache): only the tool, tool-filter and prompt reads include state a user edits (the account's removal list lives in its stored configuration), and the keep-alive routes in routeCache can include strategy and campaign state. The stats, series, sessions, facets, components, spend and kvcache reads are aggregates of traffic.

• The tenant registry takes a listener (SetOnChange); it fires after every committed account write (any Update that changes a field, including the toolfilter switch, PUT /api/me, adapt apply, keep-alive control) and every strategy or campaign write, never for a refused or no-op write.
• API.InvalidateStateCaches clears toolsCache, toolFilterCache and promptCache on an account write, and also routeCache on a strategy or campaign write. A compute that began before the write cannot store its (old) body afterwards (generation check).
• Tests: toggle then reload shows the change at once (TestToolFilterWriteShowsOnTheNextRead, fails without the wiring), the registry listener fires only for committed writes, and a late old compute does not repopulate the cache.

[OsherElhadad]

Review: REQUEST CHANGES at ade961c. (Posted as a comment: GitHub does not let the author account request changes.)

Verified:

• Single-flight still holds. Only the first stale reader calls beginRefresh, and endRefresh is deferred. A failed refresh keeps the stale body inside its cap. Past the cap nothing stale is served (miss, Age: 0).
• Cache keys are unchanged (tenant + manager + rounded query, so manager ?tenant= is in the key). No cross-tenant body, and background refreshes compute only the requesting key's scope.
• Memory is unchanged. Entries were already kept until the 256-entry wipe; only the serve window grew.
• Tests catch regressions. With Age forced to 0 on stale answers, or the cap back at 30 s, TestServeJSONServesAStaleBodyWithItsAge fails.
• Checks. gofmt and go vet are clean. go test ./dash passes in full, and the cache subset passes under -race. CI is green, and the commit is signed off.
• Adapt routes are untouched. They don't use serveJSON, so their per-account report limits are unaffected.

Blocking:

1. The 10-minute cap also applies to state-bearing routes, so a user's own change can be shown undone. /api/toolfilter (toolFilterCache) caches a body that includes the account's current removal list (toolFilterStateForScope is baked into the compute). The POST answer is fresh, but a reload or tab switch afterwards can serve the pre-change switches for up to 10.5 minutes, where before it was 60 s. /api/tools (toolsCache) is the same. Neither route shows an age (only the Overview reads Age), so the page presents the old state as current. Fix one of these ways:
   • drop the account's toolFilterCache/toolsCache entries on every removal-list write (POST /api/toolfilter and the adapt apply/revert writers);
   • or keep these two caches on the old short cap;
   • or compute the state outside the cached body and merge it per request.

Non-blocking:
2. The age can understate. serveJSON reads the body with c.load(key) and then the age with a separate c.age(key). If a refresh's set lands between the two locks, a stale body is labelled with the new entry's age, close to 0, which contradicts "never shown newer than it is". Returning the entry's at from load/get under the same lock fixes it.
3. The benefit is mostly the all-time view. cacheKey rounds since/until to 30 s buckets, so a relative range ("last 24h") moves to a new key every 30 s and the longer stale window rarely applies there. That is fine; the PR text could say the gain is mainly the all-time view.
4. The retry can repeat for up to 10 minutes. loadOverview's stale retry re-arms every 10 s while the data stays over 45 s old. If refreshes keep failing, that is about 60 cheap cache reads over the cap window. Acceptable, but consider stopping after one or two retries.

[OsherElhadad]

Re-review: REQUEST CHANGES at b033366, for one small gap. (Posted as a comment: GitHub does not let the author account request changes.)

The hosted path is fixed:

• Registry.Update calls changed(ChangeAccount) after tx.Commit() and only on a real change. Strategy and campaign create, update, delete and archive call changed(ChangeStrategy) after a successful commit. proxy.go wires the hook to InvalidateStateCaches.
• With the wiring removed, TestToolFilterWriteShowsOnTheNextRead fails.
• The hook runs after the transaction, and InvalidateStateCaches only takes jsonCache locks, so there is no lock-order inversion with the registry.
• Clearing all tenants' entries on each write is acceptable, since writes are rare.

Blocking:

1. Single-tenant writes don't invalidate. On a local proxy there is no registry, so the hook is never set. reloadLocal (proxy/adaptapply.go), which adaptWriteLocal and adaptRevertLocal use, swaps the pipeline without clearing anything. A /context-guru:remove-mcp-and-skills apply, save-more config apply or a revert on the plugin's own proxy can therefore show the old removal list in the local dashboard's Inventory and Tool filter tabs for up to the 10-minute stale window. That is the original bug, on the plugin's main path. One line fixes it: call h.api.InvalidateStateCaches(tenant.ChangeAccount) (nil-guarded) at the end of a successful reloadLocal. Add a test like the hosted one.

Non-blocking:
2. setIfCurrent checks the generation under the lock, unlocks, and then set locks again. A clear() landing between the two lets a pre-write body be stored after the clear. Doing the compare and the store under one lock closes it.
3. Only the method is pinned, not the call sites. TestInvalidateStateCachesDropsReadsAndIgnoresALateOldCompute calls setIfCurrent directly. With both serveJSON call sites switched back to set, every test still passes.
4. The earlier age note still stands. The body and c.age(key) are read under separate locks. After a clear() between the two, age reads 0 for an old body.

Checks: gofmt and go vet are clean. ./tenant passes, the dash cache/stale/invalidate tests and the proxy toolfilter/adapt tests pass under -race, and CI is green. The commit is signed off with no AI attribution.

[OsherElhadad]

Pushed b699b9c: single-tenant apply/revert (reloadLocal) now invalidates (test fails without it); check+store and body+age each under one lock; a clear during a slow compute is not undone (foreground flight and background refresh, test added). The single-flight key fix moved to its own hotfix, #574.

[OsherElhadad]

Re-review at b699b9c: the code is APPROVED. Please re-run CI before merging: build-test is red, on what looks like an unrelated timing flake (details at the end). (Posted as a comment: GitHub does not let the author account approve.)

Last blocker fixed:

• reloadLocal (proxy/adaptapply.go) now calls h.api.InvalidateStateCaches(tenant.ChangeAccount), nil-guarded, after a successful pipeline swap. Without it, TestAdaptApplyLocalDropsTheDashboardsCachedReads fails.

Earlier non-blocking notes, all fixed and pinned:

• setIfCurrent compares the generation and stores under one lock hold (setLocked). With the check turned into if true, TestInvalidateStateCachesDropsReadsAndIgnoresALateOldCompute fails.
• The live routes use it. I switched each serveJSON call site back to set, the foreground flight (api.go:400) and the background refresh (api.go:506), one at a time. Each makes TestAClearDuringASlowComputeIsNotUndoneByIt fail.
• Body and age come from one lock hold (loadAged). Writing a stale answer with age 0 makes TestServeJSONServesAStaleBodyWithItsAge fail.

Other checks:

• The single-flight key fix is not in this PR. It merges cleanly both with main and with #574's head, so there is no dependency or conflict.
• gofmt and go vet are clean on dash, proxy and tenant.
• go test -race ./dash -run 'Cache|Stale|Invalidate|Age' and go test -race ./proxy -run 'ToolFilter|Adapt' pass, and ./tenant passes.
• The commit is signed off with no AI attribution.

CI: build-test fails only TestDashboardAddsNoRequestLatencyWithContentCapture. The dashboard added 57.69 ms against a 57.10 ms budget, on a 571 ms median baseline. That latency-budget test is known to be timing-sensitive under load, and this PR touches nothing on the request goroutine (caches and write hooks only). Please re-run it.

This approval covers b699b9c, once CI is green.

[OsherElhadad]

Rebased onto #564's branch (875d960) and the PR base set to feat/dashboard-ux-overhaul so the two merge in either order: the only change is the Age handling in dash/ui/app.js, now on #564's lean-then-full Overview read (the lean read sets 'Updated N ago' from the Age header; the full read sets it again and schedules the one catch-up refresh when the answer was stale by more than 45 s). Tests on this head: dash UI, savings, nav, freshness and cache tests, the browser smoke test, proxy tool-filter and local-apply tests, tenant on-change test: ok. When #564 merges I retarget this to main and rebase; the diff should be identical (range-diff).

[OsherElhadad]

Re-review: APPROVE at ca1477d (restacked on #564's 875d960). git range-diff against the approved b699b9c: 21 patches are identical, and the two marked "!" differ only in context. One is the #574 single-flight key line already on the base. The other is the dash/ui/app.js Age hunk moved into #564's lean-then-full loadOverview. In that hunk, state.loadedAt is set from Date.now() - ageMs.stats after both the lean and the full read. Both calls use the path stats, so each takes its own response's Age. The single 10 s catch-up refresh is scheduled only after the full read and only when the data is more than 45 s old. Invalidation, reloadLocal, the one-lock guard and the server Age header are unchanged. CI is green. This approval covers ca1477d.