Skip to content

ci: prove the macos-arm64 baseline on a second machine, and keep it proved - #204

Merged
tylerkron merged 4 commits into
mainfrom
ci/baseline-second-machine-188
Sep 3, 2026
Merged

tylerkron merged 4 commits into
mainfrom
ci/baseline-second-machine-188

Conversation

@tylerkron

@tylerkron tylerkron commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Not merging — for review.

Closes #188.

What was wrong

tools/parity-audit/baselines/macos-arm64.sha256 is the visual gate's reference: 18
SHA-256 hashes, one per captured screen. Its name is keyed on OS and architecture, which
asserts that any osx-arm64 host reproduces those exact bytes. Nothing had ever checked
that. It was recorded on one Mac, and #188's point is that the name was writing a cheque
the evidence could not cash.

The way that hurts someone is concrete. A developer runs ./run.sh --check-baseline on
their own machine, gets eighteen red screens, and has to decide whether they broke the UI
or whether their laptop simply renders differently. The README does warn them;
realistically they conclude the harness is untrustworthy and stop running it, which is the
exact failure #89 exists to prevent. It also left the CI question permanently
unanswerable: while the reference is a single-machine artifact wearing an OS+arch name, no
gate built on it can tell a regression from a host difference.

What changed

The measurement came back clean, so the work is to make CI keep it that way. A GitHub
macos-latest runner reproduces all eighteen hashes byte for byte — a different
Apple-silicon machine on a different macOS build (26.5.2 / 25F84, image macos26) from
the one that recorded them (26.5 / 25F71). The macos-arm64 name is earned.

So the Desktop head on macOS job now checks the baseline as well as determinism, and the
name stops being an assumption maintained by hope: the day two Apple-silicon hosts
disagree, a red tick says so on the pull request that did it.

It does that over the same captures, not a new one. --determinism already runs five
times and proves the five agree; the new step verifies determinism/r1 — one of the sets
every other run was just shown to byte-match — against the committed manifest. One
statement about one set of bytes, and it costs ~0.3 s, because capturing is the
expensive part and it had already happened.

The ordering is load-bearing rather than tidy. A baseline check on a host whose captures
race is a machine for manufacturing differences that are not regressions — which is why
this could not have been added before #202, when ~1 capture run in 5 on this runner
carried a six-pixel flip on the SplitView pane edge.

run.sh gains --check-captured <dir>: --check-baseline's verification against a
directory that has already been captured. Both modes now share one resolve_baseline
preflight and one baseline_check, so the manifest lookup, the both-directions comparison
and the failure text exist once instead of twice. The failure-artifact steps fire for
either check now, since a baseline failure wants the same PNGs and they are already
sitting in determinism/.

Nothing was loosened. The determinism gate is untouched; this is strictly an
additional assertion on the same run.

What this asks of everyone else

A pull request that changes what any of the 18 screens looks like must now re-record
macos-arm64.sha256 in the same commit, or the macOS job goes red. That was already the
documented contract — this is the first time it is enforced.

When it does go red, the causes in order of likelihood are: a UI change that forgot the
re-record; a real visual regression; or the macos-latest image moving under us. The
third is the one your own Mac cannot reproduce, so the step prints the runner's macOS
build and image version on every run, green ones included — a red run is diagnosed by
comparing it against a green one, and that needs both sides on the record.

There is a fourth, rarer case and the comments name it rather than pretending it away: a
red that does not reproduce is a capture race that survived five determinism samples
(which miss a 50/50 per-run flip ~6% of the time). That is a fourth instance of the class
#179 and #201 each turned out to be, and it wants its own ticket, not a re-run.

Verification

The claim this PR makes, on real runners. Four Desktop head on macOS jobs, every one
green on both steps:

run head determinism baseline
33692427866 (dispatch) acc8da3 18/18 across 5 runs baseline OK
33693741764 b849080 same same
33694204203 1b1d2a8 same same
33694476081 d8b71af same same

(The 1b1d2a8 run shows as cancelled overall — the concurrency group superseded it when
d8b71af was pushed — but its macOS job had already completed green. Commits after
acc8da3 change comments and README prose only; the workflow logic and run.sh are
identical across all four.)

From the first of those:

host: macOS 26.5.2 (25F84), arm64, runner image macos26/20260728.0273.1
== baseline (macos-arm64) ==
desktop-1-livegraph.png: OK
... 18 of 18 ...
baseline OK: every screen in .../determinism/r1 matches .../baselines/macos-arm64.sha256

A correction to an earlier measurement while I was here. #191 reported 89 of 90
screen-by-screen comparisons between this Mac and the runner as byte-identical. That was
before #202. It is now 90 of 90: determinism/r1 matches the manifest, and runs r2–r5
are byte-identical to r1, so all 18 screens agree across all five runs and with the
recording Mac.

Locally, on the recording Mac (macOS 26.5 / 25F71):

  • --check-baseline end-to-end after the refactor: all 18 OK, baseline OK.
  • --check-captured on that same capture dir: identical verdict.
  • All three failure directions, against a real capture set: a changed screen (one byte
    appended) → desktop-7…: FAILED and non-zero; a missing screen → FAILED open or read;
    an extra screen the manifest does not list → captured but absent from the baseline — the screen set has grown. Clean again once the extra is removed.
  • Argument handling: no argument, a path that is not a directory, and a directory holding
    no PNGs each fail with their own message rather than as eighteen missing screens.
  • The workflow YAML was parsed before pushing, to check the two if: expressions and the
    new step id.

Build gate, under build.2: Desktop build, iOS restore, iOS
-c Release -r iossimulator-arm64 --no-restore — all green.
git status --short -- '**/packages.lock.json' empty afterwards. The Android head is not
buildable on this Mac; CI covers it. No first-party C# changed, so no warning-count claim
is made either way.

Not verified: that a third Apple-silicon Mac agrees. Two hosts is two hosts, and the
README says exactly that rather than rounding it up to "portable".

Also, from the same investigation

#201 is closed on the CI record rather than on #202's title: 85 capture runs across 11
macos-latest jobs and 6 heads since the fix landed, zero flips, against a pre-fix rate of
7 in 30. Full table on the issue.

Diff

git diff --numstat origin/main...HEAD — .github/workflows/build.yml 91/19,
tools/parity-audit/run.sh 105/37, tools/parity-audit/README.md 49/15. Well over half
of the first two is comment prose, and no C# is touched.

🤖 Generated with Claude Code

tylerkron and others added 2 commits September 2, 2026 16:51
…d machine

tools/parity-audit/baselines/macos-arm64.sha256 is keyed on OS and architecture,
which asserts that any osx-arm64 host reproduces those exact bytes. It had only
ever been reproduced on the one Mac that recorded it (#188). A developer running
--check-baseline on their own machine and getting eighteen red screens would most
likely distrust the harness rather than read the caveat in the README, and a
distrusted visual gate stops being run at all.

Two changes on 2026-09-02 made the measurement cheap. #199 put the capture harness
on macos-latest at all; #202 made the saved bytes a function of the visual tree
rather than of the compositor's redraw history, which removed the ~1-in-5 six-pixel
flip that would have made any baseline check on that runner a false-positive
machine.

So the macOS job now checks the committed baseline as well as determinism, and it
does it over the SAME captures: --determinism proves the five runs agree with each
other, then the new step verifies determinism/r1 against the manifest. That makes
the verdict one statement about one set of bytes, and it costs no extra capture.

run.sh gains --check-captured <dir>, which is --check-baseline's verification
against a directory that has already been captured. Both modes now share one
resolve_baseline preflight and one baseline_check, so the manifest lookup, the
both-directions comparison and the failure text exist once.

The failure-artifact steps now fire for either check, since a baseline failure
wants exactly the same PNGs and they are already in determinism/.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…fied

The README's Baselines section said cross-machine reproducibility was plausible but
untested and told readers to treat a mismatch on another machine as "compare the
environments" first. Both halves are out of date: a GitHub macos-latest runner
(macOS 26.5.2 / 25F84, image macos26) reproduces all eighteen hashes recorded on
macOS 26.5 / 25F71, and CI now checks that on every pull request.

Says what a red baseline step means, in order of likelihood, including the one
explanation a developer's own Mac cannot reproduce — the runner image moving —
and notes that the step prints the host on green runs precisely so a red one has
something to be compared against. Documents --check-captured next to
--check-baseline in the Run block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Gate macOS ARM64 baseline with deterministic CI captures

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

Grey Divider

AI Description

• Adds --check-captured to validate existing screenshots against the host-specific baseline.
• Gates macOS ARM64 baseline reproducibility after deterministic captures pass.
• Reuses failure artifacts and documents cross-machine guarantees and diagnostics.
Diagram

sequenceDiagram
  participant CI as macOS Job
  participant H as Audit Script
  participant C as Stable Captures
  participant M as Baseline Manifest
  participant A as Failure Artifact
  CI->>H: Run determinism
  H->>C: Write five runs
  C-->>H: Byte-identical set
  H-->>CI: Stable verdict
  CI->>H: Check captured r1
  H->>C: Read r1
  H->>M: Verify hashes
  M-->>H: Expected hashes
  H-->>CI: Baseline verdict
  alt Baseline mismatch
    CI->>A: Upload captures and logs
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Capture a sixth baseline run
  • ➕ Keeps baseline verification operationally separate from determinism captures.
  • ➕ Provides another sample from the runner.
  • ➖ Adds unnecessary capture time.
  • ➖ Compares the manifest against bytes not included in the determinism proof.
  • ➖ Weakens the single-set verdict provided by reusing r1.
2. Fold baseline checks into determinism mode
  • ➕ Provides one command and one atomic visual-gate operation.
  • ➕ Could centralize ordering within the audit script.
  • ➖ Couples host stability checks to committed-baseline availability.
  • ➖ Reduces reuse for callers that only need determinism validation.
  • ➖ Eliminates the general-purpose ability to verify arbitrary existing captures.

Recommendation: Keep the PR’s two-stage approach. Reusing determinism/r1 gives CI one coherent claim about the same bytes without another expensive capture, while the standalone --check-captured mode preserves separation between capture production and baseline verification.

Files changed (3) +232 / -71

Enhancement (1) +105 / -37
run.shVerify existing captures against committed baselines +105/-37

Verify existing captures against committed baselines

• Adds the read-only '--check-captured' mode with explicit path validation. Refactors manifest resolution and bidirectional baseline comparison into shared functions used by both fresh and existing-capture checks, including clearer empty-directory and cross-host failure diagnostics.

tools/parity-audit/run.sh

Documentation (1) +44 / -15
README.mdDocument cross-machine baseline verification +44/-15

Document cross-machine baseline verification

• Documents '--check-captured', the ordered CI gate, and the reuse of deterministic captures. Records successful reproduction on a second Apple-silicon machine and explains how to diagnose future host or rendering differences.

tools/parity-audit/README.md

Other (1) +83 / -19
build.ymlGate committed macOS baseline after determinism +83/-19

Gate committed macOS baseline after determinism

• Adds a baseline verification step after the macOS determinism gate and records runner OS, build, architecture, and image details. Baseline failures reuse the determinism capture logs and PNG artifact for diagnosis.

.github/workflows/build.yml

@qodo-code-review

qodo-code-review Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Determinism guarantee is overstated ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new comment says a baseline failure can never result from a capture race after determinism
passes, but the workflow itself documents that five runs still miss a 50/50 race about 6% of the
time. This certainty can cause maintainers to misclassify an intermittent capture race as a real
visual or cross-host difference.
Code

.github/workflows/build.yml[R609-612]

+      # The ordering is load-bearing, not just convenient. A baseline check on a host
+      # whose captures race is a machine for manufacturing differences that are not
+      # regressions; placing it after --determinism means a red tick here can never be
+      # one of those, and can be reported as a real difference in the bytes.
Relevance

●●● Strong

Finite determinism sampling cannot guarantee no intermittent race; the comment’s certainty conflicts
with the documented ~6% miss probability.

PR-#179
PR-#154

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow explicitly states that five determinism runs miss a 50/50 flip approximately 6% of the
time, which directly contradicts the new assertion that a later baseline failure can never come from
such a race.

.github/workflows/build.yml[523-525]
.github/workflows/build.yml[609-612]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The workflow comment incorrectly guarantees that passing five determinism runs excludes capture races. Five samples reduce the probability but do not eliminate it, so a subsequent baseline mismatch can still be caused by nondeterministic capture output.

## Issue Context
The same workflow documents that five runs miss a 50/50 per-run flip approximately 6% of the time. Avoid telling maintainers that a baseline failure can therefore never be a flake; describe determinism as increasing confidence rather than proving the absence of every intermittent race.

## Fix Focus Areas
- .github/workflows/build.yml[609-612]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

Comment thread .github/workflows/build.yml Resolved
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 1b1d2a8

The new step's comment said that running the baseline check after --determinism
means a red tick "can never" be a capture race, and the ::error:: text said the
same in the place a maintainer actually reads it. The step above them documents
the opposite: five runs miss a 50/50 per-run flip about 6% of the time. Told that
a failure cannot be a flake, the natural response to an unreproducible one is to
assume the message is wrong and re-run until green.

Both now say likelier rather than certain, and name the case explicitly: a red
that does not reproduce is a capture race that survived five samples — a fourth
instance of the class #179 and #201 each turned out to be — and it wants a ticket
rather than a retry. The README's list of what red means gains the same fourth
entry.

Found by Qodo on b849080..1b1d2a8.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

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

@tylerkron

Copy link
Copy Markdown
Contributor Author

Qodo-clean, CI green — ready for review

Sweep verification, independent of the author: head is still d8b71af; all six checks green on that exact SHA; MERGEABLE/CLEAN; the single qodo-code-review[bot] review thread is resolved. This PR was also the control that proves the thread query actually returns rows, so the zero on #203/#205 is a real zero and not the silent one you get from filtering on the bare login without the [bot] suffix.

The new macOS step this PR adds — Committed baseline must be reproduced on this runner — ran and passed on d8b71af, so the gate is exercised rather than merely added. I also drove the new --check-captured mode by hand against a real 18-screen capture: it passes on matching captures, and it fails closed (exit 1, with the "holds no PNGs at all" message rather than a confusing eighteen-line miss) on an empty directory under macOS bash 3.2.

Merge-order note — not a defect. This is the only one of #203 / #204 / #205 that changes the pipeline. The moment it lands, #203 and #205 are green on a pipeline that no longer exists: neither ever ran the new baseline step. I closed that gap locally rather than leaving it to a re-run — a #203+#205 test-merge onto cd44a11 reproduces all 18 committed baseline screens byte-for-byte, and this PR's own --check-captured accepts exactly those captures. So no re-run is strictly required, but if you merge this one first, that is why the other two will look a step short.

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.

tooling: the macos-arm64 baseline is named for OS+arch but has only ever been reproduced on one Mac

1 participant