[tylerkron]

Not merging — for review.

closes #242

Deletes three DaqifiViewModel members that nothing in the app can reach.
0 lines added, 31 deleted, one file touched.

0	31	Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs

#242 lists three commands. Two are deleted here; the third (ShutdownCommand) is
deliberately left in place — see "What is not deleted" below. A fourth member,
LatestFirmwareVersionText, was carried forward from PR #235 and is deleted here
because it is in the same file and clears the same bar.

Why the issue's evidence was not taken on trust

#242 was filed by tools/parity-audit/coverage.py --bindings, which is itself
unmerged and under review in #244, and whose earlier parser is documented as having
silently miscounted four different ways. Every symbol below was re-proved from
scratch against this checkout. The method was an exhaustive substring scan of
every file in the repo (all 443 files, all extensions, skipping only
.git/, bin/, obj/, node_modules/) rather than a git grep over *.cs and
*.axaml — so the enumeration is closed by construction, not by my guessing which
binding syntaxes to look for.

That scan searched the base names (RemoveChannel, ToggleDebugMode, Shutdown,
LatestFirmwareVersion), which are substrings of the source generator's
…Command names, so it necessarily catches both the method-name and
FooCommand spellings, in any of the forms XAML can use them:
Command="{Binding X}", element syntax <Binding Path="X"/>, MultiBinding
children, $parent[…].DataContext.X, TemplatedParent, x:Key/StaticResource,
CommandParameter, a bare name in a converter parameter, a nameof(...), a
[NotifyCanExecuteChangedFor]/[NotifyPropertyChangedFor] argument, or a plain
string literal built for a runtime Binding. Anything containing the name in any
file would have appeared.

Proof, per symbol

1. RemoveChannelCommand — [RelayCommand] private void RemoveChannel(IChannel)

