Co-authored-by: Copilot <[email protected]>

Copilot-Session: 6ea38c04-6f68-4c42-91d9-8a03b49bdd81

Co-authored-by: Copilot <[email protected]>

Copilot-Session: 6ea38c04-6f68-4c42-91d9-8a03b49bdd81

Co-authored-by: Copilot <[email protected]>

Copilot-Session: 6ea38c04-6f68-4c42-91d9-8a03b49bdd81

Co-authored-by: Copilot <[email protected]>

Copilot-Session: 6ea38c04-6f68-4c42-91d9-8a03b49bdd81

Co-authored-by: Copilot <[email protected]>

Copilot-Session: 6ea38c04-6f68-4c42-91d9-8a03b49bdd81

Co-authored-by: Copilot <[email protected]>

Copilot-Session: 6ea38c04-6f68-4c42-91d9-8a03b49bdd81

Offload direct probe reads, aggregate attempt diagnostics, and prefer live initialization before same-code cache fallback.

Co-authored-by: Copilot <[email protected]>
Copilot-Session: 6ea38c04-6f68-4c42-91d9-8a03b49bdd81

Co-authored-by: Copilot <[email protected]>
Copilot-Session: 6ea38c04-6f68-4c42-91d9-8a03b49bdd81

Delay state pruning until an ID is absent from consecutive settings snapshots, and retry the HRESULT form of invalid continuous VCP ranges.

Review of the maximum-compatibility discovery rework surfaced several defects
introduced by this branch. Each is fixed at its cause rather than at the symptom.

VcpDiscoveryEvidence.Reconcile used "the parsed caps advertise this code" as a
proxy for "a live read was already attempted for this code". Those coincide only
when the caps string is unusable, which is the only case where the probe runs. On
the caps-parsed path no probe runs, so a cached code the caps string omits was
applied to the monitor, advertised to the UI, and never read from hardware — and
its cache entry could never be refreshed. PreferLiveRead now derives from whether
the probe actually touched the code.

A probe reply carrying an unusable range no longer hides the feature: the device
answered, which proves support (unimplemented codes fail with
DDCCI_VCP_NOT_SUPPORTED), so the code stays reachable as it did before the
rework. The reply is recorded on the observation rather than inferred.

Cache evidence could overwrite VcpCodeInfo entries parsed from the capabilities
string, discarding discrete value lists and custom names. Marking support is now
add-if-absent through a single helper, so evidence can only widen capabilities.

Known-good observations had no invalidation: a monitor that loses DDC/CI support
for a code while keeping its DevicePath would keep advertising it indefinitely.
LastSuccessfulUtc was persisted but never read; it now bounds how long an
observation may stand in for a live read. Successful reads restamp it, so a
feature that still works never expires.

Monitor-state pruning deleted entries by absence from the rebuilt settings list.
A missing or corrupt settings.json makes GetSettingsOrDefault persist and return
an empty monitor list that is indistinguishable from a real one, so every
disconnected monitor's saved brightness, contrast, volume and known-good cache
was deleted. Pruning is now driven by the entries the rebuild observably dropped,
so an unreadable snapshot drops nothing.

The discrete VCP stage no longer discards a monitor. Handle liveness is already
decided by the probe and the continuous stage; repeating the decision after the
continuous values were applied meant one bad 0xD6 read removed a working monitor
from the flyout, in normal mode as well as maximum compatibility mode.

The restore gate compared only the monitor snapshot, so a profile apply issued
while a debounced slider commit was pending was skipped and then overwritten by
that commit. It now also requires the optimistic UI value to agree.

A throwing native read is contained to its own VCP code instead of unwinding into
the pipeline-wide catch that drops every monitor sharing the hMonitor.

Also fixes a mis-indented XML doc block in MonitorStateRetentionPlanner.

Validation: built PowerDisplay.Lib.UnitTests and PowerDisplay for x64 Debug with
VS MSBuild; ran PowerDisplay.Lib.UnitTests.dll with vstest.console.exe — 268
passed, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

… the cache

Second-pass review of the previous fix commit found that the 30-day freshness
bound on known-good VCP observations was worse than the problem it addressed.

