feat: surface recorded bandwidth history, expose the driver filter, and make the progress bars real - #1704
Merged
Conversation
…story-and-feedback
…nd make the progress bars real Four verified issues, each re-confirmed against current source before any code was touched. #1609 Bandwidth Monitor wrote a throughput sample to disk every ~5s and pruned to a 7-day window, but LoadAsync and Downsample had ZERO production callers — so the file grew for a week and the user could never see any of it. The service's own doc comment promised the opposite ("so the Bandwidth Monitor can draw the last hour/day/week"). Added a range picker (Live plus last hour / 24 hours / 7 days, capped at the service's own retention so no range can promise data that was pruned) and a summary of what actually moved over the period. The summary integrates each rate over the gap to the next sample rather than summing rates, and skips gaps longer than 4x the write cadence: samples exist only while the tab is open, so crediting the last known rate across a closed-tab hour would fabricate ~3.7 GB of traffic. The axis labeller adapts to the range too — a bare "HH:mm:ss" repeats seven times across a week and hides which day a spike was on. #1602 Drivers had a working HideSystemDrivers filter — change handler, filtering logic, and the status text that adjusts the count — with zero bindings in the view. Nobody could reach it. Now a toolbar checkbox. Also split the empty state: on a machine where every driver is Microsoft-supplied, the single shared state told the user to click a button they had already clicked (same defect I fixed in the Logs tab). #1604 Disable recorded a service's previous startup type in a property on ServiceEntry — and GetAllServices builds new instances on every scan. So Disable, Refresh, Enable brought an Automatic service back as Manual (StartTypeToScToken maps an unknown value to "demand") while the status line reported success. Silent, unrequested change to the machine's configuration. Now persisted via ServiceStartupLedgerService, same shape as the three sibling preference services. Rehydration applies ONLY to services Windows currently reports as Disabled, so if the user re-enabled one outside SysManager a stale entry cannot override reality. #1603 CpuAffinity, DisplayProfile and StandbyMemory each drew a progress bar bound to IsBusy that their VM never assigned — structurally incapable of appearing, and the sidebar spinner was dead for them too. TimerResolution correctly needs none (sub-millisecond calls), so it is left alone, and Standby's 2s auto-purge tick is deliberately excluded with a comment so the next refactor does not "fix" the asymmetry back. PROPAGATE: grepping every view that binds IsBusy found FOUR more with the identical defect that the issue never named — AudioMixer, Cleanup, NotificationBlocker, Privacy. Fixed the class, not the instances; the sweep now reports zero dead bars app-wide. Cleanup was the worst of them: SFC and DISM run for minutes behind a bar that could not appear. Its flag is DERIVED from the four per-operation flags rather than assigned per command, so overlapping operations cannot clear the bar out from under each other, and it stays determinate for SFC/DISM which report a real percentage. DisplayProfile's overlapping mode loads got a generation counter for the same reason. Verification, not assertion. A 41-check console harness ran against the BUILT assembly (allowed on this workstation; not the xUnit suite): ledger persistence across instances, the history round-trip through a real temp file, range filtering, downsampling, the gap-integration maths, axis labels, the driver predicate, live IsBusy transitions, and DI resolution — 41/41, and the DI check exists because nothing else in the suite exercises the container and I changed two container-resolved constructors. Two harness findings were mine, not the code's. It seeded history samples out of order, violating a precondition both history services document ("append-only and time-ordered") and that LoadAsync relies on to avoid parsing a 120k-line file per load; the production writer always stamps DateTime.Now, so the file cannot be out of order. Fixed the harness, then made the precondition mechanical with an assert in the test seeding helper so a future test cannot silently reproduce it. It also hung driving Cleanup's pre-scan, which on this machine walks 20,776 temp files (13.4 GB) plus 163,188 in the Recycle Bin — measured, and precisely why that bar was worth fixing. Those assertions were reshaped to not depend on the developer's disk state. BandwidthHistoryService's directory is now injectable (matching VolumePresetService and the other preference services), so tests exercise the real append/load/prune paths without touching the user's own history file. ResourceHistoryViewModel's private RangeOption record became the shared HistoryRange model rather than being duplicated, so the two pickers cannot drift. All four projects build with 0 warnings and 0 errors. Full leak scan against all 32 patterns in leak-terms.txt: zero hits. Author headers verified on every one of the 20 touched files.
…ayProfile's chained init Two CI failures, both mine (2 failed / 4033). CleanupViewModelTests.IsProgressIndeterminate_TogglesCleanly — a long-standing test asserting the flag starts false. PreScanAsync set IsBusy/IsProgressIndeterminate BEFORE its first await, so the CONSTRUCTOR returned with the bar already on. The test was right and the code was wrong: the tab is not busy until the scan is actually running, and construction should have no visible side effect. Fixed with an await Task.Yield() before the assignment, not by touching the test. My own new test had asserted the synchronous raise, so it was corrected to state the real contract. DisplayProfileViewModelTests.AfterInit_TheBusyFlagIsClear — my own test, wrong about the seam. Init CHAINS: LoadDisplaysAsync assigns SelectedDisplay, whose handler calls InitializeAsync again, which REPLACES InitializationComplete with the mode-load task. Awaiting the handle once therefore returns while the mode load is still running with the flag legitimately raised — the code was correct, the assertion was racing. Re-reading the property after the first await yields the second task; awaiting that too makes it deterministic. Checked the class, not the instance: swept every VM I touched for the same "assign IsBusy before the first await" shape. It is present in CpuAffinity, DisplayProfile, Privacy and NotificationBlocker too, but none of their tests probe the raw constructor (all await init first), so only Cleanup could fail. Left as-is rather than churning four more files on a pattern nothing observes; Cleanup is the one with a test pinning the contract. Verified, not assumed: both cases were added to the runtime harness and now pass against the built assembly — 43/43. All four projects build with 0 warnings and 0 errors.
…racing the constructor Second CI failure on the same test, so the previous fix was wrong rather than incomplete. Task.Yield() before raising the flag was a timing fix for a structural problem. The pre-scan is fire-and-forget from the constructor, so whether the flag was observable at construction depended on whether that continuation resumed first — the yield posts to the thread pool, which the harness lost and CI won. It passed locally and failed on CI for exactly that reason, and it also broke my own new test, which had asserted the synchronous raise. Removed the timing dependency instead of adjusting it: PreScanAsync takes an explicit reportProgress flag. The user-pressed Rescan passes true (a button press must visibly do something), and the startup scan passes false — its progress is already visible in the "Scanning…" size labels, which is why nobody missed a bar there. The flag's value at construction is now a property of the code path, not of scheduler timing. Found two more call sites the compiler flagged: the re-scans after Temp cleanup and after emptying the Recycle Bin. Both run while the operation's own flag still holds the bar (it clears in finally), so both pass false — otherwise the refresh would take the bar over from the operation that is still running. The finally now only hands the flag back to the derived value when this call raised it. Verified across 200 constructions rather than one: a single green run cannot distinguish "fixed" from "won the race", which is precisely how the first attempt escaped. 200/200 return with the bar off; the full harness is 43/43 against the built assembly. All four projects build with 0 warnings and 0 errors.
…story-and-feedback
laurentiu021
added a commit
that referenced
this pull request
Aug 5, 2026
… chart buffers CI failed with "Collection was modified; enumeration operation may not execute" thrown from LiveCharts' CollectionDeepObserver. A real concurrency bug in the history feature I added in #1704, not a test artifact. The poll loop appends to _downBuffer/_upBuffer every second while ReloadHistoryAsync rebuilds those same buffers with ReplaceWith, and LiveCharts observes both. The existing guard tested ShowingHistory, which is only assigned AFTER the load's await completes — so the window between entering the reload and that assignment was unguarded. In the app both run on the UI dispatcher and interleave harmlessly; a test host has no dispatcher, so ConfigureAwait(true) resumes on the thread pool and the two genuinely overlap. Now gated on a _historyOwnsChart flag claimed BEFORE the first await and released on the cancellation path, so a cancelled load cannot leave the live chart permanently frozen. I was wrong about the cause first. My initial diagnosis was that ReplaceWith let a Reset subscriber observe the collection mid-rebuild — plausible, and it matched the stack trace's shape, but the harness disproved it: those assertions passed against the UNFIXED helper, because notification suppression already means the Reset fires after the rebuild. The stack trace says OnItemsAdded, an Add notification, which is the poll loop, not the replace. Corrected the fix and the comment rather than shipping a rationale that reads convincingly and is false. Investigating it did surface two genuine ReplaceWith defects, both now fixed and pinned: passing the collection as its own source emptied it (Items.Clear() runs before anything is added back, and the lazy source was that same collection), and a source that threw part-way left the collection half-replaced while still bound to the UI. Both are red against the old code and green against the new one — 20 call sites pass a lazy LINQ query, so the shape was reachable. What is NOT claimed: the harness does not reproduce the race. 60 reload cycles with the poll loop live pass against the unfixed code too, so that check is labelled a smoke test, not a red/green proof. The race has only ever been observed on CI, so CI is where this fix has to be judged. All four projects build with 0 warnings and 0 errors.
laurentiu021
added a commit
that referenced
this pull request
Aug 5, 2026
…mismatch cannot recur (#1706) * fix: align the version with the 1.57.0 tag and add a CI guard so the mismatch cannot recur The v1.57.0 release failed at "Extract release notes from CHANGELOG", leaving a pushed tag with no published release. Root cause is mine, not the pipeline's. I titled #1704 feat:, so auto-release computed a MINOR bump and tagged v1.57.0 — while I had hand-written the csproj version and the CHANGELOG header as 1.56.15, assuming a patch. release.yml then looked for a "## [1.57.0]" section, found none, and correctly refused rather than publishing a release with placeholder notes. The guard did its job; the inputs were wrong. Fixed by moving the version to what the tag actually says: csproj Version/FileVersion/ AssemblyVersion to 1.57.0, and the CHANGELOG header renamed. The notes themselves are unchanged and already describe exactly what shipped — the feature additions are also what justify a minor bump, so 1.57.0 is the correct number and 1.56.15 was the mistake. Nothing about the release is rewritten: the tag stays, and pushing this lets the release be re-dispatched against it. Turned the failure into a mechanical rule rather than a resolution to remember. CI now cross-checks, on every PR, that Version/FileVersion/AssemblyVersion agree with each other and that the newest CHANGELOG header matches — the exact drift that reached a published tag today, caught where it costs one edit instead of a stuck release. Nothing in the repo verified this before; that absence is why it got through. The check was proven, not assumed. Run against three fixtures with the real PowerShell host: the corrected tree passes; the actual bug (csproj 1.56.15 vs CHANGELOG 1.57.0) exits 1 with "disagree"; and a partial edit that updates Version but leaves FileVersion behind exits 1 with "FileVersion (1.56.15.0) does not match Version (1.57.0)". Red before, green after. * fix: stop the bandwidth poll loop from racing a history reload on the chart buffers CI failed with "Collection was modified; enumeration operation may not execute" thrown from LiveCharts' CollectionDeepObserver. A real concurrency bug in the history feature I added in #1704, not a test artifact. The poll loop appends to _downBuffer/_upBuffer every second while ReloadHistoryAsync rebuilds those same buffers with ReplaceWith, and LiveCharts observes both. The existing guard tested ShowingHistory, which is only assigned AFTER the load's await completes — so the window between entering the reload and that assignment was unguarded. In the app both run on the UI dispatcher and interleave harmlessly; a test host has no dispatcher, so ConfigureAwait(true) resumes on the thread pool and the two genuinely overlap. Now gated on a _historyOwnsChart flag claimed BEFORE the first await and released on the cancellation path, so a cancelled load cannot leave the live chart permanently frozen. I was wrong about the cause first. My initial diagnosis was that ReplaceWith let a Reset subscriber observe the collection mid-rebuild — plausible, and it matched the stack trace's shape, but the harness disproved it: those assertions passed against the UNFIXED helper, because notification suppression already means the Reset fires after the rebuild. The stack trace says OnItemsAdded, an Add notification, which is the poll loop, not the replace. Corrected the fix and the comment rather than shipping a rationale that reads convincingly and is false. Investigating it did surface two genuine ReplaceWith defects, both now fixed and pinned: passing the collection as its own source emptied it (Items.Clear() runs before anything is added back, and the lazy source was that same collection), and a source that threw part-way left the collection half-replaced while still bound to the UI. Both are red against the old code and green against the new one — 20 call sites pass a lazy LINQ query, so the shape was reachable. What is NOT claimed: the harness does not reproduce the race. 60 reload cycles with the poll loop live pass against the unfixed code too, so that check is labelled a smoke test, not a red/green proof. The race has only ever been observed on CI, so CI is where this fix has to be judged. All four projects build with 0 warnings and 0 errors. --------- Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four verified issues plus the propagated class of one of them. Every claim in each issue was re-confirmed against current source before any code was written.
Closes #1609
Closes #1602
Closes #1604
Closes #1603
#1609 — Bandwidth history was written for 7 days and never read
BandwidthMonitorViewModelwrote a throughput sample every ~5s and pruned to a 7-day window, butLoadAsyncandDownsamplehad zero production callers — so the file grew for a week and the user could never see any of it. The service's own doc comment promised the opposite: "so the Bandwidth Monitor can draw the last hour/day/week".Chose option (a) from the issue (surface it) over (b) (delete the writer), because "where did my data cap go?" is exactly what the target persona asks and the collection half was already shipped.
RetentionDaysso no range can promise data the pruner already deleted (there is a test asserting that invariant).Downloaded 4.2 GB · Uploaded 380 MB · Peak ↓ … ↑ ….The maths matters here. Each sample is a rate, not a volume, so the totals integrate each rate over the gap to the next sample rather than summing them. Gaps longer than 4× the write cadence are skipped entirely: samples exist only while the tab is open, so crediting the last known rate across a closed-tab hour would fabricate ~3.7 GB of traffic the user never used. Both behaviours are pinned by tests.
The axis labeller now adapts to the range — a bare
HH:mm:ssrepeats seven times across a week and hides which day a spike was on.#1602 — the Drivers filter had no UI at all
HideSystemDrivershad a change handler, filtering logic, and the status text that adjusts the count — and zero bindings inDriversView.xaml. Nobody could reach it. Now a toolbar checkbox matching theTaskSchedulerView/ContextMenuViewpattern.Also split the empty state while in there: on a machine where every driver is Microsoft-supplied, the single shared state told the user to click a button they had already clicked. Same defect I fixed in the Logs tab, so it uses the same two-state shape (
HasNotScannedvsHasNoResults).#1604 — Enable silently downgraded Automatic services to Manual
Disable recorded the previous startup type in a plain property on
ServiceEntry, andGetAllServicesbuilds new instances on every scan. So: disable an Automatic service, refresh (or restart), press Enable → it came back as Manual, becauseStartTypeToScTokenmaps an unknown value todemand, and the status line reported success the whole time. A silent, unrequested change to the machine's configuration.Now persisted by
ServiceStartupLedgerService, the same shape as the three sibling preference services (injectable directory, pureSerialize/Parse, IO that never throws). Two deliberate constraints:Disabledvalue is refused rather than stored, so a restore Windows would reject is never attempted.Disabled. If the user re-enabled one outside SysManager, the machine is the authority — a stale ledger entry cannot override it.#1603 — three progress bars that could not appear, and four more the issue never found
CpuAffinityView,DisplayProfileViewandStandbyMemoryVieweach drew aProgressBarbound toIsBusythat their VM never assigned (grep -c IsBusy→ 0 for all of them). Structurally incapable of appearing, and sinceNavItem.WireBusyforwards the same flag, the sidebar spinner was dead for those tabs too — while the work behind them is genuinely slow (enumerating every process and reading its affinity;ChangeDisplaySettingsExblocking for seconds while the panel re-trains, possibly on a black screen).Following the issue's own guidance,
TimerResolutionis left alone — its calls are sub-millisecond, so a bar there would only flicker. Standby's 2 s auto-purge tick is likewise excluded, with a comment and a test so the next refactor doesn't "fix" the asymmetry back.Propagate
Grepping every view that binds
IsBusyagainst its VM found four more with the identical defect that the issue never named: AudioMixer, Cleanup, NotificationBlocker, Privacy. Fixed the class rather than the instances — the sweep now reports zero dead bars app-wide.Cleanup was the worst of them: SFC and DISM run for minutes behind a bar that could not appear. Its flag is derived from the four per-operation flags rather than assigned per command, so two overlapping operations can't clear the bar out from under each other, and it stays determinate for SFC/DISM which report a real percentage through the runner.
DisplayProfile's overlapping mode loads got a generation counter for the same class of reason: only the newest load may release the flag.Verification
Local
dotnet testis not run on this workstation, so behaviour was proven by a 41-check console harness against the built assembly (not the xUnit suite): ledger persistence across instances, the history round-trip through a real temp file, range filtering, downsampling, the gap-integration maths, axis labels, the driver predicate, liveIsBusytransitions, and DI resolution. 41/41 passed. The DI checks exist because nothing else in the suite exercises the container and this PR changes two container-resolved constructors — a wiring break would otherwise only surface at app startup.Two harness findings turned out to be mine, not the code's, and are worth stating since they shaped the tests:
LoadAsyncrelies on to stop at the window boundary instead of parsing a 120k-line file per load. The production writer always stampsDateTime.Now, so the file cannot be out of order. Fixed the harness — then made the precondition mechanical with an assert in the test seeding helper, so a future test can't silently reproduce it and read as a range-filtering bug.Also in scope, both to avoid duplication rather than as drive-by refactoring:
BandwidthHistoryService's directory is now injectable (matchingVolumePresetServiceand the other preference services), so tests exercise the real append/load/prune paths without touching the user's own history file.ResourceHistoryViewModel's privateRangeOptionrecord became the sharedHistoryRangemodel instead of being copied into a second tab, so the two pickers can't drift.Checks run
leak-terms.txt: zero hits.Console.WriteLine/TODO/HACK/MessageBox.Show; the two genericcatch (Exception)blocks flagged in the touched files are pre-existing async-void/poll-loop last-resort nets, confirmed absent from this diff.Deferred to the secondary workstation
Launching the published
.exe, FlaUI, and screenshot recapture — this workstation never runs the app. The Bandwidth Monitor range picker and the Drivers checkbox are new visible UI, so a screenshot pass is worth doing there.Docs
README (Bandwidth Monitor + Drivers sections), ARCHITECTURE (the new ledger service and the bandwidth read path, including why the gap rule exists), CHANGELOG, and the version bump to 1.56.15 are all in this PR.