Skip to content

refactor: delete the keep-awake service abstraction nothing ever wired - #235

Merged
tylerkron merged 2 commits into
mainfrom
dead-code/unreachable-sweep
Sep 4, 2026
Merged

tylerkron merged 2 commits into
mainfrom
dead-code/unreachable-sweep

Conversation

@tylerkron

@tylerkron tylerkron commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Not merging — for review.

Services/KeepAwake/ is a four-file abstraction that no code path can reach. Deleting it removes the second, dead copy of the Windows keep-awake logic the app already performs inline.

 Daqifi.Avalonia/Daqifi.Desktop/Device/AbstractStreamingDevice.cs              |  2 +-  3 -
 Daqifi.Avalonia/Daqifi.Desktop/Services/KeepAwake/IKeepAwakeService.cs        |  0 +- 21 -
 Daqifi.Avalonia/Daqifi.Desktop/Services/KeepAwake/KeepAwakeServiceFactory.cs  |  0 +- 14 -
 Daqifi.Avalonia/Daqifi.Desktop/Services/KeepAwake/NoOpKeepAwakeService.cs     |  0 +- 15 -
 Daqifi.Avalonia/Daqifi.Desktop/Services/KeepAwake/WindowsKeepAwakeService.cs  |  0 +- 26 -

5 files changed, 2 insertions(+), 79 deletions(-) — net −77 lines, 4 files removed.

Why it is dead

AbstractStreamingDevice.InitializeStreaming and StopStreaming call SetThreadExecutionState through their own file-local NativeMethods shim (lines 1537 and 1680). That is the live path. WindowsKeepAwakeService wraps the same kernel32 API, and nothing calls it — so the repo carries two implementations of one behaviour and only the inline one ever runs. This deletes the copy that does not.

Proof, per symbol

Symbol C# references XAML DI Reflection / serialization
KeepAwakeServiceFactory none — the string occurs exactly once repo-wide, its own declaration none none none
WindowsKeepAwakeService only new-ed inside that unreachable factory none none none
NoOpKeepAwakeService only new-ed inside that unreachable factory none none none
IKeepAwakeService only the factory return type + the two backends' base list none none none

