[tylerkron]

Closes #444.

Which option I took, and why

Deleted the settings and the unreachable handling. The issue offered an alternative — wire one up if the UI appears to offer a non-default and never plumbs it through — and I checked for that specifically. It does not: DeviceLogsViewModel exposes exactly four commands (RefreshFilesCommand, ImportFile, ImportAllFiles, CopyDiagnosticInfoAsync), there is no per-file delete, no overwrite prompt and no rename-on-import anywhere in the .axaml. There is no gap behind these three settings, so there was nothing to report instead and nothing to leave for its own issue.

The issue's body matches what I found; it contradicted nothing in my brief. Its one sequencing condition — wait for #437 — was already satisfied (#437 merged as 6dff1d8 on 2026-10-27), and its RecordingSessionImporter fake loses the parameter in this PR as instructed.

How the dead-setting claim was verified

A git grep 'new ImportOptions(' is not sufficient here — this codebase uses target-typed new(...), so the call can be spelled ImportOptions opts = new() { ... } and a type-name grep misses it. So I searched three ways, repo-wide, third_party/ excluded:

search	result
each property name (DeleteFromDeviceAfterImport, OverwriteExistingSession, SessionNameOverride)	0 production setters. Only 4 test setters, all OverwriteExistingSession = true in SdCardImportAtomicityTests.cs
every bare new() in the repo	100+ hits, not one of them an ImportOptions
options: named arguments, and tools/ + all three heads (Desktop, Android, iOS)	nothing; the only options: hits outside tests are FirmwareUpdateServiceConfig and wifiOptions

All three production call sites pass null positionally (DaqifiViewModel.cs:1504, DeviceLogsViewModel.cs:592 and :690), so every import the app has ever performed ran with false, false, null.

Defaults, precisely: DeleteFromDeviceAfterImport → false (so the delete block never ran), OverwriteExistingSession → false (so superseded was always [] and ReplaceOverwrittenSessions always returned true immediately, making OutcomeGuidance always string.Empty on that branch), SessionNameOverride → null (so the ?? "SD Import - …" fallback always won). Only the branches that could not be taken are gone; the live path is untouched.

What this deletes

• ImportOptions and its three properties.
• The options parameter on ISdCardSessionImporter.ImportFromDeviceAsync, ImportFromDeviceAsync, ImportFromFileAsync and ImportSessionAsync, plus both options ??= new ImportOptions() lines.
• The delete-after-import block.
• The superseded-session selection in CreateSession, which now returns a LoggingSession instead of a (Session, Superseded) tuple.
• ReplaceOverwrittenSessions and the OutcomeGuidance ternary that called it.

−89 production lines, −28 test lines (−117 net). The importer goes from 1,069 to 980 lines.

Kept deliberately:

• RemoveSessions — two callers remain (the empty-log path and the threw-before-first-batch path). Its doc said "three callers"; now it says two. Its parameter is a collection that is now never given more than one element; narrowing it to a single int is a real follow-on cleanup but would change the generated SQL from IN (…) to =, so I left it out of a behaviour-preserving PR rather than widen the blast radius.
• ImportAllOutcome.Notices — SD import: one NaN sample fails the whole import; the live path drops it (#294/#298), the importer has no guard #412's dropped-NaN notice is now its only producer. Several comments used the overwrite duplicate as their worked example; they now cite the NaN case, which is what actually reaches them.
• IStreamingDevice.DeleteSdCardFileAsync — worth flagging: the deleted block was its only production consumer, so the device-side capability is now reachable only from test doubles. I did not follow the chain and delete it. It is a real, Core-backed device capability that a future "delete file" affordance would use, and removing a device API is a different decision from removing a dead option.

Two comments that survived the mechanical edit were stale on a second read and are fixed here: ImportSessionAsync warned that reporting success "would let the device path delete the source file" — nothing deletes it any more — and the SessionPersisted doc carried the same parenthetical.

The four // @port: markers on the deleted class go with it. The file header says not to delete markers manually, but a marker whose symbol no longer exists is a dangling link; the correspondence map itself is untracked.

Verification

A real SD import, before and after. Rather than trust the tests, I pulled a log straight off the bench Nyquist over USB serial (SYSTem:STORage:SD:GET, 5,416 bytes, echo prefix stripped and the __END_OF_FILE__ marker truncated) and ran it through ImportFromFileAsync against a real migrated SQLite database on both sides of the change:

	before	after
SamplesImported	540	540
TimestampQuality.TotalEntries	271	271
sample rows / channels	540 / AI0,AI1	540 / AI0,AI1
sum of all values	5998	5998
first / last tick	639244735560083724 / 639244735582679724	identical
SessionPersisted, Status, OutcomeGuidance	True, Complete, ''	identical

The two digests diff clean — no difference at all, down to individual row values and colours.

Builds and tests. ~/.dotnet/dotnet test Daqifi.Avalonia.Tests → 1227 passed, 0 failed. Desktop head, iOS head (-r iossimulator-arm64 --no-restore) and Android head (-p:AndroidSdkDirectory) all build with 0 errors; no lock file is dirtied. No new compiler warnings in the three changed files — the remaining ones there are pre-existing sites outside every hunk.

Test coverage that was at risk

The four overwrite tests exercised a path production cannot reach, so they go. But one of them was pinning something real — that an import must never destroy the committed batches of a concurrent import of the same file — so rather than drop that coverage I re-expressed it through the surviving default path. Three tests replace the four:

• An_import_leaves_a_session_another_import_is_writing_alone — the concurrency invariant, now asserted from the outside instead of through the option.
• An_import_that_fails_leaves_an_earlier_session_of_the_same_name_alone — a failed import removes or flags its own row and no other.
• Importing_the_same_file_twice_keeps_both_sessions — states the behaviour that dropping overwrite makes permanent: a re-import adds a session rather than replacing one, so a future change to CreateSession cannot quietly make it destructive.

SdCardImportNaNTests, SdCardImportTimestampFrequencyTests and ImportAllSummaryTests only lose an argument or an example sentence.

Not a hidden parity gap

Worth ruling out explicitly, since "dead code" and "a feature we never ported" look identical from inside this repo: none of the three is an upstream capability we simply failed to wire up. Checked against the daqifi-desktop checkout on main:

property	every upstream reference
DeleteFromDeviceAfterImport	Daqifi.Desktop/Loggers/SdCardSessionImporter.cs only — the branch at :205 and the property at :607. No UI, no test.
OverwriteExistingSession	same file only — :525 and :608. No UI, no test.
SessionNameOverride	the importer (:521, :609) plus one test, SdCardSessionImporterTests.cs:118. No UI.

So upstream never wired any of them to a user-facing control either. Deleting them erases no capability the Windows app has and this port lacks, and it costs nothing for parity — it shrinks what the Core SD-log pipeline migration (#334) would have to carry.

🤖 Generated with Claude Code

[qodo-code-review]

ⓘ Qodo reviews are paused because your workspace is out of credits. Ask your workspace admin to add credits to resume reviews. Manage billing