[tylerkron]

Not merging — for review. Routine: coverage-gap. Tests only, no production change.

What was untested

DiskSpaceMonitorCoordinator decides what a low disk does: whether the logging toggle may turn on, which dialog the user sees, and whether a running session gets stopped before SQLite fills the volume mid-write. DiskSpaceMonitorTests already pins the monitor's thresholds (50/100/500 MB). Nothing pinned the coordinator. It was referenced once in the test project, in a doc comment.

How I checked it was untested: a case-insensitive search of Daqifi.Avalonia.Tests/ for the class name and for each member and seam: EvaluateStartLogging, DiskSpaceStartDecision, OnCriticalSpaceReached, IDiskSpaceMonitorHost, ShowDiskSpaceMessage. The only hit was that comment (DeviceRefusalCrashTests.cs:93). Positive control: the same search does find the event names LowSpaceWarning/CriticalSpaceReached in DiskSpaceMonitorTests.cs, which really is tested.

What this adds

One file, Daqifi.Avalonia.Tests/DiskSpace/DiskSpaceMonitorCoordinatorTests.cs, +313/−0. That's 13 test cases (9 methods; two are theories). They use hand-rolled monitor and host doubles, with no timer and no dispatcher. Net-additive because it is a coverage PR. It adds no production code, no abstraction and no helper shared outside the file.

Plain characterisation (these pass on main by design)

• The start gate, level by level: Ok → Allowed with no dialog. PreSessionWarning → AllowedWithWarning. Warning → AllowedWithWarning. Critical → Blocked. Each case also asserts the dialog title.
• StartMonitoring passes the suppress flag straight through, and StopMonitoring is forwarded.
• A low-space event during a session shows only a dialog. Logging keeps going, and the message names the 50 MB hard stop.

Tests that pin something subtle

