feat(backend): redesign HardwareMonitorUpdate for multi-GPU (#1298)#1332
Merged
Conversation
- Add GpuMonitorData struct with gpu_id, gpu_name, and per-GPU metrics - Replace 6 flat GPU fields in HardwareMonitorUpdate with gpus: Vec<GpuMonitorData> - Add gpu_id field to GpuSample with platform-specific conventions: - Windows NVAPI: nvapi:<name>, ADL: pci:<bus>:<device>:<function>, PDH: pdh:<name> - Linux: pci:<bdf> or drm:card<id> - macOS: iokit:<name> - Key gpu_usage_histories/gpu_temperature_histories by gpu_id instead of name - Fix gpu_usage_history to read from per-GPU HashMap (accepts gpu_id param) - Remove dead gpu_history field from HardwareMonitorState - Extract build_gpu_payloads() pure function from emit_hardware_update() - Remove GpuCapabilities struct (Option fields communicate availability) - Apply temperature unit conversion per-GPU - Add gpu_id column to GPU_DATA_ARCHIVE (migration v5) - Add gpu_name_map to MonitorResources for archive gpu_id→name lookup - Add 24 new tests (236→260), all passing
… into feat/redesign-gpu-update-event
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughImplements multi‑GPU support: adds per‑GPU IDs and Changes
Sequence DiagramsequenceDiagram
participant Drivers as Drivers (OS/Platform)
participant MS as MonitoringService
participant SM as SystemMonitor
participant AS as ArchiveService
participant DB as Database
participant UI as Frontend UI
Drivers->>MS: sample GPUs -> Vec<GpuSample { gpu_id, name, metrics }>
MS->>MS: update_gpu_histories(key=gpu_id) & update gpu_name_map
MS->>SM: deliver gpu samples
SM->>SM: build_gpu_payloads -> Vec<GpuMonitorData>
SM->>UI: emit_hardware_update(payload with gpus[])
UI->>UI: consume payload.gpus (select primary GPU for UI atoms)
SM->>AS: archive_current_snapshot(gpus keyed by gpu_id + gpu_name_map)
AS->>DB: insert GPU archive rows (include gpu_id)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Rust Backend Coverage ReportCoverage Details |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR redesigns the backend hardware monitoring payload and related history/archive plumbing to properly support multiple GPUs by introducing a per-GPU data structure and keying GPU histories by a stable-ish gpu_id rather than GPU name.
Changes:
- Introduces
GpuMonitorDataand updatesHardwareMonitorUpdateto emitgpus: Vec<GpuMonitorData>instead of flat single-GPU fields. - Adds
gpu_idtoGpuSample, rekeys GPU histories bygpu_id, and updates theget_gpu_usage_historycommand accordingly. - Extends GPU archive persistence with a new
gpu_idcolumn + insertsgpu_idand maintains agpu_id → gpu_namemap for archive naming.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src-tauri/src/workers/system_monitor.rs | Builds per-GPU payloads and emits multi-GPU HardwareMonitorUpdate; adds unit tests for payload building. |
| src-tauri/src/services/monitoring_service.rs | Adds gpu_id to GpuSample, rekeys histories to gpu_id, and updates GPU usage history lookup + tests. |
| src-tauri/src/services/archive_service.rs | Archives GPU metrics by gpu_id and uses a gpu_id → gpu_name map for naming. |
| src-tauri/src/models/hardware.rs | Adds GpuMonitorData, updates HardwareMonitorUpdate, removes dead single-GPU history field, adds serialization tests. |
| src-tauri/src/models/hardware_archive.rs | Adds gpu_id to archived GPU model and adds gpu_name_map to shared monitor resources. |
| src-tauri/src/lib.rs | Wires up the new gpu_name_map resource and removes the unused GPU history buffer initialization. |
| src-tauri/src/infrastructure/database/migration.rs | Adds migration v5 to add gpu_id column and introduces migration-related unit tests. |
| src-tauri/src/infrastructure/database/gpu_archive.rs | Inserts gpu_id into GPU_DATA_ARCHIVE. |
| src-tauri/src/commands/hardware.rs | Updates get_gpu_usage_history command signature to accept gpu_id. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/services/monitoring_service.rs (1)
119-127:⚠️ Potential issue | 🔴 CriticalUse
gpu.gpu_id()instead of GPU model name to ensure unique identifiers for identical NVIDIA GPUs.The code currently assigns
gpu_idasnvapi:{name}wherenameisgpu.full_name()(e.g., "NVIDIA GeForce RTX 4090"). This approach produces duplicate IDs for systems with multiple identical NVIDIA GPUs, causing their usage histories to be merged into a single entry.The
PhysicalGpureference has access togpu_id() -> Result<u32>, which returns NVAPI's unique per-GPU identifier. This method is already used elsewhere in the codebase (seenvapi_provider.rs:187inget_nvidia_gpu_info). Update line 120 to usegpu.gpu_id()instead:gpu_id: format!("nvapi:{}", gpu.gpu_id().unwrap_or(0))This ensures each physical GPU gets a unique identifier regardless of model name, maintaining separate history tracking for multi-GPU systems.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/services/monitoring_service.rs` around lines 119 - 127, The GpuSample currently uses the GPU model name for gpu_id which yields duplicate IDs for identical NVIDIA cards; update the GpuSample construction to use the NVAPI per-GPU identifier by calling gpu.gpu_id() (e.g., format!("nvapi:{}", gpu.gpu_id().unwrap_or(0))) so each PhysicalGpu gets a unique gpu_id; change the gpu_id field in the GpuSample creation (the block that builds GpuSample with name/usage/temperature/dedicated_memory_kb/cooler_level/source) to format the id from gpu.gpu_id() instead of using the model name.
🤖 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-tauri/src/commands/hardware.rs`:
- Around line 186-194: The command get_gpu_usage_history signature changed to
include gpu_id but its doc comment and the TypeScript binding were not updated;
add/update the function doc comment to document the new gpu_id parameter,
annotate the function with #[specta::specta] (or ensure it is included in the
specta export) so the new signature is picked up, then regenerate the TypeScript
bindings so the frontend typing reflects (gpu_id: string, seconds: number) and
update any frontend callers to pass the gpu_id argument accordingly.
In `@src-tauri/src/models/hardware.rs`:
- Around line 20-27: The frontend listener in useHardwareEventListener is still
destructuring old flat GPU fields (gpuUsage, gpuName, gpuTemperature, gpuSource,
gpuDedicatedMemoryUsageKb, gpuCoolerLevel) but the backend now emits
HardwareMonitorUpdate with gpus: Vec<GpuMonitorData>; update the event handler
to iterate/map over event.gpus, extract the required properties from each
GpuMonitorData object, and feed those mapped values into the existing
merge/history/fan-temperature logic instead of relying on the old flat
destructured variables; remove the old flat destructuring and replace usages
with the mapped per-GPU data so all downstream functions (history tracking,
temperature display, fan updates) receive the correct shape.
---
Outside diff comments:
In `@src-tauri/src/services/monitoring_service.rs`:
- Around line 119-127: The GpuSample currently uses the GPU model name for
gpu_id which yields duplicate IDs for identical NVIDIA cards; update the
GpuSample construction to use the NVAPI per-GPU identifier by calling
gpu.gpu_id() (e.g., format!("nvapi:{}", gpu.gpu_id().unwrap_or(0))) so each
PhysicalGpu gets a unique gpu_id; change the gpu_id field in the GpuSample
creation (the block that builds GpuSample with
name/usage/temperature/dedicated_memory_kb/cooler_level/source) to format the id
from gpu.gpu_id() instead of using the model name.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f11f9af5-a8bd-476c-b295-90d85db6e225
📒 Files selected for processing (9)
src-tauri/src/commands/hardware.rssrc-tauri/src/infrastructure/database/gpu_archive.rssrc-tauri/src/infrastructure/database/migration.rssrc-tauri/src/lib.rssrc-tauri/src/models/hardware.rssrc-tauri/src/models/hardware_archive.rssrc-tauri/src/services/archive_service.rssrc-tauri/src/services/monitoring_service.rssrc-tauri/src/workers/system_monitor.rs
gpu_id was derived from the GPU model name, which produces duplicate IDs when multiple identical GPUs are installed. Use gpu.gpu_id() to ensure each physical GPU gets a unique identifier.
Update useHardwareEventListener to handle the new gpus array in HardwareMonitorUpdate. Uses gpus[0] to maintain single-GPU display until full multi-GPU frontend support is implemented (#1299). Also includes regenerated TypeScript bindings for the updated backend types.
Update makePayload helper and all GPU-related test cases to use the new gpus array structure instead of flat GPU fields.
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
… in useHardwareEventListener tests
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/features/hardware/hooks/useHardwareEventListener.ts (1)
56-69: Consider handling emptygpusarray explicitly.The code handles the case when
gpus[0]is undefined via?? null, but whengpusis an empty array,gpuSourceis explicitly set tonull(line 69) while other GPU-related atoms likegpuHistory,gpuTemp,gpuDedicatedMemory, andgpuFanSpeedretain their previous values. This creates an inconsistent state where the source shows "no GPU" but stale metrics persist.Consider resetting all GPU atoms when no GPU data is available:
♻️ Suggested improvement
// TODO: Multi-GPU support (`#1299`) — for now use the first GPU const gpu = gpus[0] ?? null; setCpuHistory((prev) => padHistory([...prev, cpuUsage])); setMemoryHistory((prev) => padHistory([...prev, memoryUsage])); - if (gpu?.gpuUsage != null) { - setGpuHistory((prev) => padHistory([...prev, gpu.gpuUsage as number])); - } + if (gpu?.gpuUsage != null) { + setGpuHistory((prev) => padHistory([...prev, gpu.gpuUsage as number])); + } else if (!gpu) { + // No GPU available - could optionally clear history or push null + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/hardware/hooks/useHardwareEventListener.ts` around lines 56 - 69, The current handler only nulls gpuUsageSource when gpus is empty but leaves stale GPU atoms (gpuHistory, gpuTemp, gpuDedicatedMemory, gpuFanSpeed) unchanged; update the logic in useHardwareEventListener where gpus and gpu are derived so that when gpus.length === 0 (or gpu is null) you explicitly reset setGpuHistory to an empty/padded history, setGpuTemp to an empty array, setGpuDedicatedMemory to null/0 (matching your atom type), setGpuFanSpeed to null/0, and setGpuUsageSource to null so all GPU-related state is consistent; otherwise keep the existing per-field updates (setGpuHistory, setGpuTemp, setGpuDedicatedMemory, setGpuFanSpeed, setGpuUsageSource) when a gpu is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/features/hardware/hooks/useHardwareEventListener.ts`:
- Around line 56-69: The current handler only nulls gpuUsageSource when gpus is
empty but leaves stale GPU atoms (gpuHistory, gpuTemp, gpuDedicatedMemory,
gpuFanSpeed) unchanged; update the logic in useHardwareEventListener where gpus
and gpu are derived so that when gpus.length === 0 (or gpu is null) you
explicitly reset setGpuHistory to an empty/padded history, setGpuTemp to an
empty array, setGpuDedicatedMemory to null/0 (matching your atom type),
setGpuFanSpeed to null/0, and setGpuUsageSource to null so all GPU-related state
is consistent; otherwise keep the existing per-field updates (setGpuHistory,
setGpuTemp, setGpuDedicatedMemory, setGpuFanSpeed, setGpuUsageSource) when a gpu
is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 00f84af5-d5f0-4fdc-8fea-3d31739d5cce
📒 Files selected for processing (4)
src-tauri/src/commands/hardware.rssrc-tauri/src/services/monitoring_service.rssrc/features/hardware/hooks/useHardwareEventListener.tssrc/rspc/bindings.ts
This was referenced Apr 29, 2026
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
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
Related Issues
Close #1298
Type of Change
fix/branch)feat/branch)refactor/branch)docs/branch)chore/branch)Screenshots / Videos
Test Plan
Checklist
npm run lint && npm run format/cargo tauri-lint && cargo tauri-fmt)npm test/cargo tauri-test)Summary by CodeRabbit
New Features
Database
API
Tests