How each column was checked:

  • C# references — grep -rn for every type name and for the member names PreventSleep / AllowSleep across the whole repo excluding .git, obj, bin and third_party/. The complete set of hits outside Services/KeepAwake/ was two prose comment lines in AbstractStreamingDevice.cs. Since nothing calls Create(), the factory is the only entry point into the cluster and it is unreachable, so the whole graph is closed.
  • XAML — grepped every .axaml for each type name, for PreventSleep/AllowSleep, and for x:Key / StaticResource / DynamicResource mentions. Zero hits. (Worth stating explicitly: views here without x:DataType bind by reflection, so a zero-C#-reference member can still be live. Not the case for any of these — none is a view-model member and none is named in markup.)
  • DI — App.cs holds the only ServiceCollection in the app; it never registers IKeepAwakeService. No other AddSingleton/AddTransient/AddScoped site exists outside tools/ and the per-platform icon providers.
  • Reflection / serialization / EF — none of these types is an EF entity, appears in a migration or the model snapshot, carries a converter, or is named in any .csproj, .json or .xml. The project uses default compile globbing, so no build file lists them individually.

The one outside mention was a comment in InitializeStreaming calling the abstraction the "proper wiring". It is reworded so it no longer names deleted types; the surrounding behaviour is untouched.

Verification

  • Desktop build: 235 → 235 warnings, 0 errors. Per-code tallies are byte-identical before and after (both measured cold with --no-incremental in this worktree against merge-base 5414c07). No delta to attribute — the deleted files were warning-free.
  • iOS Release iossimulator-arm64: 236 warnings, 0 errors, measured cold. A warm rebuild reports 216; the 20-warning gap is the vendored oxyplot re-emitting only when its own obj/ is cold, not a real change. Both figures were reproduced here to confirm that is all it is.
  • dotnet test: 462 passed, 0 failed.
  • Android head is not buildable on this Mac; CI covers it. The deleted namespace has no Android references, so no head-specific risk.

Restore introduced no packages.lock.json changes.

Scope

Touches only Services/KeepAwake/ and one comment in AbstractStreamingDevice.cs — disjoint from every open PR (#224/#225/#226/#233 tests + DiskSpace + PlotLogger, #227 converters, #229 tools/+docs/, #232 SummaryFlyout.axaml), each checked with gh pr diff --name-only.

Review history

Qodo round 1 flagged that the reworded comment claimed non-Windows heads "log-and-skip" while the guard has no else branch and logs nothing. Correct, and worth recording where it came from: the original comment attributed that behaviour to NoOpKeepAwakeService, which also only returned true without logging — so the claim was already wrong, and rewording it into a statement about this code made it directly misleading. Fixed in 74829dd; the comment now says other heads skip the block entirely and log nothing.

Noted, not acted on

DaqifiViewModel.LatestFirmwareVersionText (line 1593) also has zero C# and zero XAML references. Left alone — it belongs to a live class rather than this cluster, and folding an unrelated one-off into a deletion PR would weaken the argument. Flagging it for whoever picks up the next pass.

Services/KeepAwake/ holds a four-file abstraction — IKeepAwakeService, a
platform factory, a Windows SetThreadExecutionState backend and a no-op
fallback — that no code path reaches. AbstractStreamingDevice.InitializeStreaming
and StopStreaming call the same kernel32 API through their own local
NativeMethods shim instead, so the repo ships two copies of the Windows
keep-awake logic and only the inline one runs.

Proof, per symbol:
- KeepAwakeServiceFactory: the string occurs exactly once repo-wide, its
  own declaration. Nothing calls Create(), so nothing can reach either
  backend.
- WindowsKeepAwakeService / NoOpKeepAwakeService: constructed only inside
  that unreachable factory.
- IKeepAwakeService: referenced only by the factory, its two backends and
  a prose comment in AbstractStreamingDevice.
- No XAML binding — grepped every .axaml for each type name, the member
  names PreventSleep/AllowSleep, and StaticResource/DynamicResource/x:Key.
- No DI registration — App.cs is the only ServiceCollection and never
  registers the interface.
- No reflection or serialization path: no EF entity, no converter, no
  JSON contract, not named in any .csproj, .json or .xml.

The one outside mention was a comment in InitializeStreaming pointing at
the abstraction as the 'proper wiring'; it is reworded so it no longer
names deleted types.

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

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Remove unreachable keep-awake service abstraction

✨ Enhancement 🕐 Less than 10 minutes

Grey Divider

AI Description

• Removes the unreachable keep-awake interface, factory, and platform implementations.
• Retains the live Windows sleep-prevention path and clarifies non-Windows behavior.
Diagram

graph TD
  Device["Streaming Device"] --> Guard{"Windows?"} -->|Yes| Native["NativeMethods"] --> API["kernel32 API"]
  Guard -->|No| Skip["Log and skip"]
Loading
High-Level Assessment

Deleting the unreachable abstraction is the best approach because the existing streaming lifecycle already owns and invokes the live keep-awake behavior. Wiring the service instead would expand scope into dependency injection and lifecycle management without adding platform support or changing behavior.

Files changed (1) +3 / -4

Refactor (1) +3 / -4
AbstractStreamingDevice.csRemove references to the deleted keep-awake abstraction +3/-4

Remove references to the deleted keep-awake abstraction

• Rewords the streaming sleep-prevention comment so it describes the retained Windows guard and non-Windows behavior without referencing deleted service types. The existing native keep-awake implementation remains unchanged.

Daqifi.Avalonia/Daqifi.Desktop/Device/AbstractStreamingDevice.cs

@qodo-code-review

qodo-code-review Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Informational

1. Comment claims nonexistent logging ✓ Resolved 🐞 Bug ◔ Observability
Description
The updated comment says non-Windows heads “log-and-skip,” but the surrounding conditional has no
non-Windows branch and performs no logging when OperatingSystem.IsWindows() is false. This can
mislead maintainers about the observability of streaming startup on mobile and other non-Windows
platforms.
Code

Daqifi.Avalonia/Daqifi.Desktop/Device/AbstractStreamingDevice.cs[R1531-1533]

+        // is Windows-guarded, and non-Windows heads log-and-skip. Mobile
+        // sleep-prevention (a wake lock) is a future divergence, not
+        // required to stream.
Relevance

●●● Strong

Recent history accepts correcting misleading comments and observability claims in
AbstractStreamingDevice.

PR-#59
PR-#4

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed comment explicitly says non-Windows heads log-and-skip, while the implementation only
enters the body when OperatingSystem.IsWindows() is true; there is no else branch or non-Windows
log in the surrounding code.

Daqifi.Avalonia/Daqifi.Desktop/Device/AbstractStreamingDevice.cs[1529-1545]

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

## Issue description
The comment claims that non-Windows heads log and skip keep-awake setup, but the code silently skips the Windows-only block and emits no log.

## Issue Context
Keep the comment aligned with the actual behavior after removing the keep-awake abstraction.

## Fix Focus Areas
- Daqifi.Avalonia/Daqifi.Desktop/Device/AbstractStreamingDevice.cs[1531-1533]

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


Grey Divider

Context sources
Review mode: 🚀 Fast: This is a single localized comment-only change with no runtime behavior or control-flow impact, so a lightweight review is sufficient.

Grey Divider

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 ↗

Grey Divider

Qodo Logo

Qodo round 1: the reworded comment claimed non-Windows heads log and
skip, but the guard has no else branch and emits no log when
OperatingSystem.IsWindows() is false. The old comment attributed
'log-and-skip' to NoOpKeepAwakeService (which also only returned true);
restating it as a claim about this code made it directly misleading.

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

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 74829dd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant