From 9124a2ccdd869f63eadb4d53ef1acd821c67ddb4 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 2 Sep 2026 10:27:23 -0600 Subject: [PATCH 1/7] ci: run the visual capture harness on macOS and require it to be deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #179 gave the visual gate a macOS capture leg and deliberately added no CI job to run it. The reason it gave was about --check-baseline specifically: the committed macos-arm64 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 and asserts nothing about any other machine's bytes. So the `Desktop head on macOS` job now 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. That is the first thing in this workflow that executes this repo's UI code rather than compiling it, and it is what stops the next self-race in the capture harness reaching main — the two #179 found (a pane saved mid-fade, ~1 run in 8; the shared SplitView reopened mid-close, a 6-pixel edge flip, 50/50) were found by nothing except this check. Two things the step has to get right, and does: * It asserts the capture produced 18 screens. 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. #179 guards that inside the harness (ExpectedScreens); this is a deliberate SECOND copy of the contract, because the harness's list is a variable a change can edit downwards. * It uploads /determinism/ when — and only when — this step is the step that failed. A determinism failure is unactionable without the differing PNGs, and #179 already had to fix a cleanup that destroyed the evidence its own failure message asked for. The harness is also restored explicitly, so an NU1004 lands on a restore step and the existing lock-file guard covers AvaloniaCapture's lock file on a macOS derivation host — which nothing did before (#106 was exactly a host-dependent key). Out of scope, and the job header now says so in place of the stale claim that the harness is not run here at all: gating --check-baseline (blocked on #188) and a Linux capture gate (never watched to succeed there). Co-Authored-By: Claude Opus 5 --- .github/workflows/build.yml | 167 ++++++++++++++++++++++++++++++----- tools/parity-audit/README.md | 6 ++ 2 files changed, 150 insertions(+), 23 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a7e1d7fd..ae091207 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -156,11 +156,13 @@ jobs: # not be RUN here; it is RID-less since #89, so this now builds it for the # runner's own platform and it WOULD run. # - # Deliberately still only COMPILED, not run. Running it would be a stronger gate - # — it boots the real app and renders 18 screens, which a compile cannot check — - # but the harness has never been executed on a Linux runner (headless Skia, font - # availability), and adding a CI step nobody has watched succeed is how a gate - # arrives broken. That is its own change, with its own evidence. + # Deliberately still only COMPILED, not run — HERE. Running it would be a + # stronger gate — it boots the real app and renders 18 screens, which a compile + # cannot check — and since #191 the macos job does exactly that. This one still + # does not, because the harness has never been executed on a LINUX runner + # (headless Skia, font availability), and adding a CI step nobody has watched + # succeed is how a gate arrives broken. That is its own change, with its own + # evidence. - name: Build run: | dotnet build Daqifi.Avalonia/Daqifi.Avalonia.csproj \ @@ -281,13 +283,22 @@ jobs: # * It builds the Desktop head for an osx-* RID, which nothing did before. A # RID is not a no-op here — it selects a runtime pack, an apphost, and each # package's runtimes/osx-arm64/ assets. - # * It asserts those macOS native assets actually landed (see the last step). - # That is the part a green compile genuinely does not prove. + # * It asserts those macOS native assets actually landed (see the arm64-natives + # step). That is the part a green compile genuinely does not prove. + # * It RUNS the visual capture harness, five times, and requires all 18 screens + # to be byte-identical across the five (#191, the determinism step at the end + # of this job). That is the first thing in this workflow that executes any of + # this repo's UI code rather than compiling it. # - # It does NOT catch the separator bug above, or anything else that is wrong only - # at runtime; nothing that merely compiles can. What it buys is that the macOS - # target stops being a claim nobody re-checks, and it gives the runtime checks a - # place to land when there is something to run. + # A green compile does NOT catch the separator bug above, or anything else that is + # wrong only at runtime. What this job buys is that the macOS target stops being a + # claim nobody re-checks — and, since #191, that it is also where the runtime + # checks land, which is what the previous version of this comment said it was + # waiting for. + # + # The job NAME is load-bearing: `Desktop head on macOS` is listed as a required + # check (.github/merge-queue/README.md), so renaming it to describe the new step + # would silently stop that check gating anything. # # Deliberately NOT covered, so the green tick does not imply more than it means: # * osx-x64. The head declares it; nobody builds or ships it. macos-latest is @@ -303,16 +314,18 @@ jobs: # a matter of taste: the first test whose VERDICT can differ by host — a path # separator, a native asset, a culture or filesystem-case assumption — is the # one that belongs here, and #88's separator bug is exactly that shape. - # * tools/parity-audit/AvaloniaCapture. It is no longer a Windows tool — #89 - # dropped the win-x64 pin so it captures on macOS, and this runner is - # therefore the natural home of a macOS visual gate. Left out anyway, for - # now, and the reason is evidence rather than cost: the harness has been run - # on exactly one Mac (verified deterministic there, 18/18 screens - # byte-identical across two runs), and whether a GitHub macOS runner - # reproduces those same bytes is untested. A baseline check added on that - # assumption would fail on its first run and read as a UI regression, which - # is precisely the false positive #89 exists to stop. Compiling it here would - # be strictly redundant with the `desktop` job. + # * `tools/parity-audit/run.sh --check-baseline`. HALF of the visual gate now + # runs here — see the determinism step at the end of this job (#191) — and + # this is the half that still does not, for the reason #89 and #179 both gave: + # the committed baselines/macos-arm64.sha256 has been reproduced on exactly + # one Mac, and whether a GitHub macOS runner produces those same bytes is + # untested. A baseline check added on that assumption would fail on its first + # run and read as a UI regression, which is precisely the false positive #89 + # exists to stop. #188 is the ticket that measures a second Apple-silicon + # machine; gating the baseline is a third one, and if the bytes disagree it is + # the manifest's OS+arch NAME that needs fixing rather than the gate. + # `--determinism` carries none of that risk: it compares this runner against + # ITSELF, so nothing in its verdict depends on another machine's bytes. # * A dependency-graph upload for `avalonia-graph`. That gate reads the # `libraries` section, which comes from a RID-LESS restore and so is # byte-identical to the one `desktop` already uploads. Uploading it would add @@ -338,10 +351,20 @@ jobs: # instead, against the per-RID assets a RID-less restore has already produced # for every declared RuntimeIdentifier. # - # The head alone is enough to restore — Daqifi.Avalonia and the vendored + # The head alone is enough to restore the app — Daqifi.Avalonia and the vendored # OxyPlot.Avalonia arrive as project references. + # + # The capture harness is restored EXPLICITLY rather than implicitly by the + # `dotnet run` inside run.sh, for two reasons. An NU1004 lock-file failure gets + # attributed to a restore step instead of surfacing as "the visual gate broke"; + # and the guard below then covers this lock file on a macOS derivation host, + # which nothing did before — #106 was a lock-file key that macOS and Linux + # derived with different casing, and AvaloniaCapture's lock file is otherwise + # only ever derived on ubuntu. - name: Restore - run: dotnet restore Daqifi.Avalonia.Desktop/Daqifi.Avalonia.Desktop.csproj + run: | + dotnet restore Daqifi.Avalonia.Desktop/Daqifi.Avalonia.Desktop.csproj + dotnet restore tools/parity-audit/AvaloniaCapture/AvaloniaCapture.csproj # Same guard as every other job. - name: Lock files must be unchanged by restore @@ -485,6 +508,104 @@ jobs: fi echo "arm64 apphost and all five macOS natives present, each with an arm64 slice" + # ---- The visual gate's determinism half (#191) ---- + # + # The first step in this workflow that RUNS this repo's UI code instead of + # compiling it. It boots the real Daqifi.Avalonia app headless five times over + # and requires every PNG to be byte-identical to the first run's. + # + # What it catches, and nothing else did: a harness that races its own animations + # produces differences indistinguishable from a real regression, and this + # project has been burned by that twice — a 65.6% "visual regression" that was + # an unfinished fade-in, and the two live settle-loop races #179 found by + # running exactly this mode on macOS (a pane saved mid-fade, ~1 run in 8; the + # shared SplitView reopened mid-close, a 6-pixel edge flip, 50/50). Both are + # fixed, and until now nothing stopped the next one landing. + # + # Five runs is the harness default and the sample size is arithmetic rather than + # taste: two runs miss a 50/50 per-run flip half the time, five miss it ~6% of + # the time. DETERMINISM_RUNS raises it when someone is chasing a flake. + # + # This asserts NOTHING about cross-machine reproducibility — it compares this + # runner against itself — which is exactly why it can be here while + # --check-baseline cannot. See the job header, and #188. + # + # `bash tools/parity-audit/run.sh`, not `./run.sh`: the script is committed mode + # 644, so invoking it through its exec bit would fail on a fresh checkout. It + # needs bash specifically (arrays, `shopt`), not sh. + # + # No RID anywhere in here, and none needed: run.sh restores implicitly through + # `dotnet run`, a RID-less restore cannot prune a lock file the way #85's `-r` + # one did, and the harness was restored explicitly above anyway. + - name: Visual capture harness must be deterministic + id: determinism + env: + # Under RUNNER_TEMP rather than the workspace: each pass writes a throwaway + # DAQIFI_DATA_DIR and ~3.5 MB of PNGs, and none of that should be anywhere a + # later `git diff` check can see it. Must stay in step with the upload path. + PARITY_OUT: ${{ runner.temp }}/parity-audit + run: | + set -euo pipefail + bash tools/parity-audit/run.sh --determinism "${PARITY_OUT}" + + # ASSERT THE SIZE OF THE CAPTURE SET, which the byte comparison above cannot: + # it compares whatever BOTH runs contain, so a consistently absent screen + # reads as a clean "17/17 byte-identical" rather than as an incomplete + # capture. #179 put a guard for that inside the harness (ExpectedScreens) and + # this is deliberately a SECOND, independent statement of the same contract, + # because the harness's list is a variable that a change can edit downwards: + # if the only record of the expected count sits beside the code that + # produces it, a gate that shrinks stays green. A gate that quietly shrinks + # its own scope is worse than no gate, because it is believed. + # + # Counting r1 alone is sufficient. run.sh compares every later run against r1 + # in BOTH directions, so a run whose set of names differs has already failed. + # + # Adding a screen therefore means editing two places on purpose: the + # harness's ExpectedScreens, and this number. + expected=18 + actual=$(find "${PARITY_OUT}/determinism/r1" -maxdepth 1 -type f -name '*.png' \ + | wc -l | tr -d '[:space:]') + if [ "${actual}" != "${expected}" ]; then + echo "::error::the capture produced ${actual} screens, not ${expected}." \ + "Every run agreed with every other, so this is NOT a determinism" \ + "failure — it is an incomplete capture that passed the comparison by" \ + "being incomplete in the same way twice. Either a capture site" \ + "stopped firing, or the screen set changed on purpose and both this" \ + "number and AvaloniaCapture's ExpectedScreens need updating in the" \ + "same commit." + ls -la "${PARITY_OUT}/determinism/r1" + exit 1 + fi + echo "${actual} screens, byte-identical across every run" + + # The PNGs, on failure only. A determinism failure names the screens that + # differed and is close to unactionable without the two encodings in hand — and + # #179 already had to fix a cleanup that destroyed the very evidence its own + # failure message told you to go and fetch. Not a thing to leave for later. + # + # Conditioned on THIS step rather than a bare `failure()`. `failure()` is + # required — an `if:` with no status function has an implicit `success()` and + # would never run here — but on its own it would also fire after an earlier + # failure (restore, the arm64 check), uploading nothing and adding a second, + # misleading red step to a job that already said what was wrong. + # + # if-no-files-found: warn, for that same reason. The only way this fires against + # an empty tree is a harness that captured nothing at all, which run.sh already + # fails with its own message ("run 1 produced no PNGs at all, so there is + # nothing to compare"); a second failure here would restate a diagnosis that is + # already on screen. + # + # Hidden files are excluded by upload-artifact's own default, which is what + # keeps each run's throwaway .appdata (SQLite DB + logs) out of the artifact. + - name: Upload the determinism captures + if: ${{ failure() && steps.determinism.outcome == 'failure' }} + uses: actions/upload-artifact@v4 + with: + name: parity-determinism-macos + path: ${{ runner.temp }}/parity-audit/determinism + if-no-files-found: warn + android: name: Android head runs-on: ubuntu-latest diff --git a/tools/parity-audit/README.md b/tools/parity-audit/README.md index 8974714f..d1f68781 100644 --- a/tools/parity-audit/README.md +++ b/tools/parity-audit/README.md @@ -87,6 +87,12 @@ It captures the Avalonia side N times into `/determinism/r1..rN` and requir every PNG to be byte-identical to the first run's, failing non-zero if any differs, is missing, or if there is nothing to compare at all. +Since #191 this is also a **CI gate**: the `Desktop head on macOS` job runs it on every +pull request, additionally asserts that the capture produced all 18 screens, and uploads +`/determinism/` as an artifact when it fails, so the differing PNGs are in hand. +`--check-baseline` is deliberately *not* gated — it compares against bytes recorded on +one particular Mac, and #188 is the ticket that measures a second one. + This is not ceremony. Avalonia's animation clock advances with wall time, so whether a fade-in has finished when the shutter opens is a race, and a racing harness produces differences indistinguishable from a real regression: on this project two From 1f08028ea525bba64c7c32bb0bbc8df7fa8c47a4 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 2 Sep 2026 10:45:45 -0600 Subject: [PATCH 2/7] fix(parity): the settle loop drives the animation, so one still pair is not settled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CI determinism gate added in the previous commit failed on its first run on a GitHub macos-latest runner — a host this harness had never been executed on — and uploaded the evidence, which is exactly what it is for (run 33655089083, job 100331537976). What it caught: run 4 of 5 wrote desktop-8-livegraph-settings-flyout with 6 pixels one unit off, every one of them in the SplitView pane's left edge column (x=1059). That is 0.0005% of the image and the same signature as the flyout-edge flip #179 fixed for desktop-7 and desktop-9. The other four runs, 20 consecutive runs on an M-series Mac, and the committed baselines/macos-arm64.sha256 all agree byte for byte — so 89 of the 90 screen-vs-screen comparisons between this Mac and that runner are identical, and the one that is not is this. Cause: the headless render timer is MANUAL, so an animation advances only while Pump() ticks it. The settle loop is therefore driving the UI, not observing it, and stopping the loop freezes whatever frame the animation had reached. Near the end of a transition the remaining motion is sub-pixel, two samples 50 ms apart render identically, and the loop stops with the animation parked a hair short of its target — saving a frame that is stable, plausible, and wrong. It is the quietest member of this family: the 65.6% and 81% cases before it announced themselves. Fix: require two CONSECUTIVE identical comparisons (SettleStableSamples), resetting the count on any change. That buys 50 ms more of ticked animation clock past the point where the picture stopped moving, which is longer than any tail in this app. It cannot change a frame that had genuinely finished, and does not: with this in place 20 local runs produce all 18 screens byte-identical to the 20-run set captured before it, and `run.sh --check-baseline` still passes against the committed manifest. Cost is one extra 50 ms round per screen, ~0.9 s per capture pass (measured: 7.1 s to 8.2 s a run on this Mac). Co-Authored-By: Claude Opus 5 --- tools/parity-audit/AvaloniaCapture/Program.cs | 46 +++++++++++++++++-- tools/parity-audit/README.md | 18 +++++++- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/tools/parity-audit/AvaloniaCapture/Program.cs b/tools/parity-audit/AvaloniaCapture/Program.cs index 18fbd69f..b05ca220 100644 --- a/tools/parity-audit/AvaloniaCapture/Program.cs +++ b/tools/parity-audit/AvaloniaCapture/Program.cs @@ -402,8 +402,9 @@ private static void Pump() // because one run caught the pane at ~50% opacity mid-fade and the other caught it settled. // Diffed against another Avalonia version that reads as a catastrophic visual regression. // - // So instead of pumping a fixed number of times and hoping, pump until two consecutive frames - // are byte-identical. Comparing encoded PNGs is exact and needs no pixel-buffer access. + // So instead of pumping a fixed number of times and hoping, pump until the frame stops + // changing (SettleStableSamples below says how much stillness counts as stopped). Comparing + // encoded PNGs is exact and needs no pixel-buffer access. private const int SettleMaxRounds = 40; // ...and let REAL TIME pass between the two frames being compared, which is the half that @@ -422,10 +423,36 @@ private static void Pump() // interval, so an unfinished one is guaranteed to move between two samples. // // 50 ms because it must exceed a frame and stay well under the shortest transition. The cost - // is bounded and small: a settling screen takes 2-4 rounds (~100-200 ms), and only a screen + // is bounded and small: a settling screen takes 3-5 rounds (~150-250 ms), and only a screen // that never settles — which fails the capture rather than saving it — pays the full 2 s. private static readonly TimeSpan SettleSampleInterval = TimeSpan.FromMilliseconds(50); + // How many CONSECUTIVE identical comparisons end the loop. Two, not one, and the reason is a + // third defect of this same family — found by the CI determinism gate (#191) on its first run + // on a GitHub macos-latest runner, which is a host this harness had never been executed on. + // + // The headless render timer is MANUAL: an animation advances only while Pump() ticks it. So + // the loop is not merely observing the UI, it is DRIVING it, and stopping the loop freezes + // whatever frame the animation had reached. Near the end of a transition the remaining motion + // is sub-pixel, two samples 50 ms apart therefore render identically, and the loop stops — + // leaving the animation parked a hair short of its target and saving THAT frame. + // + // It is a much quieter failure than the two above, because the frame it saves is a stable + // one: run 4 of 5 on the runner wrote desktop-8-livegraph-settings-flyout with 6 pixels one + // unit off, all in the SplitView pane's left edge column (x=1059) — the same signature as the + // flyout-edge flip #179 fixed for desktop-7 and desktop-9, and 0.0005% of the image. The + // other four runs, and 20 consecutive runs on an M-series Mac, agree byte for byte with each + // other and with the committed baseline. A one-unit difference on six antialiased pixels is + // sub-perceptual and still exactly what a byte-identity gate must not manufacture. + // + // Requiring a second identical comparison buys 50 ms more of ticked animation clock past the + // point where the picture stopped moving, which is longer than any tail this app has. It + // cannot change a frame that had genuinely finished — verified: with this in place, 20 local + // runs produce all 18 screens byte-identical to the set recorded before it. + // + // Cost: one extra 50 ms round per screen, so ~0.9 s per capture pass. + private const int SettleStableSamples = 2; + private static byte[] Encode(Window w) { using var buffer = new MemoryStream(); @@ -448,6 +475,7 @@ private static (byte[] Frame, bool Settled, int Rounds) SettledFrame(Window w) { Pump(); var previous = Encode(w); + var stable = 0; for (var round = 1; round <= SettleMaxRounds; round++) { Thread.Sleep(SettleSampleInterval); @@ -455,7 +483,17 @@ private static (byte[] Frame, bool Settled, int Rounds) SettledFrame(Window w) var current = Encode(w); if (current.Length == previous.Length && current.AsSpan().SequenceEqual(previous)) { - return (current, true, round); + // Consecutive, and reset on ANY change. A screen that moves again after a + // pair of identical samples has not settled, it has paused — which is the + // whole point of counting more than one. + if (++stable >= SettleStableSamples) + { + return (current, true, round); + } + } + else + { + stable = 0; } previous = current; } diff --git a/tools/parity-audit/README.md b/tools/parity-audit/README.md index d1f68781..67e9608a 100644 --- a/tools/parity-audit/README.md +++ b/tools/parity-audit/README.md @@ -113,9 +113,23 @@ the race**, both fixed in the same change that unpinned the RID: while the previous close was still animating. That left a one-pixel-wide flip on the pane edge, 50/50 per run. -Both are in `Program.cs` with the measurements attached. The lesson generalises: a +**The first CI run of this mode, on a GitHub `macos-latest` runner, found a third** +(#191) — and it is the quietest of the three, because the frame it saved was *stable*: + +- The headless render timer is manual, so an animation advances only while the settle + loop ticks it. The loop is therefore driving the UI, not just watching it, and + stopping it freezes the animation wherever it had got to. Near the end of a + transition the remaining motion is sub-pixel, two samples 50 ms apart render + identically, and the loop stops with the animation parked a hair short of its + target. One run in five wrote `desktop-8-livegraph-settings-flyout` with **6 pixels + one unit off**, all in the `SplitView` pane's left edge column (x=1059) — the same + signature as the flip above, 0.0005% of the image. Fixed by requiring two + consecutive identical comparisons (`SettleStableSamples`). + +All three are in `Program.cs` with the measurements attached. The lesson generalises: a settle loop is only as good as the evidence that it settled, and "two samples agreed" -is not evidence unless something could have changed between them. +is not evidence unless something could have changed between them — nor is one pair of +samples evidence when the loop itself is what advances the clock. **Why five runs and not two.** Both defects above were roughly coin-flips per run, and two runs miss a 50/50 flip half the time; five gets that to ~6%, ten to ~0.2%, at about From 269b13a7ef950a28107fac0f940416cbebc81a60 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 2 Sep 2026 10:50:05 -0600 Subject: [PATCH 3/7] =?UTF-8?q?test(ci):=20TEMPORARY=20=E2=80=94=2015=20de?= =?UTF-8?q?terminism=20runs=20to=20size=20the=20settle=20fix=20on=20the=20?= =?UTF-8?q?runner?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverted in the next commit. Five green runs against a defect that showed up once in five is ~33% likely by luck; fifteen brings that to ~3.5%. This commit exists so the evidence for the previous one is on the record with a run id attached. Co-Authored-By: Claude Opus 5 --- .github/workflows/build.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ae091207..03170b36 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -544,6 +544,10 @@ jobs: # DAQIFI_DATA_DIR and ~3.5 MB of PNGs, and none of that should be anywhere a # later `git diff` check can see it. Must stay in step with the upload path. PARITY_OUT: ${{ runner.temp }}/parity-audit + # TEMPORARY, reverted in the next commit: a bigger sample on the runner to + # test the settle fix. Five green runs would be ~33% likely by luck against + # a 1-in-5 flake; fifteen is ~3.5%. + DETERMINISM_RUNS: 15 run: | set -euo pipefail bash tools/parity-audit/run.sh --determinism "${PARITY_OUT}" From b807590892dabc0d774ce0f970674ceaea9e5c80 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 2 Sep 2026 11:07:18 -0600 Subject: [PATCH 4/7] =?UTF-8?q?Revert=20"test(ci):=20TEMPORARY=20=E2=80=94?= =?UTF-8?q?=2015=20determinism=20runs=20to=20size=20the=20settle=20fix=20o?= =?UTF-8?q?n=20the=20runner"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 269b13a7ef950a28107fac0f940416cbebc81a60. --- .github/workflows/build.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 03170b36..ae091207 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -544,10 +544,6 @@ jobs: # DAQIFI_DATA_DIR and ~3.5 MB of PNGs, and none of that should be anywhere a # later `git diff` check can see it. Must stay in step with the upload path. PARITY_OUT: ${{ runner.temp }}/parity-audit - # TEMPORARY, reverted in the next commit: a bigger sample on the runner to - # test the settle fix. Five green runs would be ~33% likely by luck against - # a 1-in-5 flake; fifteen is ~3.5%. - DETERMINISM_RUNS: 15 run: | set -euo pipefail bash tools/parity-audit/run.sh --determinism "${PARITY_OUT}" From 1d111b6fca533cf010b7c99ad874225fb474834f Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 2 Sep 2026 11:07:33 -0600 Subject: [PATCH 5/7] Revert "fix(parity): the settle loop drives the animation, so one still pair is not settled" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That fix was wrong, and the gate it was written for is what proved it. Requiring two consecutive identical comparisons passed a 5-run job on the runner, then failed a 15-run one three times (run 33657356313) at the same ~1-in-5 rate as before. The flyout-edge flip is not an animation stopping one sample early. A settle FLOOR was tried next and was worse: with 10 rounds (500 ms) forced before any frame is accepted, the same 6-pixel flip reproduced on a Mac that had been clean for 40 consecutive runs (2 failures in 12), and desktop-7-notifications-flyout settled to a THIRD encoding that no previous run had produced. So the picture is still changing after half a second, and waiting longer selects a different wrong answer rather than the right one. Both experiments are dead ends and neither belongs in the tree. What is left is the measurement, which is in the follow-up issue: two stable encodings of column x=1059 — the SplitView pane's own left edge — differing by one unit on six pixels, on whichever flyout the run happens to hit. Co-Authored-By: Claude Opus 5 --- tools/parity-audit/AvaloniaCapture/Program.cs | 46 ++----------------- tools/parity-audit/README.md | 18 +------- 2 files changed, 6 insertions(+), 58 deletions(-) diff --git a/tools/parity-audit/AvaloniaCapture/Program.cs b/tools/parity-audit/AvaloniaCapture/Program.cs index b05ca220..18fbd69f 100644 --- a/tools/parity-audit/AvaloniaCapture/Program.cs +++ b/tools/parity-audit/AvaloniaCapture/Program.cs @@ -402,9 +402,8 @@ private static void Pump() // because one run caught the pane at ~50% opacity mid-fade and the other caught it settled. // Diffed against another Avalonia version that reads as a catastrophic visual regression. // - // So instead of pumping a fixed number of times and hoping, pump until the frame stops - // changing (SettleStableSamples below says how much stillness counts as stopped). Comparing - // encoded PNGs is exact and needs no pixel-buffer access. + // So instead of pumping a fixed number of times and hoping, pump until two consecutive frames + // are byte-identical. Comparing encoded PNGs is exact and needs no pixel-buffer access. private const int SettleMaxRounds = 40; // ...and let REAL TIME pass between the two frames being compared, which is the half that @@ -423,36 +422,10 @@ private static void Pump() // interval, so an unfinished one is guaranteed to move between two samples. // // 50 ms because it must exceed a frame and stay well under the shortest transition. The cost - // is bounded and small: a settling screen takes 3-5 rounds (~150-250 ms), and only a screen + // is bounded and small: a settling screen takes 2-4 rounds (~100-200 ms), and only a screen // that never settles — which fails the capture rather than saving it — pays the full 2 s. private static readonly TimeSpan SettleSampleInterval = TimeSpan.FromMilliseconds(50); - // How many CONSECUTIVE identical comparisons end the loop. Two, not one, and the reason is a - // third defect of this same family — found by the CI determinism gate (#191) on its first run - // on a GitHub macos-latest runner, which is a host this harness had never been executed on. - // - // The headless render timer is MANUAL: an animation advances only while Pump() ticks it. So - // the loop is not merely observing the UI, it is DRIVING it, and stopping the loop freezes - // whatever frame the animation had reached. Near the end of a transition the remaining motion - // is sub-pixel, two samples 50 ms apart therefore render identically, and the loop stops — - // leaving the animation parked a hair short of its target and saving THAT frame. - // - // It is a much quieter failure than the two above, because the frame it saves is a stable - // one: run 4 of 5 on the runner wrote desktop-8-livegraph-settings-flyout with 6 pixels one - // unit off, all in the SplitView pane's left edge column (x=1059) — the same signature as the - // flyout-edge flip #179 fixed for desktop-7 and desktop-9, and 0.0005% of the image. The - // other four runs, and 20 consecutive runs on an M-series Mac, agree byte for byte with each - // other and with the committed baseline. A one-unit difference on six antialiased pixels is - // sub-perceptual and still exactly what a byte-identity gate must not manufacture. - // - // Requiring a second identical comparison buys 50 ms more of ticked animation clock past the - // point where the picture stopped moving, which is longer than any tail this app has. It - // cannot change a frame that had genuinely finished — verified: with this in place, 20 local - // runs produce all 18 screens byte-identical to the set recorded before it. - // - // Cost: one extra 50 ms round per screen, so ~0.9 s per capture pass. - private const int SettleStableSamples = 2; - private static byte[] Encode(Window w) { using var buffer = new MemoryStream(); @@ -475,7 +448,6 @@ private static (byte[] Frame, bool Settled, int Rounds) SettledFrame(Window w) { Pump(); var previous = Encode(w); - var stable = 0; for (var round = 1; round <= SettleMaxRounds; round++) { Thread.Sleep(SettleSampleInterval); @@ -483,17 +455,7 @@ private static (byte[] Frame, bool Settled, int Rounds) SettledFrame(Window w) var current = Encode(w); if (current.Length == previous.Length && current.AsSpan().SequenceEqual(previous)) { - // Consecutive, and reset on ANY change. A screen that moves again after a - // pair of identical samples has not settled, it has paused — which is the - // whole point of counting more than one. - if (++stable >= SettleStableSamples) - { - return (current, true, round); - } - } - else - { - stable = 0; + return (current, true, round); } previous = current; } diff --git a/tools/parity-audit/README.md b/tools/parity-audit/README.md index 67e9608a..d1f68781 100644 --- a/tools/parity-audit/README.md +++ b/tools/parity-audit/README.md @@ -113,23 +113,9 @@ the race**, both fixed in the same change that unpinned the RID: while the previous close was still animating. That left a one-pixel-wide flip on the pane edge, 50/50 per run. -**The first CI run of this mode, on a GitHub `macos-latest` runner, found a third** -(#191) — and it is the quietest of the three, because the frame it saved was *stable*: - -- The headless render timer is manual, so an animation advances only while the settle - loop ticks it. The loop is therefore driving the UI, not just watching it, and - stopping it freezes the animation wherever it had got to. Near the end of a - transition the remaining motion is sub-pixel, two samples 50 ms apart render - identically, and the loop stops with the animation parked a hair short of its - target. One run in five wrote `desktop-8-livegraph-settings-flyout` with **6 pixels - one unit off**, all in the `SplitView` pane's left edge column (x=1059) — the same - signature as the flip above, 0.0005% of the image. Fixed by requiring two - consecutive identical comparisons (`SettleStableSamples`). - -All three are in `Program.cs` with the measurements attached. The lesson generalises: a +Both are in `Program.cs` with the measurements attached. The lesson generalises: a settle loop is only as good as the evidence that it settled, and "two samples agreed" -is not evidence unless something could have changed between them — nor is one pair of -samples evidence when the loop itself is what advances the clock. +is not evidence unless something could have changed between them. **Why five runs and not two.** Both defects above were roughly coin-flips per run, and two runs miss a 50/50 flip half the time; five gets that to ~6%, ten to ~0.2%, at about From 9496201c39087071a75a7234f106c5eba6f7f909 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 2 Sep 2026 12:44:33 -0600 Subject: [PATCH 6/7] ci: the failure artifact dropped the one file a pre-PNG failure leaves behind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The determinism gate uploads `/determinism/` when it fails, so a reviewer has the differing PNGs. But `run.sh`'s `run_capture` writes each pass's full stdout and stderr to `/.capture-determinism-N.log` — at the output ROOT, one level above the directory being uploaded, and as a dotfile. Two independent reasons nothing under the upload's `path:` ever matched them, which is why `include-hidden-files` on its own would not have fixed it either. 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 `/determinism/rN` empty, so the upload found no files at all, `if-no-files-found: warn` shrugged, and no artifact was produced. #191 asked for this gate because 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 in new clothes. A new failure-only step copies the logs into `/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: the glob would force `include-hidden-files: true`, and that flag is not selective — it would also sweep in each run's throwaway `.appdata` (SQLite DB + app logs) from inside the tree being uploaded. Copying keeps that default exclusion doing its job, and puts the evidence where a plain `ls` on the unzipped artifact shows it, rather than under a name `ls` hides — a trap this README already warns readers about. The step is the only place that knows `run_capture`'s log naming, so it refuses to fail silently: 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. Co-Authored-By: Claude Opus 5 --- .github/workflows/build.yml | 89 ++++++++++++++++++++++++++++++++---- tools/parity-audit/README.md | 7 +++ 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ae091207..e3c67849 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -579,10 +579,79 @@ jobs: fi echo "${actual} screens, byte-identical across every run" - # The PNGs, on failure only. A determinism failure names the screens that - # differed and is close to unactionable without the two encodings in hand — and - # #179 already had to fix a cleanup that destroyed the very evidence its own - # failure message told you to go and fetch. Not a thing to leave for later. + # The capture logs, so the artifact is never empty when it matters most. + # + # run.sh's run_capture() sends each pass's whole stdout+stderr to + # `/.capture-determinism-N.log`, and its failure line points straight at it + # ("!! determinism-3 failed (see …)"). Two things put those logs out of reach of + # the upload below, and both had to be true for the artifact to be right: + # + # 1. They are written at the OUTPUT ROOT — one level ABOVE `/determinism`, + # which is the only directory the upload collects. Nothing under `path:` + # ever matched them, so `include-hidden-files` alone would not have helped. + # 2. They are dotfiles, which upload-artifact drops by default. + # + # The case that suffers is exactly the one with the least to go on: a pass that + # dies BEFORE writing a single PNG — a crash during boot, or a build error out of + # `dotnet run` — leaves `/determinism/rN` empty, so the upload found no + # 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 differing PNGs in hand; a failure with no PNGs is only + # actionable with the log, and that was the one thing being thrown away. It is + # #179's cleanup-destroys-the-evidence trap wearing different clothes. + # + # COPIED to a non-hidden name rather than added to `path:`. Adding the glob would + # force `include-hidden-files: true`, and that flag is not selective: it would + # also sweep in each run's throwaway `.appdata` (SQLite DB + app logs) — one per + # run — from inside the very tree being uploaded. Copying keeps that default + # exclusion doing its job, and puts the evidence somewhere a plain `ls` on the + # unzipped artifact will show, which is the same dotfile trap + # tools/parity-audit/README.md already warns readers about. + # + # The leading dot is the only thing dropped, so `capture-determinism-3.log` here + # is the `.capture-determinism-3.log` the failing step named on screen. + - name: Collect the capture logs + if: ${{ failure() && steps.determinism.outcome == 'failure' }} + env: + PARITY_OUT: ${{ runner.temp }}/parity-audit + run: | + set -euo pipefail + dest="${PARITY_OUT}/determinism/capture-logs" + mkdir -p "${dest}" + found=0 + for log in "${PARITY_OUT}"/.capture-determinism-*.log; do + # An unmatched glob stays literal; skip it rather than copying a filename + # that does not exist. + [ -e "${log}" ] || continue + name="$(basename "${log}")" + cp "${log}" "${dest}/${name#.}" + found=$((found + 1)) + done + + # This step is the one place that knows run.sh's log naming, so a rename + # there must not quietly produce a plausible-looking artifact with the logs + # missing — that is the failure mode this whole step exists to end. Say so, + # and say so INSIDE the artifact too, where whoever downloads it is looking. + if [ "${found}" -eq 0 ]; then + echo "::warning::no .capture-determinism-*.log found under ${PARITY_OUT}." \ + "run_capture()'s log naming in tools/parity-audit/run.sh has moved and" \ + "this step needs updating; the artifact carries the PNGs only." + printf '%s\n' \ + "No capture logs were collected." \ + "" \ + "This step looked for .capture-determinism-*.log at the parity-audit output" \ + "root, which is where run_capture() in tools/parity-audit/run.sh writes each" \ + "pass's stdout+stderr. Nothing matched. If that naming changed, update the" \ + "'Collect the capture logs' step in .github/workflows/build.yml." \ + > "${dest}/NO-CAPTURE-LOGS.txt" + fi + echo "collected ${found} capture log(s) into ${dest}" + + # The PNGs and those logs, on failure only. A determinism failure names the + # screens that differed and is close to unactionable without the two encodings in + # hand — and #179 already had to fix a cleanup that destroyed the very evidence + # its own failure message told you to go and fetch. Not a thing to leave for + # later. # # Conditioned on THIS step rather than a bare `failure()`. `failure()` is # required — an `if:` with no status function has an implicit `success()` and @@ -590,13 +659,13 @@ jobs: # failure (restore, the arm64 check), uploading nothing and adding a second, # misleading red step to a job that already said what was wrong. # - # if-no-files-found: warn, for that same reason. The only way this fires against - # an empty tree is a harness that captured nothing at all, which run.sh already - # fails with its own message ("run 1 produced no PNGs at all, so there is - # nothing to compare"); a second failure here would restate a diagnosis that is - # already on screen. + # if-no-files-found: warn, and it should now be unreachable: the collect step + # above writes into this tree unconditionally, so there is always at least a + # note. It stays `warn` rather than `error` for the reason above — the only way + # to arrive here empty is the collect step having failed first, and a second red + # step would restate a diagnosis already on screen. # - # Hidden files are excluded by upload-artifact's own default, which is what + # Hidden files are still excluded by upload-artifact's own default, which is what # keeps each run's throwaway .appdata (SQLite DB + logs) out of the artifact. - name: Upload the determinism captures if: ${{ failure() && steps.determinism.outcome == 'failure' }} diff --git a/tools/parity-audit/README.md b/tools/parity-audit/README.md index d1f68781..b4762ee0 100644 --- a/tools/parity-audit/README.md +++ b/tools/parity-audit/README.md @@ -93,6 +93,13 @@ pull request, additionally asserts that the capture produced all 18 screens, and `--check-baseline` is deliberately *not* gated — it compares against bytes recorded on one particular Mac, and #188 is the ticket that measures a second one. +That artifact also carries each pass's `.capture-determinism-N.log`, copied in under +`capture-logs/` with the leading dot dropped. The job has to copy them because +`run_capture` writes them at the output *root*, one level above the directory being +uploaded, and as dotfiles — so nothing under the upload's `path:` matched them. Without +that copy the one failure with nothing else to go on, a pass that dies before writing a +single PNG, produced an artifact with no files in it at all. + This is not ceremony. Avalonia's animation clock advances with wall time, so whether a fade-in has finished when the shutter opens is a race, and a racing harness produces differences indistinguishable from a real regression: on this project two From c6dfa35743587cb6c264769c1f3376c56e452bbe Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 2 Sep 2026 12:56:21 -0600 Subject: [PATCH 7/7] ci: say where else the parity out-dir is spelled, and why it cannot be hoisted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The collect step makes three copies of this path in one job. The obvious tidy-up — a job-level env: — silently does not work, because the runner context is not available to jobs..env (only to steps). Name both facts where someone would try it. --- .github/workflows/build.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e3c67849..44d6c367 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -542,7 +542,12 @@ jobs: env: # Under RUNNER_TEMP rather than the workspace: each pass writes a throwaway # DAQIFI_DATA_DIR and ~3.5 MB of PNGs, and none of that should be anywhere a - # later `git diff` check can see it. Must stay in step with the upload path. + # later `git diff` check can see it. + # + # This path is spelled out in THREE places in this job — here, the collect + # step below, and the upload's `path:` — and they must stay in step. It + # cannot be hoisted to a job-level `env:`, because the `runner` context is + # not available there; only steps can read `runner.temp`. PARITY_OUT: ${{ runner.temp }}/parity-audit run: | set -euo pipefail