feat(replay_soak): PR gate consecutive-green ≥ 7d + docs (#403 C) - #424
Closed
yoshi280 wants to merge 0 commit into
Closed
feat(replay_soak): PR gate consecutive-green ≥ 7d + docs (#403 C)#424yoshi280 wants to merge 0 commit into
yoshi280 wants to merge 0 commit into
Conversation
Reviewer's GuideAdds a replay soak "consecutive green ≥ 7 days" merge gate implemented as a JSONL status file consumer script, a GitHub Actions PR workflow that enforces the gate against main’s status file, and documentation that defines this soak gate as the canonical operational definition. Sequence diagram for PR replay soak gate check executionsequenceDiagram
actor Developer
participant GitHub
participant PR_Gate_Workflow as replay-soak-gate.yml
participant Main_Repo as main_repo_checkout
participant Status_File as replay_soak_status_jsonl
participant Streak_Script as replay_soak_streak_py
participant Branch_Protection as branch_protection_rules
Developer->>GitHub: open/synchronize PR targeting main
GitHub->>PR_Gate_Workflow: trigger pull_request event
PR_Gate_Workflow->>Main_Repo: actions/checkout ref=main
Main_Repo-->>PR_Gate_Workflow: workspace with .replay-soak-status.json (if exists)
PR_Gate_Workflow->>Status_File: check existence
alt status file missing
PR_Gate_Workflow-->>PR_Gate_Workflow: set streak=0, emit warning
else status file present
PR_Gate_Workflow->>Streak_Script: run python3 scripts/replay_soak_streak.py --quiet
Streak_Script->>Status_File: load_rows()
Status_File-->>Streak_Script: JSONL rows
Streak_Script-->>PR_Gate_Workflow: print streak n, exit 0/1/2
end
PR_Gate_Workflow-->>GitHub: set output streak=n, log notice
alt streak < 7
PR_Gate_Workflow-->>GitHub: job failure, check status failure
else streak ≥ 7
PR_Gate_Workflow-->>GitHub: job success, check status success
end
GitHub->>Branch_Protection: evaluate required check consecutive-green ≥ 7d
Branch_Protection-->>Developer: allow or block merge of PR
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - name: Enforce threshold | ||
| run: | | ||
| set -euo pipefail | ||
| n="${{ steps.streak.outputs.streak }}" |
|
|
||
| import argparse | ||
| import json | ||
| import sys |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
scripts/replay_soak_streak.py,load_rowsraisingSystemExitmakes it awkward to reuse programmatically (and forces the tests to expectSystemExit); consider raising a regular exception (e.g. a customReplaySoakError) and mapping that to exit code 2 only inmain(). - The liberal
list[dict] # type: ignore[type-arg]usage inscripts/replay_soak_streak.pyand tests makes the data shape opaque to tooling; defining a TypedDict (or at leastdict[str, Any]) for the status rows would remove the ignores and clarify expected fields.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `scripts/replay_soak_streak.py`, `load_rows` raising `SystemExit` makes it awkward to reuse programmatically (and forces the tests to expect `SystemExit`); consider raising a regular exception (e.g. a custom `ReplaySoakError`) and mapping that to exit code 2 only in `main()`.
- The liberal `list[dict] # type: ignore[type-arg]` usage in `scripts/replay_soak_streak.py` and tests makes the data shape opaque to tooling; defining a TypedDict (or at least `dict[str, Any]`) for the status rows would remove the ignores and clarify expected fields.
## Individual Comments
### Comment 1
<location path="scripts/replay_soak_streak.py" line_range="24-35" />
<code_context>
+from pathlib import Path
+
+
+def streak(rows: list[dict]) -> int: # type: ignore[type-arg]
+ n = 0
+ for row in reversed(rows):
+ if row.get("replay_full_equality_result") != "pass":
+ break
+ if int(row.get("mismatched", 0)) + int(row.get("derived_orphan", 0)) != 0:
+ break
+ n += 1
+ return n
+
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Handle non-integer `mismatched` / `derived_orphan` values explicitly so malformed rows become exit 2 instead of a generic failure.
`int(row.get("mismatched", 0)) + int(row.get("derived_orphan", 0))` will raise `ValueError` if those fields are present but non-numeric, causing an unhandled exception and exit code 1. To align with the existing behavior for malformed JSON/shape (exit 2), consider catching `ValueError` (here or in `load_rows`) and mapping it to the same exit-2 path so all malformed status files are treated consistently.
```suggestion
from pathlib import Path
def streak(rows: list[dict]) -> int: # type: ignore[type-arg]
n = 0
for row in reversed(rows):
if row.get("replay_full_equality_result") != "pass":
break
try:
mismatched = int(row.get("mismatched", 0))
derived_orphan = int(row.get("derived_orphan", 0))
except (TypeError, ValueError) as exc:
# Treat non-integer fields as a malformed status file so we exit 2,
# consistent with JSON/shape errors.
print(
"malformed status file: non-integer mismatched/derived_orphan",
file=sys.stderr,
)
raise SystemExit(2) from exc
if mismatched + derived_orphan != 0:
break
n += 1
return n
```
</issue_to_address>
### Comment 2
<location path=".github/workflows/replay-soak-gate.yml" line_range="39-50" />
<code_context>
+ echo "::warning::.replay-soak-status.json does not exist on main yet; treating streak as 0"
+ n=0
+ else
+ n=$(python3 scripts/replay_soak_streak.py --quiet)
+ fi
+ echo "streak=$n" >> "$GITHUB_OUTPUT"
</code_context>
<issue_to_address>
**suggestion:** Consider pinning the Python version in the workflow for reproducibility.
This step depends on whatever `python3` happens to be on `ubuntu-latest`, which can change as the image updates. Please add an `actions/setup-python` step and run this with an explicitly pinned version (e.g., 3.11) to keep CI behavior aligned with your local target Python version.
```suggestion
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.11'
- name: Compute streak
id: streak
run: |
set -euo pipefail
if [ ! -f .replay-soak-status.json ]; then
echo "::warning::.replay-soak-status.json does not exist on main yet; treating streak as 0"
n=0
else
n=$(python scripts/replay_soak_streak.py --quiet)
fi
echo "streak=$n" >> "$GITHUB_OUTPUT"
echo "::notice::replay-soak consecutive-green streak: $n"
```
</issue_to_address>
### Comment 3
<location path="tests/test_replay_soak_streak.py" line_range="101-105" />
<code_context>
+ assert replay_soak_streak.streak(replay_soak_streak.load_rows(p)) == 1
+
+
+def test_malformed_jsonl_raises(tmp_path: Path) -> None:
+ """Hypothesis: a non-JSON line raises SystemExit (exit code 2 in main)."""
+ p = tmp_path / "status.json"
+ p.write_text("not-json\n")
+ with pytest.raises(SystemExit):
+ replay_soak_streak.load_rows(p)
</code_context>
<issue_to_address>
**suggestion (testing):** Also cover the non-dict JSON row case that `load_rows` explicitly guards against
You already cover malformed JSON text. Please also add a test where the JSON parses successfully but is not an object (e.g. a line containing `"42\n"` or `"[]\n"`), and assert that `load_rows` raises `SystemExit` for that line. This will exercise the additional guard and the second exit-code-2 failure path in `main()`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+24
to
+35
| from pathlib import Path | ||
|
|
||
|
|
||
| def streak(rows: list[dict]) -> int: # type: ignore[type-arg] | ||
| n = 0 | ||
| for row in reversed(rows): | ||
| if row.get("replay_full_equality_result") != "pass": | ||
| break | ||
| if int(row.get("mismatched", 0)) + int(row.get("derived_orphan", 0)) != 0: | ||
| break | ||
| n += 1 | ||
| return n |
There was a problem hiding this comment.
suggestion (bug_risk): Handle non-integer mismatched / derived_orphan values explicitly so malformed rows become exit 2 instead of a generic failure.
int(row.get("mismatched", 0)) + int(row.get("derived_orphan", 0)) will raise ValueError if those fields are present but non-numeric, causing an unhandled exception and exit code 1. To align with the existing behavior for malformed JSON/shape (exit 2), consider catching ValueError (here or in load_rows) and mapping it to the same exit-2 path so all malformed status files are treated consistently.
Suggested change
| from pathlib import Path | |
| def streak(rows: list[dict]) -> int: # type: ignore[type-arg] | |
| n = 0 | |
| for row in reversed(rows): | |
| if row.get("replay_full_equality_result") != "pass": | |
| break | |
| if int(row.get("mismatched", 0)) + int(row.get("derived_orphan", 0)) != 0: | |
| break | |
| n += 1 | |
| return n | |
| from pathlib import Path | |
| def streak(rows: list[dict]) -> int: # type: ignore[type-arg] | |
| n = 0 | |
| for row in reversed(rows): | |
| if row.get("replay_full_equality_result") != "pass": | |
| break | |
| try: | |
| mismatched = int(row.get("mismatched", 0)) | |
| derived_orphan = int(row.get("derived_orphan", 0)) | |
| except (TypeError, ValueError) as exc: | |
| # Treat non-integer fields as a malformed status file so we exit 2, | |
| # consistent with JSON/shape errors. | |
| print( | |
| "malformed status file: non-integer mismatched/derived_orphan", | |
| file=sys.stderr, | |
| ) | |
| raise SystemExit(2) from exc | |
| if mismatched + derived_orphan != 0: | |
| break | |
| n += 1 | |
| return n |
Comment on lines
+39
to
+50
| - name: Compute streak | ||
| id: streak | ||
| run: | | ||
| set -euo pipefail | ||
| if [ ! -f .replay-soak-status.json ]; then | ||
| echo "::warning::.replay-soak-status.json does not exist on main yet; treating streak as 0" | ||
| n=0 | ||
| else | ||
| n=$(python3 scripts/replay_soak_streak.py --quiet) | ||
| fi | ||
| echo "streak=$n" >> "$GITHUB_OUTPUT" | ||
| echo "::notice::replay-soak consecutive-green streak: $n" |
There was a problem hiding this comment.
suggestion: Consider pinning the Python version in the workflow for reproducibility.
This step depends on whatever python3 happens to be on ubuntu-latest, which can change as the image updates. Please add an actions/setup-python step and run this with an explicitly pinned version (e.g., 3.11) to keep CI behavior aligned with your local target Python version.
Suggested change
| - name: Compute streak | |
| id: streak | |
| run: | | |
| set -euo pipefail | |
| if [ ! -f .replay-soak-status.json ]; then | |
| echo "::warning::.replay-soak-status.json does not exist on main yet; treating streak as 0" | |
| n=0 | |
| else | |
| n=$(python3 scripts/replay_soak_streak.py --quiet) | |
| fi | |
| echo "streak=$n" >> "$GITHUB_OUTPUT" | |
| echo "::notice::replay-soak consecutive-green streak: $n" | |
| - name: Set up Python | |
| uses: actions/setup-python@v5 | |
| with: | |
| python-version: '3.11' | |
| - name: Compute streak | |
| id: streak | |
| run: | | |
| set -euo pipefail | |
| if [ ! -f .replay-soak-status.json ]; then | |
| echo "::warning::.replay-soak-status.json does not exist on main yet; treating streak as 0" | |
| n=0 | |
| else | |
| n=$(python scripts/replay_soak_streak.py --quiet) | |
| fi | |
| echo "streak=$n" >> "$GITHUB_OUTPUT" | |
| echo "::notice::replay-soak consecutive-green streak: $n" |
Comment on lines
+101
to
+105
| def test_malformed_jsonl_raises(tmp_path: Path) -> None: | ||
| """Hypothesis: a non-JSON line raises SystemExit (exit code 2 in main).""" | ||
| p = tmp_path / "status.json" | ||
| p.write_text("not-json\n") | ||
| with pytest.raises(SystemExit): |
There was a problem hiding this comment.
suggestion (testing): Also cover the non-dict JSON row case that load_rows explicitly guards against
You already cover malformed JSON text. Please also add a test where the JSON parses successfully but is not an object (e.g. a line containing "42\n" or "[]\n"), and assert that load_rows raises SystemExit for that line. This will exercise the additional guard and the second exit-code-2 failure path in main().
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
robotrocketscience
force-pushed
the
feat/issue-403-replay-soak-C
branch
from
May 5, 2026 06:43
7a98607 to
e1c3dcb
Compare
Collaborator
Author
|
Landed via FF-push to Effectively merged. |
5 tasks
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
3 participants
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.
Closes deliverable C public-repo scope of #403. Companion to PR #423 (deliverable A).
Implements the operator ratification at #403 (comment) (2026-05-04T19:54:28Z): committed JSONL status file consumer + PR check.
What ships
scripts/replay_soak_streak.py— pure script that walks the tail of.replay-soak-status.jsonand counts consecutive rows wherereplay_full_equality_result == "pass"ANDmismatched + derived_orphan == 0. Both conditions required — a row that claimspassbut has nonzero drift counters still breaks the streak. Exit 0 if streak ≥ threshold (default 7), exit 1 otherwise, exit 2 on malformed file. Six tests intests/test_replay_soak_streak.py..github/workflows/replay-soak-gate.yml— fires on every PR; produces the checkconsecutive-green ≥ 7d. Per the ratification, this name is what gets added to the branch ruleset'srequired_status_checkslist (admin step, separate from this PR). The workflow checks outmain(not the PR branch) so it reads the authoritative status file — the cron only writes onmain.docs/v2_replay.md— new "Soak gate (operational definition — [v2.x] Tighten replay-soak gate — operationalize #262's '≥1 week green' as actual CI probe #403)" section anchors the canonical "≥ 1 week green" definition:.replay-soak-status.json+ the consecutive-green check. Supersedes any prior comment-text calendar arithmetic ("soak-locked through YYYY-MM-DD" notes).Dependencies
.replay-soak-status.jsonto exist onmain. Until then this PR's gate workflow correctly fails because there's no streak to read.consecutive-green ≥ 7dto the branch ruleset's required-checks list. Out of scope for this PR; needs therobotrocketscienceidentity (yoshi280 hasadmin: false).Out of scope
aelf-claim.shSTALE-check rewrite — sister change in the HOME repo (~/.claude/scripts/aelf-claim.sh) per the two-repo workflow. Ships separately; this PR is the public-repo half of deliverable C.main, every PR's gate fails. That's the intended behavior — the check is fail-safe before evidence exists.Test plan
uv run pytest tests/test_replay_soak_streak.py -v— 6 passed.uv run pytest --ignore=tests/bench_gate -q— 2440 passed (+6 new), 22 skipped.python3 scripts/replay_soak_streak.py --status-file <empty> --threshold 7→ exit 1 (streak=0).Summary by Sourcery
Introduce a replay soak merge gate based on a computed consecutive green streak and document the operational definition of the soak window.
New Features:
Enhancements:
Documentation:
Tests: