morningman opened a new issue, #70256: URL: https://github.com/apache/doris/issues/70256
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/doris/issues?q=is%3Aissue) and found no similar issues. ### Description On a number of PRs the automated review (the CI Code Review Runner, `.github/workflows/code-review-runner.yml`) does not converge. Each `/review` carries forward a finding that the author has already answered, or the required `code-review` check fails although the review found nothing blocking. Four mechanisms in the runner (line numbers are for master [`315c2e0`](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml)) account for most of it. **1. The PR description and top-level comments are not part of the review context.** The context the prompt gives the agent ([L724-L735](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml#L724-L735)) is the inline review threads, the raw inline-comment JSON, the review focus, the diff and changed-file list, the AGENTS.md files and the shared ledger. The PR JSON is fetched only for `head.sha` and `base.sha` ([L278-L280](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml#L278-L280), [L294-L296](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml#L294-L296)). Design contracts and known limits written in the PR description, and the author's top-level summaries ("Addressed in ..."), are therefore not given to the agent, and it reports them again as findings. When it does look beyond the context it reads the commit message (round 2 on #70232 cites "the current-head commi t description"), which can be older than the PR description. **2. Replies are truncated in the thread summary that the prompt makes mandatory.** "Fetch existing PR review threads" ([L575-L621](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml#L575-L621)) keeps only the 30 most recently started threads (`MAX_THREADS=30`) and cuts every comment, replies included, at 1200 characters (`MAX_BODY_CHARS=1200`, followed by `...(truncated)`). The prompt requires the agent to read that file and to treat it as the already-known review context ([L772](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml#L772)); the untruncated `pr_review_comments.json` is listed but not required. On #70232 the three author replies were 1692, 1969 and 1807 characters long, so the second half of each was cut from that summary. On long PRs, older threads fall out of the 30-thread window and their findings can be raised again. Resolving a thread does not reach the agent either: the REST review-comments API the step reads h as no resolution state. **3. An answered P0/P1 is counted again on every review.** The output contract asks for `existing_blocking_comment_ids`: "every already-reported P0/P1 inline finding you independently confirmed still applies to this head" ([L782](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml#L782)). A reply such as "works as designed, consistent with branch-4.2" or "out of scope for this PR" does not change whether the mechanism still applies, so the same finding is carried forward on each `/review` and `code-review` stays red. Nothing records that the author or a committer has dispositioned a finding. Example: round 2 on #70232 (review 5779067696) found nothing new and carried P1 4474784302 by id. **4. Non-convergence fails the check even with zero P0/P1, including when the cause is model capacity.** A review converges only when every subagent of the last round returns `NO_NEW_VALUABLE_FINDINGS` ([L722](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml#L722)), and "a non-converged delivery blocks the review check even with zero P0/P1" ([L784](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml#L784), [L1229](https://github.com/apache/doris/blob/315c2e003317ef44e8a8fbb445a1ec8452d5647e/.github/workflows/code-review-runner.yml#L1229)). When subagents fail on model capacity, the round cannot converge. Example: on #70197, review 5770744204 delivered P0=0, P1=0 and converged=false because the risk subagent and its replacements hit capacity, and `code-review` became "Review did not converge for PR #70197 at 0eb5131db62c." A run whose main agent never gets a model ends with "Selected model is at capacity", and nothing retries it: #70164 lost four con secutive `/review` runs on 2026-11-04, and its check stayed pending until `/review` was posted again by hand. Each `/review` also samples independently. On #69996 two rounds over identical patches (only a rebase in between, every commit `=` in `git range-diff`) reported 0 and 2 findings. Together with 1-4, the state of the check depends on how often `/review` is posted rather than on the code alone. ### Solution Suggestions, roughly by impact: 1. Put the PR title and description into the review context (optionally also top-level comments by the PR author and committers), and tell the agent that design contracts and known limits stated there are context, not findings. 2. Stop truncating the replies under P0/P1 threads (or raise `MAX_BODY_CHARS` substantially), and always include every thread that carries a P0/P1 finding regardless of the 30-thread window; alternatively, require the agent to read `pr_review_comments.json` for threads that have replies. 3. Add a disposition step for existing blocking findings: when a P0/P1 thread has a reply from the PR author or a committer, the agent has to address that reply and may carry the finding only with a concrete rebuttal written into the summary. Additionally, let a committer accept a finding explicitly (for example with a reply command, or by resolving the thread, read through GraphQL) so that it is no longer counted in `existing_blocking_comment_ids`. 4. Treat non-convergence caused by infrastructure (model capacity, subagent failures) differently from non-convergence caused by open review questions: keep the check pending and retry with backoff instead of failing it, and retry a run that ended with "Selected model is at capacity" automatically instead of waiting for a manual `/review`. ### Are you willing to submit PR? - [ ] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
