fix(dashboard): limit automatic provider health checks - #363
Conversation
Provider health fans out to every configured provider, so keep automatic checks to an hourly cadence while preserving the explicit forced re-check action. Fixes #358
GitHub Actions
Inactive
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe dashboard provider-health hook now refreshes hourly, pages use the hook’s fixed policy, tests verify cached health is not refetched within an hour, and the HTML entrypoint references a rebuilt React/Router bundle. ChangesProvider health refresh
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR reduces unnecessary provider fan-out from the dashboard by throttling automatic provider health checks to a once-per-hour cadence, while keeping the manual “Re-check all” action as an immediate live refresh.
Changes:
- Updated
useProviderHealthto cache results for 1 hour and automatically refetch at most hourly. - Removed the Overview page’s 60s health polling, using the hook’s new default behavior instead.
- Added a regression test ensuring cached provider health prevents repeated automatic checks within an hour, and updated the committed dashboard entrypoint asset hash.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| web/src/pages/ProvidersPage.test.tsx | Adds a Vitest regression test to confirm provider health isn’t automatically re-fetched within an hour when cached. |
| web/src/pages/OverviewPage.tsx | Stops passing a 60s polling interval; relies on the hook’s new throttled behavior. |
| web/src/api/hooks.ts | Introduces a 1-hour refresh interval for provider health (staleTime + refetchInterval) and simplifies the hook signature. |
| src/gateway/static/dashboard/index.html | Updates the hashed dashboard JS entrypoint reference to match the rebuilt bundle. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/src/pages/ProvidersPage.test.tsx (2)
396-412: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the one-hour boundary explicitly.
A 31-second-old cache would also pass with any policy longer than 31 seconds, so this test does not protect the stated hourly behavior. Seed data just under the shared hourly interval, then advance past it with fake timers and assert the automatic refresh.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/pages/ProvidersPage.test.tsx` around lines 396 - 412, Update the test around the “does not automatically re-check all providers within an hour” case to seed provider-health data just under the shared one-hour interval, using fake timers. Advance the timer beyond that interval, remount or trigger the query lifecycle, and assert that healthRequestCount increases, while preserving the existing no-refresh assertion before the boundary.
405-405: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the provider-health query-key constant.
The test duplicates
["provider-health"]fromweb/src/api/hooks.ts. Export and reuse a shared query-key constant in the hook, mutation, and test so cache seeding cannot drift from the production key.As per coding guidelines, keep query keys as module constants.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/pages/ProvidersPage.test.tsx` at line 405, Define and export a module-level provider-health query-key constant in the API hooks module, then update the provider-health hook, mutation, and ProvidersPage test to reuse it instead of inline ["provider-health"] arrays. Keep the query key centralized and preserve existing cache behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/api/hooks.ts`:
- Around line 139-144: Update useProviderHealth to set gcTime to
PROVIDER_HEALTH_REFRESH_MS alongside staleTime, keeping provider-health data
cached for the full hourly interval. Add or update the remount test for
useProviderHealth to verify an inactive query remains cached and does not
trigger another provider-health probe within that interval.
---
Nitpick comments:
In `@web/src/pages/ProvidersPage.test.tsx`:
- Around line 396-412: Update the test around the “does not automatically
re-check all providers within an hour” case to seed provider-health data just
under the shared one-hour interval, using fake timers. Advance the timer beyond
that interval, remount or trigger the query lifecycle, and assert that
healthRequestCount increases, while preserving the existing no-refresh assertion
before the boundary.
- Line 405: Define and export a module-level provider-health query-key constant
in the API hooks module, then update the provider-health hook, mutation, and
ProvidersPage test to reuse it instead of inline ["provider-health"] arrays.
Keep the query key centralized and preserve existing cache behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 496d8b98-8612-46b9-a6ea-c471758ce377
📒 Files selected for processing (5)
src/gateway/static/dashboard/assets/index-DnD03gwo.jssrc/gateway/static/dashboard/index.htmlweb/src/api/hooks.tsweb/src/pages/OverviewPage.tsxweb/src/pages/ProvidersPage.test.tsx
| export function useProviderHealth() { | ||
| return useQuery({ | ||
| queryKey: [PROVIDER_HEALTH], | ||
| queryFn: () => apiFetch<ProviderHealthResponse>("/v1/providers/health"), | ||
| // A provider's reachability does not move second to second; keep it briefly | ||
| // fresh so navigating back does not re-dial every provider. | ||
| staleTime: 30_000, | ||
| refetchInterval, | ||
| staleTime: PROVIDER_HEALTH_REFRESH_MS, | ||
| refetchInterval: PROVIDER_HEALTH_REFRESH_MS, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n -C 3 'new QueryClient|gcTime|cacheTime' web package.jsonRepository: mozilla-ai/otari
Length of output: 8683
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== web/src/provider.tsx =="
cat -n web/src/provider.tsx | sed -n '1,120p'
echo
echo "== web/src/api/hooks.ts (health query area) =="
cat -n web/src/api/hooks.ts | sed -n '120,170p'
echo
echo "== ProvidersPage test around hourly re-check =="
cat -n web/src/pages/ProvidersPage.test.tsx | sed -n '360,460p'
echo
echo "== TanStack Query dependency version =="
rg -n '"(`@tanstack/react-query`|`@tanstack/query-core`)"' web/package.json web/pnpm-lock.yaml web/package-lock.json web/yarn.lock 2>/dev/null || trueRepository: mozilla-ai/otari
Length of output: 8417
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== web/src/provider.tsx =="
sed -n '1,80p' web/src/provider.tsx
echo
echo "== web/src/pages/ProvidersPage.test.tsx around the hourly re-check test =="
sed -n '380,450p' web/src/pages/ProvidersPage.test.tsx
echo
echo "== any direct use of useProviderHealth =="
rg -n "useProviderHealth|PROVIDER_HEALTH_REFRESH_MS" web/src -g '!**/*.map'Repository: mozilla-ai/otari
Length of output: 4605
Keep provider-health cached for the full hour in web/src/api/hooks.ts.
staleTime only controls freshness; inactive queries can still be garbage-collected before the hourly window ends, so a remount can trigger another all-provider probe. Add gcTime: PROVIDER_HEALTH_REFRESH_MS and a remount test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/api/hooks.ts` around lines 139 - 144, Update useProviderHealth to set
gcTime to PROVIDER_HEALTH_REFRESH_MS alongside staleTime, keeping
provider-health data cached for the full hourly interval. Add or update the
remount test for useProviderHealth to verify an inactive query remains cached
and does not trigger another provider-health probe within that interval.
khaledosman
left a comment
khaledosman
left a comment
There was a problem hiding this comment.
Reviewed the change against CONTRIBUTING.md and the dashboard frontend standards. LGTM — correct, clean, and well-scoped.
Traced the concerns and they hold:
- Manual re-check preserved —
useRecheckProviderHealthstill hits?refresh=true+setQueryData. - No freshness regression on credential changes —
invalidateProviderViewsinvalidatesPROVIDER_HEALTH, andinvalidateQueriesforces an immediate refetch on active queries regardless ofstaleTime, so add/edit/delete still re-checks right away. The 1hstaleTimeonly suppresses passive navigation refetches, which is the intended fix for #358. - No focus-refetch interaction — global default is
refetchOnWindowFocus: false. - Both callers handled — Overview dropped its arg; Providers gains a harmless hourly background poll. No other callers.
One optional test nit inline; not blocking.
Review created by Claude Code.
|
|
||
| first.unmount(); | ||
| client.setQueryData(["provider-health"], healthResponse([{ instance: "openai", ok: true, model_count: 3, error: null, checked_at: null }]), { | ||
| updatedAt: Date.now() - 31_000, |
There was a problem hiding this comment.
Optional: 31_000 (31s) encodes the previous 30s staleTime as a magic number — now that the threshold is 1h it reads as arbitrary. Also, await waitFor(() => expect(healthRequestCount(...)).toBe(1)) resolves on the first passing tick, so it is a weak guard against a delayed refetch. Consider a comment tying the value to "older than any plausible fresh window" and/or asserting the count holds after a short flush. Not blocking.
Comment created by Claude Code.
GitHub Actions
Inactive
Description
Limits automatic dashboard provider-health checks to once per hour, preventing the Providers page and Overview from repeatedly fanning out model-list requests to every configured provider. The manual Re-check all action still forces an immediate live check.
PR Type
Relevant issues
Fixes #358
Checklist
tests/unit,tests/integration).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).AI Usage
AI Model/Tool used:
Codex
Any additional AI details you'd like to share:
Implemented the focused dashboard caching change, added a Vitest regression test, rebuilt the committed dashboard bundle, and ran the local checks listed above.
NOTE:
When responding to reviewer questions, please respond yourself rather than copy/pasting reviewer comments into an AI and pasting back its answer. We want to discuss with you, not your AI :)
Summary
Validation