-
-
Notifications
You must be signed in to change notification settings - Fork 71
feat: add cpu temp as optional metric #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
… or logout or reisntall and login
WalkthroughThis PR adds CPU temperature monitoring to the dashboard system. It introduces a Temperature field to backend CPUStats that calculates average temperature from host sensors, creates frontend components to display temperature with color-coded gauges, adds skeleton loading states, and includes translations for five languages. Changes
Sequence DiagramsequenceDiagram
participant Host as Host System
participant Backend as Backend (Go)
participant Frontend as Frontend (React)
participant UI as UI Component
Host->>Backend: CPU sensor data available
Backend->>Backend: Calculate average CPU temperature<br/>from sensors in valid range
Backend->>Backend: Set cpuStats.Temperature
Frontend->>Backend: Fetch system metrics
Backend-->>Frontend: Return CPUStats with Temperature
Frontend->>Frontend: Call useSystemMetric('cpu.temperature')<br/>with fallback to DEFAULT_METRICS.cpu
Frontend->>Frontend: Determine status: Cool/Normal/Warm/Hot<br/>based on temperature range
Frontend->>UI: Render CPUTemperatureDisplay<br/>with TemperatureGauge
UI-->>Frontend: Display temperature value<br/>and color-coded gauge (0–100°C)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
view/app/dashboard/components/system/cpu-temperature.tsx (1)
56-83: Consider expanding temperature range for modern CPUs.The gauge uses a 0-100°C range (line 58), but modern high-performance CPUs can legitimately reach 100-110°C or higher under load. The current implementation will max out the gauge at 100°C, potentially masking critical temperature conditions.
Consider expanding the range to 0-120°C:
const TemperatureGauge: React.FC<TemperatureGaugeProps> = ({ temperature }) => { - // Calculate percentage based on 0-100°C range - const percentage = Math.min((temperature / 100) * 100, 100); + // Calculate percentage based on 0-120°C range + const percentage = Math.min((temperature / 120) * 100, 100); const gaugeColor = getTemperatureColor(temperature); return ( <div className="space-y-2"> <div className="flex justify-between text-xs text-muted-foreground"> <span>0°C</span> <span>50°C</span> - <span>100°C</span> + <span>120°C</span> </div>Also update the skeleton (view/app/dashboard/components/system/skeletons/cpu-temperature.tsx lines 21-23) to match.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
api/api/versions.json(1 hunks)api/internal/features/dashboard/system_stats.go(2 hunks)api/internal/features/dashboard/types.go(1 hunks)view/app/dashboard/components/system/cpu-temperature.tsx(1 hunks)view/app/dashboard/components/system/skeletons/cpu-temperature.tsx(1 hunks)view/app/dashboard/components/utils/constants.ts(1 hunks)view/app/dashboard/page.tsx(4 hunks)view/lib/i18n/locales/en.json(1 hunks)view/lib/i18n/locales/es.json(1 hunks)view/lib/i18n/locales/fr.json(1 hunks)view/lib/i18n/locales/kn.json(1 hunks)view/lib/i18n/locales/ml.json(1 hunks)view/redux/types/monitor.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
api/internal/features/dashboard/types.go (1)
view/redux/types/monitor.ts (1)
CPUCore(59-62)
api/internal/features/dashboard/system_stats.go (2)
api/internal/features/dashboard/types.go (1)
CPUCore(60-63)view/redux/types/monitor.ts (1)
CPUCore(59-62)
view/app/dashboard/components/system/cpu-temperature.tsx (6)
view/redux/types/monitor.ts (1)
SystemStatsType(92-105)view/components/ui/typography.tsx (2)
TypographyMuted(114-122)TypographySmall(99-112)view/app/dashboard/hooks/use-system-metric.ts (1)
useSystemMetric(21-41)view/app/dashboard/components/utils/constants.ts (1)
DEFAULT_METRICS(12-34)view/app/dashboard/components/system/system-metric-card.tsx (1)
SystemMetricCard(16-36)view/app/dashboard/components/system/skeletons/cpu-temperature.tsx (1)
CPUTemperatureCardSkeletonContent(11-33)
view/app/dashboard/components/system/skeletons/cpu-temperature.tsx (4)
view/components/ui/typography.tsx (1)
TypographyMuted(114-122)view/app/dashboard/hooks/use-system-metric.ts (1)
useSystemMetric(21-41)view/app/dashboard/components/utils/constants.ts (1)
DEFAULT_METRICS(12-34)view/app/dashboard/components/system/system-metric-card.tsx (1)
SystemMetricCard(16-36)
🔇 Additional comments (12)
view/app/dashboard/components/utils/constants.ts (1)
20-21: LGTM! Clean addition of temperature metric.The temperature field is properly initialized to 0 and typed consistently with other CPU metrics. The trailing comma on line 20 follows JavaScript/TypeScript best practices.
view/lib/i18n/locales/es.json (1)
796-798: LGTM! Spanish translations are accurate.The translations "Temperatura de CPU" and "Temperatura Actual" are grammatically correct and appropriately convey the intended meaning.
api/api/versions.json (1)
6-6: LGTM! Version timestamp updated.The release date update correctly reflects the new feature release.
api/internal/features/dashboard/types.go (1)
65-68: LGTM! Backend type definition is correct.The Temperature field is properly defined as a public float64 with the correct JSON tag. This aligns well with the frontend TypeScript interface where
temperature: numbermaps correctly to Go'sfloat64.view/lib/i18n/locales/kn.json (1)
793-795: LGTM! Kannada translations added.The translations are properly structured and follow the same key naming convention used across all other locales.
view/redux/types/monitor.ts (1)
67-67: LGTM! Frontend type definition matches backend.The
temperature: numberfield correctly mirrors the backend'sTemperature float64, ensuring type safety across the full stack.view/lib/i18n/locales/fr.json (1)
797-799: LGTM! French translations are accurate.The translations "Température du CPU" and "Température Actuelle" are grammatically correct with proper gender agreement and article usage.
view/lib/i18n/locales/ml.json (1)
812-814: LGTM! Malayalam translations added.The translations are properly structured and complete the i18n coverage for the new CPU temperature feature.
view/lib/i18n/locales/en.json (1)
813-815: LGTM! Translation keys properly added.The new CPU temperature translation keys follow the existing naming convention and are correctly integrated into the dashboard.cpu block. They align with the usage in the CPU temperature component.
api/internal/features/dashboard/system_stats.go (1)
168-170: LGTM! Proper initialization.The CPUStats struct is correctly initialized with Temperature set to 0.0 as a safe default value.
view/app/dashboard/components/system/cpu-temperature.tsx (1)
85-118: LGTM! Well-structured component with proper error handling.The component correctly handles the loading state with skeletons, gracefully displays when temperature sensors are unavailable, and provides clear visual feedback with color-coded temperature indicators.
view/app/dashboard/page.tsx (1)
9-9: LGTM! CPU temperature widget properly integrated.The new widget is correctly:
- Imported and wired into the dashboard
- Hidden by default (good UX for optional/platform-dependent metric)
- Registered in the widget selector for user opt-in
- Following the established pattern for dashboard widgets
Also applies to: 48-48, 93-93, 260-264
| // Get CPU temperature using host.SensorsTemperatures | ||
| if temps, err := host.SensorsTemperatures(); err == nil && len(temps) > 0 { | ||
| // Calculate average temperature from all sensors | ||
| var totalTemp float64 | ||
| var validReadings int | ||
|
|
||
| for _, temp := range temps { | ||
| // Filter for CPU-related sensors and valid temperature readings | ||
| if temp.Temperature > 0 && temp.Temperature < 150 { // Reasonable temperature range in Celsius | ||
| totalTemp += temp.Temperature | ||
| validReadings++ | ||
| } | ||
| } | ||
|
|
||
| if validReadings > 0 { | ||
| cpuStats.Temperature = totalTemp / float64(validReadings) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment is misleading; consider filtering by sensor name.
The comment on line 201 states "Filter for CPU-related sensors" but the code only filters by temperature range (0-150°C) and doesn't distinguish between CPU, GPU, disk, or other thermal sensors. This can result in an average that includes non-CPU temperatures, making the "CPU Temperature" metric inaccurate.
Consider filtering by sensor name/type to only include CPU sensors:
for _, temp := range temps {
- // Filter for CPU-related sensors and valid temperature readings
- if temp.Temperature > 0 && temp.Temperature < 150 { // Reasonable temperature range in Celsius
+ // Filter for CPU-related sensors by name and valid temperature readings
+ sensorName := strings.ToLower(temp.SensorKey)
+ isCPUSensor := strings.Contains(sensorName, "cpu") ||
+ strings.Contains(sensorName, "core") ||
+ strings.Contains(sensorName, "proc")
+ if isCPUSensor && temp.Temperature > 0 && temp.Temperature < 150 {
totalTemp += temp.Temperature
validReadings++
}
}You'll need to add "strings" to the imports if not already present.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api/internal/features/dashboard/system_stats.go around lines 194 to 211, the
code claims to "Filter for CPU-related sensors" but only filters by temperature
range, so non-CPU sensors may be included; update the loop to also filter sensor
names (e.g., check temp.SensorKey or temp.SensorKey/Label contains substrings
like "cpu", "core", "package" case-insensitively) before aggregating, and add
"strings" to the imports if not present; ensure the name check is
case-insensitive and still falls back to the temperature-range check so only
relevant CPU sensors are averaged.
| export function CPUTemperatureCardSkeletonContent() { | ||
| return ( | ||
| <div className="space-y-6"> | ||
| <div className="text-center"> | ||
| <TypographyMuted className="text-xs">Current Temperature</TypographyMuted> | ||
| <Skeleton className="h-12 w-32 mx-auto mt-1" /> | ||
| <Skeleton className="h-4 w-16 mx-auto mt-1" /> | ||
| </div> | ||
| <div className="space-y-2"> | ||
| <div className="flex justify-between text-xs text-muted-foreground"> | ||
| <span>0°C</span> | ||
| <span>50°C</span> | ||
| <span>100°C</span> | ||
| </div> | ||
| <Skeleton className="h-4 w-full rounded-full" /> | ||
| <div className="flex justify-between"> | ||
| <Skeleton className="h-3 w-16" /> | ||
| <Skeleton className="h-3 w-16" /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use translation key instead of hardcoded text.
Line 15 hardcodes "Current Temperature" while the actual component uses t('dashboard.cpu.current_temp'). This creates an inconsistency between the skeleton and loaded states.
Apply this diff:
-export function CPUTemperatureCardSkeletonContent() {
+export function CPUTemperatureCardSkeletonContent({ label }: { label: string }) {
return (
<div className="space-y-6">
<div className="text-center">
- <TypographyMuted className="text-xs">Current Temperature</TypographyMuted>
+ <TypographyMuted className="text-xs">{label}</TypographyMuted>
<Skeleton className="h-12 w-32 mx-auto mt-1" />Then pass the translated label from CPUTemperatureCardSkeleton:
return (
<SystemMetricCard
title={t('dashboard.cpu.temperature')}
icon={Thermometer}
isLoading={true}
- skeletonContent={<CPUTemperatureCardSkeletonContent />}
+ skeletonContent={<CPUTemperatureCardSkeletonContent label={t('dashboard.cpu.current_temp')} />}
>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In view/app/dashboard/components/system/skeletons/cpu-temperature.tsx around
lines 11 to 33, the skeleton hardcodes "Current Temperature"; change it to
accept a translated label prop (e.g., label: string) and render that instead of
the literal string, then update the component's callers
(CPUTemperatureCardSkeleton / parent) to pass t('dashboard.cpu.current_temp')
into the skeleton's label prop so the skeleton and loaded state use the same
translation key.

Issue
Link to related issue(s):
Description
Short summary of what this PR changes or introduces.
Scope of Change
Select all applicable areas impacted by this PR:
Screenshot / Video / GIF (if applicable)
Attach or embed screenshots, screen recordings, or GIFs demonstrating the feature or fix.
Related PRs (if any)
Link any related or dependent PRs across repos.
Additional Notes for Reviewers (optional)
Anything reviewers should know before testing or merging (e.g., environment variables, setup steps).
Developer Checklist
To be completed by the developer who raised the PR.
Reviewer Checklist
To be completed by the reviewer before merge.
Summary by CodeRabbit
New Features
Documentation