The bound assumed every successful read restamps LastSuccessfulUtc, but on the
one path where the cache is load-bearing no read happens at all: once the probe
has touched a code, PreferLiveRead is false and ContinuousVcpInitializer applies
the cached value and returns without reading or upserting. So the clock only
advances on a successful probe -- the very event whose absence makes the cache
necessary. After thirty days of transient probe failures the entry expired, the
reconciled capabilities went null, and DdcCiController dropped the monitor
entirely rather than merely hiding one slider. The failure was absorbing: only a
successful probe could rebuild the entry, and that was what had been failing.
Restamping when the cached value is applied would fix the regression but make the
bound unreachable, since an entry can then only expire while its monitor is
undiscovered, which settings retention already handles. The bound is removed.

That leaves cached observations without invalidation, which was the original
objection. Fixing it properly means letting a definitive DDCCI_VCP_NOT_SUPPORTED
reply outrank the cache, and that contradicts a deliberate, test-pinned decision
in this branch that positive cache evidence survives such a reply in maximum
compatibility mode. It is a design question for review, not a unilateral change,
and is called out in the PR description instead.

Monitor state collection is also narrowed. Removing a whole state entry deleted
the user's saved brightness, contrast, volume, color temperature and capabilities
for any monitor that aged out of settings -- values that survived indefinitely
before this branch introduced the cache. The known-good cache is discovery state
this feature owns, so only that sub-object is collected now; saved user values
are never removed. RemoveMonitorStates becomes RemoveKnownGoodFeatures.

Validation: built PowerDisplay.Lib.UnitTests and PowerDisplay for x64 Debug with
VS MSBuild; ran PowerDisplay.Lib.UnitTests.dll with vstest.console.exe -- 264
passed, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

The branch introduces three tokens check-spelling does not know. DDCCI appears
as an all-caps run in the DDCCI_VCP_NOT_SUPPORTED comments; main only ever had
the PascalCase DdcCi, which tokenizes to ddc + ci, and ddc is already expected.
AOCB and XMN come from the AOC Q27G3XMN device paths and friendly name used by
the new test fixtures.

spelling2.yml does not set only_check_changed_files, so the Check Spelling job
fails on all three until they are expected.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

IsTransient carried eight bare 0xC02625xx literals plus a bare 1460 with no
direct test, so neither its membership nor the reasoning behind the gaps in the
sequence was reviewable. Move it next to IsPhysicalMonitorUnavailable in
DdcErrorClassifier, give every code its winerror.h name, and record why each
neighbouring code stays out: VCP_NOT_SUPPORTED is the device's final answer,
I2C_NOT_SUPPORTED and I2C_DEVICE_DOES_NOT_EXIST are permanent bus-level facts,
MCA_INVALID_CAPABILITIES_STRING belongs to the capabilities path rather than to a
VCP read, the handle-class codes are owned by IsPhysicalMonitorUnavailable and
must abort rather than retry, and MONITOR_RETURNED_INVALID_TIMING_STATUS_BYTE is
raised only by the get-timing-report command, never by
GetVCPFeatureAndVCPFeatureReply.

CURRENT_CURRENT_VALUE_GREATER_THAN_MAXIMUM_VALUE stays in on purpose and now says
why: a device that genuinely reports current > maximum simply exhausts the
budget, but the same code also results from a corrupted reply, which a retry does
fix.

A data-driven test pins both sets and asserts they stay disjoint, since an
overlap would keep retrying against a handle already known to be gone.

No behaviour change: the membership is identical.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…l write

The cache was only ever written from a successful read, so a slider move left it
holding the pre-write value. Maximum compatibility mode then republished that
stale value on a later discovery whose probe touched the code but could not read
it: PreferLiveRead is false once the probe has touched a code, so
ContinuousVcpInitializer applies the cached value without re-reading. The flyout
does not gate on ReadValues, so the user saw the stale value presented as live,
and a one-notch slider move snapped the panel to it. This needed no restart --
UpdateMonitorList rebuilds every MonitorViewModel on any display-change refresh.

