refactor(core): Phase 5 — MonitoringState machine and lifecycle decoupling (#1408)#1420
Merged
Merged
Conversation
…pling Closes #1408. Introduce `core::monitoring::MonitoringState` (Running / Paused / Stopped) and move the close/quit decision out of the inline `on_window_event` handler into `src-tauri/src/lifecycle.rs`. The process lifetime is no longer tied to the main window's `CloseRequested` event — that path is reached only through an explicit Quit action, which the lifecycle module owns. Default user-visible behavior is unchanged: closing the window still quits the app. The new "hide window, keep monitoring" path is gated by the env var `HARDVIZ_CLOSE_TO_BACKGROUND=1`. The persisted setting and tray UX that flip this default ship with #1275 / Phase 6. Layout - `core/src/monitoring/state.rs` — `MonitoringState` with explicit `pause` / `resume` / `stop` transitions. Same-state and post- `Stopped` transitions are rejected with `InvalidTransition`. Pure Rust, runnable via `cargo test -p hwviz-core` without a Tauri runtime; covers the full transition grid. - `src-tauri/src/lifecycle.rs` — `on_close_requested`, `handle_close_request`, `request_quit`, plus `should_close_to_background`. The close handler dispatches based on the env var; `request_quit` transitions the state to `Stopped`, drains workers, then `app.exit(0)`. - `src-tauri/src/workers/mod.rs` — `WorkersState` gains a `monitoring_state: Mutex<MonitoringState>` field, colocated with the worker handles so the lifecycle module locks once on quit. - `src-tauri/src/commands/system.rs` — new `quit_app` Tauri command delegates to `lifecycle::request_quit`. Reachable from the generated bindings; Phase 6's tray will be the first user-facing caller. Persistence flush on Quit - `ArchiveController` now writes a final summary on shutdown when there are unwritten samples (tracked via a `dirty` flag on `ArchiveTracker`). Without this, samples accumulated since the last 60-second tick were lost on Quit. Satisfies the Phase 5 DoD "DB writes flushed". Out of scope (per #1408): tray UI (Phase 6); the user-facing "close to tray" setting and first-run dialog (#1275). The Run / Pause / Resume runtime hookup and frontend rendering pause when hidden are both #1275 territory — Phase 5 only ships the state machine and the quit path.
Contributor
📝 WalkthroughWalkthroughThis PR introduces a Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend
participant Tauri App
participant Lifecycle
participant Workers
participant Core
User->>Frontend: Close window
Frontend->>Tauri App: CloseRequested event
Tauri App->>Lifecycle: on_close_requested(window)
alt HARDVIZ_CLOSE_TO_BACKGROUND enabled
Lifecycle->>Frontend: Hide window
Note over Workers,Core: Background monitoring continues
else Default: Clean shutdown
Lifecycle->>Core: Stop MonitoringState
Lifecycle->>Workers: terminate_all()
Workers->>Core: Flush persistence
Workers->>Tauri App: app.exit(0)
end
User->>Frontend: Click quit button
Frontend->>Tauri App: quit_app command
Tauri App->>Lifecycle: request_quit(app_handle)
Lifecycle->>Core: Stop MonitoringState
Lifecycle->>Workers: terminate_all()
Workers->>Core: Flush persistence
Workers->>Tauri App: app.exit(0)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
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)
Review rate limit: 3/5 reviews remaining, refill in 15 minutes and 54 seconds. Comment |
Contributor
Coverage Report
File CoverageNo changed files found. |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements Phase 5 of the Core/App split by introducing a Tauri-independent MonitoringState state machine in hwviz-core, decoupling app process lifetime from the main window close event via a new lifecycle module, and adding an explicit “Quit” command that performs an ordered shutdown (including a final archive flush).
Changes:
- Add
hwviz_core::monitoring::MonitoringStatewith explicit transitions and unit tests. - Move window close handling into
src-tauri/src/lifecycle.rs, adding aquit_appcommand for explicit shutdown and gating “close-to-background” viaHARDVIZ_CLOSE_TO_BACKGROUND. - Update persistence archive worker to perform a shutdown-time final write when new samples arrived since the last tick.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/rspc/bindings.ts | Regenerated tauri-specta bindings to include quitApp(). |
| src-tauri/src/workers/mod.rs | Adds monitoring_state: Mutex<MonitoringState> to WorkersState for lifecycle-owned transitions. |
| src-tauri/src/lifecycle.rs | New lifecycle module handling CloseRequested policy and explicit quit ordering. |
| src-tauri/src/lib.rs | Wires lifecycle close handler and registers the new quit_app command. |
| src-tauri/src/commands/system.rs | Adds quit_app Tauri command delegating to lifecycle::request_quit. |
| core/src/persistence/archive.rs | Adds dirty tracking and shutdown-time final archive write path. |
| core/src/monitoring/state.rs | Implements MonitoringState + InvalidTransition with exhaustive tests. |
| core/src/monitoring/mod.rs | Exposes the monitoring module and re-exports state types. |
| core/src/lib.rs | Switches monitoring from a placeholder module to a real module. |
Contributor
Rust Backend Coverage ReportCoverage Details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/persistence/archive.rs (1)
318-355:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
dirtyset until the archive write succeeds.Clearing the flag at the start means a partial DB failure logs an error but still looks “clean,” so the shutdown flush at lines 118-125 is skipped and the last batch can be lost.
🔧 Suggested fix
async fn write_archive(&mut self) { use crate::infrastructure::database; use crate::log_error; - self.dirty = false; + let mut write_succeeded = true; let cpu = StatsCalculator::compute_stats(self.cpu_history.iter().copied()); let memory = StatsCalculator::compute_stats(self.memory_history.iter().copied()); if let Err(e) = database::hardware_archive::insert(cpu, memory).await { + write_succeeded = false; log_error!( "Failed to insert hardware archive data", "persistence::archive::write_archive", Some(e.to_string()) ); } @@ for gpu in self.collect_gpu_data() { if let Err(e) = database::gpu_archive::insert(gpu).await { + write_succeeded = false; log_error!( "Failed to insert GPU hardware archive data", "persistence::archive::write_archive", Some(e.to_string()) ); @@ if !process_stats.is_empty() && let Err(e) = database::process_stats::insert(process_stats).await { + write_succeeded = false; log_error!( "Failed to insert process stats data", "persistence::archive::write_archive", Some(e.to_string()) ); } + + if write_succeeded { + self.dirty = false; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/persistence/archive.rs` around lines 318 - 355, The method write_archive clears self.dirty immediately, causing partial DB failures to leave state marked clean; change it so self.dirty remains true until all database inserts succeed: move clearing of self.dirty to after all insert operations complete successfully (e.g., compute cpu/memory stats, call database::hardware_archive::insert, loop over collect_gpu_data() calling database::gpu_archive::insert, and call database::process_stats::insert for collect_process_stats()), and if any insert returns Err set (or keep) self.dirty = true and log the error; only set self.dirty = false when every insert returned Ok (or track a success boolean and set false at the end when success is true).
🧹 Nitpick comments (1)
src-tauri/src/lifecycle.rs (1)
80-87: Centralize shutdown state transition to avoid lifecycle inconsistency.Two callers invoke
terminate_all()with inconsistent state management:request_quit()transitionsmonitoring_statetoStoppedbefore draining workers (lifecycle.rs:85), butrestart_app()in system_service.rs drains workers without this transition. Whilemonitoring_stateisn't currently read elsewhere, consolidating both the state transition and worker drain into a single helper onWorkersStateprevents future code from observing stale state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/lifecycle.rs` around lines 80 - 87, Centralize the shutdown transition by adding a new helper on WorkersState (e.g., stop_and_terminate_all or shutdown) that performs the monitoring_state stop() transition under the existing mutex and then awaits terminate_all(); replace the direct calls in request_quit() (which currently does ws.monitoring_state.lock().unwrap().stop(); ws.terminate_all().await) and in restart_app() so both call the new WorkersState helper, ensuring the state transition and worker drain are performed atomically through the WorkersState API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@core/src/persistence/archive.rs`:
- Around line 318-355: The method write_archive clears self.dirty immediately,
causing partial DB failures to leave state marked clean; change it so self.dirty
remains true until all database inserts succeed: move clearing of self.dirty to
after all insert operations complete successfully (e.g., compute cpu/memory
stats, call database::hardware_archive::insert, loop over collect_gpu_data()
calling database::gpu_archive::insert, and call database::process_stats::insert
for collect_process_stats()), and if any insert returns Err set (or keep)
self.dirty = true and log the error; only set self.dirty = false when every
insert returned Ok (or track a success boolean and set false at the end when
success is true).
---
Nitpick comments:
In `@src-tauri/src/lifecycle.rs`:
- Around line 80-87: Centralize the shutdown transition by adding a new helper
on WorkersState (e.g., stop_and_terminate_all or shutdown) that performs the
monitoring_state stop() transition under the existing mutex and then awaits
terminate_all(); replace the direct calls in request_quit() (which currently
does ws.monitoring_state.lock().unwrap().stop(); ws.terminate_all().await) and
in restart_app() so both call the new WorkersState helper, ensuring the state
transition and worker drain are performed atomically through the WorkersState
API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 2260a20f-df69-4c82-9efc-033e857155a4
📒 Files selected for processing (9)
core/src/lib.rscore/src/monitoring/mod.rscore/src/monitoring/state.rscore/src/persistence/archive.rssrc-tauri/src/commands/system.rssrc-tauri/src/lib.rssrc-tauri/src/lifecycle.rssrc-tauri/src/workers/mod.rssrc/rspc/bindings.ts
9 tasks
This was referenced Apr 30, 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.
Closes #1408. Part of #1402 (Epic).
Summary
core::monitoring::MonitoringState(Running / Paused / Stopped) with explicit transitions and exhaustive unit tests.on_window_eventhandler intosrc-tauri/src/lifecycle.rs. The process lifetime is no longer tied to the main window'sCloseRequestedevent.quit_appTauri command.lifecycle::request_quittransitions the state machine toStopped, drains workers, thenapp.exit(0).ArchiveControllerwrites a final summary on shutdown so samples accumulated since the last 60-second tick are not lost on Quit (DoD "DB writes flushed").User-visible behavior
Unchanged in this phase. Closing the window still quits the app by default. The new "hide window, keep monitoring" path is gated by the env var
HARDVIZ_CLOSE_TO_BACKGROUND=1. The persisted setting + tray UX that flip the default ship with #1275 / Phase 6.Path note
The #1408 issue body says `src-tauri/src/app/lifecycle.rs`, but #1402's 2026-04-30 changelog supersedes it ("`app::lifecycle` → `lifecycle`"). Following the parent epic — `src-tauri/src/lifecycle.rs` directly. `src-tauri/src/app/startup.rs` (DB startup error dialog) is unrelated and stays as-is.
Layout
core/src/monitoring/state.rs—MonitoringStatewithpause/resume/stop. Same-state transitions and any transition out ofStoppedare rejected withInvalidTransition. Pure Rust, no Tauri runtime needed.src-tauri/src/lifecycle.rs—on_close_requested,handle_close_request,request_quit,should_close_to_background.src-tauri/src/workers/mod.rs—WorkersStategains amonitoring_state: Mutex<MonitoringState>field, colocated with the worker handles so the lifecycle module locks once on quit.src-tauri/src/commands/system.rs—quit_appcommand delegates tolifecycle::request_quit. Reachable via the generated bindings; Phase 6's tray will be the first user-facing caller.core/src/persistence/archive.rs—ArchiveTrackergains adirtyflag; the spawn loop runs a finalwrite_archiveon shutdown signal when dirty.Tests
InvalidTransitionDisplayand per-method coverage. Runnable viacargo test -p hwviz-corewithout a Tauri runtime.lifecycle::is_truthyparser tests (no env-var mutation, parallel-safe).ArchiveTrackertests (fresh_tracker_is_not_dirty,ingest_marks_tracker_dirty).Out of scope (per #1408)
Pausedto actually halt sensor emission, and stopping frontend render work when the window is hidden — also feat: add background monitoring mode with system tray support #1275.Test plan
cargo tauri-fmt -- --checkcargo tauri-lintcargo test --workspace --lib --bins -- --test-threads=1(theadl_diagnosticexample fails to build on macOS; this is pre-existing ondevelopand unrelated)npm run lintnpm run testquitAppadded).HARDVIZ_CLOSE_TO_BACKGROUND=1, close the window, verify the process keeps running and the collector keeps polling. Callingcommands.quitApp()from devtools must terminate cleanly with a final archive row written.Summary by CodeRabbit
New Features
Chores