Implement LLM-driven review assigner for PR bot

[kennknowles]

Beam's existing PR reviewer assigner works like so:

• matches directory paths against coarse labels (e.g. java, python, kafka)
• and assigns one reviewer per matching label via round-robin

The downsides:

• Reviewer bloat on multi-directory PRs
• Doesn't stay up to date
• Is pretty coarse

This PR is meant to introduce a little more intelligence:

• Read the history and do some heuristics on who has edited in a meaningful way recently
• Provide that heuristic along with the history to an LLM to choose a good reviewer
• Give the reviewer a one-liner about why they were chosen

---

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

• Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
• Update CHANGES.md with noteworthy changes.
• If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

[kennknowles]

@damccorm I need to give this a more thorough review, but the biggest picture is that I don't really know what it looks like for Beam's infra to make the gemini calls that are in this using a GEMINI_API_KEY or if there is an entirely different appropriate architecture.

[damccorm]

I haven't done a full review, but I don't think that this has the information needed to do this optimally. Specific things which aren't covered, but seem important:

1. How many times has this reviewer been assigned recently (right now we just round robin)
2. Committership
3. Does this person actually want to be a reviewer for the area/are they willing to respond in a timely manner (I can think of contributors who commit a good amount of code, but aren't necessarily ideal reviewers).

[damccorm]

Looking at the problems, do we need a full system overhaul or can we just make smaller less-invasive changes? The gemini infra is going to be painful since it will need to be a secret we regularly rotate.

Reviewer bloat on multi-directory PRs

Could this be solved by just choosing one of the reviewers from the set of available ones (probably with a preference for code-related expertise)?

Doesn't stay up to date

Is this really an issue? I have not seen this one cause meaningful problems since reviewers tend to opt out fairly quickly once they are out of the loop.

I think the ability to opt in/out is a pretty useful feature.

Is pretty coarse

This is less avoidable (though we certainly could have tighter categories).

[kennknowles]

We could ditch gemini and just go with the heuristic score, for example.

[kennknowles]

And TBH I think the selected reviewers are just almost always not the right person. I'm biased because I get on there for "java" and "website" and "fallback" and realistically there's always a better choice. I don't think load balancing to people who are that vaguely related is necessarily good.

[damccorm]

The gemini infra is going to be painful since it will need to be a secret we regularly rotate.

We could ditch gemini and just go with the heuristic score, for example.

Ok, actually I thought a bit more about this and I don't think this will be an issue - we just shouldn't use a GEMINI_API_KEY, we can just use Vertex + gcloud auth. We do this in our model handler if no API key is provided, and our ITs rely on this -

beam/sdks/python/apache_beam/ml/inference/gemini_inference.py

Line 193 in 562273c

else:

The idea has grown on me, I think my primary concern is more around the context and how we're prompting the model.

I'm biased because I get on there for "java" and "website" and "fallback" and realistically there's always a better choice. I don't think load balancing to people who are that vaguely related is necessarily good.

Could you give examples of 1 or 2 prs as case studies of where we could do better and what would have led to a better outcome? I think the most important part of this is that we need to pick the right set of things to prioritize when choosing a reviewer. The criteria I can think of are:

1. Competence: Don't assign a reviewer who will be incapable of providing a good review.
2. Fairness: Don't assign a reviewer to too many PRs.
3. Opt-out: Allow reviewers to opt out of being assigned too many PRs (or limit the volume).
4. Affinity: Try to assign reviewers who are most active in an area.
5. Inclusion: Try to include/train reviewers who may not be the best option, but are contributing more.

I tried to order those criteria from most to least important. I think right now, the prompt fully addresses (4) and partially addresses (1)/(5). It doesn't try to address (2) or (3).

I think the current system generally accomplishes (1) and (3). It tries to accomplish (2), (4), and (5), but probably doesn't do a great job with any of them.

Fairness is the one that I think is hardest. For reference, if you look at the last calendar year and filter out both reviews done on the reviewer's own PR and dependabot reviews (usually 1-liners):

• Yi has reviewed the most PRs with 501
• I am second with 397
• For comparison, you are at 105.

I did a check of other common reviewers and couldn't find anyone with more than 129 reviews

For both Yi and me, this is driven by a lot of manual assignment, so I'm not sure if the system can totally fix it. But I worry that an affinity based assigner will overassign Yi because he is very active across the codebase (and is likely the best reviewer by heuristics)

[damccorm]

FWIW, out of the 3 examples you gave, I think the current bot assigned a good reviewer in the first 2. It clearly got stuck on the third one (that just looks like a bug), but I don't think the LLM solution (assigning a GSoC contributor) is better. It is also unclear who a good reviewer there is (Cham is probably as good as anyone).

FWIW I'm deliberately taking the blameless premise that if someone didn't respond to the "R: " assignment then they had a good reason (too busy, not the right reviewer, the PR was not ready, etc). There could, of course, be other reasons that are less blameless, but I don't think it is productive. In any case being strictly pragmatic makes sense, in the sense of "if they didn't do it, they didn't do it and we change the system not the people".

I do think this is a problem worth addressing. Ideally we would have better expectations here for what is reasonable. I'll also note that one of the big problems with the current setup is distributed ownership (if 3 reviewers are assigned to a PR, no reviewers are assigned to the PR)

In my experience, it is more common to have a qualified reviewer who doesn't review in a timely manner than to have a set of reviewers with nobody capable of providing a review

[kennknowles]

I reached for LLM because it is something where a quick glance at code history typically makes it very obvious who should review. It may be that all the extra parts of the system are actually worse than just saying "choose a good reviewer" and letting it do its thing.

[kennknowles]

Tried it... and yes it is better, but burns tokens for a while and lots of tool calls, plus I didn't attempt to restrict the context away from the existing PR comments and reviewers.

The funniest result would be if it said "me! I'm the ultimate code reviewer assistant" but alas...

Suggest a good reviewer for #39156

The best reviewer for #39156 #39156 is @ahmedabu98 (Ahmed Abualsaud).
──────

Primary Recommendation: @ahmedabu98 (Ahmed Abualsaud)

• Original Author: Authored WriteToPartitions.java https://github.com/apache/beam/blob/master/sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/WriteToPartitions.java in PR #38061
#38061 when introducing hash distribution mode for Iceberg writes.
• Component Lead: Primary maintainer and most active committer across the sdks/java/io/iceberg https://github.com/apache/beam/tree/master/sdks/java/io/iceberg module.
• Context / Current Blocker: Ahmed already reviewed this PR on June 29 and left an inline design question on WriteToPartitions.java #39156 (comment):
│ "Were you seeing OOMs even with autosharding enabled? AFAIK autosharding should distribute to different keys, so the state for any one key shouldn't get too large. If it makes sense, we can keep
│ the GroupIntoBatches.withShardedKey() path for autosharding batch writes. Otherwise maybe we can add a num_shards config to do fixed sharding per partition, and stick with GBK"
The PR is currently stalled awaiting the author's response to this question, which has caused the Beam PR bot to repeatedly cycle reviewers (@ahmedabu98 ↔ @chamikaramj ↔ @Abacn). Once addressed,
@ahmedabu98 is best positioned to approve and merge it.
──────

Alternative Reviewers

1. @chamikaramj (Chamikara Jayalath)
   • Beam PMC member and senior I/O specialist.
   • Reviewed the original PR [IcebergIO] Support hash distribution mode when writing rows #38061 [IcebergIO] Support hash distribution mode when writing rows #38061 implementing WriteToPartitions.
   • Deep expertise in Beam core transforms, batch vs. streaming execution semantics, and runner shuffle behavior (GroupByKey vs. GroupIntoBatches).
2. @clairemcginty (Claire McGinty)
   • Active contributor to IcebergIO, having recently modified WriteToPartitions.java https://github.
   com/apache/beam/blob/master/sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/WriteToPartitions.java in PR (IcebergIO) bugfix: propagate config properties to RecordWriter #39250 (IcebergIO) bugfix: propagate config properties to RecordWriter #39250.
   • Colleague of the PR author at Spotify, with direct context on large-scale production Iceberg write pipelines.

Suggest a good reviewer for #37535

Considering Stream Completion Logic
Based on the changes in PR #37535 #37535 ("Fix temp table cleanup for continuous read in BigQueryIO" by @stankiewicz), here are the best reviewers to suggest:
──────

Primary Recommendations

1. Yi Hu (@Abacn https://github.com/Abacn) — Recommended primary reviewer

• Why:
• Already active on this PR: Yi previously reviewed this PR in April 2026 and requested a load test regarding the overhead of the stateful CleanupTempTableDoFn, as well as handling the zero-
stream edge case.
• Addressed feedback: The author (Radek) has since conducted the benchmark, posted the load test doc https://docs.google.com/document/d/1yUxqlv2118xc-6AN-
8OxFgBi9iZRg0HcDToVElWoetY/edit?usp=sharing, and added tests for zero streams.
• Code ownership: Designated reviewer for the Java label and frequent contributor to BigQueryStorageStreamSource and BigQuery IO.
• Next step: A ping to Yi to review the benchmark results and verify the zero-stream edge case test.

2. Chamikara Jayalath (@chamikaramj https://github.com/chamikaramj) — Recommended for runner/fault-tolerance validation

• Why:
• Pending review thread: Chamikara reviewed the PR in July 2026 and raised a critical question regarding runner failure and retry semantics:
│ "Should there be a shuffle between 'ReadDynamicStreamSource' and 'CleanupTempTableDoFn'? What if there's a runner failure of the downstream output processing and a retry after cleanup
│ operation already performed? ... Just want to make sure that we considered and ruled out the case of premature deletion of temp tables before they are fully processed and corresponding
│ workitems are completed."

  • Radek replied on July 23 noting that CleanupTempTableDoFn is a stateful DoFn (introducing a key-by shuffle on the cleanup messages), but Chamikara has not yet weighed in on whether that fully
  guarantees downstream consumers won't attempt a re-read if an uncheckpointed bundle fails.
  • Chamikara's sign-off is essential to ensure this doesn't risk data loss under runner worker failure scenarios.

──────

Secondary / Alternative Reviewers

3. Danny McCormick (@damccorm https://github.com/damccorm)

• Why:
• Danny reviewed and approved Radek's original PRs that implemented this entire feature:
• #36668 #36668: Streaming read for BigQuery
• #36912 #36912: Fix retry scenario for query to table materialization
• He already possesses full context on the design and lifecycle of BigQuery dynamic streaming reads.

4. Reuven Lax (@reuvenlax https://github.com/reuvenlax)

• Why:
• Primary architect and maintainer of BigQueryIO and Dataflow streaming/stateful semantics (including the original batch temp table cleanup transform PassThroughThenCleanup).
• If there is any remaining question about the stateful DoFn approach vs. stream completion guarantees across runners, Reuven's review would be authoritative.

──────

Suggested Action

Since @Abacn and @chamikaramj are already assigned and have pending review threads on the PR, the best step is to re-ping both of them on the PR:

R: @Abacn @chamikaramj

Specifically highlighting that the load test results and the response to the runner-retry question are ready for their re-evaluation.

Suggest a good reviewer for #39363

Examining artifact staging issues
The recommended reviewer for #39363 #39363 is:

Primary Recommendation: @Abacn (Yi Hu)

Why:

• Active Engagement & Verification: @Abacn is already involved with this PR. They verified the changes on Windows (running Spark portable runner tests) and commented on the PR.
• Direct Context on Artifact Staging & Windows: They reviewed and merged the closely related PR #39367 #39367 (handling InvalidPathException during artifact
staging on Windows) and subsequently opened #39916 #39916 addressing the broader Windows artifact staging filename and CI fixes.
• Committer Status: As an Apache Beam committer and PMC member, @Abacn has the necessary permissions to guide the PR to completion and merge it once open comments are resolved.
──────

Domain Expert Alternative: @robertwb (Robert Bradshaw)

• Why: Robert is the original author of ArtifactStagingService.java https://github.com/apache/beam/blob/master/runners/java-fn-
execution/src/main/java/org/apache/beam/runners/fnexecution/artifact/ArtifactStagingService.java and leads the Fn Execution / Portability artifact management architecture. He is the best escalation
point if there are questions about cross-language artifact retrieval contracts or naming semantics across SDKs.
──────

Context on Existing Reviews

• @Eliaaazzz (Elia Liu) already performed an in-depth review on this PR, pointing out that the Splitter pattern ([^A-Za-z-_.]]) contains a stray bracket causing base to carry unescaped characters,
and suggesting handling of control characters. While not currently a committer with merge bits, their feedback is the primary blocker @Abacn cited as needing resolution before merge.

[kennknowles]

I'll try it on incoming stuff that doesn't have so much history, to see if it is worth adding as a requestable service.

[damccorm]

Let me step back for a moment. I think we need to start with what we're optimizing for; everything else falls out from that.

I like your criteria. Of those, I'm privileging affinity above all else, which I think proxies for and causes other good criteria - particularly competence and lower latency. Fairness is tricky because different people have different amounts of their attention currently focused on Beam or on a particular area.

I disagree with prioritizing affinity above everything. I think it does proxy for competence. Its not obvious to me that it prioritizes latency - I can see an argument that more active code contributors provide faster reviews, but I'm not sure I've seen that in practice.

I also intentionally would not prioritize latency. This rewards people who do reviews quickly (or in this case, commit a bunch of code) with... more reviews. That doesn't seem right. Conversely, I am committing less code these days than I have before, but I am not particularly interested in doing fewer reviews. I think this is a pretty natural directional progression over time (shifting workload toward less code, more reviews).

I agree that fairness is tricky, but I don't think we can ignore it - I think it is highly important, and was one of the core goals of the project initially. I think we should move towards optimizing for it more, not less. There are alternative ways of achieving this (e.g. surfacing review metrics, making those contributions more visible, making it easier for reviewers to opt out), but I'd also love to incorporate it into this bot.

I think we need to align on what we're prioritizing, and then we can figure out the how. I agree that an LLM-based solution will likely help.