Draft
Conversation
Use full unit names (KB, MB, GB) with proper spacing and show used/total with percentages in the memory graph header and stats. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughMemory formatting was changed to use two-letter unit labels with a space (e.g., "12.3 MB"); MemoryGraph now shows used/total as bytes plus percentage and free bytes with computed free percentage. A blinking LIVE indicator and new AlertBar UI were added, plus related CSS. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use your project's `biome` configuration to improve the quality of JS/TS/CSS/JSON code reviews.Add a configuration file to your project to customize how CodeRabbit runs |
Adds a green dot that flashes on/off every second with a "LIVE" label, giving a visual heartbeat indicator in the bottom status bar. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
New AlertBar component sits above the status bar and displays system health alerts. It flashes when CPU or memory usage exceeds thresholds (70% warn, 90% critical) and shows "All systems nominal" when healthy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/App.css (1)
453-467: Add a reduced-motion fallback for alert flashing.The flash effect should be disabled for users with reduced-motion preferences.
♿ Suggested CSS tweak
.alert-bar.alert-flash-off { opacity: 0.4; } + +@media (prefers-reduced-motion: reduce) { + .alert-bar { + transition: none; + } + .alert-bar.alert-flash-off { + opacity: 1; + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.css` around lines 453 - 467, Add a reduced-motion fallback: detect prefers-reduced-motion and disable the alert flashing by removing/overriding the transition and any animation on .alert-bar (and ensure .alert-bar.alert-flash-off still sets the final opacity) so users with reduced-motion see the non-animated state; update the CSS to add a `@media` (prefers-reduced-motion: reduce) rule that sets .alert-bar { transition: none; animation: none; } and preserves .alert-bar.alert-flash-off { opacity: 0.4; } so the flash is effectively disabled for those users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Line 75: metrics.cpuUsage is an array of CpuUsage objects but AlertBar (and
its getAlerts) expects number[]; update the prop passed to AlertBar so you
convert metrics.cpuUsage into an array of numeric percentages (e.g., map
CpuUsage objects to their percent/value field) before passing it in, or change
the producer to expose a cpuPercent array; specifically modify the usage around
AlertBar to pass a number[] derived from metrics.cpuUsage (refer to AlertBar and
metrics.cpuUsage) so .reduce in getAlerts operates on numbers.
In `@src/components/AlertBar.tsx`:
- Around line 8-11: The avgCpu calculation in getAlerts uses
cpuUsage.reduce(...) / cpuUsage.length which yields NaN for an empty cpuUsage
array; update getAlerts to handle empty samples before averaging by checking
cpuUsage.length === 0 and either early-return appropriate alerts (or set avgCpu
= 0) so downstream CPU alert logic uses a safe numeric value; locate the
getAlerts function and adjust the avgCpu computation/flow to guard against empty
cpuUsage arrays (reference: getAlerts, avgCpu, cpuUsage).
---
Nitpick comments:
In `@src/App.css`:
- Around line 453-467: Add a reduced-motion fallback: detect
prefers-reduced-motion and disable the alert flashing by removing/overriding the
transition and any animation on .alert-bar (and ensure
.alert-bar.alert-flash-off still sets the final opacity) so users with
reduced-motion see the non-animated state; update the CSS to add a `@media`
(prefers-reduced-motion: reduce) rule that sets .alert-bar { transition: none;
animation: none; } and preserves .alert-bar.alert-flash-off { opacity: 0.4; } so
the flash is effectively disabled for those users.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c2a3c2b3-9962-4a1f-8a5b-18debf3012c5
📒 Files selected for processing (3)
src/App.csssrc/App.tsxsrc/components/AlertBar.tsx
📜 Review details
🔇 Additional comments (1)
src/components/AlertBar.tsx (1)
36-42: Effect cleanup is implemented correctly.The interval is cleared on dependency changes/unmount, which prevents timer leaks.
|
|
||
| <EnvironmentPanel filter={filter} /> | ||
|
|
||
| <AlertBar cpuUsage={metrics.cpuUsage} memPercent={metrics.memPercent} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the cpuUsage type contract across call site and component props.
rg -n -C3 'interface AlertBarProps|cpuUsage:' src/components/AlertBar.tsx
rg -n -C3 'interface SystemMetrics|cpuUsage:' src/types.ts
rg -n -C3 '<AlertBar|cpuUsage=\{metrics\.cpuUsage' src/App.tsxRepository: coderabbit-demo/btop
Length of output: 1174
Pass numeric CPU percentages into AlertBar.
At line 75, metrics.cpuUsage is CpuUsage[] but AlertBar expects number[]. The component's getAlerts function will fail when it tries to compute the average with .reduce((a, b) => a + b, 0).
Fix
- <AlertBar cpuUsage={metrics.cpuUsage} memPercent={metrics.memPercent} />
+ <AlertBar cpuUsage={metrics.cpuUsage.map((core) => core.usage)} memPercent={metrics.memPercent} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <AlertBar cpuUsage={metrics.cpuUsage} memPercent={metrics.memPercent} /> | |
| <AlertBar cpuUsage={metrics.cpuUsage.map((core) => core.usage)} memPercent={metrics.memPercent} /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.tsx` at line 75, metrics.cpuUsage is an array of CpuUsage objects but
AlertBar (and its getAlerts) expects number[]; update the prop passed to
AlertBar so you convert metrics.cpuUsage into an array of numeric percentages
(e.g., map CpuUsage objects to their percent/value field) before passing it in,
or change the producer to expose a cpuPercent array; specifically modify the
usage around AlertBar to pass a number[] derived from metrics.cpuUsage (refer to
AlertBar and metrics.cpuUsage) so .reduce in getAlerts operates on numbers.
Comment on lines
+8
to
+11
| function getAlerts(cpuUsage: number[], memPercent: number): { message: string; level: 'info' | 'warn' | 'critical' }[] { | ||
| const alerts: { message: string; level: 'info' | 'warn' | 'critical' }[] = []; | ||
| const avgCpu = cpuUsage.reduce((a, b) => a + b, 0) / cpuUsage.length; | ||
|
|
There was a problem hiding this comment.
Handle empty CPU samples before averaging.
At Line 10, dividing by cpuUsage.length can produce NaN when the array is empty, which suppresses CPU alerting logic.
💡 Proposed fix
- const avgCpu = cpuUsage.reduce((a, b) => a + b, 0) / cpuUsage.length;
+ const avgCpu =
+ cpuUsage.length > 0
+ ? cpuUsage.reduce((a, b) => a + b, 0) / cpuUsage.length
+ : 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/AlertBar.tsx` around lines 8 - 11, The avgCpu calculation in
getAlerts uses cpuUsage.reduce(...) / cpuUsage.length which yields NaN for an
empty cpuUsage array; update getAlerts to handle empty samples before averaging
by checking cpuUsage.length === 0 and either early-return appropriate alerts (or
set avgCpu = 0) so downstream CPU alert logic uses a safe numeric value; locate
the getAlerts function and adjust the avgCpu computation/flow to guard against
empty cpuUsage arrays (reference: getAlerts, avgCpu, cpuUsage).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
formatBytesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI / Style