Skip to content

fix(dashboard): route scheduled refresh through the quota cache - #142

Merged
pitimon merged 2 commits into
mainfrom
fix/140-scheduled-limits-cache
Aug 5, 2026
Merged

fix(dashboard): route scheduled refresh through the quota cache#142
pitimon merged 2 commits into
mainfrom
fix/140-scheduled-limits-cache

Conversation

@pitimon

@pitimon pitimon commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • A visible dashboard forced a full quota fan-out every 30 seconds. refreshAll now takes { forceUsageLimits }: manual refresh passes true, both scheduled passes pass false and go through the hook's cache-aware revalidate.
  • The first commit on this branch (a142542) added revalidate to the hook but nothing called it — git grep revalidate -- dashboard/src hit only a comment in use-pulse.ts. Merging it alone would have closed nothing. This commit is the wiring plus the behavioural tests.
  • use-usage-limits.test.tsx.ts: the file has no JSX, and validate:ui-hardcode was reading its TS generics (deferred<T>()) as JSX text and failing ci:local on the previous commit. Renaming fixes it at the source rather than widening the baseline.

The call site that hides from a naive test

handleAutoRefresh calls refreshAll twice — the interval tick (DashboardPage.jsx:809) and the post-sync follow-up when the background sync queued buckets (:813). With isLocalMode false, startAutoSync() returns null and :813 never executes, so a one-tick test passes whether or not that line was fixed. The suite mocks triggerLocalSync to report queued buckets so the branch actually runs.

Test plan

  • npm run ci:local — exit 0 (dashboard 42 files / 320 tests)
  • New DashboardPage.limits-refresh.test.jsx — 7 tests, renders the page with DashboardView stubbed
  • Mutation-checked, 5 ways — revert the tick → 3 fail; revert only the post-sync follow-up → 1 fail; revert both → 3 fail; make the manual path cache-aware → 2 fail; rewire handleWake to a direct forced refreshAll() → exactly the 2 wake tests fail. All restored → 7/7
  • Real local-server smoke on bin/tracker.js serve against the built bundle: manual click → exactly one ?refresh=1; auto at 30s → two scheduled requests, neither forced. (Read via performance.getEntriesByType("resource") — the network panel listing hides query strings. The smoke establishes "no scheduled request carries refresh=1"; which of the two was the tick and which the follow-up is not established by it, and does not need to be — the mutation above carries that.)
  • Limits page forced initial mount still locked by the existing LimitsPage.test.jsx:91-98 and :110-113 — no new test added, cited instead

Notes for the reviewer

  • The wake path is included, and it is broader than the issue's wording. focus / visibilitychange also run through handleAutoRefresh (:857-859), so they became cache-aware too. That is a deliberate call — they are scheduled work, not a user asking for fresh numbers — and it is inside the two-minute contract. Covered by two tests.
  • revalidate publishes with source: "page-load", which reads oddly for a scheduled tick. It is inert today: DashboardPage's useUsageLimits() leaves publishToPreloadCache falsy, and use-usage-limits.ts:52 early-returns. Left alone rather than widening the union type.
  • Pre-existing bug found during the smoke, not fixed here: handleManualRefresh (:771-780) awaits triggerLocalSync() inside the same try as refreshAll, so a failed local sync cancels the entire manual refresh — nothing refetches. Reproducible on the Vite dev server, where /api/local-auth 404s. Unrelated to limits: stop scheduled dashboard refresh from bypassing quota cache #140; worth its own issue.
  • Scope: no adaptive polling, freshness metadata, credits UI, notifications, provider registry, and no server-side fan-out coalescing — that is limits: coalesce concurrent quota provider fan-outs #141, in src/lib/usage-limits.js.

Closes #140

Review trail

/code-review clean (claim-verb audit and false-success grep both no-match). Codex QA at xhigh returned 5 Pass / 2 Fix: Edge Case (wake path untested, with a concrete mutation that would slip past every other test) and Handoff (nothing committed). Both closed — the wake tests exist because that pass asked for them.

itarun.p added 2 commits August 2, 2026 18:18
The hook change in a142542 added `revalidate` but nothing called it:
auto-refresh still went `handleAutoRefresh` -> `refreshAll` ->
`refreshUsageLimits()`, the forced path that appends `refresh=1`,
resets the CLI's two-minute cache (src/lib/usage-limits.js:28, :1922
via src/lib/local-api.js:1720-1722) and fans out to every configured
provider — every 30 seconds, per visible tab.

`refreshAll` now takes `{ forceUsageLimits }`. Manual refresh passes
true; both scheduled passes pass false — the interval tick and the
post-sync follow-up that runs when the background sync queued new
buckets. The follow-up matters: with `isLocalMode` false it never
runs at all, so a single-tick test cannot tell whether it was fixed.

Also renames use-usage-limits.test.tsx to .ts. It contains no JSX, and
validate:ui-hardcode was reading its TS generics (`deferred<T>()`) as
JSX text nodes and failing ci:local on the previous commit.

Closes #140
@pitimon

pitimon commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

Final merge review

Verdict: PASS — no merge-blocking correctness, security, performance, or test-quality findings.

Verified in this session:

  • Independent fresh-context review returned PASS at head 55edce7be747a84c3467d161fc79abc3a9de4ae8.
  • Manual refresh remains forced; interval, post-sync follow-up, focus, and visible-tab wake paths are cache-aware.
  • The new tests exercise both scheduled passes and mutation-discriminate the relevant call sites.
  • Local npm run ci:local completed successfully: dashboard 42 files / 320 tests, root 1008 tests, all validators and OpenWiki checks green.
  • Static added-line scan found no secrets, dangerous eval/exec, debug artifacts, or conflict markers.

The pre-existing manual-refresh cancellation found during review is deliberately out of scope and is now tracked as #146.

Proceeding with squash merge under the user's explicit authorization.

@pitimon
pitimon merged commit 12579c7 into main Aug 5, 2026
1 check passed
@pitimon
pitimon deleted the fix/140-scheduled-limits-cache branch August 5, 2026 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

limits: stop scheduled dashboard refresh from bypassing quota cache

1 participant