Skip to content

[Throughput] Review dispatch burns budget re-dispatching CONFLICTING PRs whose merge tree can never materialize #1972

Description

@seonghobae

Summary

The review-dispatch scheduler re-dispatches pull requests whose merge tree cannot be materialized. Those dispatches cannot produce coverage evidence under any circumstances, yet nothing stops them from repeating. Because REVIEW_ADMISSION_DISPATCH_BUDGET defaults to 1, each wasted dispatch consumes an entire tick's budget.

This is the mirror image of #1219. That issue says the budget is too small; this one says a large share of it is spent on work that cannot succeed.

Evidence

Across the 400 most recent opencode-review-dispatch.yml runs, 55 were repeat dispatches at an already-dispatched head. Cross-referencing the 21 repeated subjects against their current merge state:

merge state subjects redundant dispatches
CONFLICTING / DIRTY 4 35
MERGEABLE / BEHIND 12 14
other 5 6

Four conflicting pull requests produce 64% of all redundant dispatch.

The worst single case is .github#1529 at head c352014a: 19 dispatches, zero successes (7 failed, 12 cancelled). A peer's wider date-range window found 27 over 100.8 hours with the same zero-success result, so the figure here is the conservative one. The triggering actor was github-actions[bot] on the large majority, and the gaps run 1.3 to 8.7 hours, so this is the scheduler on its normal cadence, not human retries.

Corrected after review. An earlier revision of this issue said every attempt failed identically at the same step. That overstated the evidence, and peer verification caught it. Splitting the runs by whether they ever started the job:

  • 8 reached coverage-source-tree (7 that concluded failure, plus 1 cancelled mid-job). All 7 failures died there with the merge-tree error below.
  • 11 never started that job at all. They were cancelled while queued, which this repository also does through bulk-cancel sweeps, so they cannot be attributed to the conflict.

The claim the fix rests on is unaffected: all 19 spent dispatch budget on a head that cannot succeed, and every run that got far enough to try hit the conflict. The job log names the cause:

CONFLICT (content): Merge conflict in CHANGELOG.md
Automatic merge failed; fix conflicts and then commit the result.
##[error]Coverage merge tree could not be materialized for base 69481751 and head c352014a;
         resolve merge conflicts or rerun after GitHub can synthesize the PR merge commit.

The failing step is Materialize pull request merge tree for coverage measurement in coverage-source-tree. coverage-evidence then fails at its Report coverage source materialization failure step, which is the intended reporting of the upstream failure.

Why nothing stops it

Cross-run idempotency does not exist. The admission controller rejects a repeat identity as idempotent, but its state file is ${RUNNER_TEMP}/review-admission/state.json, which is discarded when the job ends. pr_review_merge_scheduler_core.py:6034 documents the intent precisely: state is "shared by scheduler processes in this run". Across runs, every request looks new.

The one cross-run debounce is scoped too narrowly. coverage_retry_wait_reason is the only guard consulting completed same-head dispatch history. It returns None immediately when the pull request has no current-head coverage change-request review. A pull request whose dispatches die before producing any review never reaches the debounce. Its docstring states it exists so that "a repeated coverage-only review cannot create an unbounded dispatch loop" — the hazard is already recognized, but this path is outside the guard.

Proposed direction

Do not dispatch a review for a pull request whose merge tree cannot be materialized. Merge-tree materialization is a hard precondition of coverage-source-tree, so a CONFLICTING pull request is a guaranteed failure that is knowable before the dispatch is spent.

This adds no new policy. The merge scheduler already treats DIRTY pull requests as needing repair rather than review, and gives them repair guidance instead of merging them. Only the dispatch path is unaware of that state.

Conflicting pull requests would still get repair guidance; they would simply stop consuming review-dispatch budget while unrepairable.

🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions