feat: warn user when disk space is running low by tylerkron · Pull Request #456 · daqifi/daqifi-desktop

tylerkron · 2026-04-04T16:18:33Z

Summary

Adds disk space monitoring during logging sessions with three protection tiers: pre-session warning at <500 MB, active warning at <100 MB, and automatic hard stop at <50 MB
Blocks logging from starting when disk space is critically low (<50 MB) to prevent system crashes
All warnings suggest corrective actions (deleting old logging sessions or freeing disk space)

Closes #448

Implementation

New DiskSpaceMonitor service (Daqifi.Desktop/DiskSpace/) with injectable free-space provider for testability
Monitors every 15 seconds during active logging via System.Threading.Timer
Warning events fire only once per session; critical events immediately pause the timer to prevent duplicates
Integrates into DaqifiViewModel.IsLogging setter for pre-session checks and start/stop lifecycle
22 unit tests covering threshold classification, event behavior, lifecycle, and edge cases

Test plan

Verify build succeeds and all tests pass in CI
Start a logging session with >500 MB free — no warning shown
Start a logging session with <500 MB free — warning dialog shown, logging proceeds
Start a logging session with <50 MB free — blocked with error dialog
During active logging, space drops below 100 MB — non-blocking warning shown once
During active logging, space drops below 50 MB — logging auto-stopped immediately with notification

🤖 Generated with Claude Code

Add disk space monitoring with three tiers of protection: - Pre-session check warns at <500 MB, blocks at <50 MB - Active monitoring warns at <100 MB (once per session) - Auto-stops logging at <50 MB with no user confirmation required Closes #448 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

qodo-code-review · 2026-04-04T16:18:53Z

Review Summary by Qodo

Add disk space monitoring with three-tier protection system

✨ Enhancement

Walkthroughs

Description

• Adds disk space monitoring service with three protection tiers
  - Pre-session check warns at <500 MB, blocks at <50 MB
  - Active monitoring warns at <100 MB (once per session)
  - Auto-stops logging at <50 MB without user confirmation
• Integrates monitoring into logging lifecycle via IsLogging setter
• Includes 22 comprehensive unit tests covering thresholds and edge cases
• Provides injectable free-space provider for testability

Diagram

flowchart LR
  A["User starts logging"] --> B["CheckPreLoggingSpace"]
  B --> C{Space level?}
  C -->|Critical| D["Block logging"]
  C -->|Warning| E["Show warning"]
  C -->|OK| F["Start monitoring"]
  F --> G["Monitor every 15s"]
  G --> H{Space drops?}
  H -->|Critical| I["Auto-stop logging"]
  H -->|Warning| J["Show warning once"]
  H -->|OK| G

File Changes

1. Daqifi.Desktop/DiskSpace/DiskSpaceLevel.cs ✨ Enhancement +27/-0

Disk space severity level enumeration
• Defines enum with four severity levels: Ok, PreSessionWarning, Warning, Critical
• Maps to thresholds: 500 MB (pre-session), 100 MB (active), 50 MB (hard stop)
Daqifi.Desktop/DiskSpace/DiskSpaceLevel.cs

2. Daqifi.Desktop/DiskSpace/DiskSpaceCheckResult.cs ✨ Enhancement +28/-0

Pre-logging disk space check result model
• Result object for pre-logging disk space checks
• Stores available bytes and computed megabytes property
• Includes disk space level classification
Daqifi.Desktop/DiskSpace/DiskSpaceCheckResult.cs

3. Daqifi.Desktop/DiskSpace/DiskSpaceEventArgs.cs ✨ Enhancement +28/-0

Disk space event arguments model
• Event arguments for disk space threshold events
• Provides available bytes and computed megabytes property
• Includes threshold level that was crossed
Daqifi.Desktop/DiskSpace/DiskSpaceEventArgs.cs

View more (4)

4. Daqifi.Desktop/DiskSpace/IDiskSpaceMonitor.cs ✨ Enhancement +39/-0

Disk space monitor interface contract

• Interface defining disk space monitoring contract
• Events: LowSpaceWarning (100 MB) and CriticalSpaceReached (50 MB)
• Methods: CheckPreLoggingSpace(), StartMonitoring(), StopMonitoring()
• Property: IsMonitoring to check active state

Daqifi.Desktop/DiskSpace/IDiskSpaceMonitor.cs

5. Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs ✨ Enhancement +174/-0

Disk space monitor implementation with timer-based checks

• Implements IDiskSpaceMonitor with three threshold constants (500/100/50 MB)
• CheckPreLoggingSpace() performs pre-session validation
• StartMonitoring() begins 15-second interval timer checks
• OnTimerTick() classifies space level and raises events; critical events pause timer to prevent
 duplicates
• Warning events fire only once per session via _warningRaised flag
• Includes injectable Func<string, long> for free-space provider (testability)

Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs

6. Daqifi.Desktop.Test/DiskSpace/DiskSpaceMonitorTests.cs 🧪 Tests +348/-0

Comprehensive disk space monitor unit tests

• 22 unit tests covering threshold classification logic
• Tests for ClassifyLevel() with boundary conditions (exactly at thresholds, zero bytes)
• Tests for CheckPreLoggingSpace() returning correct levels and megabyte conversions
• Tests for monitoring lifecycle: start, stop, double-start, dispose
• Event tests: critical/warning event firing, no events when space OK, warning fires once per
 session
• Edge case: space jumps from OK to critical (critical event only, no warning)
• Constructor validation tests for null parameters
• Threshold constant verification tests

Daqifi.Desktop.Test/DiskSpace/DiskSpaceMonitorTests.cs

7. Daqifi.Desktop/ViewModels/DaqifiViewModel.cs ✨ Enhancement +81/-0

Integrate disk space monitoring into logging lifecycle

• Adds _diskSpaceMonitor field initialized in constructor
• IsLogging setter now calls CheckPreLoggingSpace() before starting logging
• Blocks logging if critical space (<50 MB) with error dialog
• Shows warning dialog if pre-session or warning level space detected
• Calls StartMonitoring() when logging starts, StopMonitoring() when stops
• Adds event handlers OnDiskSpaceLowWarning() and OnDiskSpaceCritical()
• OnDiskSpaceCritical() auto-stops logging and shows notification
• ShowDiskSpaceMessage() helper displays dialogs via MetroWindow

Daqifi.Desktop/ViewModels/DaqifiViewModel.cs

qodo-code-review · 2026-04-04T16:18:54Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. _diskSpaceMonitor never disposed 📘 Rule violation ☼ Reliability

Description

DaqifiViewModel creates an IDiskSpaceMonitor (which is IDisposable) and subscribes to its
events, but never disposes it or unsubscribes handlers. This can cause resource leaks (timer/event
handler retention) and violates deterministic cleanup requirements.

Code

Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[R483-487]

+                    // Disk space monitoring
+                    _diskSpaceMonitor = new DiskSpaceMonitor(App.DaqifiDataDirectory);
+                    _diskSpaceMonitor.LowSpaceWarning += OnDiskSpaceLowWarning;
+                    _diskSpaceMonitor.CriticalSpaceReached += OnDiskSpaceCritical;
+

Evidence
PR Compliance IDs 23 and 30 require proper disposal and deterministic cleanup/unsubscription. The
code constructs a DiskSpaceMonitor, attaches event handlers, and since IDiskSpaceMonitor is
IDisposable, it must be disposed/unsubscribed when the view model is torn down.
CLAUDE.md
Daqifi.Desktop/DiskSpace/IDiskSpaceMonitor.cs[6-6]
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[483-487]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DaqifiViewModel` creates an `IDiskSpaceMonitor` instance and subscribes to its events but never disposes it and never unsubscribes the event handlers.

## Issue Context
`IDiskSpaceMonitor` inherits `IDisposable`, and the monitor owns timer-based resources and event subscriptions that should be cleaned up deterministically.

## Fix Focus Areas
- Daqifi.Desktop/DiskSpace/IDiskSpaceMonitor.cs[6-6]
- Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[483-487]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~IsLogging UI not updated~~ ☑ 🐞 Bug ≡ Correctness

Description

DaqifiViewModel.IsLogging never raises PropertyChanged when _isLogging changes, so other
bindings to IsLogging (e.g., session status visibility) won’t refresh. Additionally, when disk
space is critical the setter returns early without notifying the binding system, so the TwoWay
ToggleSwitch can remain visually ON even though logging did not start.

Code

Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[R168-199]

+            if (value && _diskSpaceMonitor != null)
+            {
+                var check = _diskSpaceMonitor.CheckPreLoggingSpace();
+                if (check.Level == DiskSpaceLevel.Critical)
+                {
+                    _ = ShowDiskSpaceMessage(
+                        "Cannot Start Logging",
+                        $"Only {check.AvailableMegabytes} MB of disk space remaining. " +
+                        "Logging cannot start because the disk is critically low.\n\n" +
+                        "Please free disk space by deleting old logging sessions or removing other files.");
+                    return;
+                }
+
+                if (check.Level == DiskSpaceLevel.PreSessionWarning || check.Level == DiskSpaceLevel.Warning)
+                {
+                    _ = ShowDiskSpaceMessage(
+                        "Low Disk Space Warning",
+                        $"Only {check.AvailableMegabytes} MB of disk space remaining. " +
+                        "Logging may be stopped automatically if space runs out.\n\n" +
+                        "Consider freeing disk space by deleting old logging sessions or removing other files.");
+                }
+            }
+
            _isLogging = value;
            LoggingManager.Instance.Active = value;
            if (_isLogging)
            {
+                _diskSpaceMonitor?.StartMonitoring();
+
                foreach (var device in ConnectedDevices)
                {
                    if (device.Mode == DeviceMode.StreamToApp)

Evidence
IsLogging assigns _isLogging = value but does not call OnPropertyChanged() (unlike other
properties in the same class), and it can return; early on critical disk space without any
notification. The UI binds multiple elements to IsLogging (ToggleSwitch IsOn TwoWay and a Border
Visibility), which rely on INotifyPropertyChanged to update when the source changes/refuses a
change.
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[152-236]
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[168-189]
Daqifi.Desktop/MainWindow.xaml[194-260]
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[2272-2280]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DaqifiViewModel.IsLogging` does not raise `PropertyChanged`, and in the critical-disk-space early-return path it never notifies bindings to revert the TwoWay toggle. This breaks WPF UI synchronization for all UI elements bound to `IsLogging`.

### Issue Context
Other properties in `DaqifiViewModel` explicitly call `OnPropertyChanged()` after assignment, indicating this is the intended pattern. The UI binds multiple elements to `IsLogging`.

### Fix Focus Areas
- Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[163-225]
- Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[2272-2280]
- Daqifi.Desktop/MainWindow.xaml[194-260]

### Suggested fix
- Change `IsLogging` to use `SetProperty(ref _isLogging, value)` (or assign + `OnPropertyChanged(nameof(IsLogging))`) when the value actually changes.
- In the critical disk space block path, explicitly notify bindings (e.g., call `OnPropertyChanged(nameof(IsLogging))`) so the UI toggle reverts to the actual value (`false`).
- Once `IsLogging` consistently raises notifications, remove the manual `OnPropertyChanged(nameof(IsLogging))` in `OnDiskSpaceCritical` to avoid double-notifications.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Unhandled pre-check exceptions~~ ☑ 🐞 Bug ☼ Reliability

Description

DiskSpaceMonitor.CheckPreLoggingSpace() performs IO/drive queries without a try/catch, and it is
called directly from the IsLogging setter on the UI thread. Any exception from
DriveInfo/free-space lookup can bubble up and crash logging start instead of showing a safe
warning/blocking dialog.

Code

Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[R64-72]

+    public DiskSpaceCheckResult CheckPreLoggingSpace()
+    {
+        var available = GetAvailableSpace();
+        var level = ClassifyLevel(available, preSession: true);
+
+        _appLogger.Information($"Pre-logging disk space check: {available / (1024 * 1024)} MB available, level={level}");
+
+        return new DiskSpaceCheckResult(available, level);
+    }

Evidence
The monitoring path (OnTimerTick) wraps disk space checks in a try/catch, but the pre-session path
does not. The view model calls CheckPreLoggingSpace() synchronously when the user attempts to
enable logging.
Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[64-72]
Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[113-141]
Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[168-172]
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[168-172]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DiskSpaceMonitor.CheckPreLoggingSpace()` can throw exceptions when querying free disk space, and these exceptions are currently unhandled on the pre-session path (UI thread), risking crashes when the user toggles logging on.

### Issue Context
`OnTimerTick` already has a try/catch around the same disk-space query, suggesting exceptions are expected and should be handled similarly during the pre-session check.

### Fix Focus Areas
- Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[64-72]
- Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[168-172]
- Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[168-172]

### Suggested fix
- Wrap the body of `CheckPreLoggingSpace()` in try/catch.
- On exception: log an error and return a conservative result (e.g., `availableBytes = 0` and `level = DiskSpaceLevel.Critical`) so logging is blocked safely rather than crashing.
- Optionally harden `GetAvailableFreeSpaceForPath` by validating `Path.GetPathRoot(path)` and throwing a more descriptive exception if it’s null/empty (then handled by the new try/catch).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. DiskSpaceMonitor not DI-injected 📘 Rule violation ⚙ Maintainability

Description

DaqifiViewModel directly instantiates DiskSpaceMonitor instead of receiving IDiskSpaceMonitor
via dependency injection. This reduces testability and violates the DI-for-testability requirement.

Code

Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[R483-486]

+                    // Disk space monitoring
+                    _diskSpaceMonitor = new DiskSpaceMonitor(App.DaqifiDataDirectory);
+                    _diskSpaceMonitor.LowSpaceWarning += OnDiskSpaceLowWarning;
+                    _diskSpaceMonitor.CriticalSpaceReached += OnDiskSpaceCritical;

Evidence

PR Compliance ID 24 requires external collaborators/services be injected for testability. The new
code constructs new DiskSpaceMonitor(...) directly rather than accepting/resolving an
IDiskSpaceMonitor abstraction via DI.

CLAUDE.md
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[483-486]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DaqifiViewModel` constructs `DiskSpaceMonitor` directly instead of accepting an `IDiskSpaceMonitor` dependency.

## Issue Context
The repository standard expects DI for external services to enable mocking and isolation in unit tests.

## Fix Focus Areas
- Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[483-486]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Duplicate warning dialogs~~ ☑ 🐞 Bug ☼ Reliability

Description

When free space is already below 100MB at logging start, the view model shows a warning dialog from
the pre-session check and then monitoring immediately fires LowSpaceWarning on the first timer
tick (dueTime=0), showing a second warning dialog back-to-back. This is noisy and undermines the
“warn once per session” intent from a user perspective.

Code

Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[R168-197]

+            if (value && _diskSpaceMonitor != null)
+            {
+                var check = _diskSpaceMonitor.CheckPreLoggingSpace();
+                if (check.Level == DiskSpaceLevel.Critical)
+                {
+                    _ = ShowDiskSpaceMessage(
+                        "Cannot Start Logging",
+                        $"Only {check.AvailableMegabytes} MB of disk space remaining. " +
+                        "Logging cannot start because the disk is critically low.\n\n" +
+                        "Please free disk space by deleting old logging sessions or removing other files.");
+                    return;
+                }
+
+                if (check.Level == DiskSpaceLevel.PreSessionWarning || check.Level == DiskSpaceLevel.Warning)
+                {
+                    _ = ShowDiskSpaceMessage(
+                        "Low Disk Space Warning",
+                        $"Only {check.AvailableMegabytes} MB of disk space remaining. " +
+                        "Logging may be stopped automatically if space runs out.\n\n" +
+                        "Consider freeing disk space by deleting old logging sessions or removing other files.");
+                }
+            }
+
            _isLogging = value;
            LoggingManager.Instance.Active = value;
            if (_isLogging)
            {
+                _diskSpaceMonitor?.StartMonitoring();
+
                foreach (var device in ConnectedDevices)

Evidence
The pre-session path shows a low disk space message for DiskSpaceLevel.Warning. Immediately after,
StartMonitoring() schedules a timer with TimeSpan.Zero dueTime, and OnTimerTick raises
LowSpaceWarning once per monitoring session, which the view model also converts into a
dialog—resulting in two dialogs for the same condition at session start.
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[168-188]
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[191-207]
Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[74-84]
Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[113-135]
Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[2260-2269]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
At logging start with <100MB free, users can get two low-space dialogs: one from the pre-session check and one from the monitor’s immediate first tick.

### Issue Context
`StartMonitoring()` uses an immediate tick (dueTime=0) for safety, which is good for critical-stop responsiveness, but it also causes warning-level events right away.

### Fix Focus Areas
- Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[168-197]
- Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[74-84]
- Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[113-135]
- Daqifi.Desktop/ViewModels/DaqifiViewModel.cs[2260-2269]

### Suggested fix
Implement one of:
- If the pre-session check already displayed a `<100MB` warning, start monitoring in a mode that suppresses the first `Warning` event unless the level worsens to `Critical`.
- Or have the view model skip showing the pre-session dialog for `<100MB` and rely on the monitor’s immediate tick for that tier, keeping pre-session warnings only for the 500MB tier.

Maintain immediate critical detection (<50MB) while avoiding duplicate warning dialogs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. DiskSpaceMonitor missing XML docs ☑ 📘 Rule violation ⚙ Maintainability

Description

New public members on DiskSpaceMonitor (e.g., CheckPreLoggingSpace(), StartMonitoring(),
StopMonitoring(), Dispose()) lack XML documentation comments. This violates the requirement that
public APIs be documented, reducing maintainability and developer usability.

Code

Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[R64-100]

+    public DiskSpaceCheckResult CheckPreLoggingSpace()
+    {
+        var available = GetAvailableSpace();
+        var level = ClassifyLevel(available, preSession: true);
+
+        _appLogger.Information($"Pre-logging disk space check: {available / (1024 * 1024)} MB available, level={level}");
+
+        return new DiskSpaceCheckResult(available, level);
+    }
+
+    public void StartMonitoring()
+    {
+        if (_timer != null)
+        {
+            return;
+        }
+
+        _warningRaised = false;
+        _timer = new System.Threading.Timer(OnTimerTick, null, TimeSpan.Zero, TimeSpan.FromMilliseconds(MONITOR_INTERVAL_MS));
+        _appLogger.Information("Disk space monitoring started");
+    }
+
+    public void StopMonitoring()
+    {
+        if (_timer == null)
+        {
+            return;
+        }
+
+        _timer.Dispose();
+        _timer = null;
+        _warningRaised = false;
+        _appLogger.Information("Disk space monitoring stopped");
+    }
+
+    public void Dispose()
+    {

Evidence
PR Compliance ID 16 requires XML documentation comments on public APIs; the new DiskSpaceMonitor
public methods are added without /// XML comments.
CLAUDE.md
Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[64-100]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DiskSpaceMonitor` introduces multiple public members without XML documentation comments.

## Issue Context
The repository requires XML docs for public APIs to keep behavior self-documented.

## Fix Focus Areas
- Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[64-100]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. ~~Timer state not thread-safe~~ ☑ 🐞 Bug ☼ Reliability

Description

DiskSpaceMonitor accesses _timer and _warningRaised from both the UI thread (start/stop) and
the timer callback without synchronization. This can cause rare races (e.g., multiple warning
events) if callbacks overlap start/stop or if the callback re-enters.

Code

Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[R74-136]

+    public void StartMonitoring()
+    {
+        if (_timer != null)
+        {
+            return;
+        }
+
+        _warningRaised = false;
+        _timer = new System.Threading.Timer(OnTimerTick, null, TimeSpan.Zero, TimeSpan.FromMilliseconds(MONITOR_INTERVAL_MS));
+        _appLogger.Information("Disk space monitoring started");
+    }
+
+    public void StopMonitoring()
+    {
+        if (_timer == null)
+        {
+            return;
+        }
+
+        _timer.Dispose();
+        _timer = null;
+        _warningRaised = false;
+        _appLogger.Information("Disk space monitoring stopped");
+    }
+
+    public void Dispose()
+    {
+        if (_disposed)
+        {
+            return;
+        }
+
+        StopMonitoring();
+        _disposed = true;
+        GC.SuppressFinalize(this);
+    }
+    #endregion
+
+    #region Private Methods
+    private void OnTimerTick(object? state)
+    {
+        try
+        {
+            var available = GetAvailableSpace();
+            var level = ClassifyLevel(available, preSession: false);
+
+            switch (level)
+            {
+                case DiskSpaceLevel.Critical:
+                    // Stop the timer first to prevent duplicate critical events
+                    // before the UI thread can call StopMonitoring()
+                    _timer?.Change(Timeout.Infinite, Timeout.Infinite);
+                    _appLogger.Warning($"Disk space critically low: {available / (1024 * 1024)} MB — triggering hard stop");
+                    CriticalSpaceReached?.Invoke(this, new DiskSpaceEventArgs(available, DiskSpaceLevel.Critical));
+                    break;
+
+                case DiskSpaceLevel.Warning when !_warningRaised:
+                    _appLogger.Warning($"Disk space low: {available / (1024 * 1024)} MB");
+                    _warningRaised = true;
+                    LowSpaceWarning?.Invoke(this, new DiskSpaceEventArgs(available, DiskSpaceLevel.Warning));
+                    break;
+            }
+        }

Evidence

StartMonitoring/StopMonitoring mutate _timer and _warningRaised, while OnTimerTick
reads/mutates them concurrently, with no lock/Interlocked usage. The warning ‘only once’ behavior
depends on _warningRaised being seen consistently across threads.

Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[74-97]
Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[113-135]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DiskSpaceMonitor` uses shared mutable state (`_timer`, `_warningRaised`) across threads without synchronization.

### Issue Context
The timer callback runs on a ThreadPool thread; start/stop is typically on the UI thread.

### Fix Focus Areas
- Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[74-97]
- Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs[113-135]

### Suggested fix
- Use `Interlocked`/`Volatile` for `_warningRaised` (e.g., `Interlocked.Exchange` / `CompareExchange`).
- Consider guarding `_timer` lifecycle with a private lock or a dedicated `_isMonitoring` atomic flag checked early in `OnTimerTick`.
- Optionally throw `ObjectDisposedException` in `StartMonitoring()` if `_disposed` is true, to prevent restarting a disposed monitor.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

Daqifi.Desktop/ViewModels/DaqifiViewModel.cs

Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs

- Dispose monitor and unsubscribe events on window close (qodo #1) - Notify bindings on critical early-return so toggle reverts (qodo #2) - Wrap CheckPreLoggingSpace in try/catch to avoid crash on IO error (qodo #3) - Fix flaky test that depended on timer firing twice in 500ms Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Dispatcher.Invoke blocks the timer callback thread until the UI thread processes the delegate. BeginInvoke queues it asynchronously, which is safer when the UI thread may be busy showing a MahApps dialog. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

tylerkron · 2026-04-05T02:46:07Z

Addressing remaining Qodo feedback from the summary comment:

4. Missing XML docs — Fixed in 33c8137. Added /// <inheritdoc /> to all public methods on DiskSpaceMonitor since the interface already has full XML doc comments.

5. Not DI-injected — Disagree. DaqifiViewModel itself is not DI-resolved (it's new'd in MainWindow.xaml.cs), and the existing loggers (DatabaseLogger, PlotLogger, SummaryLogger) are all directly instantiated in the same constructor block. Wiring IDiskSpaceMonitor through DI would be inconsistent with the codebase. Testability is already addressed via the internal constructor that accepts a Func<string, long> for the free-space provider.

Verifies the try/catch returns DiskSpaceLevel.Ok when the free-space provider throws, so logging is not blocked by transient IO errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Add suppressInitialWarning parameter to StartMonitoring so the pre-session warning and the first timer tick don't both show a warning dialog (qodo #6) - Add lock around timer state and _warningRaised to prevent races between the timer callback and UI-thread start/stop calls (qodo #7) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

tylerkron · 2026-04-05T03:01:02Z

Addressing remaining Qodo feedback (items 6 and 7) from the summary comment:

6. Duplicate warning dialogs — Fixed in 16b80c7. Added suppressInitialWarning parameter to StartMonitoring(). When the pre-session check already showed a warning, we pass true so the first timer tick (which fires immediately at dueTime=0) doesn't show a second dialog.

7. Timer state not thread-safe — Fixed in 16b80c7. Added a lock around _timer/_warningRaised access in StartMonitoring, StopMonitoring, and OnTimerTick to prevent races between the timer callback thread and UI-thread callers.

github-actions · 2026-04-05T03:03:02Z

📊 Code Coverage Report

Summary


Generated on:	4/5/2026 - 3:02:54 AM
Coverage date:	4/5/2026 - 3:02:25 AM - 4/5/2026 - 3:02:50 AM
Parser:	MultiReport (4x Cobertura)
Assemblies:	3
Classes:	110
Files:	141
Line coverage:	19.9% (1206 of 6046)
Covered lines:	1206
Uncovered lines:	4840
Coverable lines:	6046
Total lines:	18479
Branch coverage:	18% (398 of 2207)
Covered branches:	398
Total branches:	2207
Method coverage:	Feature is only available for sponsors

Coverage

DAQiFi - 19.7%

Name	Line	Branch
DAQiFi	19.7%	17.9%
Daqifi.Desktop.App	6.3%	0%
Daqifi.Desktop.Channel.AbstractChannel	40.9%	27.7%
Daqifi.Desktop.Channel.AnalogChannel	58.7%	25%
Daqifi.Desktop.Channel.Channel	11.5%	0%
Daqifi.Desktop.Channel.ChannelColorManager	100%	100%
Daqifi.Desktop.Channel.DataSample	91.6%
Daqifi.Desktop.Channel.DigitalChannel	65.2%	25%
Daqifi.Desktop.Commands.CompositeCommand	0%	0%
Daqifi.Desktop.Commands.HostCommands	0%
Daqifi.Desktop.Commands.WeakEventHandlerManager	0%	0%
Daqifi.Desktop.Configuration.FirewallConfiguration	90.6%	66.6%
Daqifi.Desktop.Configuration.WindowsFirewallWrapper	64%	68.4%
Daqifi.Desktop.ConnectionManager	43%	43.1%
Daqifi.Desktop.Converters.BoolToActiveStatusConverter	0%	0%
Daqifi.Desktop.Converters.BoolToConnectionStatusConverter	0%	0%
Daqifi.Desktop.Converters.BoolToStatusColorConverter	0%	0%
Daqifi.Desktop.Converters.ConnectionTypeToColorConverter	0%	0%
Daqifi.Desktop.Converters.ConnectionTypeToUsbConverter	0%	0%
Daqifi.Desktop.Converters.InvertedBoolToVisibilityConverter	0%	0%
Daqifi.Desktop.Converters.ListToStringConverter	0%	0%
Daqifi.Desktop.Converters.NotNullToVisibilityConverter	0%	0%
Daqifi.Desktop.Converters.OxyColorToBrushConverter	0%	0%
Daqifi.Desktop.Converters.StringRightConverter	0%	0%
Daqifi.Desktop.Device.AbstractStreamingDevice	42.8%	38.6%
Daqifi.Desktop.Device.DeviceMessage	0%
Daqifi.Desktop.Device.Firmware.BootloaderSessionStreamingDeviceAdapter	0%	0%
Daqifi.Desktop.Device.Firmware.WifiPromptDelayProcessRunner	0%	0%
Daqifi.Desktop.Device.NativeMethods	100%
Daqifi.Desktop.Device.SerialDevice.SerialStreamingDevice	27.6%	30.8%
Daqifi.Desktop.Device.WiFiDevice.DaqifiStreamingDevice	40.9%	39.4%
Daqifi.Desktop.DialogService.DialogService	0%	0%
Daqifi.Desktop.DialogService.ServiceLocator	0%	0%
Daqifi.Desktop.DiskSpace.DiskSpaceCheckResult	100%
Daqifi.Desktop.DiskSpace.DiskSpaceEventArgs	100%
Daqifi.Desktop.DiskSpace.DiskSpaceMonitor	88.2%	86.6%
Daqifi.Desktop.DuplicateDeviceCheckResult	100%
Daqifi.Desktop.Exporter.OptimizedLoggingSessionExporter	30.2%	35.4%
Daqifi.Desktop.Exporter.SampleData	0%
Daqifi.Desktop.Helpers.BooleanConverter`1	0%	0%
Daqifi.Desktop.Helpers.BooleanToInverseBoolConverter	0%	0%
Daqifi.Desktop.Helpers.BooleanToVisibilityConverter	0%
Daqifi.Desktop.Helpers.EnumDescriptionConverter	100%	100%
Daqifi.Desktop.Helpers.IntToVisibilityConverter	0%	0%
Daqifi.Desktop.Helpers.MyMultiValueConverter	0%
Daqifi.Desktop.Helpers.NaturalSortHelper	100%	100%
Daqifi.Desktop.Helpers.VersionHelper	98.2%	66.2%
Daqifi.Desktop.Logger.DatabaseLogger	0%	0%
Daqifi.Desktop.Logger.LoggedSeriesLegendItem	0%	0%
Daqifi.Desktop.Logger.LoggingContext	0%
Daqifi.Desktop.Logger.LoggingManager	0%	0%
Daqifi.Desktop.Logger.LoggingSession	26.6%	0%
Daqifi.Desktop.Logger.PlotLogger	0%	0%
Daqifi.Desktop.Logger.SummaryLogger	0%	0%
Daqifi.Desktop.Logger.TimestampGapDetector	95%	83.3%
Daqifi.Desktop.Loggers.ImportOptions	0%
Daqifi.Desktop.Loggers.ImportProgress	0%	0%
Daqifi.Desktop.Loggers.SdCardSessionImporter	0%	0%
Daqifi.Desktop.MainWindow	0%	0%
Daqifi.Desktop.Migrations.InitialSQLiteMigration	0%
Daqifi.Desktop.Migrations.LoggingContextModelSnapshot	0%
Daqifi.Desktop.Models.AddProfileModel	0%
Daqifi.Desktop.Models.DaqifiSettings	80.5%	83.3%
Daqifi.Desktop.Models.DebugDataCollection	6.6%	0%
Daqifi.Desktop.Models.DebugDataModel	0%	0%
Daqifi.Desktop.Models.Notifications	0%
Daqifi.Desktop.Models.SdCardFile	0%	0%
Daqifi.Desktop.Services.WindowsPrincipalAdminChecker	0%
Daqifi.Desktop.Services.WpfMessageBoxService	0%
Daqifi.Desktop.UpdateVersion.VersionNotification	0%	0%
Daqifi.Desktop.View.AddChannelDialog	0%	0%
Daqifi.Desktop.View.AddProfileConfirmationDialog	0%	0%
Daqifi.Desktop.View.AddprofileDialog	0%	0%
Daqifi.Desktop.View.ConnectionDialog	0%	0%
Daqifi.Desktop.View.DebugWindow	0%	0%
Daqifi.Desktop.View.DeviceLogsView	0%	0%
Daqifi.Desktop.View.DuplicateDeviceDialog	0%	0%
Daqifi.Desktop.View.ErrorDialog	0%	0%
Daqifi.Desktop.View.ExportDialog	0%	0%
Daqifi.Desktop.View.FirmwareDialog	0%	0%
Daqifi.Desktop.View.Flyouts.ChannelsFlyout	0%	0%
Daqifi.Desktop.View.Flyouts.DevicesFlyout	0%	0%
Daqifi.Desktop.View.Flyouts.FirmwareFlyout	0%	0%
Daqifi.Desktop.View.Flyouts.LiveGraphFlyout	0%	0%
Daqifi.Desktop.View.Flyouts.LoggedSessionFlyout	0%	0%
Daqifi.Desktop.View.Flyouts.NotificationsFlyout	0%	0%
Daqifi.Desktop.View.Flyouts.SummaryFlyout	0%	0%
Daqifi.Desktop.View.Flyouts.UpdateProfileFlyout	0%	0%
Daqifi.Desktop.View.SelectColorDialog	0%	0%
Daqifi.Desktop.View.SettingsDialog	0%	0%
Daqifi.Desktop.View.SuccessDialog	0%	0%
Daqifi.Desktop.ViewModels.AddChannelDialogViewModel	0%	0%
Daqifi.Desktop.ViewModels.AddProfileConfirmationDialogViewModel	0%	0%
Daqifi.Desktop.ViewModels.AddProfileDialogViewModel	0%	0%
Daqifi.Desktop.ViewModels.ConnectionDialogViewModel	21.6%	19.5%
Daqifi.Desktop.ViewModels.DaqifiViewModel	14.4%	8.5%
Daqifi.Desktop.ViewModels.DeviceLogsViewModel	0%	0%
Daqifi.Desktop.ViewModels.DeviceSettingsViewModel	0%	0%
Daqifi.Desktop.ViewModels.DuplicateDeviceDialogViewModel	0%
Daqifi.Desktop.ViewModels.ErrorDialogViewModel	0%
Daqifi.Desktop.ViewModels.ExportDialogViewModel	0%	0%
Daqifi.Desktop.ViewModels.FirmwareDialogViewModel	0%	0%
Daqifi.Desktop.ViewModels.SelectColorDialogViewModel	0%	0%
Daqifi.Desktop.ViewModels.SettingsViewModel	0%
Daqifi.Desktop.ViewModels.SuccessDialogViewModel	85.7%
Daqifi.Desktop.WindowViewModelMapping.IWindowViewModelMappingsContract	0%
Daqifi.Desktop.WindowViewModelMapping.WindowViewModelMappings	0%
Sentry.Generated.BuildPropertyInitializer	100%

Daqifi.Desktop.Common - 33.3%

Name	Line	Branch
Daqifi.Desktop.Common	33.3%	22.7%
Daqifi.Desktop.Common.Loggers.AppLogger	31.3%	22.7%
Daqifi.Desktop.Common.Loggers.NoOpLogger	60%

Daqifi.Desktop.IO - 100%

Name	Line	Branch
Daqifi.Desktop.IO	100%	****
Daqifi.Desktop.IO.Messages.MessageEventArgs`1	100%

Coverage report generated by ReportGenerator • View full report in build artifacts

tylerkron requested a review from a team as a code owner April 4, 2026 16:18

qodo-code-review bot reviewed Apr 4, 2026

View reviewed changes

Daqifi.Desktop/ViewModels/DaqifiViewModel.cs Show resolved Hide resolved

Daqifi.Desktop/ViewModels/DaqifiViewModel.cs Show resolved Hide resolved

Daqifi.Desktop/DiskSpace/DiskSpaceMonitor.cs Show resolved Hide resolved

tylerkron and others added 4 commits April 4, 2026 10:31

docs: add rule to always reply to Qodo review comments on PRs

73fa66a

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docs: add XML doc comments to DiskSpaceMonitor public methods

33c8137

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

tylerkron and others added 2 commits April 4, 2026 20:52

test: add coverage for CheckPreLoggingSpace exception handling

a53c2f3

Verifies the try/catch returns DiskSpaceLevel.Ok when the free-space provider throws, so logging is not blocked by transient IO errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

tylerkron merged commit 706a87e into main Apr 5, 2026
2 checks passed

tylerkron deleted the claude/vigorous-visvesvaraya branch April 5, 2026 03:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: warn user when disk space is running low#456

feat: warn user when disk space is running low#456
tylerkron merged 7 commits intomainfrom
claude/vigorous-visvesvaraya

tylerkron commented Apr 4, 2026 •

edited

Loading

Uh oh!

qodo-code-review bot commented Apr 4, 2026

Uh oh!

qodo-code-review bot commented Apr 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tylerkron commented Apr 5, 2026

Uh oh!

tylerkron commented Apr 5, 2026

Uh oh!

github-actions bot commented Apr 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

tylerkron commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Implementation

Test plan

Uh oh!

qodo-code-review bot commented Apr 4, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-code-review bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tylerkron commented Apr 5, 2026

Uh oh!

tylerkron commented Apr 5, 2026

Uh oh!

github-actions bot commented Apr 5, 2026

📊 Code Coverage Report

Summary

Coverage

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tylerkron commented Apr 4, 2026 •

edited

Loading

qodo-code-review bot commented Apr 4, 2026 •

edited

Loading