Skip to content

One way to do each of the things the driver was doing several ways - #100

Merged
amrali-eg merged 1 commit into
masterfrom
chore/simplify-gui-driver
Sep 10, 2026
Merged

One way to do each of the things the driver was doing several ways#100
amrali-eg merged 1 commit into
masterfrom
chore/simplify-gui-driver

Conversation

@amrali-eg

@amrali-eg amrali-eg commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Cleanup pass over EcGuiDriver.cs, which has been edited across six commits in
three pull requests, so duplication that spans those edits was invisible to any
single diff. Behaviour is unchanged everywhere except one message that is now
built later than it was.
No production code changes.

Fixed

Rendered text was read three ways. ReviewText, StatusText and the body of
StatusLine were the same LINQ chain, identical but for the root element. One
VisibleText holds it; the three callers keep the doc comments explaining why
each wants a different root.

The select-all wait built its failure message eagerly. An automation call sat
unguarded inside a message - the one thing Safely exists to prevent, and the
same hazard SmokeSuite documents for its own Check. WaitUntil gained an
overload taking the message as a function, evaluated only on failure and routed
through Safely.

It also did two full walks of a thousand result rows on every passing call, but
that is a correctness-of-diagnostics fix rather than a performance one: timing
--phase I on both builds gives 7-8s against 8s, so the saving is about a second,
and only in the one phase that has a thousand rows. Not a speedup worth citing.

"Has the run ended" had two spellings, one named and one inlined in the cancel
probe. FinalConversionStatus() answers it once; ConversionHasFinished() is the
null test over it, and the stranded summary moved onto the member it describes.

Smaller: ResultCount hand-rolled what ResultItems already does (including
an is var pattern that always matches); the safe status read was inlined at
three failure sites beside a sibling wrapper that already had the name;
SetToggle's confirmation refetched the toggle pattern each poll instead of
reading the one it holds; two comments described code that is no longer there.

Deliberately not done

Caching the status bar and results-list elements would remove a whole-window walk
from every 50 ms poll - the single largest win available here. A stale held
AutomationElement is exactly the hypothesis EC-28 is
open on
, so adding more of them while that is unexplained would be reckless.

Provider-side ControlType filtering, a narrower scope for the review-window
lookup, and a handle-valued PropertyCondition for FindWindow are all cheaper,
but each changes UI Automation query semantics on the timing-sensitive path whose
cancellation margin is a tracked number.

Also skipped: collapsing the four process-scoped finders, where the
Children/Descendants differences are load-bearing; extracting the
review-close prologue; and two larger structural changes - a first-class one-shot
attempt in the wait mechanism, and unifying Safely / Ask /
DescribeWindowsSafely.

Evidence

Check Result
Build 0 warnings, 0 errors
Unit tests 756 passed
Full suite 8/8
Cancellation margin 48-56 of 1000, unchanged

EC-28 remains open; nothing here explains it.

🤖 Generated with Claude Code

Cleanup only; behaviour is unchanged everywhere except one message that is now
built later than it was.

Reading rendered text was written out three times - for the review, the whole
window, and the status bar - identical but for the root element. One VisibleText
holds it, and the three callers keep the doc comments that say why each wants a
different root.

The select-all wait built its failure message eagerly, so two full walks of a
thousand result rows ran on every passing call, and an automation call sat
unguarded inside a message. WaitUntil gained an overload taking the message as a
function, evaluated only on failure and routed through Safely. That is the same
hazard the suite documents for its own Check.

"Has the run ended" had two spellings, one named and one inlined in the cancel
probe; FinalConversionStatus now answers it once and ConversionHasFinished is
the null test over it. ResultCount hand-rolled what ResultItems already does,
including an is-var pattern that always matched. The safe status read was
inlined at three failure sites and now has the name its sibling
DescribeWindowsSafely already had. SetToggle's confirmation refetched the toggle
pattern each poll instead of reading the one it holds.

Two comments described code that is no longer there: the idle-wait remark still
warned about the defect the parameter below it fixed, and an inline comment
restated the remark six lines above it.

Not done, and why: caching the status bar or results list element would remove
a whole-window walk from every 50 ms poll, but a stale held element is exactly
the hypothesis EC-28 is open on, so adding more of them now would be reckless.
Provider-side ControlType filtering and a narrower scope for the review lookup
would both be cheaper, but they change UIA query semantics on the timing-
sensitive path whose margin is a tracked number.

EC is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@amrali-eg
amrali-eg merged commit d3fc36e into master Sep 10, 2026
3 checks passed
@amrali-eg
amrali-eg deleted the chore/simplify-gui-driver branch September 10, 2026 05:44
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