• Warning is in the gate table on purpose. The pre-session check returns Warning (not PreSessionWarning) for 50–100 MB, so if that leg were dropped, a nearly full disk would start logging with no warning at all.
• The gate on a critical disk does not call StopLogging. Nothing is running yet, and the host's StopLogging writes IsLogging = false from inside the very setter that is asking.
• Dialogs quote whole megabytes, truncated. At 50 MB − 1 byte free, the dialog says "Only 49 MB". If it rounded, a dialog refusing to start because space is under 50 MB would say "50 MB". (Scope: this is asserted on the gate's Cannot Start Logging message only. The other three messages are exercised with whole-MB values, so a rounding change confined to one of them would not be caught.)
• The critical event stops logging before it shows the dialog, and logs one Warning line. The dialog is fire-and-forget, so the stop cannot depend on it.
• If the host's StopLogging throws, the exception escapes and no dialog is shown. This is current behaviour, pinned but not endorsed — the test's doc comment says so and that a change should have to update the test on purpose. It is not a live defect: DiskSpaceMonitor.OnTimerTick catches and logs the exception, and the production host (DaqifiViewModel's IDiskSpaceMonitorHost.StopLogging) sets IsLogging = false directly on the UI thread and otherwise posts to the dispatcher; the monitor's tick arrives on a timer thread, so it posts. So it's pinned, not filed.
• Dispose unsubscribes from both events and disposes the monitor exactly once, even when called twice. A tick already in flight cannot then stop a session or open a dialog on a view model that has been torn down.

Verification

• Mutation testing, 12 mutants, all killed. Each was made on DiskSpaceMonitorCoordinator.cs, then the file was restored from git and touched.

  #	Mutation	Killed by
  1	Blocked → Allowed on the critical branch	gate theory (Critical)
  2	drop || Level == Warning	gate theory (Warning)
  3	show the dialog before StopLogging	both critical-event tests
  4	Dispose keeps the critical subscription	Dispose test
  5	Dispose never latches _disposed	Dispose test (monitor disposed twice)
  6	pre-session critical also calls StopLogging	gate theory + the no-stop test
  7	StartMonitoring(true) constant	suppress theory (false)
  8	low-space handler also stops logging	low-space test
  9	MB rounded, not truncated (gate critical message)	truncation test
  10	StopMonitoring becomes a no-op	StopMonitoring test
  11	drop the critical Warning log line	critical ordering test
  12	Dispose keeps the low-space subscription	Dispose test

  Mutants 7–9 and 11–12 ran in two combined builds. Each mutant in a combined build was in a different method, and each went red in exactly the test that claims it.

• Shepherd re-run (independent, single fresh edit per mutant, touch after restore, each build confirmed to recompile Daqifi.Avalonia.Tests.dll and run all 13 cases): mutants 2, 3, 9 and 12 re-made and re-measured. feat(sd): enable SD-card offload over WiFi/TCP (closes #1) #2 → 12/13, the Warning theory row failed. reliability: device layer never notifies IsConnected transitions (stale command/UI state on silent drops) #3 → 11/13, both critical-event tests failed. fix(mobile): nav tab labels & count diverge from desktop (Stream≠Live Graph, Storage≠Logged Data, 4 vs 5) #9 (Math.Round on the gate's critical message) → 12/13, the truncation test failed. polish(desktop): assorted flyout/pane parity nits (settings highlight, scrollbar, wrap width, glyphs, gridlines) #12 → 12/13, the Dispose test failed. Unmutated baseline and post-restore run: 13/13. No sleeps, wall-clock waits or timer dependence anywhere in the file; the never-completing dialog Task makes any path that awaited a dialog hang rather than pass.

• Gate (under a build.2 lock): desktop build, iOS restore, and iOS Release iossimulator-arm64 --no-restore all exited 0. dotnet test on the full suite: 1006 passed, 0 failed, 0 skipped.

• Warnings: the diff only touches the test project, and neither head compiles it. The dotnet test build shows 0 warnings from the new file. That comes from a forced --no-incremental rebuild of the test project: it printed 338 warning lines, none from Daqifi.Avalonia.Tests/.

• No lock-file changes. No device involvement, so no bench run.

• Base and collisions: the PR's base is 642caef, which is current main, so CI's run on 8e2bb7e is a build of main + this PR. None of the other open PRs (test(#327): one dialog-DataContext guard — the shared helper follows new+DataContext, DuplicateDeviceDialog's 412-line copy goes (−369) #387, fix(#327): LoggedSessionsMobileView's 51 bindings were all on reflection — one declared scope, two inferred, three $parent hops #389–fix(#327): LoggedDataPanePrototype's 102 bindings were all on reflection — root scope, two declared template scopes, two existing item scopes #396, chore(deps): Bump the minor-and-patch group with 17 updates #363) adds or touches a DiskSpace file or any interface these doubles implement.

• Test-double note: RecordingLogger is the tenth private IAppLogger double in the test project; there is no shared one to reuse, so this follows the existing convention rather than duplicating a helper.

🤖 Generated with Claude Code

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Add characterization coverage for disk-space coordination

🧪 Tests 🕐 20-40 Minutes

AI Description

• Characterize start decisions and dialogs for every disk-space level.
• Verify warning and critical event handling, including stop-before-dialog ordering.
• Pin monitoring forwarding, exception behavior, and idempotent disposal.

Diagram

graph TD
  T["Coordinator Tests"] --> M["Fake Monitor"] -->|check or event| C["Coordinator"] --> G{"Input Path"}
  G -->|start gate| D["Start Decision"]
  G -->|low event| H["Host Dialog"]
  G -->|critical event| S["Stop Logging"] --> H
  C -->|critical event| L["Warning Logger"]

Loading

High-Level Assessment

The current approach is appropriate: local hand-rolled monitor, host, and logger doubles isolate coordinator policy while avoiding timers, dispatchers, and asynchronous dialog completion. Broader integration tests with the production monitor were considered, but would duplicate threshold coverage and make ordering and failure-path assertions less deterministic.

Files changed (1) +313 / -0

Tests (1) +313 / -0

DiskSpaceMonitorCoordinatorTests.csCharacterize disk-space coordinator decisions and side effects +313/-0

Characterize disk-space coordinator decisions and side effects

• Adds 13 synchronous cases covering start-gate mappings, dialog content, low and critical event handling, stop-before-dialog ordering, monitoring delegation, exception propagation, and idempotent disposal. File-local monitor, host, and logger doubles record interactions without timers or UI dispatching.

Daqifi.Avalonia.Tests/DiskSpace/DiskSpaceMonitorCoordinatorTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 8e2bb7e 🚀 Fast

Results up to commit 8e2bb7e 🚀 Fast

No changes from previous review

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8e2bb7e

[tylerkron]

Qodo-clean, CI green — ready for review. Head 8e2bb7e. CI (all 6 jobs, including iOS and Android) ran on this SHA. Two Qodo rounds on this head: both clean, both settled, 0 qodo threads. Grade disclosure: the rounds were graded Fast, not Deep. To cover that, every test in the file has been mutation-checked against the production line it claims (12/12 mutants killed, table in the body). Not merging.

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8e2bb7e

[tylerkron]

Qodo-clean, CI green — ready for review. Head 8e2bb7e, unchanged; CI (all jobs incl. iOS and Android) ran on exactly this SHA, whose base 642caef is current main. Shepherd round: /agentic_review at 22:56:55Z fired (summary updated_at 22:57:31Z, anchored to 8e2bb7e), 0 findings; settled 3+ min and re-checked both surfaces — summary unchanged, 0 qodo review threads. Standing grade: Fast ("No changes from previous review" — no new delta, so a deeper grade is unreachable without a push, and none was made). Covered by hand instead: re-ran mutants 2, 3, 9 and 12 myself, each killed by the test that claims it, 13/13 on baseline and after restore; read the file for timing dependence (none) and for the pinned-not-endorsed case (labelled in its doc comment). Pushed nothing, so the verification is CI's on 8e2bb7e plus those local mutant runs. Body updated in place (no commit) to scope the truncation claim to the one message it covers and record the re-run. Not merging.

[qodo-code-review]

Qodo-clean, CI green — ready for review. Head 8e2bb7e, unchanged; CI (all jobs incl. iOS and Android) ran on exactly this SHA, whose base 642caef is current main. Shepherd round: /agentic_review at 22:56:55Z fired (summary updated_at 22:57:31Z, anchored to 8e2bb7e), 0 findings; settled 3+ min and re-checked both surfaces — summary unchanged, 0 qodo review threads. Standing grade: Fast ("No changes from previous review" — no new delta, so a deeper grade is unreachable without a push, and none was made). Covered by hand instead: re-ran mutants 2, 3, 9 and 12 myself, each killed by the test that claims it, 13/13 on baseline and after restore; read the file for timing dependence (none) and for the pinned-not-endorsed case (labelled in its doc comment). Pushed nothing, so the verification is CI's on 8e2bb7e plus those local mutant runs. Body updated in place (no commit) to scope the truncation claim to the one message it covers and record the re-run. Not merging.

The PR remains clean at 8e2bb7e: CI is green, the review found 0 findings, and no new delta requires another review. The added tests and verification are sufficient for review.

[tylerkron]

Sweep (13-way merge build): all 13 PRs are clean together on main 642caef

Covered: #387 259a829, #389 808752a, #390 a7c89ec, #391 4370e80, #392 01764b9, #393 f1599e0, #394 4b13ea8, #395 9a0bfa4, #396 8f4f29e, #398 8e2bb7e, #399 df20ae1, #400 c810ddf, #401 998da59.

Per PR: every head matches its ready note. Every PR is MERGEABLE/CLEAN, and its Build run on that exact SHA passed. No PR has an unresolved qodo review thread. I validated the thread-author filter against #196 first: it finds 3 qodo threads there, as expected. None of the green runs are stale: the last workflow change on main is 94fcac3 (09-08), older than all of them.

• CI base: 12 of the 13 CI runs merged into 642caef. test(#327): one dialog-DataContext guard — the shared helper follows new+DataContext, DuplicateDeviceDialog's 412-line copy goes (−369) #387 is the exception. Its run merged into 25df7b8, one commit behind, because fix(#327): LiveGraphPane's 40 bindings were all on reflection — a tracker scope the compiler can't infer, one hop it can't follow #386 landed on main afterwards. The merge build below covers that gap.

Merge build: I merged 642caef ← 387, 389–396 first, as a checkpoint. It produced tree 94b85fe, the same tree the previous sweep reported, so that result now holds up. Adding #398, #399, #400 and #401 gave tree b0f2033. Merging all 13 in reverse order gives the same b0f2033. Neither order had a text conflict.

• Desktop build: exit 0.
• iOS Release build (iossimulator-arm64, --no-restore): exit 0.
• Tests: 1039/1039 passed. No lock-file drift.

Test count reconciles exactly: 993 on main + #387 0 + #389–#394 +3 each (+18) + #395 +3 + #396 +4 + #398 +13 + #399 0 + #400 +8 + #401 0 = 1039. Each delta comes from that PR's own CI total minus its base's total. For #387 that is 989 − 989 on 25df7b8.

Cross-PR hazards checked:

• Same thing added twice: fix(#327): ChannelsMobileView's 76 bindings were all on reflection — two declared templates, three cast hops, and a drawer label bound to a member IChannel never had #392 and fix(#327): ChannelsPanePrototype's 81 bindings were all on reflection — three declared templates, and a drawer label bound to a member IChannel never declared #394 both add IChannel.ScaleExpressionError. Their hunks are byte-identical (same blob c63a719), so git merges them as one change. This is the only file two PRs share. No new top-level type name appears in more than one PR.
• chore(warning-zero): CS0067 x2 — the two never-raised events get empty accessors, not a delegate field nobody invokes #399 (CS0067, production change): none of the other 12 diffs mentions DeviceRemoved, MessageReceived, NoOpDeviceWatcher or BootloaderSessionStreamingDeviceAdapter. So no open PR raises or subscribes to the events it gives empty accessors.
• test: nine private IAppLogger fakes → one RecordingAppLogger (net −240, test code only) #401 (shared RecordingAppLogger): none of the other 12 PRs edits any of its 9 files, and none adds a type called RecordingAppLogger. test(coverage-gap): pin DiskSpaceMonitorCoordinator — the class that decides whether a full disk stops your session (13 cases, 12/12 mutants killed) #398 adds its own private sealed class RecordingLogger : IAppLogger, nested inside the test class. That cannot clash, and the build confirms it. It is simply one more private fake that a later clean-up could fold into RecordingAppLogger. It does not block either PR.
• Repo-wide scanning guard: test(#327): one dialog-DataContext guard — the shared helper follows new+DataContext, DuplicateDeviceDialog's 412-line copy goes (−369) #387's BindingFacts guard scans Daqifi.Avalonia/**/*.cs. chore(warning-zero): CS0067 x2 — the two never-raised events get empty accessors, not a delegate field nobody invokes #399 is the only PR here that changes production .cs, and fix(#327): LiveGraphPane's 40 bindings were all on reflection — a tracker scope the compiler can't infer, one hop it can't follow #386 is the commit that landed after test(#327): one dialog-DataContext guard — the shared helper follows new+DataContext, DuplicateDeviceDialog's 412-line copy goes (−369) #387's CI. The guard passes in the merged test run under both merge orders.
• Removed members: chore(warning-zero): CS0067 x2 — the two never-raised events get empty accessors, not a delegate field nobody invokes #399 replaces 2 event fields with accessor events (same public signature). test: nine private IAppLogger fakes → one RecordingAppLogger (net −240, test code only) #401 removes only private nested fakes. test(#327): one dialog-DataContext guard — the shared helper follows new+DataContext, DuplicateDeviceDialog's 412-line copy goes (−369) #387 removes 1 test method. The 13-way build compiles with all of them.

No merge was performed and nothing was pushed to any PR branch.