Skip to content

Include available CI failures in review-round coder follow-ups - #758

Merged
wwind123 merged 3 commits into
mainfrom
codex/round-ci-feedback
Sep 10, 2026
Merged

Include available CI failures in review-round coder follow-ups#758
wwind123 merged 3 commits into
mainfrom
codex/round-ci-feedback

Conversation

@wwind123

@wwind123 wwind123 commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Summary

Include already-failed CI checks in the coder follow-up without waiting for unfinished CI, and prevent the pending-CI filter from silently discarding real reviewer findings.

Why

In PR #757, the coder addressed reviewer findings and pushed another commit while an existing GitHub failure in test_completion_recovery.py remained unaddressed. Both CI runs failed on the same test. The pre-coder path already fetched a post-review check snapshot, but used it only for infrastructure-stall guidance; actionable CI findings were normally added at the approval gate.

Investigation also found that Sol's raw round-1 review contained one blocker and its round-3 review contained two blockers, but the tool published those reviews as approved with the findings removed. _is_pending_ci_only_review() accepted any substring matching a pending check name or generic CI keyword. With a check named test, genuine code findings requesting regression tests were misclassified as CI-wait-only objections. The checked-in regression payloads preserve those real summaries and findings; no provider calls are needed to replay them.

Behavior

  • Reuse the post-review, reviewed-head CI snapshot at the coder handoff. Add observed failures, check links, and head identity to the unresolved-item ledger alongside reviewer findings.
  • Tell the coder to inspect the failure logs, fix the failures, and run local regressions without waiting for queued or running CI.
  • Do not turn missing checks, API unavailability, or recognized infrastructure stalls into code defects. Mixed failed/pending boards contribute only the failures.
  • Preserve existing merge-conflict query suppression, managed intermediate-check filtering, and final approval/CI gates.
  • Reuse an existing CI item instead of duplicating it; subsequent reviewers disposition the item through the normal ledger.
  • Apply this behavior whether or not auto-merge is enabled. No new flag, CI dispatch, watcher, or LLM call is introduced.
  • Replace pending-CI substring matching with conservative whole-statement matching. Check names may appear only in the statement's subject, not as arbitrary keyword matches across prose.
  • Preserve all findings when any finding or substantive summary is mixed or ambiguous. Existing same-PR follow-up and unresolved-prior-item guards remain in place. An unrecognized phrasing stays blocking instead of being silently approved.
  • Verify both early parallel publication and final reconciliation retain the real blockers and pass them to the coder's unresolved-item ledger.

Validation

  • Regression matrix covers failures appearing during review, mixed failed/pending checks, pending/missing/unavailable checks, infrastructure stalls, and both auto-merge settings.
  • End-to-end regression covers coder handoff followed by re-review and resolution of both the reviewer and CI items.
  • Targeted CI-health, PR-orchestration, and parallel-review suite: 282 passed.
  • python3 -m pytest -q --tb=short: 2,713 passed in 164.63 seconds, the complete local suite on the expanded fix, including completion recovery and parallel publication tests.
  • git diff --check: passed.

-- OpenAI Codex

@wwind123

Copy link
Copy Markdown
Owner Author

Review verdict: Blocking

The CI-failure handoff is otherwise well covered, but the new pending-only classifier fails to recognize a canonical unavailable-status statement unless a check happens to be named status.

Blocking issues

  • [src/coding_review_agent_loop/ci_health.py:104] Recognize generic GitHub (PR) check status is unavailable wording independently of check names. On an unavailable board, check_names is commonly empty, so the current subject grammar rejects this unambiguous CI-only statement and _is_pending_ci_only_review leaves the review blocking, dispatching a pointless coder round. The positive test at tests/test_ci_health.py:34 masks the defect by including status as a fictitious check name. Add check status to the generic GitHub subject grammar and cover the unavailable case with no pending or missing check names.

-- OpenAI Codex: gpt-5.6-sol (medium)

@wwind123

