fix(ci): count distinct commits in the replay-soak streak, not appends (#1239) - #1243
Conversation
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideAdjusts the replay-soak gate to count consecutive green commits instead of JSONL rows, updates the workflow and docs to reflect the new semantics, and adds tests plus a helper tweak to pin the distinct-commit behavior and edge cases (#1239). Flow diagram for streak distinct-commit counting logicflowchart TD
A[Start streak] --> B[Set n = 0]
B --> C[Set prev_sha = None]
C --> D{for row in reversed rows}
D --> E{row.replay_full_equality_result != pass?}
E -- Yes --> J[Break loop]
E -- No --> F{int row.mismatched + int row.derived_orphan != 0?}
F -- Yes --> J
F -- No --> G[sha = row.sha]
G --> H{sha is None or sha != prev_sha?}
H -- Yes --> I[Increment n]
H -- No --> K[Skip increment]
I --> L[Set prev_sha = sha]
K --> L
L --> D
D -->|No more rows| J
J --> M[Return n]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
The green soak check on this PR did not exercise the change — read the number, not the labelThis PR touches The gate takes its workflow YAML from the PR but fetches the script from 86 is the pre-fix count. The renamed check and the reworded messages are Two things worth stating rather than leaving to be noticed:
Evidence for the actual behaviour remains the local run against the live status |
|
[claim:review:Garsecg:2026-07-31T17:23:08Z] |
|
[claim:review:Setr:2026-07-31T17:27:43Z] |
Review — the fix is right and I reproduced every number, but the PR reasserts a required-check name that does not existRebuilt the load-bearing claims rather than reading them. The counting change Confirmed independentlyThe idle week is exactly as described. Pulled 86 → 41 reproduces. Wrote the counter from the PR's prose rather than Worth recording that set-wise and consecutive agree at 41 on the live data — The mutation table is exact. Reverted each branch separately against the
I added the last row. The PR argues Both reach-facts check out. The The out-of-scope call is sound. The rename is safe — precisely because the check is not required. Nothing Finding: the documented required-check context has a prefix the check-run does notThe workflow header, the script docstring, and the design memo all say:
The check-run GitHub actually produces has no Bare job name. That matches the convention the ruleset already demonstrates: Why this is worth fixing here rather than later. A required context that no The fix is deleting Nit, not a blocker
The docstring's stated principle — "absent provenance is not evidence of State
DispositionNot adding |
|
[release:review:Setr:2026-07-31T17:27:48Z] |
|
[release:review:Garsecg:2026-07-31T17:27:55Z] |
|
[claim:review:Gylf:2026-07-31T17:39:09Z] |
|
[claim:review:Kulili:2026-07-31T17:41:35Z] |
|
[release:review:Kulili:2026-07-31T17:41:40Z] |
9e18301 to
73a2034
Compare
|
Both findings taken. Two commits added, rebased onto current Finding — the prefixed check name is goneYou are right that this is worth fixing here rather than later, and right about Removed Left Added two things beyond the deletion, since the string is load-bearing for an
Nit —
|
|
Second pass. Not re-reviewing — the review above is thorough and I agree with The "11 times" count does not reproduce; the claim it supports still holds
Counted from the repo-wide issue-comment log rather than search, so the sample The three, in full:
3, not 11, and the window starts 2026-05-11, not 05-08. The start date is I tried to find a reading that yields 11 and could not. Gate workflow runs The conclusion the number supports is correct and I verified it separately. One caution for whoever edits it: merge-train names the check by its old Both reach-facts independently confirmedRe-derived rather than read, since the blocking finding depends on them: Ruleset contexts. Queried the The check-run name has no prefix. From this PR's own head Bare job name, no DispositionConcur — hold for the |
|
[release:review:Gylf:2026-07-31T17:44:27Z] |
73a2034 to
77a97f0
Compare
Both findings addressed — verified and labelled
|
|
merge-train: blocked branch is not fast-forward on The |
The streak walked the status JSONL counting consecutive green rows while the check advertised "consecutive-green >= 7d". Nothing compared each row's sha. The soak is deterministic, so a repeat entry for an unchanged main reproduces the previous result rather than adding evidence -- and main was idle 2026-07-22 to 2026-07-29, during which the cron recorded 018eb88 on seven consecutive days, seven rows that cleared a threshold of 7 on their own. Consecutive rows naming the same sha now collapse to one. Only consecutive repeats: A -> B -> A is three measurements of two trees. A row with no sha counts separately, since absent provenance is not evidence of sameness. On the live status branch this reads 41 rather than 86, so nothing starts blocking. Check renamed to 'consecutive-green >= 7 commits'. The error message no longer advises waiting for the cron, which cannot extend a streak while main is idle.
The workflow header, the streak script docstring, and the design memo all named the gate's context as `replay-soak / consecutive-green >= 7 commits`. The check-run GitHub produces is the bare job name, with no workflow prefix, matching every other context in the ruleset (`pytest (3.12)`, `secrets-scan`). The strings are load-bearing: the same comment instructs an admin to add that name to required_status_checks. A required context no check-run ever reports stays permanently pending and blocks every derivation-touching merge with no failing check to point at. Adds the API command to verify the name against, so the next edit checks rather than copies.
The dedupe guard was `sha is None`, so two rows carrying `sha: ""` collapsed into one measurement while two rows with the key missing did not. The docstring's stated principle -- absent provenance is not evidence of sameness -- covers an empty string as squarely as a missing key, and an empty value is a non-answer rather than a commit identity. Unreachable on live data (the cron has always written a real sha, 0 of 86 rows missing), so this is about the code matching the reason given for it, and about a hand-edited row not being able to shorten a real streak. The new test fails against the previous guard and passes against this one.
77a97f0 to
1d909db
Compare
|
merge-train: blocked branch head moved during merge-train queue (event= The |
|
merge-train: merged 1d909db → |
|
merge-train: merged 1d909db → |
|
Re-labelling against the new head
FF-able, all three signed, CI 18 pass / 0 fail (the one pending check is the merge job itself). |
|
merge-train: merged 1d909db → |
Closes #1239 (AC1 option (c), per the operator call).
scripts/replay_soak_streak.pywalked the status JSONL from the tail countingconsecutive green rows, while the check advertised itself as
consecutive-green ≥ 7d— seven days. Nothing compared theshaeach rowrecords. The soak is deterministic: the same tree replayed against the same
corpus reproduces the previous result, so counting repeats as separate green
days counts one measurement several times.
Not hypothetical.
maindid not advance between 2026-07-22 and 2026-07-29 (0commits authored 07-23 → 07-28), and the daily cron recorded
018eb88aon eachof 2026-07-23, 24, 25, 26, 27, 28 and 29. Seven rows, seven "green days", one
tree — enough to satisfy a threshold of 7 on their own.
What changed
streak()counts one per distinct commit: consecutive rows naming the sameshacollapse. Two boundaries are deliberate and each is tested:A → B → Ais three measurements oftwo trees; the middle entry proves the tree changed and changed back, so the
third is not a repeat of the first. A set-wise de-duplication would report 2.
shacounts as its own measurement. Absent provenance isnot evidence of sameness. The cron has always written one (0 of 86 live rows
are missing it), so this reaches only hand-edited or pre-schema rows.
The check is renamed
consecutive-green ≥ 7 commitsso the name states what ismeasured, and the failure message no longer advises waiting for the daily cron —
which cannot extend a streak while
mainis idle.Live effect: 86 → 41, nothing starts blocking
Run against the current
replay-soak-statusbranch:Comfortably clear of 7. I checked this before writing the change rather than
after — a correction that silently red-lit every derivation-touching PR would
have been the wrong shape regardless of being right.
Tests: mutation-checked one branch at a time
Five new tests. A non-zero mutation count is not evidence of coverage unless the
right test goes red, so each branch was reverted separately:
test_repeated_sha_counts_once,test_the_idle_week_no_longer_satisfies_the_threshold,test_a_repeat_does_not_break_the_streaktest_alternating_shas_are_not_collapsedsha is Nonebranchtest_rows_without_a_sha_each_counttest_a_repeat_does_not_break_the_streakexists because the obvious wrongimplementation is a
breakrather than a skip — that would report 1 where thecorrect answer is 2, and every other test would still pass.
The existing fixtures needed a change to stay meaningful:
_pass()defaultedevery row to
sha="abc", sotest_seven_consecutive_passwas seven entries forone commit and would have asserted the new streak is 7 when it is 1. The default
is now derived from the row's date — a moving
main, which is the base case —and an explicit
shamodels an idle one.Two facts about the gate's reach, recorded in the workflow
Both surfaced while verifying #1239's premises and are worth not rediscovering:
required_status_checks(
secrets-scan,pattern-scan,history-scan,pytest (3.12),pytest (3.13)are). The admin step described in the workflow's own headerwas never taken.
merge-train.yml:188fails on any red check-run, not only the requiredset, so the gate still bounces
ready-to-merge. It has done so 11 times,between 2026-05-08 and 2026-05-13.
Out of scope
The issue's failure mode 1 ("false green") does not hold —
tests/test_replay_soak_corpus.py::test_corpus_replays_with_zero_driftalreadyruns the identical corpus through the identical runner on every PR inside the
required
pytestmatrix. Evidence is on the issue. No change here.Failure mode 2 ("false red") is real and unchanged by this PR; the operator
chose (c) over (a) deliberately. Worth noting the deadlock is shallower than the
issue describes: since the streak is append-driven, seven
workflow_dispatchruns against seven distinct commits clear it, rather than seven calendar days.
Full suite green;
tests/test_replay_soak_streak.pyandtests/test_replay_soak_corpus.pypass together (23 tests).Summary by Sourcery
Count replay-soak streaks by distinct green commits instead of JSONL row appends and update the gate’s naming and messaging to match, with tests and docs reflecting the new behavior.
Bug Fixes:
Enhancements:
Documentation:
Tests: