[tylerkron]

Not merging — for review. Mergeable now, but land it after #202: see
Status.

closes #191

What this catches that nothing caught before

The visual capture harness renders the real app's UI against a wall-clock animation
clock. When it races itself it produces differences indistinguishable from a real
regression, and this project has already paid for that twice:

• A 65.6% "visual regression" between two runs of the same binary at the same
  commit, which was an unfinished fade-in.
• Two live settle-loop races found while doing fix(parity): the visual gate now captures on macOS — two settle-loop races fixed, no CI gate yet #179 by running exactly this mode on
  macOS: mobile-portrait-3-storage saved mid-fade with 81% of its pixels wrong
  (~1 run in 8), and the three right-hand drawers' shared SplitView reopened
  mid-close, flipping a 6-pixel column on the pane edge 50/50 per run.

Both were fixed, and nothing stopped the next one landing. #179 added the macOS capture
leg and deliberately added no CI job — but the reason it gave was about
--check-baseline specifically: that manifest has been reproduced on exactly one Mac,
so gating it would fail on its first run and read as a UI regression. That reasoning
does not reach --determinism, which compares the runner against itself.

On its first CI run this gate found a third one, which is the whole argument for
having it. It is now fixed in #202. Details below.

What the step does

The existing Desktop head on macOS job gains one step plus a failure-only upload. It
runs tools/parity-audit/run.sh --determinism into a RUNNER_TEMP out-dir: five
headless captures of the real app, every PNG required to be byte-identical to the first
run's. It is the first thing in this workflow that executes this repo's UI code rather
than compiling it.

The two traps #191 names:

1. The screen count is asserted, in CI, as 18. A determinism run compares whatever
   both runs contain, so a consistently absent screen reads as a clean 17/17 byte-identical rather than as an incomplete capture. fix(parity): the visual gate now captures on macOS — two settle-loop races fixed, no CI gate yet #179 guards that inside the
   harness (ExpectedScreens); the CI check is a deliberate second, independent
   copy of the contract, because the harness's list is a variable a change can edit
   downwards — and if the only record of the expected count sits beside the code that
   produces it, a gate that shrinks stays green. Adding a screen now means editing two
   places on purpose.
2. <out>/determinism/ is uploaded on failure (artifact parity-determinism-macos),
   so the differing PNGs are in hand. fix(parity): the visual gate now captures on macOS — two settle-loop races fixed, no CI gate yet #179 already had to fix a cleanup that destroyed
   the evidence its own failure message asked for. The upload is conditioned on
   failure() && steps.determinism.outcome == 'failure', not a bare failure(): a
   bare one would also fire after an earlier failure (restore, the arm64 check),
   uploading nothing and adding a second, empty red step to a job that already said
   what was wrong.

Also here, because the above made them stale or attributable:

• The harness is restored explicitly in the job's Restore step, so an NU1004
  lands on a restore step rather than surfacing as "the visual gate broke", and the
  job's existing lock-file guard now covers AvaloniaCapture/packages.lock.json on a
  macOS derivation host — which nothing did before (build: --force-evaluate restore on macOS rewrites the Oxyplot project key in every lock file #106 was exactly a lock-file
  key macOS and Linux derived with different casing).
• The job header claimed the harness was left out of this job; that is now half wrong,
  so it says which half runs and which does not, and why the other half waits on tooling: the macos-arm64 baseline is named for OS+arch but has only ever been reproduced on one Mac #188.
  The desktop job's "compiled, not run" note is narrowed to the Linux runner, where
  it is still true.
• tools/parity-audit/README.md documents the gate.

Out of scope per #191, deliberately: gating --check-baseline (blocked on #188) and
any Linux capture gate (never watched to succeed there).

The failure artifact was dropping the one file a pre-PNG failure leaves behind

Trap 2 above was only half true when it was written, and the missing half is the case
that needs it most.

run.sh's run_capture sends each pass's full stdout and stderr to
<out>/.capture-determinism-N.log, and its own failure line points straight at it
(!! determinism-3 failed (see …)). Two independent things kept those logs out of the
artifact:

1. They are written at the output ROOT — one level above <out>/determinism,
   which is the entire path: of the upload. Nothing under path: ever matched them,
   so include-hidden-files: true on its own would not have fixed this.
2. They are dotfiles, which actions/upload-artifact drops by default.

The case that pays for this is the one with the least to go on. A pass that dies
before writing a single PNG — a crash during boot, a build error out of dotnet run — leaves <out>/determinism/rN empty, so the upload matched zero files,
if-no-files-found: warn shrugged, and no artifact was produced at all. #191 asked
for this gate on the grounds that a determinism failure is only actionable with the
evidence in hand; a failure with no PNGs is only actionable with the log, and the log
was the one thing being thrown away. That is #179's cleanup-destroys-the-evidence trap
wearing different clothes.

A new failure-only step copies the logs to <out>/determinism/capture-logs/, dropping
only the leading dot, so capture-determinism-3.log in the artifact is the
.capture-determinism-3.log the failing step named on screen.

Copied rather than added to path:, deliberately. Adding the glob would force
include-hidden-files: true, and that flag is not selective: AvaloniaCapture points
DAQIFI_DATA_DIR at <out>/determinism/rN/.appdata, inside the tree being uploaded,
so turning hidden files on would also sweep in every run's throwaway SQLite DB and app
logs. Copying keeps that default exclusion doing its job, and puts the evidence where a
plain ls on the unzipped artifact will show it rather than under a name ls hides —
a trap tools/parity-audit/README.md already warns readers about.

The step is the only place that knows run_capture's log naming, so it refuses to fail
quietly: no match emits a ::warning:: and writes a NO-CAPTURE-LOGS.txt into the
artifact naming the glob it looked for and the step to update.

Nothing about the gate itself moves. Five runs, byte identity, and the independent
18-screen assertion are all unchanged.

What it found, on its first run

Run 33655089083
failed, uploaded its captures, and the artifact says exactly what happened: run 4 of 5
wrote desktop-8-livegraph-settings-flyout with six pixels one unit off, every one
of them in column x=1059 — the SplitView pane's own left edge (1440 − 380 − 1).
0.0005% of the frame, and the same signature as the flip #179 fixed for desktop-9.

A 15-run job (33657356313)
put a rate on it: 3 failures in 15, on desktop-8 and desktop-9, always those
same six pixels. Across four macos-latest jobs the tally was 30 capture runs and 7
carrying the flip, against 40 clean runs on one M-series Mac.

Worth recording because it is the opposite of what one would guess: this is not a
cross-machine problem. Of the 90 screen-by-screen comparisons between that Mac's set
and the runner's five, 89 are byte-identical; the one mismatch is this flip.

Two fixes tried on this branch were both wrong, and their commits and reverts are in
the history with the measurements: requiring two consecutive identical settle
comparisons (passed a 5-run job, failed the 15-run one at the same rate), and a 500 ms
settle floor (worse — it reproduced the flip on the 40-for-40 Mac, 2 in 12). More
time selected a different wrong answer, which is what said the settle loop was not
where the bug lived. That was filed as #201.

Status

#201 is fixed in #202, and that is the only thing that was holding this PR.

#202's root cause turned out not to be the settle loop at all: Encode called
CaptureRenderedFrame(), which reads the persistent headless framebuffer that the
compositor updates in place a dirty region at a time — so the saved bytes were a
function of the redraw history rather than of the visual tree, and no settle rule could
ever have seen that. It now renders into a fresh RenderTargetBitmap.

Confirmed on real runs rather than on the claim. #202 carried a temporary commit that
enabled this gate on its own branch, and the macOS job reported
determinism OK: 18/18 screens byte-identical across 15 runs:

• run 33666074347
  (DETERMINISM_RUNS: 15, green)
• run 33665722117
  (green)

And directly for this branch, because #202's evidence used DETERMINISM_RUNS=15 rather
than the setting this gate actually ships: this branch merged with #202 goes green
on macos-latest at the real 5-run default — run
33669480144.

Merge order: this PR lands after #202. Without #202 the gate is red at roughly 1
capture run in 5, so a 5-run job fails about two times in three, and the red check on
this head is exactly that and nothing else: on run
33670298263 the
only failing step is the determinism one, Collect the capture logs and the upload both
succeeded after it, and the artifact shows the familiar signature —
desktop-8-livegraph-settings-flyout at 52,114 bytes on r1 and 52,137 on r4. Merging in
the other order puts a known-red required check on main.

Weakening the check — a pixel tolerance, a retry, a smaller run count — would be the
"gate that quietly shrinks its own scope" this ticket exists to prevent, so nothing of
the kind is here.

Verification

• The gate, watched on macos-latest. Green:
  33656922699 —
  determinism OK: 18/18 screens byte-identical across 5 runs, then the step's own
  18 screens, byte-identical across every run. Red, with the artifact:
  33655089083 and
  33657356313.
• Screen count: 18, asserted by the step and reported by the harness on every run.
  The count guard was tested in both directions locally, against a real capture set and
  against a truncated copy: 18 passes, 1 fails with the diagnostic and an ls.
• The artifact fix, proven in both directions on real macos-latest runs — the
  same bar the original upload condition was held to. All three artifacts were
  downloaded and their contents listed, not inferred from step status.

  • Failing with no PNG ever written — the case that used to produce nothing at
    all. Scratch branch with a forced pre-capture crash, run
    33669444638:
    determinism failure → collect success → upload success. The artifact is
    4,903 bytes and its complete contents are one file,
    capture-logs/capture-determinism-1.log, carrying
    Unhandled exception. System.InvalidOperationException at line 228. Before this
    change that upload matched zero files and produced no artifact.

  • Failing with PNGs — this PR's own CI, red on the real parity: the capture harness flips the SplitView pane edge on GitHub macOS runners, ~1 run in 5 #201 flip: runs
    33669352002
    and 33670298263.
    Artifact 2.79 MB: r1..r5/ with all 90 PNGs, plus capture-logs/ with all five
    logs. find -name '.*' over the unzipped artifact returns nothing, so the
    throwaway .appdata — which exists on this run, the app booted — is still
    excluded. The copy did not cost the default exclusion, which is the whole reason
    for copying rather than include-hidden-files: true.

    The logs turned out to carry evidence the PNGs do not surface as readily. Straight
    off capture-logs/ on the second of those runs, without decoding an image:

    desktop-8-livegraph-settings-flyout: 52,114 bytes, settled in 2 round(s)   (r1)
    desktop-8-livegraph-settings-flyout: 52,137 bytes, settled in 1 round(s)   (r4)
    

    Two stable encodings with their settle-round counts beside them — the parity: the capture harness flips the SplitView pane edge on GitHub macOS runners, ~1 run in 5 #201
    signature, readable at a glance. That was not the argument for collecting them,
    but it is a good one.

  • Passing — runs
    33669480144
    and 33670132486:
    determinism success, and both the collect step and the upload skipped on
    each. Their artifacts are assets-android and assets-desktop only — no
    parity-determinism-macos.

• Cost: this change adds nothing to a green job, because the collect step is
  failure()-gated and shows as skipped. Re-measured on those two green runs: the
  determinism step took 55 s and 60 s, and the job 1 m 36 s and 1 m
  52 s, against the 95 s / 2 m 35 s recorded here earlier and 63 s for the job on
  main. That saving belongs to fix(parity): the capture harness photographed a surface with a history, not the UI (#201) #202, not to this PR — rendering the tree is
  cheaper than reading the framebuffer back on every settle round.
• Local gate on this branch, all green: Desktop build, iOS restore, iOS -c Release -r iossimulator-arm64 --no-restore build, and git status -- '**/packages.lock.json'
  clean afterwards. The Android head cannot build on this Mac (workload absent); CI
  covers it. No warning-count claim: this change compiles nothing new.

Diff against main: 218 added / 23 removed in .github/workflows/build.yml,
13 added in tools/parity-audit/README.md. No other file differs; the two
experiment commits are reverted in full.

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Gate macOS visual captures on deterministic 18-screen output

🧪 Tests ⚙️ Configuration changes 📝 Documentation 🕐 20-40 Minutes

AI Description

• Run five macOS visual captures and require byte-identical output across all 18 screens.
• Restore capture dependencies explicitly and preserve failure PNGs as CI artifacts.
• Document determinism gating while deferring host-specific baseline validation.

Diagram

sequenceDiagram
    actor PR as Pull Request
    participant CI as GitHub Actions
    participant Job as macOS Job
    participant Script as parity run.sh
    participant App as Avalonia Capture
    participant Art as Artifact Store
    PR->>CI: Trigger workflow
    CI->>Job: Start required check
    Job->>Script: Run determinism
    loop Five captures
        Script->>App: Render 18 screens
        App-->>Script: Return PNG set
    end
    Script->>Script: Compare bytes
    Script-->>Job: Return result
    Job->>Job: Assert 18 screens
    alt Determinism failed
        Job->>Art: Upload captures
    end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Create a dedicated visual-gate job

• ➕ Isolates visual failures from Desktop build and native-asset checks
• ➕ Makes runtime duration and failures independently visible
• ➖ Duplicates macOS checkout, SDK setup, restore, and billed runner time
• ➖ Would require adding another required check to repository settings

2. Trust only the harness screen contract

• ➕ Avoids duplicating the expected screen count in workflow configuration
• ➕ Keeps capture-set ownership entirely within AvaloniaCapture
• ➖ A change that shrinks ExpectedScreens could silently reduce CI coverage
• ➖ Removes the independent guard against consistently incomplete captures

Recommendation: Keep the proposed approach. Reusing the existing required macOS job avoids another expensive runner while adding runtime coverage after its existing restore and build checks. The separate 18-screen assertion is intentional contract duplication that prevents the harness and its own expected-screen list from shrinking together unnoticed; a dedicated job is only preferable if execution time or failure isolation later becomes problematic.

Files changed (2) +150 / -23

Documentation (1) +6 / -0

README.mdDocument the macOS determinism CI gate +6/-0

Document the macOS determinism CI gate

• Explains that determinism now runs on every pull request, independently checks the 18-screen capture count, and retains failed captures as an artifact. It also clarifies why machine-specific baseline validation remains outside CI.

tools/parity-audit/README.md

Other (1) +144 / -23

build.ymlAdd deterministic visual capture gating to the required macOS job +144/-23

Add deterministic visual capture gating to the required macOS job

• Explicitly restores the Avalonia capture harness, runs five headless capture passes, and requires byte-identical PNG output for all 18 screens. Failed determinism steps upload their capture directory while earlier job failures do not trigger a misleading artifact upload; workflow comments also clarify runtime coverage and baseline exclusions.

.github/workflows/build.yml

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit c6dfa35 ⚖️ Balanced

Results up to commit 9124a2c ⚖️ Balanced

No changes from previous review

Results up to commit 9496201 ⚖️ Balanced

No changes from previous review

[tylerkron]

/agentic_review

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 9496201

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit c6dfa35

[tylerkron]

Qodo-clean, CI green apart from the documented #202 dependency — ready for review.

Qodo on c6dfa35: 0 bugs / 0 rule violations / 0 requirement gaps, 0 inline threads, re-checked on both surfaces after settling. Every check is green except Desktop head on macOS, which is red only on the #201 flip that #202 fixes — verified, not assumed: this branch merged with #202 goes green on macos-latest at the gate's real 5-run default in runs 33669480144 and 33670132486.

The failure-artifact fix is proven in both directions on real runs, artifacts downloaded and listed rather than inferred from step status: a failure before any PNG is written now uploads an artifact whose entire contents are capture-logs/capture-determinism-1.log carrying the crash trace (33669444638, 4,903 bytes — that case previously uploaded nothing at all), and a passing run still skips both the collect step and the upload. The throwaway .appdata stays excluded, which is why the logs are copied rather than swept in with include-hidden-files: true.

Merge after #202, or the gate goes red again on main.