Copy link
Copy Markdown
Owner Author

Review verdict: Approved

Reviewed 678464a against main. The new post-review CI snapshot correctly excludes infrastructure stalls, pending and missing-required checks, applies managed-CI filtering consistently with the other get_pr_checks call sites, skips entirely while conflicted, dedupes against a same-round GitHub PR checks item, and never waits on CI. The tightened pending-only filter now fails closed on ambiguous prose, which only ever preserves blocking findings. Snapshot field replacement matches the PullRequestChecks dataclass, item numbering stays consistent with the record-derived ledger, and source_round is display-only so refreshing a carried item is safe. README documents both behaviors. Parametrized tests cover failing, mixed, pending, missing, unavailable and stall boards under both auto-merge settings, plus a real-payload regression fixture. GitHub PR checks are passing.

-- Anthropic Claude: claude-opus-5 (medium)

@wwind123

Copy link
Copy Markdown
Owner Author

PR review round 1 reconciliation: settled reviewers: Claude, Codex. Finalization continues after reconciliation.

@wwind123

Copy link
Copy Markdown
Owner Author

Coder follow-up

Implemented and pushed the requested generic GitHub check-status recognition fix.

Addressed items

  • item-1: Blocking issue from OpenAI Codex: gpt-5.6-sol (medium), round 1: [src/coding_review_agent_loop/ci_health.py:104] Recognize generic `GitHub (PR) check status is un...
    • Resolution: Added generic GitHub check-status subjects independent of dynamic check names and regression coverage with an unavailable board supplying no pending or missing check names; the focused CI-health test module covers the changed classifier and its conservative grammar cases.

Remaining items

  • None.

Tests run

  • python3 -m pytest tests/test_ci_health.py -q (agent-loop instrumented; whole-command timeout 120s)

-- OpenAI Codex: gpt-5.6-sol (medium)

@wwind123

Copy link
Copy Markdown
Owner Author

Review verdict: Approved

Reviewed d380c52. The round-2 delta adds check status to the generic GitHub subject alternation in is_canonical_pending_only_text, so an unavailable board with no pending or missing-required check names now matches without relying on a fictitious check name; the positive test no longer passes status as a check name and a dedicated case covers check_names=() for both GitHub check status is unavailable. and the GitHub PR variant. Full-match anchoring is unchanged, so the existing negative cases (mixed CI-plus-defect prose, bare test, unescaped build linux) still fail closed and substantive findings stay blocking. Round-1 review of the CI-snapshot handoff logic still holds. GitHub PR checks are pending (test in progress), so no global test-pass claim is made here.

Prior unresolved item dispositions

  • [item-1] Blocking issue from OpenAI Codex: gpt-5.6-sol (medium), round 1: [src/coding_review_agent_loop/ci_health.py:104] Recognize generic `GitHub (PR) check status is un... -> resolved

-- Anthropic Claude: claude-opus-5 (medium)

@wwind123

Copy link
Copy Markdown
Owner Author

Review verdict: Approved

The prior pending-CI classification defect is resolved, and the reviewed changes correctly add available non-infrastructure CI failures to coder follow-ups without waiting on pending checks. GitHub's test check remains in progress for the reviewed head.

Prior unresolved item dispositions

  • [item-1] Blocking issue from OpenAI Codex: gpt-5.6-sol (medium), round 1: [src/coding_review_agent_loop/ci_health.py:104] Recognize generic `GitHub (PR) check status is un... -> resolved

-- OpenAI Codex: gpt-5.6-sol (medium)

@wwind123

Copy link
Copy Markdown
Owner Author

PR review round 2 reconciliation: settled reviewers: Claude, Codex. Finalization continues after reconciliation.

@wwind123

Copy link
Copy Markdown
Owner Author

Reviewers approved PR #758; watching GitHub checks in the foreground. No coder or reviewer agents will run while checks remain pending.

-- coding-review-agent-loop

@wwind123
wwind123 merged commit 77beee8 into main Sep 10, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant