damccorm commented on code in PR #39980: URL: https://github.com/apache/beam/pull/39980#discussion_r3926571843
########## scripts/ci/pr-bot/shared/geminiReviewerAdvisor.ts: ########## @@ -0,0 +1,426 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + PrHistoryContext, + CandidateContributor, + TouchedFileContext, +} from "./gitHistory"; + +/** + * Interface representing an individual recommended reviewer. + */ +export interface ReviewerRecommendation { + readonly username: string; + readonly role: "primary" | "secondary"; + readonly isCommitter: boolean; + readonly expertise: string; + readonly coveredFiles: readonly string[]; +} + +/** + * Interface representing an alternate reviewer suggestion. + */ +export interface AlternateReviewer { + readonly username: string; + readonly expertise: string; +} + +/** + * Result structure produced by the reviewer advisor. + */ +export interface ReviewerAdviceResult { + readonly selectedReviewers: readonly ReviewerRecommendation[]; + readonly alternateReviewers: readonly AlternateReviewer[]; + readonly reasoning: string; + readonly source: "gemini" | "heuristic-fallback"; +} + +/** + * Interface for LLM clients that can generate structured JSON. + */ +export interface IGeminiClient { + generateJson<T>(prompt: string): Promise<T>; +} + +/** + * Standard HTTP Gemini client using global fetch. + */ +export class GeminiClient implements IGeminiClient { + private readonly apiKey: string; + private readonly model: string; + + constructor(apiKey: string, model: string = "gemini-2.5-flash") { + this.apiKey = apiKey; + this.model = model; + } + + async generateJson<T>(prompt: string): Promise<T> { + if (!this.apiKey) { + throw new Error("GEMINI_API_KEY is not configured."); + } + + const url = `https://generativelanguage.googleapis.com/v1beta/models/${encodeURIComponent( + this.model + )}:generateContent?key=${encodeURIComponent(this.apiKey)}`; + + const response = await fetch(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify({ + contents: [ + { + role: "user", + parts: [{ text: prompt }], + }, + ], + generationConfig: { + temperature: 0.1, + responseMimeType: "application/json", + }, + }), + }); + + if (!response.ok) { + const errorText = await response.text(); + throw new Error( + `Gemini API request failed with status ${response.status}: ${errorText}` + ); + } + + const data: any = await response.json(); + const candidateText = data?.candidates?.[0]?.content?.parts?.[0]?.text; + + if (!candidateText) { + throw new Error("Empty or invalid candidate response from Gemini API."); + } + + return JSON.parse(candidateText) as T; + } +} + +/** + * Configuration options for the Gemini Reviewer Advisor. + */ +export interface ReviewerAdvisorOptions { + readonly geminiClient?: IGeminiClient; + readonly committerCheck?: (username: string) => Promise<boolean>; + readonly exclusionList?: readonly string[]; + readonly maxReviewers?: number; +} + +/** + * Advisor that analyzes PR git history and selects optimal reviewers using Gemini or heuristic fallback. + */ +export class GeminiReviewerAdvisor { + private readonly client?: IGeminiClient; + private readonly committerCheck: (username: string) => Promise<boolean>; + private readonly exclusionList: readonly string[]; + private readonly maxReviewers: number; + + constructor(options: ReviewerAdvisorOptions = {}) { + this.client = options.geminiClient; + this.committerCheck = options.committerCheck ?? (async () => false); + this.exclusionList = options.exclusionList ?? []; + this.maxReviewers = options.maxReviewers ?? 2; + } + + /** + * Constructs the prompt instructing Gemini on how to select reviewers. + * + * @param context Extracted git and PR history. + * @param committers Map of username to committer status. + * @returns Detailed prompt string. + */ + public buildPrompt( + context: PrHistoryContext, + committers: Readonly<Record<string, boolean>> + ): string { + const fileSummaries = context.touchedFiles.map((file) => { + const commitSummaries = file.recentCommits + .slice(0, 5) + .map( + (c) => + ` - [${c.date}] ${c.authorLogin || c.authorName}: ${c.subject}` + ) + .join("\n"); + + return `- File: ${file.path} (+${file.additions}, -${ + file.deletions + }, changes: ${file.changes}${ + file.isNewFile ? " [NEW FILE]" : "" + })\n Recent Commits:\n${commitSummaries || " (No recent commits)"}`; + }); + + const candidateSummaries = context.candidates.map((c) => { + const isCommitter = committers[c.login] ?? false; + return `- @${c.login} (${c.name}): ${ + c.commitCount + } commits, last active ${ + c.lastCommitDate + }, committer=${isCommitter}. Files touched: ${c.touchedFilePaths.join( + ", " + )}`; + }); + + const exclusions = + this.exclusionList.map((e) => `@${e}`).join(", ") || "(none)"; + + return `You are the Apache Beam Code Review Assigner. +Your goal is to choose a small, optimal set of expert reviewers for a pull request based on real git history and file churn. Review Comment: 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. -- 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]