SetVcpFeatureAsync is the single funnel for all six codes and already holds the
device-native value, monitor.Id, the store and the clock, so the refresh goes
there. It only refreshes an entry a real read already established -- a successful
SetVCPFeature is not evidence that the device implements the code -- and only
when the write was scaled against the very maximum that entry holds. That guard
is load-bearing: BrightnessVcpMax and its siblings default to the placeholder
100 and Monitor is rebuilt on every discovery, so a monitor whose read failed
would otherwise overwrite a read-proven 0-50 range with the placeholder and
mis-scale every later write.

The internal constructor also gains the delayAsync seam VcpFeatureProbeService
already had, and FetchCapabilitiesWithFallbackAsync becomes internal, so the
compatibility-mode gate and the probe-observation persist loop are covered for
the first time -- InternalsVisibleTo and the injecting constructor existed but no
test used them. IntPtr.Zero is a safe handle to drive that with:
TryGetCapabilitiesString short-circuits on it without issuing a native call, so
the capabilities string is deterministically unusable and every remaining
decision comes from the injected reader, clock and store.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…ocks

MarkDirtyAndScheduleSave had no _disposed guard, unlike SaveStateToDiskAsync and
Dispose. Dispose snapshots _isDirty, sets _disposed, disposes the debouncer and
flushes, so an upsert arriving after that point mutated the state and set
_isDirty again with no path left to clear it. The reachable window is the
synchronous BuildMonitorFromPhysical block, which performs no cancellation checks
and upserts after each successful blocking VCP read. Late observations are now
dropped on purpose: the next discovery pass re-derives them. MigrateLegacyKeys
stops duplicating the schedule inline and routes through the same helper.

The locks this branch added were exercised by nothing. The one concurrency test
upserts two different monitor Ids, which GetOrAdd maps to two distinct
MonitorState instances and therefore two distinct lock objects, so the suite
passed with every lock statement deleted. The contention they actually guard is
one monitor Id: the discovery thread upserting several VCP codes while the
debounced save enumerates that same KnownGoodVcpFeatures dictionary. The new test
drives exactly that from two threads and fails with InvalidOperationException out
of the ToDictionary inside GetKnownGoodFeatures once the locks are removed.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

Three statements did not match the code they describe.

Reconcile claimed the probe "spends its full paced retry budget on every code it
touches". ProbeCodeAsync breaks after a single attempt on a non-transient error
or a throwing read, which this branch's own ProbeAsync_NonTransientFailureDoes
NotRetry asserts. The guarantee that actually holds -- at least one transaction
per code the probe touched -- is what justifies not re-reading it.

DiscreteVcpInitializer claimed handle liveness is already settled before it runs.
A capabilities string that parses but advertises none of 0x10/0x12/0x62 leaves
nothing for the continuous stage to read and suppresses the probe, so the
discrete reads are the first on that handle. The stage is still safe, for a
different reason: failing here only leaves the read flag unset.

MonitorRestorePlanner described an unflagged value as merely cached. It is just
as often the never-read backing-field default, and naming those defaults -- 0
brightness, 50 contrast, 50 volume, 0x05 color temperature -- explains why a
saved value that happens to match one still has to be written.

Two decisions are also recorded where they are made, rather than left for the
next reviewer to re-litigate. Why a handle-class error discards the monitor
outright: Monitor.Handle is captured once per discovery pass and never
refreshed, so a monitor kept here would answer every later read and write
against a handle already known to be dead. And why positive cache evidence is
deliberately permanent while the monitor stays connected: retracting it on a
DDCCI_VCP_NOT_SUPPORTED reply trades a stale control that writes into the void
for a display that disappears entirely, and that reply is not dependable
negative evidence on the hardware this mode exists for.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…known-good store

Physical-monitor handles only reach PhysicalMonitorHandleManager through monitors
that were successfully built: the map is rebuilt from the returned monitor list,
and its cleanup pass only destroys handles that were already in the previous map.
A handle dropped inside the discovery loop therefore never gets destroyed. This
branch added a new way to drop one -- a handle-class error during continuous VCP
initialization now returns PhysicalMonitorUnavailable and discards the monitor,
where main logged it, left the read flag unset and kept the monitor so its handle
entered the map. A panel that intermittently answers that way leaks one kernel
handle per discovery, and a discovery runs on every display-topology change.
ReleaseAbandonedPhysical now covers all three abandon paths, including the two
that already leaked on main.

FetchCapabilitiesWithFallbackAsync also snapshots MaxCompatibilityMode once. The
property is settable from the UI thread on every settings reload and the method
awaits several times; main read it at a single decision point, this branch grew
that to three, so a toggle arriving mid-pass could have one monitor's evidence
gathered under one mode and reconciled under the other.

NullKnownGoodVcpStore and the two `?? NullKnownGoodVcpStore.Instance` fallbacks
are removed. Nothing ever reached them -- MainViewModel always supplies the real
MonitorStateManager and every test uses the injecting constructor -- so they were
dead code whose only effect would have been to silently disable the discovery
cache if a caller ever forgot the argument. The parameter is now required, which
turns that into a compile error.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…ial-value source

LoadStateFromDisk guards stateFile.Monitors but not the KnownGoodVcpFeatures list
this branch added, even though System.Text.Json writes an explicit JSON null
straight over the member initializer and the array can carry null elements. The
resulting NullReferenceException escapes to the method-level catch, so every
monitor entry after the offending one is dropped -- and the next whole-file
rewrite in BuildStateJson makes that loss permanent. The property is now typed
`List<KnownGoodVcpFeature>?`, matching how the same file already declares
CapabilitiesRaw and reflecting what deserialization can actually produce, and the
loop null-checks both the collection and its elements.

VcpInitialValue.Source is deleted. It was populated at both construction sites in
Reconcile and read by nothing: the record's only consumers -- InitializeFeature
and the probe-outcome log line -- read Value, IsLive and PreferLiveRead only.
KnownGoodVcpFeature.Source is kept; it is read when a successful write refreshes
a cache entry, and it is a useful field diagnostic in monitor_state.json.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

Three tests added in the previous review round overclaimed.

RefreshKnownGoodAfterWrite_EmptyMonitorIdIsIgnored did not pin the guard it is
named for: the fake store did its own monitor-Id comparison, so deleting
`string.IsNullOrEmpty(monitor.Id)` from the production path left the test green.
The fake now answers any Id and counts lookups, and the test asserts the store
was never queried -- removing the guard now fails it.

HandleClassFailuresAreNeverTransient asserted, verbatim, two rows that
IsTransient_RejectsEverythingElse already covers, and its comment claimed to test
set disjointness while hardcoding the two handle codes rather than deriving them,
so an overlap introduced by widening IsPhysicalMonitorUnavailable would have
sailed past it. Deleted; the rationale moves into the DataRow comment that owns
those two inputs.

The comment on ConcurrentUpsertAndRead_OnSameMonitorDoNotTearTheFeatureMap spent
most of its lines critiquing a sibling test instead of describing its own, and
would have gone stale the moment that sibling changed. Rewritten to state what
this test drives and why two monitor Ids cannot reproduce it.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…ce a control rests on

ReleaseAbandonedPhysical missed one exit. When GetPhysicalMonitorsFromHMONITOR
returns more physicals than QueryDisplayConfig has entries for, the loop breaks
without releasing physicals[i..] -- the one abandon path in that loop still
leaking the handles the helper's own remark describes. The break now releases the
remainder before leaving.

Two observability gaps in exactly the mechanism most likely to produce a
confusing bug report -- a control the cache created rather than the hardware:

The probe outcome line reports attempts, status and lastError but not Replied,
even though Replied is the single input that flips a feature to Supported when
the device answers with a range that cannot scale a percentage. In a shipped
build a replied-but-unusable outcome was indistinguishable from three plain
failures. It is now in the message.

Nothing at all was logged when cached evidence widened a monitor's capabilities
past what the capabilities string advertised. The existing probe-outcome block is
gated on `live.Count > 0`, and live is only populated when the caps string is
unusable -- so on the caps-parsed path, which is where the cache silently adds a
code, there was no line to find. Reconcile now reports which codes it supported on
cache evidence alone, and discovery logs them with the observation's age, which
also gives the persisted LastSuccessfulUtc its first reader.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

This branch introduced IVcpFeatureReader as the seam for native VCP reads, but left
DdcCiController.TryGetVcpFeature in place. It deleted six of that helper's seven call
sites, so the controller ended up carrying two parallel paths to the same syscall:
TryGetVcpFeature and DdcCiNative.ReadVcpFeature both wrap
GetVCPFeatureAndVCPFeatureReply and both report Marshal.GetLastWin32Error().

Route the one remaining caller, GetVcpFeatureAsync, through the injected reader and
delete the helper. The error value and the log line are unchanged. Alongside removing
the duplication this puts GetBrightnessAsync, GetContrastAsync, GetVolumeAsync,
GetColorTemperatureAsync, GetInputSourceAsync and GetPowerStateAsync behind the same
injectable seam the initializers already use; they had no test coverage before because
there was no way to reach them without hardware.

VcpFeatureProbeService.TransactionInterval also drops from internal to private. It has
no reader outside its own class, in production or in the tests.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…hree claims

The commentary this branch added had grown into design essays sitting inside method
bodies, and three statements did not survive being checked against the code.

Corrections:

VcpDiscoveryEvidence said a slipped-through false positive lasts "until its settings
entry ages out", which reads as a 30-day bound. RemoveKnownGoodFeatures is driven by
MonitorStateRetentionPlanner.BuildDroppedIds, i.e. the Rebuild input minus its output,
and MonitorSettingsRebuilder re-emits every currently-discovered monitor with a fresh
LastSeenUtc and keeps IsHidden entries unconditionally. Reclamation therefore needs
30 days both undiscovered and unhidden, which a monitor in daily use never reaches.
The comment now says so.

MonitorRestorePlanner enumerated the never-read values as "0 brightness, 50 contrast,
50 volume, 0x05 color temperature". Those are the Monitor backing-field defaults, but
on the DDC path the value it actually sees is MonitorDiscoveryHelper's placeholder of
50 brightness -- the mid-slider value users routinely save -- so the stated cost of a
redundant write pointed at the wrong number.

MonitorStateManager.MarkDirtyAndScheduleSave claimed Dispose has already flushed by the
time a late caller arrives, and that discovery is the only such caller. Dispose sets
_disposed before it flushes, and UpdateMonitorParameter reaches the same path; the
remark now states what actually holds and why it is still harmless.

Trimming: the 24-line block in Reconcile drops to 17 and keeps the traps rather than
the argument -- the full asymmetry is argued in the PR description, not at the call
site. DiscreteVcpInitializer's remark loses the restatement of when its stage runs but
gains what keeping the monitor costs. MonitorViewModel.ShouldRestoreValue and the
retention call site in MainViewModel.Settings stop repeating the callee's own remarks
verbatim and link to them instead.

Also drops the SA1402 suppression from VcpDiscoveryEvidence.cs. StyleCop.json does not
override maintainabilityRules.topLevelTypes, so SA1402 uses its default of ["class"],
and the file declares one class -- VcpInitialValue is a record struct. Verified by
building without it.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…or values

Four tests added by this branch hand their subject an input it cannot distinguish from
a test that already exists, and each has a twin asserting a strict superset.

ContinuousVcpInitializer never dereferences evidence.Capabilities or
CacheSupplementedCodes -- InitializeFeature reads only InitialValues, and feature gating
comes from the Monitor -- so Initialize_OmittedCodeFallsBackToCacheWhenLiveReadFails
drives the same ApplyCachedFallback branch as its Advertised twin, which additionally
asserts BrightnessVcpMax. Its VcpNotSupported reader script remains covered by
Initialize_VcpNotSupportedUsesCacheAndContinuesRemainingReads. The sibling
Initialize_OmittedCodeCachedValueUsesFreshLiveValueAndPersists is deliberately kept: it
is the only test that feeds caps-omitted, cache-supplemented evidence through the
initializer and asserts the Monitor is populated, which is the regression e2852c0
fixed.

ProbeCodeAsync consults the error code exactly once, through IsTransient, so
ProbeAsync_CurrentValueGreaterThanMaximumThenSuccessRetriesWithPacing walks the same
path as the InvalidCommand test; its extra Disposition assertion moves to the survivor.
0xC02625D8's membership in the transient set stays pinned by DdcErrorClassifierTests.

Reconcile_MaximumCompatibilityMarksAdvertisedCachedCodeAsLiveFallback differs from the
union test only in whether the caps string advertises the cached code, and the sole
observable effect of that -- CacheSupplementedCodes -- is asserted by
Reconcile_ReportsOnlyTheCodesTheCacheAloneProved, which now also pins PreferLiveRead for
a caps-advertised cached code. Reconcile_NormalModeReportsNoCacheSupplementedCodes built
input byte-identical to Reconcile_NormalModeIgnoresCache; its assertion moves there.

Adding DdcErrorClassifierTests.Constants_MatchWinerrorValues. Every other assertion in
the suite addresses these codes by name, so the numeric values were pinned only
incidentally, by the raw-hex constants the other test files declare as inputs. A typo in
a constant would have moved production and tests together and left the suite green. With
the values now pinned in one place, VcpFeatureProbeServiceTests' local
CurrentValueGreaterThanMaximum constant goes with the test that used it.

GetKnownGoodFeatures_UsesExactDevicePathComparer uses the file's existing Feature()
helper; its inline initializer differed only in a Source it never asserts on. The
round-trip test keeps its explicit initializer, since the fields it spells out are the
ones it verifies.

Validation: built PowerDisplay.Lib.UnitTests and PowerDisplay for x64 Debug with VS
MSBuild -- 0 errors, 0 warnings; ran PowerDisplay.Lib.UnitTests.dll with
vstest.console.exe -- 296 passed, 0 failed (299 - 4 + 1).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…e redundant handle

Three cleanups to code this branch introduced, all behaviour-preserving.

The continuous VCP set had been triplicated. Base had exactly one list --
DdcCiNative.ProbeableContinuousVcpCodes -- which this branch deleted and replaced with
three byte-identical private copies in VcpDiscoveryEvidence, ContinuousVcpInitializer
and VcpFeatureProbeService. All three iterate it in declaration order, so a code added
to one and not the others is probed but never applied, or applied but never proven. The
set moves to NativeConstants.ContinuousVcpCodes, next to the three codes it is built
from; hosting it on any of the three consumers would have made the other two depend on
a peer for a constant.

Both initializers took an IntPtr handle that is provably Monitor.Handle:
MonitorDiscoveryHelper.cs:133 is the only write to that property in the module and it
stores physicalMonitor.HPhysicalMonitor, which is exactly what BuildMonitorFromPhysical
passed alongside the monitor. Beyond the duplication this was a coherence problem --
ContinuousVcpInitializer justifies discarding a monitor by reasoning about
Monitor.Handle while actually reading from the parameter, and nothing stopped a caller
from passing one that disagreed. Both Initialize signatures lose the parameter and read
Monitor.Handle directly.

The three near-identical KnownGoodVcpFeature initializers fold into
KnownGoodVcpFeature.From, the inverse of the existing ToVcpFeatureValue.
RefreshKnownGoodAfterWrite now names the VcpFeatureValue it was already constructing for
validation and reuses it.

DiscreteVcpInitializerTests' two tests also collapse to one: Initialize never inspects
read.ErrorCode -- it is used once, interpolated into a log string, and the file never
references DdcErrorClassifier -- so VcpNotSupported and the two handle-class codes are
indistinguishable inputs. The DataTestMethod survives because its handle-class rows are
what would fail if someone copied ContinuousVcpInitializer's drop-the-monitor branch
into this stage; the deleted test's two value assertions move into it.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…ate the error rationale

Reconcile_IndeterminateLiveUsesCachedPositiveEvidence and
Reconcile_VcpNotSupportedStillUsesCachedPositiveEvidence differ only in the error code
handed to VcpProbeObservation.Indeterminate. Reconcile never reads LastError -- it
consults Disposition, IsSuccess and Replied -- and Disposition routes both 0xC0262589
and 0xC0262584 to Indeterminate, since neither is handle-class. Same cached entry, same
null parsed capabilities, same includeCache, so the two calls are provably identical
inputs. The survivor is the stronger mutation-killer, because a regression where
DDCCI_VCP_NOT_SUPPORTED retracts cached evidence fails it and not the other; it inherits
the deleted test's IsLive assertion and its retry-budget comment.

ProbeAsync_UnansweredCodeIsNotMarkedAsReplied repeated ProbeAsync_NonTransientFailure
DoesNotRetry's arrange and act verbatim; its one assertion moves into that test.

The <remarks> on DdcErrorClassifier.IsTransient and the DataRow comments on
IsTransient_RejectsEverythingElse were written by the same commit and say the same five
things. The production remarks win -- that is where someone editing the predicate looks
-- and the test keeps only the reason its handle-class rows are load-bearing here, which
is specific to the test rather than to the classifier.

Validation: built PowerDisplay.Lib.UnitTests and PowerDisplay for x64 Debug with VS
MSBuild -- 0 errors, 0 warnings; ran PowerDisplay.Lib.UnitTests.dll with
vstest.console.exe -- 293 passed, 0 failed (296 - 2 methods - 1 DataRow case).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…eter

Third cleanup pass. Where the first two hunted duplicated text, this one asks whether
each shape earns its keep.

Reconcile took both a `cached` dictionary and an `includeCache` flag, but the sole
production caller derives both from the same local: DdcCiController passes
`maxCompatibility ? GetKnownGoodFeatures(monitorId) : new Dictionary<...>()` and then
restates the decision as `includeCache: maxCompatibility`. Since `!includeCache` always
implies `cached` is empty, the `includeCache &&` conjunct could never change the result
of the `cached.TryGetValue` beside it -- one decision expressed twice, in two places a
maintainer had to keep in agreement by hand. The parameter goes; the gate stays where it
has to live anyway, because in normal mode the controller must not even call
GetKnownGoodFeatures. That is what DdcCiControllerCompatibilityGateTests.NormalMode_
NeitherProbesNorTouchesTheCache pins, with `store.GetCallCount == 0` -- strictly stronger
than the unit test it replaces, which only checked that Reconcile ignored a cache it had
already been handed.

`using System;` was orphaned in both initializers when round 2 removed their IntPtr
parameter, and `using System.Threading.Tasks;` in MonitorStateManagerTests by the test
deleted below.

Four more tests hand their subject input it cannot distinguish:

- Reconcile_NormalModeIgnoresCache existed only to exercise the removed parameter.
- Reconcile_NoLiveOrCacheLeavesFeatureUnavailable: Reconcile has no branch keyed on
  emptiness, so it kills no mutant the other tests miss.
- Initialize_AdvertisedCachedCodeUsesFreshLiveValueAndPersists differs from its Omitted
  twin only in CachedEvidence's flag, which changes evidence.Capabilities --
  ContinuousVcpInitializer reads only evidence.InitialValues, and Reconcile emits the
  identical entry for both. The Omitted twin is kept, being the discriminating input; its
  one unique assertion moves across.
- ConcurrentUpserts_PreserveBothMonitorEntries raced two different monitor Ids, which the
  PR's own sibling test explains cannot contend: GetOrAdd maps them to two MonitorState
  instances and therefore two disjoint locks. It exercised ConcurrentDictionary, not this
  branch. Its non-concurrent content is already asserted by RemoveKnownGoodFeatures_
  ClearsCacheButKeepsSavedUserValues, and the real contention path by
  ConcurrentUpsertAndRead_OnSameMonitorDoNotTearTheFeatureMap.
- BuildDroppedIds_KeepsEntriesThatSurvivedTheRebuild: BuildDroppedIds has a single code
  path and two other tests already assert an empty result.

Finally the nine test-local DDC error constants and one inline literal now reference
DdcErrorClassifier by name. Round 1 blocked this because those locals were the only
value-level check on the constants; Constants_MatchWinerrorValues, added in the same
round, is now that oracle, so the duplication can go. Codes DdcErrorClassifier does not
declare stay as raw hex.

Validation: built PowerDisplay.Lib.UnitTests and PowerDisplay for x64 Debug with VS
MSBuild -- 0 errors, 0 warnings; ran PowerDisplay.Lib.UnitTests.dll with
vstest.console.exe -- 288 passed, 0 failed (293 - 5).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

…rop two suppressions

VcpFeatureProbeService.cs held six top-level types and suppressed both SA1649 and
SA1402 to do it. Moving NativeVcpFeatureReader out leaves a single class, so SA1402
no longer applies, and ordering the file after its namesake type satisfies SA1649.

The move stands on its own merit: NativeVcpFeatureReader is the production P/Invoke
seam that DdcCiController and both VCP initializers depend on, not a detail of the
probe service it happened to live inside.

… decision

VcpInitializationResult was a two-value enum in its own file whose only branch asked
one question -- the same shape as VcpProbeDisposition, dropped in b20712a. Eight of
its ten references were 'return Completed', meaning nothing to report. Initialize now
returns bool and documents the polarity.

MonitorStateRetentionPlanner was 51 lines plus a 50-line test file wrapping a set
difference with one call site. The rationale comment moves to that call site. No
coverage is lost: MonitorIdComparer has its own tests, and the case-insensitive
matching of the consumer is already pinned by
MonitorStateManagerTests.RemoveKnownGoodFeatures_MatchesIdsCaseInsensitively.

UpsertKnownGoodFeature threw ArgumentOutOfRangeException on an invalid range that no
caller can produce: all three pre-validate, and nothing tested the throw. The guard
that matters is in LoadStateFromDisk, where untrusted values actually enter.

BuildMonitorFromPhysical tested evidence.IsPhysicalMonitorUnavailable alongside the
null capabilities check. Reconcile only ever sets that flag together with null
capabilities, and the single caller already skips both cases with their own log lines,
so the disjunct could never decide anything.

…aximum compatibility mode

DdcCiController gates only the read side on MaxCompatibilityMode, so the unconditional
upsert here reads like an oversight. It is not: a monitor that reads cleanly today can
start failing after a cable or dock change, and a cache populated lazily would be empty
on exactly the first pass that needs it.

…he' into pr-49445-head

…ous codes

Review follow-ups on the known-good VCP cache.

MonitorStateManager.UpsertKnownGoodFeature no longer marks the state file
dirty when neither Current nor Maximum moved. Every discovery pass re-reads
all three continuous codes, so re-observing an unchanged value was the common
case, and each one rewrote the whole file and reset the save debouncer —
cancelling the pending Task.Delay with an OperationCanceledException — for a
timestamp only the support log reads. The entry is still replaced in memory so
LastSuccessfulUtc stays accurate; the comment records that this holds only
while the timestamp is diagnostic, and has to go with any reintroduced
freshness bound.

The probe-success upsert loop in FetchCapabilitiesWithFallbackAsync is bounded
to ContinuousVcpCodes. VcpFeatureProbeService takes its sweep list through the
constructor, so widening it without a matching arm in ContinuousVcpInitializer
would have persisted a code nothing ever applies while Reconcile kept marking
it supported. DdcCiController's internal constructor now forwards that sweep
list so the bound is testable. The same loop gained a comment explaining why a
reading taken before the handle died is still persisted even though Reconcile
discards the pass: the probe stops at the first handle-class failure, so the
value is proven even when the pass is not.

Reconcile and MarkSupported now document that a non-null parsedCapabilities is
extended in place and returned as the same instance — reference identity is
already asserted by Reconcile_ParsedCapabilitiesSurviveWhenNoProbeRan, and a
deep copy would silently lose metadata a later field addition forgot to carry.

RemoveKnownGoodFeatures records the retention blind spot: a monitor whose
handle dies mid-discovery seeds an entry but never reaches settings.json, so
it is never named for collection.

Restored the ContinuousVcpInitializer class summary dropped in the merge with
PR #49596, undid two signature reflows that changed no parameters, and dropped
a fully-qualified System.Array.Empty in favour of a using.

Tests: three pin the save short-circuit (unchanged value keeps the in-memory
timestamp, does not recreate a deleted state file, and the changed-value
control does); two cover the upsert loop (a reading that preceded a dead
handle is persisted, a widened sweep does not reach the cache). Restored the
return-value assertion on Initialize_InvalidRangeDoesNotApplyOrPersist, took
the concurrency test from 100 rounds to 20, and made the temp-directory
cleanup tolerant of a save still in flight.