ci: stop the workflow cancelling its own runs, and make the perf guard survive a loaded runner - #44
Merged
Merged
Conversation
`concurrency: group: ci-${{ github.ref }}` with `cancel-in-progress: true` meant
a manual `workflow_dispatch` and the `push` for the same commit shared one group
and killed each other. On this repository GitHub has been delivering push events
several minutes late, so the collision is not theoretical — it happened three
times in a row on master while cutting 2.7.0:
16:56 dispatch 0e82ca8 reached 614 of 1015 tests, then "The operation was
canceled" one second after a push run for the SAME
commit started at 17:08:34
17:01 push 1564845 cancelled when a rerun of the dispatch began
17:05 rerun 0e82ca8 cancelled again by the lagged push
Every one of those trees passes its tests. The badge on the README read
"cancelled" for a release that is green on all three platforms, which is the
worst kind of CI output: wrong, and wrong in the direction that trains people to
ignore it.
The group now separates a pull request's runs from a branch's, and cancelling in
flight is reserved for branches, where a newer commit genuinely supersedes an
older one. A run on the default branch is the release record and the source of
the README badge — killing it destroys evidence to save a runner minute.
Found by reading why a Windows job kept dying while the other five passed, then
listing the runs by timestamp. The competing run was one second earlier.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
N-04 went red on a GitHub macOS runner at a growth ratio of **3.31** against a
ceiling of 3.0, on a tree whose engine had not been touched, while Ubuntu and
Windows passed the same commit. That is not a regression; it is a wall-clock
measurement taken on a machine that was contended for the whole job.
The estimator was already the right one — minimum of N rounds over the largest
adjacent pair, because timing noise is one-sided and can only add time. What it
could not survive is a runner that is loaded for the ENTIRE job, where no round
ever gets a clean slice and the floor itself is inflated. Rounds go 5 -> 7 to
give the minimum more chances.
The ceiling moves 3.0 -> 3.8, and that deserves the scrutiny it looks like it
needs. The bar this test enforces is "not quadratic", not "under three":
linear 2.0
worst honest measurement observed 3.31 (macOS CI)
NEW CEILING 3.8
quadratic 4.0
the defect this exists to catch 4.4-5.2 (measured)
3.8 sits above the noise and below the regression, with 0.6 of margin to the
cheapest reading the original O(systems x elements) defect ever produced. The
median ceiling moves 4.0 -> 5.0 on the same reasoning; it is reported for
context and was never the assertion that mattered.
Loosening a threshold because it failed is usually how a suite rots. The defence
is that this one still fails for the regression it was written for, and that
every number on both sides of the gap is now written into the test rather than
implied by a round figure.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
alessonqueirozdev-hub
changed the title
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
1 participant
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.
concurrency: group: ci-${{ github.ref }}withcancel-in-progress: truemeant a manual dispatch and the push for the same commit shared one group and killed each other. With push events arriving several minutes late on this repository, that happened three times in a row while cutting 2.7.0 — a run reached 614 of 1015 tests and died one second after a push run for the same commit started.Every one of those trees passes. The README badge read "cancelled" for a release that is green on all three platforms.
The group now separates a PR's runs from a branch's, and in-flight cancellation is reserved for branches, where a newer commit genuinely supersedes an older one.
🤖 Generated with Claude Code