Repository navigation
Separate cooldown property deadlines from process bounds (#219) - #222
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
SummaryDisable Hypothesis execution deadlines on the two subprocess-backed cooldown properties in Preserve the properties’ domains, examples, budgets, assertions, tools, and isolation. See issue ValidationThe PR reports that all six Make gates passed. The full suite passed 977 tests and three snapshots, including both affected properties. Hosted CI and review remain pending. WalkthroughThe test module imports Hypothesis ChangesCooldown test timing
Suggested labels: Priority: ⬇️ Low Change: Bug fix · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Satisfy issue #219. The diff changes only tests/test_sdlc_milestone_review.py. It adds @settings(deadline=None) to the two specified subprocess-backed properties. It imports settings and adds th… |
| Out of Scope Changes check | |
Keep the change within issue #219 scope. The only changes are the required Hypothesis import, two local deadline settings, and a comment that explains the semantic boundary. The diff adds no global pr… |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. |
| Testing (Overall) | |
Pass the testing check. The pull request changes only Hypothesis configuration and adds an explanatory comment; it does not change product behaviour. The two existing properties remain substantive: th… |
| User-Facing Documentation | |
Treat the change as test-only behaviour. The pull request changes only tests/test_sdlc_milestone_review.py: it disables Hypothesis deadlines for two generated tests and retains each subprocess `time… |
| Developer Documentation | |
Treat this change as documentation-inapplicable. The pull request changes only tests/test_sdlc_milestone_review.py: it imports settings, adds two @settings(deadline=None) decorators, and adds a … |
| Module-Level Documentation | |
Keep the existing module docstring at tests/test_sdlc_milestone_review.py:1. It clearly states the module purpose and its relation to the documented local CodeRabbit CLI cooldown helper. The pull re… |
| Testing (Unit And Behavioural) | |
Pass this check. The pull request changes only Hypothesis timing configuration and a comment. The two affected tests still exercise the documented Bash helper through real subprocesses, with the indep… |
| Testing (Property / Proof) | |
The pull request changes only tests/test_sdlc_milestone_review.py: it imports settings, adds @settings(deadline=None) to two existing Hypothesis properties, and adds a timing comment. The existi… |
| Testing (Compile-Time / Ui) | |
The pull request changes only tests/test_sdlc_milestone_review.py, a Python Hypothesis test. It adds @settings(deadline=None) to two subprocess-backed properties and keeps the existing behavioural… |
| Unit Architecture | |
Pass. The pull request changes only tests/test_sdlc_milestone_review.py. It adds @settings(deadline=None) to the two existing subprocess-backed Hypothesis properties and documents the reason at li… |
| Domain Architecture | |
Pass. The pull request changes only tests/test_sdlc_milestone_review.py. It adds @settings(deadline=None) to two subprocess-backed Hypothesis tests and documents scheduler latency. It does not cha… |
| Observability | |
PASS. Restrict the change to tests/test_sdlc_milestone_review.py. It adds @settings(deadline=None) to two Hypothesis properties and imports settings; it adds no production operational behaviour,… |
| Title check | |
The title accurately describes separating Hypothesis cooldown deadlines from process bounds and references issue #219. |
| Description check | |
The description directly explains the timing fix, preserved 30-second process timeout, affected properties, validation, and issue scope. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Set deadlines aside for the run,
Keep the process timeout in place.
Let scheduler delays pass by,
Check cooldowns at their own pace.
Let both properties finish their race.
Comment @coderabbitai help to get the list of available commands.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR isolates subprocess scheduling variability from Hypothesis property timing by disabling the default 200 ms deadline on two cooldown properties, without changing their generated inputs, assertions, examples, budgets, or 30-second process bounds. All six Make validation gates passed, including the affected tests. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Keep subprocess-backed cooldown properties independent of scheduler latency while retaining their separate 30-second process timeout. Preserve generated inputs, explicit examples and semantic assertions.
f13e8d1 to
a92e312
Compare
|
@coderabbitai review Candidate head: a92e312 Please inspect the current candidate, including the changed test-framework base/input (6.168.3 to 6.168.5) and inherited installer-action pin. The earlier approval submission #5515300525 is current-head metadata, but its linked report coverage was bound to f13e8d1. Please report the actual head and scope inspected; do not transfer that prior report's coverage to this candidate. |
|
|
@coderabbitai Please reconcile the managed request's response for current head The response says “Action not completed” and “No files to review.” The The current one-file comparison All six fresh local Make gates passed on the current candidate, including Please state the actual inspected scope and whether any required inspection This asks for a service and inspection disposition, not another whole-PR |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -uo pipefail
head=a92e31269f40883a2e1d9a585d1bc9e3f84564e9
base=34f86aa35764546cee3fe4a5227077098b4f0fde
oldbase=d3da862950d9169e08454fb47d55f9bb079bf6b8
printf '\nCHECKOUT\n'
git rev-parse HEAD
printf '\nEXACT COMPARISON\n'
git diff --stat "$base...$head"
git diff "$base...$head"
printf '\nINHERITED DELTA\n'
git diff "$oldbase" "$base"
printf '\nFILE DIGEST\n'
git show "$head:tests/test_sdlc_milestone_review.py" | sha256sum
printf '\nTEST FILE OUTLINE\n'
ast-grep outline tests/test_sdlc_milestone_review.py --items all
printf '\nREFERENCES\n'
rg -n -C3 'hypothesis|timeout=30|sdlc.milestone.review' Makefile pyproject.toml tests/test_sdlc_milestone_review.pyLength of output: 3815 🏁 Script executed: #!/bin/bash
set -uo pipefail
repo=leynos/agent-helper-scripts
printf '\nMANAGED RESPONSE\n'
gh api "repos/$repo/issues/comments/6166745647" --jq '{id,created_at,updated_at,body}'
printf '\nWALKTHROUGH\n'
gh api "repos/$repo/issues/comments/6164359430" --jq '{id,created_at,updated_at,body}'
printf '\nPR CURRENT METADATA\n'
gh api "repos/$repo/pulls/222" --jq '{head:.head.sha,base:.base.sha,state}'
printf '\nHISTORICAL APPROVAL METADATA\n'
gh api "repos/$repo/pulls/222/reviews/5515300525" --jq '{id,state,commit_id,submitted_at,body}'
printf '\nCURRENT CI METADATA\n'
gh api "repos/$repo/actions/runs/38540914286" --jq '{id,head_sha,status,conclusion,created_at,updated_at,html_url}'
printf '\nCI JOBS\n'
gh api "repos/$repo/actions/runs/38540914286/jobs" --jq '.jobs[] | {name,status,conclusion,steps:[.steps[]|{name,conclusion}]}'Length of output: 19047 🏁 Script executed: #!/bin/bash
set -uo pipefail
repo=leynos/agent-helper-scripts
printf '\nEXACT COMPARISON\n'
gh api "repos/$repo/compare/34f86aa35764546cee3fe4a5227077098b4f0fde...a92e31269f40883a2e1d9a585d1bc9e3f84564e9" --jq '{status,merge_base:.merge_base_commit.sha,total_commits,files:[.files[]|{filename,status,additions,deletions,patch}]}'
printf '\nINHERITED COMPARISON\n'
gh api "repos/$repo/compare/d3da862950d9169e08454fb47d55f9bb079bf6b8...34f86aa35764546cee3fe4a5227077098b4f0fde" --jq '{status,merge_base:.merge_base_commit.sha,total_commits,files:[.files[]|{filename,status,additions,deletions,patch}]}'
printf '\nNATIVE PATCH DIGEST\n'
gh api -H 'Accept: application/vnd.github.patch' "repos/$repo/compare/34f86aa35764546cee3fe4a5227077098b4f0fde...a92e31269f40883a2e1d9a585d1bc9e3f84564e9" | sha256sumLength of output: 43241 🏁 Script executed: #!/bin/bash
set -uo pipefail
printf '\nTEST IMPLEMENTATION\n'
sed -n '1,267p' tests/test_sdlc_milestone_review.py
printf '\nMAKEFILE OUTLINE\n'
ast-grep outline Makefile --items all
printf '\nGATE REFERENCES\n'
rg -n -C4 'typecheck:|test:|uv sync|pytest|compileall|py_compile' Makefile
printf '\nHYPOTHESIS INPUTS\n'
rg -n -C3 '6\.168\.[35]|name = "hypothesis"|hypothesis==' .Length of output: 32388 🤖 web_search executed:
Length of output: 1204 🏁 Script executed: #!/bin/bash
set -uo pipefail
printf '\nSHARED ACTION DELTA\n'
gh api 'repos/leynos/shared-actions/compare/b59d7d13d29636cc9f0c33a81ba17a9a2bde9bb8...aea9c14c24b73042e99f0be8245a4f171b67c31a' --jq '{status,total_commits,files:[.files[]|{filename,status}],relevant_patches:[.files[]|select((.filename|contains("install-mdtablefix")) or (.filename|contains("dependabot-automerge")))|{filename,patch}]}'
printf '\nPINNED INSTALLER\n'
gh api 'repos/leynos/shared-actions/contents/.github/actions/install-mdtablefix/action.yml?ref=aea9c14c24b73042e99f0be8245a4f171b67c31a' --jq '.content' | base64 --decode
printf '\nCI EVIDENCE\n'
gh run view 38540914286 --repo leynos/agent-helper-scripts --log | grep -E 'hypothesis==|hypothesis-|test_documented_cooldown_range_and_sleep_match_the_requested_wait|test_malformed_cooldown_inputs_fail_before_invoking_tools|[0-9]+ passed|[0-9]+ skipped|snapshot|typecheck:|make (check-fmt|markdownlint|lint|typecheck|test|spelling)|mdtablefix' | tail -55
printf '\nORIGINAL TEST BLOB DIGEST\n'
gh api 'repos/leynos/agent-helper-scripts/contents/tests/test_sdlc_milestone_review.py?ref=f13e8d19ac334a5cc77526d2e1cd5e299feb8a44' --jq '.content' | base64 --decode | sha256sumLength of output: 43338 🏁 Script executed: #!/bin/bash
set -uo pipefail
printf '\nNATIVE GIT DIFF DIGEST\n'
git diff 34f86aa35764546cee3fe4a5227077098b4f0fde a92e31269f40883a2e1d9a585d1bc9e3f84564e9 | sha256sum
printf '\nWORKFLOW OUTLINES\n'
ast-grep outline .github/workflows/ci.yml --items all
ast-grep outline .github/workflows/dependabot-automerge.yml --items all
printf '\nWORKFLOW CONTENTS\n'
sed -n '1,110p' .github/workflows/ci.yml
sed -n '1,90p' .github/workflows/dependabot-automerge.yml
printf '\nDOCUMENTED HELPER\n'
rg -n -A65 -B4 '^wait_for_coderabbit_cli\(\)' skills/sdlc-implementation/references/milestone-review.mdLength of output: 5435 🏁 Script executed: #!/bin/bash
set -uo pipefail
printf '\nPINNED REUSABLE WORKFLOW\n'
gh api 'repos/leynos/shared-actions/contents/.github/workflows/dependabot-automerge.yml?ref=aea9c14c24b73042e99f0be8245a4f171b67c31a' --jq '.content' | base64 --decode
printf '\nAUDIT IMPLEMENTATION DELTA\n'
gh api 'repos/leynos/shared-actions/compare/b59d7d13d29636cc9f0c33a81ba17a9a2bde9bb8...aea9c14c24b73042e99f0be8245a4f171b67c31a' --jq '.files[]|select(.filename=="workflow_scripts/dependabot_commit_audit.py" or .filename=="workflow_scripts/tests/test_dependabot_foreign_commits.py")|{filename,patch}'Length of output: 17797
Actual scope inspected hereI performed read-only, bounded inspection of:
Both original and current test files match your supplied SHA-256. The native Git diff also matches your supplied patch SHA-256. The GitHub patch response uses a different representation; its digest is not the native diff digest. Inspection disposition
Thus, the skipped command alone does not establish sufficient coverage. The direct inspection above establishes coverage for the timing patch and its Hypothesis/installer context. Do not transfer the old report’s coverage to this head. Keep validation evidence separateI verified that run 38540914286 succeeded on the exact current head. Its logs show:
Keep your reported local 977 tests and three snapshots as separate evidence. I did not rerun the local gates. I confirmed that Bounded remaining agent taskGitHub read access worked. The initial shallow-history limitation was bypassed through GitHub’s comparison API. I did not access your local replay receipt. The sandbox prohibits executing repository code, so the remaining task must use static inspection or separately supplied execution evidence. Thanks for the provenance guidance. ✏️ Learnings added
You are interacting with an AI system. |
|
@coderabbitai The bounded transitive inspection requested in your response is complete for The exact trees were non-truncated. The inspection fetched and checked all The reachable route remains: workflow entry The workflow and two caller scripts are byte-identical across the revisions: The old tree's OID handling Test source No compatibility gap was found within this requested workflow tree. Arbitrary Does this resolve the specific inherited auto-merge inspection gap? If not, |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
work=$(mktemp -d)
export INSPECTION_DIR="$work"
python - <<'PY'
import base64, concurrent.futures, json, os, pathlib, subprocess
repo = "leynos/shared-actions"
old = "b59d7d13d29636cc9f0c33a81ba17a9a2bde9bb8"
new = "aea9c14c24b73042e99f0be8245a4f171b67c31a"
root = pathlib.Path(os.environ["INSPECTION_DIR"])
def api(path):
return json.loads(subprocess.check_output(["gh", "api", f"repos/{repo}/{path}"], text=True))
trees = {}
for label, revision in (("old", old), ("new", new)):
tree = api(f"git/trees/{revision}?recursive=1")
if tree.get("truncated"):
raise RuntimeError("Truncated tree")
entries = {e["path"]: e for e in tree["tree"] if e["type"] == "blob"}
trees[label] = entries
files = [p for p in entries if p.startswith("workflow_scripts/") and p.endswith(".py")]
print(label, revision, "non-truncated Python file count:", len(files))
paths = [
".github/workflows/dependabot-automerge.yml",
"workflow_scripts/dependabot_automerge.py",
"workflow_scripts/dependabot_github.py",
"workflow_scripts/dependabot_commit_audit.py",
]
for path in paths:
print("BLOB", path, trees["old"][path]["sha"], trees["new"][path]["sha"])
jobs = []
for label in ("old", "new"):
for path, entry in trees[label].items():
if (path.startswith("workflow_scripts/") and path.endswith(".py")) or path == paths[0]:
jobs.append((label, path, entry["sha"]))
def fetch(job):
label, path, sha = job
blob = api(f"git/blobs/{sha}")
data = base64.b64decode(blob["content"])
destination = root / label / path
destination.parent.mkdir(parents=True, exist_ok=True)
destination.write_bytes(data)
refs = [(i, line) for i, line in enumerate(data.decode().splitlines(), 1) if "audit_commits" in line]
return label, path, refs
with concurrent.futures.ThreadPoolExecutor(max_workers=6) as pool:
results = list(pool.map(fetch, jobs))
for label, path, refs in sorted(results):
for line, text in refs:
print("REMOVED SYMBOL REFERENCE", label, path, line, text.strip())
print("FETCHED", len(results), "source files; no repository code executed")
PY
for file in workflow_scripts/dependabot_automerge.py workflow_scripts/dependabot_github.py workflow_scripts/dependabot_commit_audit.py workflow_scripts/tests/test_dependabot_foreign_commits.py; do
ast-grep outline "$work/new/$file" --items all
done
printf '\nWORKFLOW ENTRY\n'
rg -n -C4 'dependabot_automerge|workflow_scripts|python' "$work/new/.github/workflows/dependabot-automerge.yml"
printf '\nCALL ROUTE AND IMPORTS\n'
rg -n -C5 'CommitAudit|commit_page|foreign_commits|audit_whole_branch|fetch_pull_request|live_snapshot' "$work/new/workflow_scripts/dependabot_automerge.py" "$work/new/workflow_scripts/dependabot_github.py"
printf '\nAUDIT DECISIONS\n'
sed -n '280,480p' "$work/new/workflow_scripts/dependabot_commit_audit.py"
printf '\nOID TEST SOURCE\n'
sed -n '55,150p' "$work/new/workflow_scripts/tests/test_dependabot_foreign_commits.py"
printf '\nAUDIT DELTA\n'
diff -u "$work/old/workflow_scripts/dependabot_commit_audit.py" "$work/new/workflow_scripts/dependabot_commit_audit.py" || test "$?" -eq 1Length of output: 45513
Verified scopeI inspected
Both trees were non-truncated. I fetched and searched all 36 old and 44 new Python files under
No remaining caller, decision branch, or input requires another bounded task for this gap. The missing combined test does not leave the requested source-inspection question unresolved. Preserve the evidence limits
Thanks for the precise scope clarification. ✏️ Learnings added
🧠 Learnings usedYou are interacting with an AI system. |
|
@coderabbitai approve Published head: a92e312; base: 34f86aa. The original completed full round was clean for f13e8d1; its table remains historical. Current timing and inherited-context inspection is explicitly recorded in your current-candidate disposition. The remaining transitive auto-merge gap is explicitly closed by your immutable-source inspection. These source-only confirmations do not themselves submit approval or refresh the managed full report. There are no inline findings or unresolved threads. Pre-merge checks: no remaining required inspection gap was identified in those two dispositions. The old 15-row report is not claimed as a new current-head table. Please approve if those findings are resolved. If approval remains blocked, identify the specific unresolved finding or missing required inspection. This is final approval reconciliation, not a new whole-PR or resume request. The older reassociated approval is not claimed as fresh inspection. |
|
Keep the historical report separate from current-candidate inspection. The approval workflow will check current permissions and review readiness, then report the result. This request does not trigger another review or resume reviews. 🧠 Learnings used
✅ Action performedChanges approved. Review threads were not changed. |
4 participants
Summary
The default Hypothesis deadline can reject subprocess-backed cooldown examples
because of host scheduling, even though their semantic assertions pass. This
change removes that wall-clock deadline from the two affected properties while
retaining their independent 30-second subprocess timeout.
Closes #219.
This is the independent timing prerequisite for PR #217.
PR #217 is stacked on this branch and will be restacked onto the updated parent
before merge, so its review diff retains only Tokio spelling-policy scope. The prerequisite will merge
first; the child will then be retargeted and revalidated on the landed main
revision.
Review walkthrough
Cooldown properties
add the settings import, a short scheduler-latency explanation and
@settings(deadline=None)on the requested-wait and malformed-input properties.Their bodies, generated domains, explicit examples, property budgets, fake
tools, assertions and
timeout=30remain unchanged. The change is one file.Validation
The current published candidate is
a92e31269f40883a2e1d9a585d1bc9e3f84564e9,based on main
34f86aa35764546cee3fe4a5227077098b4f0fde, with tested treec1d134c1831258fa5d3c5067957cd5f0891b575a. The one-file timing patch and387-line source are unchanged. The new main files, shared-action pins and
Hypothesis 6.168.5 dependency are inherited unchanged.
All six fresh Make gates passed sequentially on this replayed candidate:
check-fmt,markdownlint,lint,typecheck,testandspelling, withactual zero process exits and accepted final candidate parity. The full suite
passed 977 tests and three snapshots; both affected cooldown properties
executed and passed.
typecheckperforms syntax checks only.The observed test environment was Python 3.14.7, pytest 9.1.1 and Hypothesis
6.168.5. Spelling resolved builder
v0.1.3toc8a4f95d7cf7f6a1b7517f2775d122d47d5721eb. The current shared dictionary,local overlay and generated output remained unchanged (SHA-256 prefixes
5be79d9f,56da0706and1a6c4cca, respectively).Hosted CI and review for this published head remain pending; the prerequisite
has not merged.
The original failure was a 1156.06 ms example against a 200 ms deadline;
replaying the same input took 6.34 ms. The failure was deadline flakiness,
rather than a cooldown assertion failure. The independent process bound remains
30 seconds. The existing Node colour warning is tracked in
issue #218; its
producer remains unproven and no warning filter is added.
References
Summary by Sourcery
Remove Hypothesis wall-clock deadlines from subprocess-backed cooldown properties while retaining their process-level timeout.
Bug Fixes:
Enhancements: