ci: judge pull requests on the delta from main, not on absolute green - #1090
Merged
Conversation
andrewgazelka
enabled auto-merge
July 30, 2026 03:28
On 2026-07-29 two correct pull requests each displayed exactly the failures the other one fixes. 1080 repaired completions-e2e and minecraft-literals and its CI reported the three checks 1081 repairs; 1081 repaired the two dev-boot gates and its CI reported the three 1080 repairs. Neither could ever be green on its own, so neither could land, so the failures accumulated. A gate that can only say "green" cannot say "better than before", which is the only question a reviewer has when main is already red. The gate now compares this run's failing set against a baseline main publishes on every push. A pull request that leaves the set no larger passes; one that adds a name blocks and the summary names it. Flakiness is answered with a proof rather than a statistic, because a retry cannot do the job: for a check failing 30% of the time, two failures in a row happen 9% of the time, so a retry can acquit and never convict. A derivation path is a Merkle root over every input, so an unchanged path means an unchanged build closure, and the same derivation producing a different outcome is a proof of nondeterminism. Measured on this repository: differential-traces passed in run 30428441399 and failed in run 30429463435 on the byte-identical /nix/store/fczpzka3v7c9npf4a3i730ahkncla3f5-check-hyperion-differential-traces.drv, eighteen minutes apart, on a 600-tick wall-clock timeout. Three checks are proven nondeterministic by the eighteen main runs of 2026-07-28/29 (smash-hud-e2e 44%, differential-traces 39%, bedwars-bow-e2e 11%), and 61% of runs hit at least one. minecraft-literals looks like a fourth and is not: its derivation moved at the ok-to-failed edge, so it is a real regression that landed unnoticed while the gate was already red. The exclusion list is deleted. It was the same idea at lower fidelity, stored where only human attention could refresh it, and every fix was a merge conflict against every other fix in flight. This verdict is NOT a required status context and this commit does not make it one. Ruleset 566717 carries no required_status_checks at all, so one approving review remains the only gate. The entry criteria for making it binding are in the rendered summary and in the PR description.
andrewgazelka
force-pushed
the
ci/flake-gate-differential-verdict
branch
from
July 30, 2026 03:35
40c5e28 to
24cb741
Compare
andrewgazelka
added a commit
that referenced
this pull request
Jul 30, 2026
…them (#1092) Follow-up to #1090. The four measured criteria for making the differential verdict a required status check were only in the summary `nix/ci/delta-gate.sh` renders. That summary is read when the pipeline runs, and every trigger has been `workflow_dispatch` since #1088, so nobody meets them. They move into the header of `nix/ci/flake-gate.nix`, which is the file anybody asking "why does this not block anything" opens first. Each criterion carries its measured value and why the number is what it is: 1. **Flake rate at or under 5%.** Measured 0.611 over the 18 main runs of 2026-07-28/29. 5% is derived rather than picked: the merge queue's `grouping_strategy: ALLGREEN` with `max_entries_to_build: 5` means a flake ejects a batch of up to five pull requests rather than costing one author a re-run. 2. **At least 30 runs in the instability record**, from P(proven) = 1 - q^n - (1-q)^n. At n=18 an 11% flake is still 12% likely to be unproven. 3. **At most 1 in 20 pull request runs** blocked by a verdict a re-run then clears. The only one that measures the whole system, and only takeable while the verdict reports rather than blocks. 4. **p95 under 40 minutes over runs where every check reached a verdict.** Stated that way because the observed 31 to 41 minutes is over red runs only and no green run of this gate has ever been observed. The summary keeps the one-line statement that the verdict blocks nothing, plus ENG-10827, and points at the header instead of restating the criteria. One copy to keep true. No behaviour change: comments plus one summary paragraph. `checks.<system>.delta-gate` still passes 65 of 65 in the sandbox, `checks.<system>.flake-gate` builds, `nix run .#fmt` clean. Refs ENG-11433, ENG-10827.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem, with the evidence
On 2026-07-29 two correct pull requests each displayed exactly the failures the other one fixes.
completions-e2eandminecraft-literals. Its CI reportedbedwars-dev-boot-e2e,smash-dev-boot-e2e,smash-e2e.completions-e2e,minecraft-literals,smash-e2e.Neither could ever be green on its own, so neither could land, so the failures accumulated. Both were correct. Both looked broken. A gate that can only say "green" cannot say "better than before", and when main is already red that is the only question a reviewer has.
What this does
The gate compares this run's failing set against a baseline that main publishes on every push. A pull request that leaves the set no larger passes. One that adds a name blocks, and the summary names it.
Replayed against the real 1080/1081 sets, using this implementation:
fixed = [completions-e2e, minecraft-literals], three excusedfixed = [bedwars-dev-boot-e2e, smash-dev-boot-e2e], three excusedblocked = [proxy]blocked = [proxy]Flakiness is answered with a proof, not a statistic
A retry cannot do this job. For a check failing 30% of the time, two failures in a row happen 9% of the time, so a retry can acquit and never convict. Any gate whose flake answer is a retry blocks honest work 9% of the time.
A derivation path is a Merkle root over every input derivation, so an unchanged path means an unchanged build closure. Therefore:
Not evidence, not a heuristic. If a check passed on the base and fails here on the byte-identical derivation, the diff did not cause it. Measured here:
A wall-clock timeout inside the sandbox. Nothing in the derivation moved; the runner was slower.
Applied at two scopes: within a verdict when the derivation is unchanged, and across a rolling record for the common case where a pull request moves the hash. The record keys on the derivation for the proof and promotes onto the attribute for the conclusion, because nondeterminism is a property of how a check is written and survives a change to its inputs.
What the 18 main runs of 2026-07-28/29 actually say
Every one of the 18 was red. Extracted from the
Flakejob logs;.= ok,X= FAILED,-= check did not exist yet, oldest leftmost.61% of runs hit at least one flake, 2.57 gate runs expected per clean pass. That is measured, not assumed, and the implementation reproduces it:
dg flake-rateon the folded real history returns0.611over the same 18 runs.Three things here were not previously known.
differential-tracesandbedwars-bow-e2eare flaky and nobody had noticed.smash-selector-e2eis no longer flaky. Andminecraft-literalsis not one of the standing five, it is a regression that landed mid-window and stuck while the gate was already red, which is consequence 2 of a permanently red gate caught in the act.Why the identity has to be the derivation, demonstrated on that data
Folding the real 18-run history two ways:
A excuses a genuine regression as a coin flip. B does not. The derivation keying is the whole difference, on real data.
Guards, and each was watched failing
Immunity is withdrawn when the derivation does not describe the run. The general form, because the list will grow: a pull request forfeits immunity when it changes anything the derivation does not capture but the run depends on. Three known members: environment (
.github/workflows/**,nixConfig), scheduling and concurrency (nix/ci/flake-gate.nix, e2e port offsets), and a changed check set (detected from the two documents rather than a path list, because adding a check adds contention under a concurrent gate and no path list would catch every way of doing that).nix/ci/delta-gate-tests.shis 65 cases with no network, no nix and no clock, wired aschecks.<system>.delta-gate. It contains the inverse of every guard, because the failure mode of a suite like this is passing for the wrong reason. Six deliberate mutations of the verdict logic were each caught by a named case:What keeps the failing set from becoming permanent
nix/ci/delta-gate.sh. Past it nothing lands except pull requests that shrink the set. It is a committed constant, editable downward only and deliberately never derived from an observed count: a suite with proven coin flips will eventually have a lucky night, and a self-lowering cap would latch onto it and freeze the repository the next ordinary day.GATE_ISSUE_APPLY: "1"is one line in the workflow.The exclusion list is deleted
Same idea at lower fidelity, stored where only human attention could refresh it, and every fix was a merge conflict against every other fix in flight. The header of
nix/ci/flake-gate.nixnow says where "known broken" lives so nobody reinvents it. A check that genuinely cannot run in CI belongs out ofchecksinflake.nix, which is a truer home for "this cannot run here" than a CI-side exception.This is NOT a required status context, and this PR does not make it one
gh api repos/hyperion-mc/hyperion/rulesets/566717 --jq '.rules[].type'returns seven rules and norequired_status_checks; it is the only ruleset andbranches/main/protectionreturns 404. So one approving review is the only thing between any pull request and main today, red pipeline or no pipeline. Making the verdict binding is onerequired_status_checksentry naming this job. Four criteria should hold first, none of which does:bedwars-bow-e2eis still 12% likely to be unproven; n=30 puts anything at or above 10% over 97% likely to be proven.check_response_timeout_minutesis 60 while this job is allowed 90, so those two must stop disagreeing before anything is required.Criterion 4 makes #1084 (concurrent gate) a prerequisite. The gate is two long checks and a tail with a 4-second median, so the concurrency floor is about 11 minutes.
Also worth naming: the queue's
grouping_strategyisALLGREENwithmax_entries_to_build: 5, so once checks are required one flake ejects a batch of five rather than costing one author a re-run.Composing with #1087, which landed while this was being written
#1087 gave the gate a
results.jsonlof{attr, outcome, drvPath}per check, and wrote the same rationale I had arrived at separately: "the same derivation with two different outcomes is proof the change did not cause the failure." So this rebases onto their file rather than replacing it. Their concurrent build, theirdrv_map, theirrealised()store oracle and theirunquotableguard are all untouched. What this adds isexcludeddeleted, and a verdict stage at the end that readsresults.jsonland nothing else.That seam is load-bearing and now demonstrated: the build loop changed from one
nix buildper name to one concurrent--keep-goingbuild for the whole set, and no part of the verdict noticed.I adopted their vocabulary (
pass/fail, notok/fail), and the library tests forfailand treats anything else as passing, so a third spelling arriving later reads as a pass rather than silently turning every check red.This does NOT restore the CI triggers
#1088 and #1089 removed every automatic trigger while this was in flight. Three of the reasons given are measurements from this same investigation: 18 consecutive red runs, no required status context, and three checks poisoning 61% of runs so a correct change needed 2.6 attempts.
This change answers two of those three and restores none of the triggers. A differential verdict makes a red base survivable and stops the coin flips being blamed on anybody, but it does not make the checks green, and green-and-stays-green is the bar #1089 set. Restoring triggers is a decision for whoever set that bar, not a side effect of landing machinery that changes what the bar is worth.
The consequence, stated plainly: on
workflow_dispatchalone this machinery is mostly inert. No push means no published baseline, so the first pull request to run under it will find none and be judged exactly as it is today. A manual dispatch onmaindoes produce a baseline and does fold the first instability samples, and the publish conditions are written asgithub.ref == 'refs/heads/main' && github.event_name != 'pull_request'so they keep working unchanged whenever the triggers come back.Verification
nix build .#checks.aarch64-darwin.delta-gatepasses in the sandbox: 65 of 65.nix build .#checks.aarch64-darwin.flake-gatebuilds, so shellcheck passes on the gate.nix run .#fmtmakes no changes.yq.dg flake-rateon the folded history returns0.611, matching the hand measurement.Not verified, and worth knowing before approving:
nix run .#flake-gateon x86_64-linux. The verdict stage is exercised only through fixtures and replays.workflow_dispatchonmainafter this merges is where they are first exercised. That is the largest untested surface here and I am not going to describe it as working.GATE_ISSUE_APPLYis"1". Its body has been rendered locally against the real folded record; it has never posted.