1. No C# reference. The scan's only RemoveChannel hits are the declaration
   itself and IStreamingDevice.RemoveChannel / AbstractStreamingDevice.RemoveChannel
   — a different member on a different type, called by three unrelated sites
   (ChannelsPaneViewModel.cs:336, Views/MobileShellViewModel.cs:412, and the
   view-model's own now-deleted body). No occurrence of RemoveChannelCommand
   exists anywhere in the repo.
2. No .axaml binding. Zero hits in any .axaml file, in any of the forms
   listed above.
3. No DI registration. App.cs holds the only ServiceCollection; it
   registers 7 services (IFirmwareDownloadService, IHidTransport,
   IExternalProcessRunner, IFirmwareUpdateService, IBootloaderWatcher,
   LoggingManager, IDialogService) and no view-model. DaqifiViewModel is not
   DI-constructed at all — MainWindow.axaml.cs does DataContext = new DaqifiViewModel().
4. No EF / reflection / serialization path. The repo's only runtime
   member-by-name lookups are ConnectionDialogViewModel-targeted test reflection
   and DaqifiViewModel.cs:1789's GetProperty("DeviceVersion"). None names this
   member. It is a private method, not a persisted or serialized property.

Live equivalent, verified reachable: ChannelsPaneViewModel.ToggleChannel
(ChannelsPaneViewModel.cs:326) runs exactly the same two steps on deactivation —
LoggingManager.Instance.Unsubscribe(channel); device.RemoveChannel(channel);. It
is reached through ToggleChannelCommand, bound in both heads:
View/Prototype/ChannelsPanePrototype.axaml:145 (hosted by MainWindow.axaml:360)
and Views/Mobile/ChannelsMobileView.axaml:71.

Correction to the issue: #242 also cites MobileShellViewModel.cs:412 as a live
equivalent. It is not one — that call is a pre-stream reconciliation of the
device's active analog set and does not unsubscribe. The
ChannelsPaneViewModel path alone carries the claim, and it carries it on both heads.

Extra evidence it never ran: the deleted body dereferenced device on the line
before its own if (device != null && ...) check — a guaranteed
NullReferenceException for any channel whose device is not connected. That is a
latent crash no user has hit.

2. ToggleDebugModeCommand — [RelayCommand] private void ToggleDebugMode()

1. No C# reference. The scan found exactly two ToggleDebugMode lines in the
   entire repo: the @port: comment and the declaration. Nothing else, in any file.
2. No .axaml binding. Zero hits.
3. No DI registration. As above.
4. No EF / reflection / serialization path. Private void method.

Live equivalent, verified reachable: MainWindow.axaml:232 binds a
ToggleButton's IsChecked="{Binding IsDebugModeEnabled}". ToggleButton.IsChecked
registers BindingMode.TwoWay by default, so clicking it writes the property and
runs OnIsDebugModeEnabledChanged (DaqifiViewModel.cs:2825), which is where every
side effect lives (WiFi-notification cleanup, SetDebugMode fan-out to devices).
The deleted command's whole body was IsDebugModeEnabled = !IsDebugModeEnabled —
the identical effect through the identical setter. The debug UI it gates
(OpenDebugWindowCommand, the console button, the mobile debug panels) all bind
IsDebugModeEnabled, never the command.

3. LatestFirmwareVersionText — public string => _firmwareCoordinator.LatestFirmwareVersion

Carried forward from #235, re-proved here rather than taken on the handoff's word.

1. No C# reference. The only other LatestFirmwareVersion uses are
   FirmwareUpdateCoordinator.LatestFirmwareVersion (the coordinator property this
   forwarded to, which stays) and DaqifiViewModel.cs:1791, which reads the
   coordinator's property directly rather than going through this wrapper.
2. No .axaml binding. Zero hits in any view, desktop or mobile.
3. No DI registration. As above.
4. No EF / reflection / serialization path. A get-only computed property on a
   view-model; no EF model references it, no reflection names it.

Why deletion is right rather than "bind it": it is an [ObservableProperty]-less
computed property that never raises PropertyChanged — nothing anywhere calls
OnPropertyChanged(nameof(LatestFirmwareVersionText)). Binding it today would
display whatever the coordinator held at first read and never update. It is not
merely unused; as written it does not work.

Upstream note, stated plainly so the reviewer can overrule me: unlike the two
commands above, this member does exist upstream
(daqifi-desktop/Daqifi.Desktop/ViewModels/DaqifiViewModel.cs:1407, identical body).
A GitHub code search across the whole upstream repo returns that single hit — so it
is equally unbound upstream, and removing it costs no behavior. It will, however,
show up in the port's declared-level parity check as a symbol upstream has and the
port does not. If the project would rather keep the declared surface aligned with
upstream, drop this hunk and the other two still stand on their own.

What is not deleted, and why

ShutdownCommand / DaqifiViewModel.Shutdown() is left exactly as it is.

It is genuinely unreachable — the scan's Shutdown hits are all
IAppLogger.Shutdown / AppLogger.Shutdown (a different method on a different
type), Avalonia's own desktop.Shutdown() / ShutdownMode, and test fakes. There
is no ShutdownCommand reference anywhere and no C# caller of the method.

But the issue's own table says its live equivalent is "nothing", and I confirmed
that: App.OnExit() (App.cs:378, wired from App.axaml.cs:27) calls only
AppLogger.Instance.Shutdown(), and the one other exit path,
MainWindow's Closing handler → DaqifiViewModel.Dispose(), tears down timers,
coordinators and event subscriptions but never disconnects devices. So
Shutdown()'s foreach (var device in ConnectedDevices) device.Disconnect(); is not
duplicated by anything.

That makes it a product decision — wire it into app exit, or accept that devices
are never cleanly disconnected — not a dead-code deletion. Deleting it would remove
the evidence that the app-exit disconnect is missing, which is the opposite of what
this routine is for. #242 flags the same decision. Handing it back rather than
guessing.

The @port: backlinks

Verified independently against upstream's default branch via gh api (decoding the
file rather than trusting a sibling checkout on disk, which can carry other agents'
branch worktrees):

• Daqifi.Desktop.ViewModels.DaqifiViewModel.RemoveChannel — 0 hits in upstream's
  DaqifiViewModel.cs. The backlink pointed at nothing.
• Daqifi.Desktop.ViewModels.DaqifiViewModel.ToggleDebugMode — 0 hits. Same.
• Daqifi.Desktop.ViewModels.DaqifiViewModel.LatestFirmwareVersionText — exists
  (line 1407), noted above.
• Daqifi.Desktop.ViewModels.DaqifiViewModel.Shutdown — exists (line 883) as a plain
  public void, not a command. Not touched here.

So for the two commands, deleting them improves the parity ledger: it removes two
backlinks that resolve to no upstream symbol.

Gate

Measured against this branch's own merge-base, a74933d, first-party set only
(third_party/ excluded — it is not rebuilt by these projects and contributes no
first-party warning):

	merge-base a74933d	this branch	delta
Daqifi.Avalonia.Desktop	193	192	−1
Daqifi.Avalonia.iOS (Release, iossimulator-arm64)	194	193	−1

0 warnings added. The one warning removed on each head is consistent with the
deleted RemoveChannel body's unguarded device.DataChannels dereference (CS8602).

dotnet test Daqifi.Avalonia.Tests — 467 passed, 0 failed, 0 skipped.

The Android head is not buildable on this Mac; CI covers it. Nothing in this diff is
platform-specific — it deletes three members from a shared view-model with no
remaining references on any head.

packages.lock.json verified unchanged after restore (restore was run without -r).

Coordination

Checked against PR #239, which edits this same file heavily: #239 touches
DaqifiViewModel.cs around line 220 (_firmwareUploadCts) and 2476–2562
(UpdateWifiFirmwareOnly). This PR touches lines 1029–1040, 1587–1593 and
2857–2866. No overlap, and none of the deleted members is entangled with the
firmware-cancel work. tools/parity-audit/ and coverage.py are untouched.

🤖 Generated with Claude Code

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Remove unreachable DaqifiViewModel members

✨ Enhancement 🕐 10-20 Minutes

AI Description

• Removes unreachable channel-removal and debug-toggle relay commands.
• Removes the unused firmware-version display property.
• Preserves shutdown behavior because no replacement disconnects devices during application exit.

Diagram

graph TD
  VM["Daqifi ViewModel"] --> GEN["Relay generator"]
  GEN -. "removed" .-> RC["RemoveChannel command"]
  GEN -. "removed" .-> TC["ToggleDebug command"]
  VM -. "removed" .-> FV["Firmware version text"]
  GEN --> SC["Shutdown command"]

Loading

High-Level Assessment

Direct deletion is appropriate because exhaustive repository scans found no bindings, call sites, dependency injection, reflection, serialization, or generated-command consumers. Retaining ShutdownCommand is the correct exception because its disconnection behavior has no replacement path.

Files changed (1) +0 / -31

Refactor (1) +0 / -31

DaqifiViewModel.csDelete three unreachable view-model members +0/-31

Delete three unreachable view-model members

• Removes the unused RemoveChannel and ToggleDebugMode relay-command methods and the unbound LatestFirmwareVersionText property. Shutdown remains unchanged because it preserves unique device-disconnection behavior.

Daqifi.Avalonia/Daqifi.Desktop/ViewModels/DaqifiViewModel.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 route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[tylerkron]

Shepherd verification — re-derived independently, nothing taken on the builder's word

This is a deletion PR against a view-model that XAML binds by reflection, so the build
gate is structurally blind to its failure mode: a removed member that some .axaml still
binds compiles green on both heads and renders blank at runtime. So the three deletions
were re-proved from scratch rather than the builder's method being audited.

1. Deletions re-proved

Reproduced the builder's approach (it is the right one — scan the base name across
every file, so the [RelayCommand] generator's …Command spelling and every XAML binding
form are covered by construction rather than by guessing syntaxes). All 443 files, every
extension, skipping only .git/, bin/, obj/, node_modules/. Run against this head:

base name	hits	verdict
RemoveChannel	7	all on IStreamingDevice/AbstractStreamingDevice.RemoveChannel — a different member on a different type (interface + impl + ChannelsPaneViewModel.cs:336 + MobileShellViewModel.cs:412 + a test fake). Zero RemoveChannelCommand anywhere, zero .axaml.
ToggleDebugMode	0	nothing left in the repo at all.
LatestFirmwareVersion	3	the coordinator's own declaration, and DaqifiViewModel.cs:1770 reading the coordinator directly, bypassing the deleted wrapper. Zero .axaml.
Shutdown (kept)	17	all AppLogger/IAppLogger.Shutdown, Avalonia's desktop.Shutdown()/ShutdownMode, test fakes, plus its own declaration. No ShutdownCommand reference.

The scanner was validated with positive controls, not assumed to work. ToggleChannel
and IsDebugModeEnabled were scanned alongside, and between them surfaced .axaml hits in
attribute form (Command="{Binding ToggleChannelCommand}"), $parent[…]-traversal form,
InvokeCommandAction form, and element syntax
(<Binding Path="Shell.IsDebugModeEnabled"/> in DevicesPanePrototype.axaml:531). The
scan demonstrably reads XAML and finds element-syntax bindings — exactly the form a
grep over guessed attribute patterns would have missed.

2. ShutdownCommand kept — the reasoning is correct

Verified independently, in both directions:

• The app's only exit wiring is App.axaml.cs:27 → desktop.Exit += … App.OnExit(),
  and App.OnExit() (App.cs:378) is exactly AppLogger.Instance.Shutdown();.
• The other exit path, MainWindow.axaml.cs:31 Closing → DaqifiViewModel.Dispose()
  (line 2938), disposes the disk-space coordinator, two status CTS timers and the
  SD-logging DispatcherTimer, and unsubscribes the singletons and per-device events —
  and never calls Disconnect.
• Repo-wide sweep for ProcessExit / ShutdownRequested / OnExit() returns only those
  two sites.
• The only Disconnect calls in DaqifiViewModel are DisconnectDevice (line 1045, the
  user-initiated per-device command) and Shutdown() itself (line 1060).

So Shutdown()'s foreach (var device in ConnectedDevices) device.Disconnect(); is
genuinely un-duplicated. Deleting it would drop a capability, not remove a duplicate —
keeping it is the right call, and it is this PR's best judgment. Agreed and endorsed.

One accuracy note in passing: Shutdown() carries [RelayCommand] on a public method,
so the generator does emit ShutdownCommand — #242's naming is right, and upstream's
plain public void Shutdown() (no attribute) confirms #242's point that the port
manufactured the command.

3. The parity delta on LatestFirmwareVersionText — recommendation: keep the hunk (delete it)

The builder flagged this against itself so it could be dropped alone. Confirmed the
premise against the default branch via gh api, not by finding the file on disk in a
sibling checkout:

• daqifi-desktop@main, Daqifi.Desktop/ViewModels/DaqifiViewModel.cs:1407 — the member
  exists, identical body. Same fetch shows RemoveChannel and ToggleDebugMode at
  0 hits, and Shutdown at line 883.
• gh search code LatestFirmwareVersionText --repo daqifi/daqifi-desktop returns exactly
  one hit repo-wide: that declaration. It is unbound upstream too.

So the delta is between two dead symbols. My recommendation is to delete it, for three
reasons:

1. The parity that matters is behavioral. This member produces no behavior on either
   side. Holding a declared-symbol count level buys a number, not a capability.
2. Keeping it is worse than neutral — it is a trap. It is a computed property with no
   change notification: nothing calls
   OnPropertyChanged(nameof(LatestFirmwareVersionText)), and the coordinator's
   LatestFirmwareVersion is a plain => _latestFirmwareVersion getter. Bind it today and
   you latch whatever the coordinator held at first read, forever. The next person who
   needs the latest firmware version on screen would find a member that looks ready and
   silently is not. Deleting it forces them to add a properly-notifying one.
3. On net this PR improves ledger accuracy. It removes two @port: backlinks
   (RemoveChannel, ToggleDebugMode) that I confirmed resolve to no upstream symbol at
   all, against one true-but-dead symbol removed. Two false backlinks retired, one
   accurate row lost.

If the reviewer disagrees, the cost of overruling me is low and contained: it is one
self-contained 8-line hunk, and dropping it leaves the other two deletions standing
untouched.

4. Warning delta — closed from both ends

The numbers here are unusually checkable, because the diff is a pure 31-line deletion with
0 added, so the delta should be attributable to specific removed lines. It is:

• From the diff: the only deleted line capable of emitting a warning is
  var channel = device.DataChannels.FirstOrDefault(…), which dereferences device — a
  FirstOrDefault result — one line before its own if (device != null && …) guard.
• From CI on merge-base a74933d:
  DaqifiViewModel.cs(1034,23): warning CS8602: Dereference of a possibly null reference.
  Line 1034, column 23 is precisely that device token. Counts: desktop 193,
  iOS 194, Android 199.
• From CI on head cce4fcb: that warning is gone — zero occurrences of
  DaqifiViewModel.cs(1034,23), and zero CS8602 in DaqifiViewModel.cs at all. Counts:
  desktop 192, iOS 193, Android 198.

−1 on every head, attributable to one named line, 0 added. Both the merge-base and head
figures are lifted from CI's own published logs, not from the PR description.

Worth stating plainly: that CS8602 was a live NullReferenceException for any channel
whose device was not in ConnectedDevices. This deletion removes a latent crash, which is
itself further evidence the command never ran.

5. #239 interaction — built, because MERGEABLE is not enough

#239 also edits DaqifiViewModel.cs heavily (it deletes _firmwareUploadCts and its
three-site lifecycle and narrows UpdateWifiModuleAsync). Both PRs remove members from
the same file, and #189 previously sat CLEAN with zero overlap and was still ejected
from the merge queue with CS0115. Nobody had built this pair, so I did — merging #239
head 8fec8d2 into #245 head cce4fcb:

	result
git merge	auto-merged DaqifiViewModel.cs, no conflict
Desktop	succeeded, 0 errors, 192 warnings — identical to #245 alone
iOS Release / iossimulator-arm64	succeeded, 0 errors, 193 warnings — identical to #245 alone
dotnet test	472 / 472 passed (#245's 467 + #239's 5 new cancellation tests)

No CS0115, no warning regression on either head. The pair is safe to land in either
order. packages.lock.json confirmed unchanged after restore (restore run without -r).

6. #242's evidence — correction confirmed and forwarded

The builder filed a correction against the issue that filed it; spot-checked it because
#242's evidence came from coverage.py --bindings (#244) and a systematic flaw there
would affect future tickets.

Confirmed. #242 says MobileShellViewModel.cs:412 calls device.RemoveChannel(channel)
"after unsubscribing". It does not — that call sits in a pre-stream reconciliation loop
that aligns the device's active analog set with the current selection, and there is no
LoggingManager.Instance.Unsubscribe in that loop or anywhere around it.
ChannelsPaneViewModel.cs:326 alone carries the claim, and it does carry it (it runs
Unsubscribe then RemoveChannel on deactivation, on both heads). The verdict is
unaffected; one of its two citations was wrong.

The mechanism looks systematic rather than a slip — the sites were matched on the callee
name RemoveChannel while ignoring the declaring type, which is how a call on
IStreamingDevice got credited as a live equivalent for a view-model command. Recorded on
#244 for its author, since that is the direction that produces false "already covered, safe
to delete" conclusions: #244

7. Verification basis

I pushed nothing, so the gate behind this note is CI's own run on cce4fcb, not a local
re-run — re-deriving a tree CI has already built buys nothing and contends for a lock a
builder needs. I read the CI log rather than trusting the reported numbers; every warning
figure quoted above is lifted from it. The only builds I ran locally were the #245+#239
merge in §5, which CI has never seen.

CI on cce4fcb — 6 SUCCESS, 1 SKIPPED (Dependabot claims check; Android and iOS both
SUCCESS). cce4fcb is the current head, so this green covers the tree under review.

Qodo: 0 bugs / 0 rule violations / 0 requirement gaps, and 0 unresolved inline review
threads (thread filter validated against the #196 control, which returns its expected 3).
Both surfaces re-checked after settling — 20 minutes on, byte-identical, still anchored to
cce4fcb.

[tylerkron]

Qodo-clean, CI green — ready for review, on head cce4fcbc895cbb3620f7d0da2e00cf192869e519 (every check-run's own head_sha is that SHA; 6 SUCCESS, 1 SKIPPED, Android and iOS included).