[dashboard, docs] feat: make telemetry refresh opt-in - #184
Conversation
|
Warning Review limit reached
Next review available in: 40 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReplaces the dashboard's fixed 3-second polling loop with a manual "Refresh Now" action and an explicit "Auto refresh" toggle. A new ChangesDashboard Refresh Controls
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements low-power dashboard refresh controls, changing the telemetry refresh to manual by default and adding an opt-in auto-refresh toggle that polls every 10 seconds only when the browser tab is visible. It also collapses overlapping refresh requests and updates relevant documentation, tests, and static assets. The review feedback suggests safely handling caught errors in the refresh function to prevent potential runtime exceptions when accessing properties on non-Error objects.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } catch (error) { | ||
| setMessage(`Refresh warning: ${error.message}`) | ||
| } finally { |
There was a problem hiding this comment.
If error is not an instance of Error (for example, if it is a string, a plain object, or null/undefined), accessing error.message can result in undefined or throw a TypeError which would crash the catch block. It is safer to use optional chaining and a fallback, such as error?.message || String(error).
| } catch (error) { | |
| setMessage(`Refresh warning: ${error.message}`) | |
| } finally { | |
| } catch (error) { | |
| setMessage(`Refresh warning: ${error?.message || String(error)}`) | |
| } finally { |
There was a problem hiding this comment.
Fixed in 8470e5d by routing refresh failures through formatRefreshWarningMessage(), with coverage for Error, string, and null rejections in web/dashboard/src/lib/refresh.test.js. The dashboard static bundle was rebuilt after the source change.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
README.md (1)
47-49: 📐 Maintainability & Code Quality | 🔵 TrivialConsider hyphenating "auto-refresh" consistently.
The static analysis hint flags grammar at line 183. Across docs, "auto refresh" appears both hyphenated and unhyphenated. Consider standardizing on "auto-refresh" when used as a compound noun/modifier for consistency with
docs/guides/cli.mdanddocs/guides/mcp.md.Current occurrences:
- Line 48: "opt-in auto refresh" (unhyphenated)
- Line 183: "opt-in auto refresh" (unhyphenated)
docs/guides/cli.mdline 128: "Auto refresh" (UI label, unhyphenated)docs/guides/mcp.mdline 225-226: "Auto refresh" toggle / "10-second polling" (mixed)The UI label "Auto refresh" can remain unhyphenated as a proper label, but descriptive prose reads more consistently as "auto-refresh."
Also applies to: 182-183
🤖 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 `@README.md` around lines 47 - 49, Standardize the prose wording for the telemetry refresh docs by changing descriptive uses of “auto refresh” to “auto-refresh” while leaving the actual UI label “Auto refresh” unchanged. Update the affected copy in the README and any matching prose in the related docs so the phrasing is consistent with the style used in `docs/guides/cli.md` and `docs/guides/mcp.md`, using the existing telemetry refresh text as the locator.Source: Linters/SAST tools
web/dashboard/src/App.test.jsx (1)
30-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the refresh effects, not just the initial HTML.
This only proves the controls render. It still passes if polling starts again by default or if hidden-tab pausing regresses, because the interval and visibility effects never run here. Please add a mounted test that verifies: no polling before opt-in, polling begins after checking Auto refresh, and polling stops when
document.visibilityStatebecomes"hidden". Based on learnings, "Add tests when there is an existing test pattern; do not introduce a brand-new testing framework unless requested."🤖 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/dashboard/src/App.test.jsx` around lines 30 - 46, The current App test only checks static markup, so it misses the refresh polling and visibility behavior in App’s mounted effects. Add or extend a test around App that mounts it and exercises the auto-refresh logic: verify no polling happens before opting in, polling starts after enabling the “Auto refresh” checkbox, and polling stops when document.visibilityState changes to "hidden". Reuse the existing App test setup/pattern in App.test.jsx and target the refresh controls/effects rather than only renderToStaticMarkup.Source: Learnings
src/keep_gpu/mcp/static/assets/index.css (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStop linting the rebuilt CSS bundle directly.
Line 1 is generated output, and Stylelint is now flagging Tailwind-emitted font-family/vendor-prefix rules here. Fixing this file by hand will be overwritten on the next dashboard rebuild. Please lint the source stylesheet or exclude
src/keep_gpu/mcp/static/assets/index.cssfrom Stylelint so rebuilt assets stay reproducible.🤖 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 `@src/keep_gpu/mcp/static/assets/index.css` at line 1, The generated CSS bundle in index.css should not be linted or edited directly because it is rebuilt output and Stylelint is catching Tailwind-generated declarations here. Update the linting setup to target the source stylesheet instead, or exclude src/keep_gpu/mcp/static/assets/index.css from Stylelint, so the asset remains reproducible after dashboard rebuilds. Use the bundled asset entrypoint and the lint configuration as the places to adjust.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In `@README.md`:
- Around line 47-49: Standardize the prose wording for the telemetry refresh
docs by changing descriptive uses of “auto refresh” to “auto-refresh” while
leaving the actual UI label “Auto refresh” unchanged. Update the affected copy
in the README and any matching prose in the related docs so the phrasing is
consistent with the style used in `docs/guides/cli.md` and `docs/guides/mcp.md`,
using the existing telemetry refresh text as the locator.
In `@src/keep_gpu/mcp/static/assets/index.css`:
- Line 1: The generated CSS bundle in index.css should not be linted or edited
directly because it is rebuilt output and Stylelint is catching
Tailwind-generated declarations here. Update the linting setup to target the
source stylesheet instead, or exclude src/keep_gpu/mcp/static/assets/index.css
from Stylelint, so the asset remains reproducible after dashboard rebuilds. Use
the bundled asset entrypoint and the lint configuration as the places to adjust.
In `@web/dashboard/src/App.test.jsx`:
- Around line 30-46: The current App test only checks static markup, so it
misses the refresh polling and visibility behavior in App’s mounted effects. Add
or extend a test around App that mounts it and exercises the auto-refresh logic:
verify no polling happens before opting in, polling starts after enabling the
“Auto refresh” checkbox, and polling stops when document.visibilityState changes
to "hidden". Reuse the existing App test setup/pattern in App.test.jsx and
target the refresh controls/effects rather than only renderToStaticMarkup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e25d69a3-ffee-4361-ad41-73c75e54edf0
📒 Files selected for processing (11)
AGENTS.mdREADME.mddocs/guides/cli.mddocs/guides/mcp.mddocs/plans/dashboard-refresh-controls.mdsrc/keep_gpu/mcp/static/assets/dashboard.jssrc/keep_gpu/mcp/static/assets/index.cssweb/dashboard/src/App.jsxweb/dashboard/src/App.test.jsxweb/dashboard/src/lib/refresh.jsweb/dashboard/src/lib/refresh.test.js
|
CodeRabbit nitpick follow-up:
|
1 participant
Summary
Test Plan
Local Review
Summary by CodeRabbit