fix(dev): paginate main CI incident evidence - #8445
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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-actions
Bot
added
the
review-bot-auto
🔍 PR Contract Check SummaryOverall Status: 🟢 PASSED
ℹ️ Info
This check is mechanized. Please resolve any blockers to pass CI. |
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head self-review
Reviewed PR #8445 at exact head 3b136e011ff8696e5b2acf045400abbe5c56c596 against
origin/main 5f476625eba9c4ae2fa5770781852b78c865fd8b.
Intended design and scope
This is a self-contained maintenance-tooling change. It replaces the cancellation-sensitive raw
Actions run cutoff in the main-CI incident reconciler with bounded REST pagination, while keeping
the existing two-newer-green closure contract and fail-closed mutation guards. It makes no
benchmark, model, checkpoint, or paper-facing claim.
Findings
No actionable P0–P3 finding. The implementation, tests, documentation, and narrowed support-tooling
scope agree. Remaining generated files under output/ are ignored local validation output and are
not durable evidence artifacts.
Exact-head validation
pytest -q tests/dev/test_reconcile_main_ci_incidents.py tests/dev/test_main_ci_incident_reconcile.py tests/dev/test_main_ci_is_green.py— 50 passed.uvx --from 'ruff==0.16.5' ruff check scripts/dev/reconcile_main_ci_incidents.py tests/dev/test_reconcile_main_ci_incidents.py— passed.uvx --from 'ruff==0.16.5' ruff format --check scripts/dev/reconcile_main_ci_incidents.py tests/dev/test_reconcile_main_ci_incidents.py— passed.git diff --check origin/main...HEADandgit merge-tree --write-tree origin/main HEAD— passed.- Live report-only run returned
status=ok,open_incident_count=0, and no mutation. - Live REST Actions probe fetched 100 current
CIruns with 14 decisive results. check_pr_ci_status.py 8445reported completed success with 35 successful checks and one skipped coverage gate; hosted docs, compatibility, CodeQL, determinism, and changed-coverage checks passed.check_base_sensitive_gates.py --pr 8445 --jsonreported no base-sensitive files; the PR merge tree is clean.- Independent exact-head review found no actionable P0–P3 finding.
Decision
High-confidence exact-head review. The single-account internal-review waiver applies because this
repository is operated through one effective GitHub account and no distinct implementation-review
identity is available. Apply the merge-ready label after promoting the draft, subject to the
guarded final receipt and compare-and-swap merge.
gate-verdict: accepted @ 3b136e0
pr-metadata: reconciled @ e60481d140e3c08d2c9ae6917267533f189ca878b39dd8b94f45930731bd6e3d
## Summary This successor closes the remaining boundedness gap in the CI `ci_uv_sync_diag` probe. It removes the unbounded `uv cache size` traversal and makes both advisory `du` probes deadline-aware and diagnostically explicit. ## Linked Issues - Relates to #8249 (successor follow-up to merged PR #8365; the issue is reopened for this work). ## Stack / Dependency - Base dependency: none. - Required prior PRs and stack follow-up issues: none. - Safe to review independently: yes; the branch is based on current `main`. ## What Changed - Removed the redundant `uv cache size` cache-root traversal. - Require a positive `ROBOT_SF_DIAG_DU_TIMEOUT_SECONDS` value, defaulting invalid values (including zero) to 10 seconds. - Detect GNU `timeout(1)` explicitly and use `--kill-after=2s` after the configured deadline. - Emit `ok`, `timed-out`, or `error` sizing status markers and exit codes for cache and virtualenv probes. - Added deterministic tests for the unbounded-`uv` regression, zero timeout, hard timeout timing, non-timeout errors, no-GNU fallback behavior, and the script contract. ## Why It Matters The diagnostic remains advisory and exits zero, while its expensive cache and virtual-environment walks no longer hide failures or consume the full readiness test budget. Hosts without GNU `timeout(1)` retain the prior direct-`du` fallback and ordinary output keys. ## Research / Evidence Notes Not applicable — this is a CI/tooling reliability change with no research, benchmark, metric, or paper-facing claim. ## Validation / Proof - `tests/dev/test_ci_uv_sync_diag.py`: 12 passed. - `tests/test_ci_script_contract.py`: 153 passed. - `bash -n scripts/dev/ci_uv_sync_diag.sh`: passed. - Ruff check and format checks for both changed Python test files: passed. - `BASE_REF=origin/main PR_READY_MODE=final ... scripts/dev/pr_ready_check.sh`: passed on committed head `8e7015c578279c1d4e1dd31279b0143bf9161eec` against base `5f476625eba9c4ae2fa5770781852b78c865fd8b`. ## Risks / Rollback The no-GNU fallback remains intentionally unbounded for compatibility with stock macOS-style hosts. Revert commit `8e7015c57` to restore the preceding diagnostic implementation if needed. ## Docs / Provenance No durable benchmark or model artifacts were produced. Readiness receipts remain worktree-local ignored output; the source contract and reproducible test fixtures are tracked in this PR. ## Downstream Propagation Not applicable — support/tooling change; no benchmark, metric, model, claim-map, registry, or durable evidence update is required. ## Follow-Up / Residual Scope No deferred work remains for this implementation slice. Issue #8249 is reopened for this PR; this is the immediate successor requested for the remaining `uv sync` diagnostic gaps. ## Reviewer Notes - Verify that `uv cache size` is absent from the executable path and that the later bounded `du` remains the sole cache-size traversal. - Verify that timeout expirations are not attributed to host contention without evidence and that non-timeout tool failures remain visible. <!-- Keep this marker and its keys unchanged. Update values to match the human narrative and changed files. --> <!-- pr-contract:v2 change_class: tooling linked_issues: closes: [] relates: [8249] deferred_work: status: none issues: [] reason: "" evidence: applicability: na tier: null result: na domain_approval: required: false status: not_required domains: [] note: "NA - support/tooling change; no experimental claim." performance: claimed: false -->
…s://github.com/ll7/robot_sf_ll7 into autopilot/sync-pr8445-20260904-cycle43
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed PR #8445 at exact head 21e5d39778a625a7a109ad325d6eef383344dc58 against fresh
origin/main a2bf33373e5775eacbeed5049c15b987ad5af9df after synchronizing the branch with the
merged #8446 base.
This is a support/tooling-only change to paginate main continuous-integration incident evidence
past cancellation-heavy pages while preserving the existing two-newer-green, fail-closed closure
rule. It makes no research, benchmark, metric, model, or paper-facing claim. The implementation
and focused proof were previously reviewed at the pre-sync head; the synchronization merge adds
only the already-merged #8446 base and preserves the reviewed PR diff.
Focused tests passed (122 passed across the reconciler and follow-up contract suites), with Ruff
0.16.5 check/format, diff, and merge-tree checks clean. Final hosted CI and the repository’s
exact-head merge receipt remain required after this new head.
No actionable P0–P3 implementation finding is present in this refreshed slice. Accept
provisionally for the guarded merge path, subject to current-head hosted checks and receipt.
Decision: accept this tooling slice provisionally; no source repair is required.
gate-verdict: accepted @ 21e5d39
pr-metadata: reconciled @ 92fc98325b22cc9afafb7a50ef8a5937f52bf0bee1d2258bd3ce1a8288ff41ef
|
review-claim: codex-goal-autopilot-20260905 @ 21e5d39 until 2026-09-04T23:57:44Z |
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head self-review
Reviewed PR #8445 at exact head 21e5d39778a625a7a109ad325d6eef383344dc58 against current
origin/main a2bf33373e5775eacbeed5049c15b987ad5af9df in a dedicated linked review worktree.
Intended design and scope
This is a support/tooling-only change that paginates main continuous-integration incident evidence
past cancellation-heavy pages while preserving the existing two-newer-green, fail-closed closure
rule. It makes no research, benchmark, metric, model, or paper-facing claim.
Findings
No actionable P0-P3 finding. The implementation, tests, documentation, and narrowed support-tooling
scope agree. Ignored local output is not durable evidence.
Exact-head validation
pytest -q tests/dev/test_reconcile_main_ci_incidents.py tests/dev/test_main_ci_incident_reconcile.py tests/dev/test_main_ci_is_green.py— 50 passed.ruff check scripts/dev/reconcile_main_ci_incidents.py tests/dev/test_reconcile_main_ci_incidents.py— passed with pinned shared Ruff 0.16.5.ruff format --check scripts/dev/reconcile_main_ci_incidents.py tests/dev/test_reconcile_main_ci_incidents.py— passed.bash -n scripts/dev/ci_uv_sync_diag.sh— passed.git diff --check origin/main...HEADandgit merge-tree --write-tree origin/main HEAD— passed.- Current hosted receipt evidence reports 35 successful checks and one intentional skipped coverage gate; no unresolved review threads or requested reviewers.
check_base_sensitive_gates.py --pr 8445 --jsonreports no base-sensitive files. The current-base CAS is passed.
Decision
High-confidence exact-head review. The single-account internal-review waiver applies because no
distinct implementation-review identity is available. The ordinary stale-base CAS path is safe at
this exact head and current main; the missing policy carrier is supplied by this review.
gate-verdict: accepted @ 21e5d39
base-policy: ordinary-cas @ 21e5d39
pr-metadata: reconciled @ 92fc98325b22cc9afafb7a50ef8a5937f52bf0bee1d2258bd3ce1a8288ff41ef
|
review-claim: released @ 21e5d39 |
1 participant
Summary
Repair the scheduled main-CI incident reconciler's evidence acquisition so cancellation-heavy
workflow history cannot hide the decisive runs required by the two-green closure rule.
Linked Issues
closed and this PR does not change its state).
Stack / Dependency
What Changed
gh run listevidence cutoff with bounded GitHub REST pagination.decisive completed green/red runs are visible.
routing; document the new
--max-run-pagesoption and legacy--run-limitbehavior.Why It Matters
The prior ten-raw-run window could contain only cancelled runs under the repository's
latest-main-wins concurrency policy, producing a pending report despite decisive greens just
outside the window. The new default searches a bounded evidence window without treating
cancellations as failures or making a current-head-green claim.
Research / Evidence Notes
Not applicable — no research claim. This changes only incident-reconciliation tooling and its
operational evidence-window contract.
Validation / Proof
python -m pytest -q tests/dev/test_reconcile_main_ci_incidents.py— 19 passed.python scripts/dev/check_docs_evidence_integrity.py --full— 2052 Markdown files passed.bash scripts/dev/check_context_notes.sh— passed.CIruns andidentified the current decisive successes/failures.
Risks / Rollback
The default may make additional read-only Actions requests when cancellations fill a page; the
ten-page ceiling bounds that work. If workflow inventory or run pagination cannot be verified, the
reconciler returns an error and performs no issue mutation. Rollback is a single revert of this
tooling commit.
Docs / Provenance
Updated
docs/dev_guide_reference.mdwith the paginated evidence-window contract. No benchmark,model, checkpoint, raw log, or generated evidence artifact is changed.
Downstream Propagation
Not applicable — support change.
Follow-Up / Residual Scope
None; current-head CI failures remain separate incidents keyed to their own deciding runs.
Reviewer Notes
success/failureruns count as decisive evidence.is green.