reliability: replace stale ignored-Rust deadline branch on current main - #736
reliability: replace stale ignored-Rust deadline branch on current main#736seonghobae wants to merge 15 commits into
Conversation
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (6)
📝 WalkthroughWalkthroughThe PR adds operation-specific subprocess deadlines for the ignored Rust shard runner. It validates overrides, cleans up timed-out process groups, emits redacted timeout evidence, preserves test failure semantics, and adds documentation and integration coverage. ChangesSubprocess deadline enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Main as run_ignored_rust_shard.main
participant Runner as run_bounded
participant Cargo as Cargo subprocess
Main->>Runner: Run metadata, test listing, or statistical test
Runner->>Cargo: Start operation with resolved deadline
Cargo-->>Runner: Completed result or timeout
Runner-->>Main: Result or BoundedSubprocessTimeout
Main-->>Main: Emit redacted evidence and return 124 on timeout
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
@opencode-agent address Bounded deterministic release-note handoff for exact Draft #736 head Do only deterministic managed changelog completion: run |
|
@coderabbitai review Review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected_head='b925ec385525d98bf993f015f007de073f8f29e1'
actual_head="$(git rev-parse HEAD)"
printf 'HEAD=%s\nEXPECTED=%s\n' "$actual_head" "$expected_head"
git status --short
git diff --check 7b57f77747b9a9bd911660c76b1be0ca0dc2d145 "$expected_head"
git diff --name-status 7b57f77747b9a9bd911660c76b1be0ca0dc2d145 "$expected_head"
fd -a '_subprocess_deadlines\.py|run_ignored_rust_shard\.py' .
fd -a -E .git -E node_modules | rg '(^|/)(test|tests)(/|$)|subprocess|deadline|ignored_rust'
ast-grep outline . --items all --type function,class --match 'run_bounded|BoundedSubprocessTimeout|run_ignored_rust_shard' 2>/dev/null || trueLength of output: 20793 🏁 Script executed: #!/bin/bash
set -euo pipefail
for file in \
scripts/_subprocess_deadlines.py \
scripts/run_ignored_rust_shard.py \
tests/test_subprocess_deadlines.py \
tests/test_subprocess_deadline_edges.py \
tests/test_subprocess_deadline_process_group_leak.py \
tests/test_ignored_rust_subprocess_deadlines.py
do
printf '\n===== %s =====\n' "$file"
nl -ba "$file"
done
printf '\n===== subprocess call contexts in scripts/run_ignored_rust_shard.py =====\n'
rg -n -C 8 --pcre2 '\bsubprocess\.(?:run|Popen|check_call|check_output)\s*\(' scripts/run_ignored_rust_shard.py
printf '\n===== bounded helper call contexts =====\n'
rg -n -C 8 --pcre2 '\brun_bounded\s*\(' scripts tests
printf '\n===== helper diff =====\n'
git diff --unified=80 7b57f77747b9a9bd911660c76b1be0ca0dc2d145 -- \
scripts/_subprocess_deadlines.py scripts/run_ignored_rust_shard.pyLength of output: 1249 🏁 Script executed: #!/bin/bash
set -euo pipefail
for file in \
scripts/_subprocess_deadlines.py \
scripts/run_ignored_rust_shard.py \
tests/test_subprocess_deadlines.py \
tests/test_subprocess_deadline_edges.py \
tests/test_subprocess_deadline_process_group_leak.py \
tests/test_ignored_rust_subprocess_deadlines.py
do
printf '\n===== %s =====\n' "$file"
awk '{printf "%6d\t%s\n", NR, $0}' "$file"
done
printf '\n===== subprocess call contexts in scripts/run_ignored_rust_shard.py =====\n'
rg -n -C 8 --pcre2 '\bsubprocess\.(?:run|Popen|check_call|check_output)\s*\(' scripts/run_ignored_rust_shard.py
printf '\n===== bounded helper call contexts =====\n'
rg -n -C 8 --pcre2 '\brun_bounded\s*\(' scripts tests
printf '\n===== helper diff =====\n'
git diff --unified=80 7b57f77747b9a9bd911660c76b1be0ca0dc2d145 -- \
scripts/_subprocess_deadlines.py scripts/run_ignored_rust_shard.pyLength of output: 49943
I verified these items:
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/_subprocess_deadlines.py`:
- Around line 152-166: Bound the final reaping in the subprocess timeout cleanup
paths around process-group and direct-process termination: replace unbounded
process.communicate() calls with a bounded timeout, and close inherited pipe
handles if reaping still exceeds that bound so cleanup can return or raise
BoundedSubprocessTimeout with exit code 124. Update
tests/test_subprocess_deadlines.py lines 165-199,
tests/test_subprocess_deadline_edges.py lines 123-148, and
tests/test_subprocess_deadline_process_group_leak.py lines 59-65 to assert
bounded cleanup; document the bound and fallback in
docs/doctoring/operation-specific-subprocess-deadlines.md lines 40-49, and
retain the changelog claim in
docs/changelog.d/555-operation-specific-subprocess-deadlines.md lines 5-7 only
once enforced.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5072250b-02ef-4d9e-8770-48e7dad88da1
📒 Files selected for processing (8)
docs/changelog.d/555-operation-specific-subprocess-deadlines.mddocs/doctoring/operation-specific-subprocess-deadlines.mdscripts/_subprocess_deadlines.pyscripts/run_ignored_rust_shard.pytests/test_ignored_rust_subprocess_deadlines.pytests/test_subprocess_deadline_edges.pytests/test_subprocess_deadline_process_group_leak.pytests/test_subprocess_deadlines.py
|
@coderabbitai review Review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected_head='105ef8271be89adb171a7141101cae90d268b9eb'
expected_base='7b57f77747b9a9bd911660c76b1be0ca0dc2d145'
printf '%s\n' '=== Repository state ==='
printf 'HEAD=%s\n' "$(git rev-parse HEAD)"
printf 'EXPECTED_HEAD=%s\n' "$expected_head"
printf 'BASE_OBJECT=%s\n' "$(git rev-parse "$expected_base")"
printf 'EXPECTED_BASE=%s\n' "$expected_base"
git status --short
git merge-base --is-ancestor "$expected_base" "$expected_head"
printf 'BASE_IS_ANCESTOR=yes\n'
printf '%s\n' '=== PR metadata and latest comments ==='
gh pr view 736 --repo ContextualWisdomLab/fast-mlsirm \
--json number,state,isDraft,headRefOid,baseRefOid,files,comments \
--jq '{number,state,isDraft,headRefOid,baseRefOid,files:[.files[].path],comments:[.comments[-5:][]|{author:.author.login,body:.body}]}'
printf '%s\n' '=== Changed files ==='
git diff --name-status "$expected_base" "$expected_head"
printf '%s\n' '=== Whitespace check ==='
git diff --check "$expected_base" "$expected_head"
printf '%s\n' '=== Relevant file discovery ==='
fd -a -E .git '_subprocess_deadlines\.py|run_ignored_rust_shard\.py|subprocess.*deadline|deadline.*subprocess|operation-specific-subprocess-deadlines|CHANGELOG\.md|render_changelog_fragments\.py' .
printf '%s\n' '=== Symbol outlines ==='
for file in scripts/_subprocess_deadlines.py scripts/run_ignored_rust_shard.py; do
if [ -f "$file" ]; then
printf '\n--- %s ---\n' "$file"
ast-grep outline "$file" --items all --type function,class,method --view expanded || true
fi
done
printf '%s\n' '=== All subprocess launch and bounded-run call contexts ==='
rg -n -C 6 --pcre2 '\b(?:subprocess\.(?:run|Popen|check_call|check_output)|run_bounded)\s*\(' \
scripts tests -g '*.py' || trueLength of output: 51254 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='7b57f77747b9a9bd911660c76b1be0ca0dc2d145'
head='105ef8271be89adb171a7141101cae90d268b9eb'
printf '%s\n' '=== Complete changed-file diff ==='
git diff --unified=35 "$base" "$head" -- \
scripts/_subprocess_deadlines.py \
scripts/run_ignored_rust_shard.py \
docs/changelog.d/555-operation-specific-subprocess-deadlines.md \
docs/doctoring/operation-specific-subprocess-deadlines.md \
tests/test_subprocess_deadlines.py \
tests/test_subprocess_deadline_edges.py \
tests/test_subprocess_deadline_process_group_leak.py \
tests/test_ignored_rust_subprocess_deadlines.py
printf '%s\n' '=== Deadline implementation ==='
sed -n '1,320p' scripts/_subprocess_deadlines.py
printf '%s\n' '=== Ignored Rust runner: imports, commands, routing, and error handling ==='
sed -n '1,430p' scripts/run_ignored_rust_shard.py
printf '%s\n' '=== Focused deadline tests ==='
for file in \
tests/test_subprocess_deadlines.py \
tests/test_subprocess_deadline_edges.py \
tests/test_subprocess_deadline_process_group_leak.py \
tests/test_ignored_rust_subprocess_deadlines.py
do
printf '\n===== %s =====\n' "$file"
sed -n '1,360p' "$file"
done
printf '%s\n' '=== Documentation and changelog fragment ==='
sed -n '1,260p' docs/doctoring/operation-specific-subprocess-deadlines.md
sed -n '1,160p' docs/changelog.d/555-operation-specific-subprocess-deadlines.md
printf '%s\n' '=== Changelog integration and fragment-contract evidence ==='
rg -n -C 4 --pcre2 'operation-specific|subprocess deadline|changelog.*fragment|render_changelog_fragments' \
CHANGELOG.md tests scripts -g '*.py' -g '*.md' || true
printf '%s\n' '=== Changed Python definitions without immediately preceding docstring ==='
python3 - <<'PY'
import ast
from pathlib import Path
for path in (
Path("scripts/_subprocess_deadlines.py"),
Path("scripts/run_ignored_rust_shard.py"),
):
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
print(f"\n--- {path} ---")
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
if not ast.get_docstring(node, clean=False):
print(f"{node.__class__.__name__} {node.name} at line {node.lineno}")
PYLength of output: 117435 🏁 Script executed: #!/bin/bash
set -euo pipefail
expected_head='105ef8271be89adb171a7141101cae90d268b9eb'
expected_base='7b57f77747b9a9bd911660c76b1be0ca0dc2d145'
printf '%s\n' '=== Renderer behavior and current managed changelog block ==='
sed -n '1,240p' scripts/render_changelog_fragments.py
rg -n -C 8 --fixed-strings '<!-- changelog fragments: begin -->' CHANGELOG.md || true
rg -n -C 8 --fixed-strings '<!-- changelog fragments: end -->' CHANGELOG.md || true
rg -n -C 3 --fixed-strings 'Operation-specific ignored Rust subprocess deadlines' CHANGELOG.md || true
printf '%s\n' '=== Fragment contract and release-parity policy ==='
sed -n '1,220p' tests/test_changelog_fragment_contract.py
sed -n '1,100p' tests/test_release_changelog_serialization_contract.py
rg -n -C 4 --pcre2 'fragment_paths|docs/changelog\.d|CHANGELOG\.md|--check' \
.github scripts tests -g '*.yml' -g '*.yaml' -g '*.py' || true
printf '%s\n' '=== Docstring coverage configuration and repository implementation ==='
if [ -f .coderabbit.yaml ]; then
sed -n '1,280p' .coderabbit.yaml
fi
rg -n -C 4 --pcre2 'docstring|coverage.*80|80.*coverage|interrogate|pydocstyle' \
. -g '!CHANGELOG.md' -g '!docs/**' -g '!**/.git/**' || true
printf '%s\n' '=== Docstrings in all new Python tests ==='
python3 - <<'PY'
import ast
from pathlib import Path
paths = (
Path("tests/test_ignored_rust_subprocess_deadlines.py"),
Path("tests/test_subprocess_deadline_edges.py"),
Path("tests/test_subprocess_deadline_process_group_leak.py"),
Path("tests/test_subprocess_deadlines.py"),
)
for path in paths:
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
missing = []
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
if not ast.get_docstring(node, clean=False):
missing.append(f"{node.__class__.__name__} {node.name} at line {node.lineno}")
print(f"--- {path} ---")
print("\n".join(missing) if missing else "All definitions have docstrings.")
PY
printf '%s\n' '=== Helper imports and module-loading sites ==='
rg -n -C 4 --pcre2 '(?:from\s+_subprocess_deadlines\s+import|import\s+_subprocess_deadlines|spec_from_file_location)' \
scripts tests -g '*.py'
printf '%s\n' '=== Final reference verification ==='
printf 'HEAD=%s\n' "$(git rev-parse HEAD)"
printf 'BASE=%s\n' "$(git rev-parse "$expected_base")"
printf 'PR_HEAD=%s\n' "$(gh pr view 736 --repo ContextualWisdomLab/fast-mlsirm --json headRefOid --jq .headRefOid)"Length of output: 69099
I found no other actionable issue in the requested scope. The timeout policies, POSIX and non-POSIX cleanup paths, bounded final reap with pipe closure, redacted timeout evidence, caller routing, and changed production docstrings are consistent with the implementation and focused tests. I verified the exact requested head and base.
|
|
Absorbed by #740 (bounded ignored-Rust subprocess deadlines + PROCESS_REAP_TIMEOUT reaps on current main). |
1 participant
Controlled clean-lineage replacement for stale Draft #578 / issue #555
This Draft starts directly from protected
main7b57f77747b9a9bd911660c76b1be0ca0dc2d145. Old #578 carries a previously validated deadline design but is long-diverged from protected main and its writer handoffs are bound to predecessor bases. No checks/reviews transfer.Intentional exact-current RED boundary
The first five commits add the bounded deadline test contract plus only a deliberately incomplete
_subprocess_deadlines.pysurface. The tests reach real subprocess policy/cleanup boundaries rather than failing import/setup. The fail-first helper resolves operation-specific deadline values and emits package-owned timeout evidence, but intentionally does not yet create isolated process groups, terminate surviving descendants, or wirerun_ignored_rust_shard.pythrough the bounded runner.Expected exact-current failures therefore prove the missing production boundaries:
subprocess.runrather than operation-specificrun_bounded.Minimum GREEN after RED proof
Port only the already-reviewed non-duplicative #578 design onto this current lineage:
start_new_sessionplus SIGTERM→5s grace→group liveness probe→SIGKILL and reap;BoundedSubprocessTimeoutevidence without command/stdout/stderr reflection;check=Trueand nonzero return semantics;scripts/run_ignored_rust_shard.pymetadata, target inventory and actual statistical-test subprocesses through the helper; andScientific workload sizes, recovery iterations, dependencies/workflows, credentials, branch protection and other subprocess classes do not change. Authoritative doctoring/changelog material is added only after GREEN. Keep Draft until focused and full Python 3.12/3.14 plus repository-required Rust/PyO3/package/GPU/fuzz/Security/SAST/current-head review gates pass on one unchanged head.
Once this replacement is established, stale #578 may close with lineage preserved. Advances #555.
Summary by CodeRabbit
New Features
Documentation