From 0ebfb36699c5b2b4f939fa628d91f7ce8445ae75 Mon Sep 17 00:00:00 2001 From: Harjot Gill Date: Mon, 20 Mar 2023 14:17:54 -0700 Subject: [PATCH 1/2] refactor code --- dist/index.js | 96 +++++++++++++++++++++++-------------------- src/commenter.ts | 53 ++++++++++++++++++++++++ src/review-comment.ts | 47 +++------------------ 3 files changed, 110 insertions(+), 86 deletions(-) diff --git a/dist/index.js b/dist/index.js index 4a5d950f..1a93674d 100644 --- a/dist/index.js +++ b/dist/index.js @@ -27069,10 +27069,9 @@ class Bot { /* harmony export */ "Es": () => (/* binding */ Commenter), /* harmony export */ "Rp": () => (/* binding */ SUMMARIZE_TAG), /* harmony export */ "Rs": () => (/* binding */ COMMENT_TAG), -/* harmony export */ "aD": () => (/* binding */ COMMENT_REPLY_TAG), -/* harmony export */ "pK": () => (/* binding */ COMMENT_GREETING) +/* harmony export */ "aD": () => (/* binding */ COMMENT_REPLY_TAG) /* harmony export */ }); -/* unused harmony exports DESCRIPTION_TAG, DESCRIPTION_TAG_END */ +/* unused harmony exports COMMENT_GREETING, DESCRIPTION_TAG, DESCRIPTION_TAG_END */ /* harmony import */ var _actions_core__WEBPACK_IMPORTED_MODULE_0__ = __nccwpck_require__(2186); /* harmony import */ var _actions_github__WEBPACK_IMPORTED_MODULE_1__ = __nccwpck_require__(5438); /* harmony import */ var _octokit_action__WEBPACK_IMPORTED_MODULE_2__ = __nccwpck_require__(1231); @@ -27191,6 +27190,54 @@ ${COMMENT_TAG}`; _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to post review comment: ${e}`); } } + async review_comment_reply(pull_number, top_level_comment, message) { + const reply = `${COMMENT_GREETING} + +${message} + +${COMMENT_REPLY_TAG} +`; + try { + // Post the reply to the user comment + await octokit.pulls.createReplyForReviewComment({ + owner: repo.owner, + repo: repo.repo, + pull_number, + body: reply, + comment_id: top_level_comment.id + }); + } + catch (error) { + _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to reply to the top-level comment ${error}`); + try { + await octokit.pulls.createReplyForReviewComment({ + owner: repo.owner, + repo: repo.repo, + pull_number, + body: `Could not post the reply to the top-level comment due to the following error: ${error}`, + comment_id: top_level_comment.id + }); + } + catch (e) { + _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to reply to the top-level comment ${e}`); + } + } + try { + if (top_level_comment.body.includes(COMMENT_TAG)) { + // replace COMMENT_TAG with COMMENT_REPLY_TAG in topLevelComment + const newBody = top_level_comment.body.replace(COMMENT_TAG, COMMENT_REPLY_TAG); + await octokit.pulls.updateReviewComment({ + owner: repo.owner, + repo: repo.repo, + comment_id: top_level_comment.id, + body: newBody + }); + } + } + catch (error) { + _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to update the top-level comment ${error}`); + } + } async get_comments_at_line(pull_number, path, line) { const comments = await this.list_review_comments(pull_number); return comments.filter((comment) => comment.path === path && comment.line === line && comment.body !== ''); @@ -29142,7 +29189,7 @@ class Options { openai_retries; openai_timeout_ms; openai_concurrency_limit; - constructor(debug, max_files = '40', review_comment_lgtm = false, path_filters = null, system_message = '', openai_model = 'gpt-3.5-turbo', openai_model_temperature = '0.0', openai_retries = '3', openai_timeout_ms = '60000', openai_concurrency_limit = '4') { + constructor(debug, max_files = '60', review_comment_lgtm = false, path_filters = null, system_message = '', openai_model = 'gpt-3.5-turbo', openai_model_temperature = '0.0', openai_retries = '3', openai_timeout_ms = '60000', openai_concurrency_limit = '4') { this.debug = debug; this.max_files = parseInt(max_files); this.review_comment_lgtm = review_comment_lgtm; @@ -29347,47 +29394,8 @@ const handleReviewComment = async (bot, prompts) => { } } const [reply] = await bot.chat(prompts.render_comment(inputs), next_comment_ids); - const message = `${_commenter_js__WEBPACK_IMPORTED_MODULE_2__/* .COMMENT_GREETING */ .pK} - -${reply} - -${_commenter_js__WEBPACK_IMPORTED_MODULE_2__/* .COMMENT_REPLY_TAG */ .aD} -`; if (topLevelComment) { - const topLevelCommentId = topLevelComment.id; - try { - // Post the reply to the user comment - await octokit.pulls.createReplyForReviewComment({ - owner: repo.owner, - repo: repo.repo, - pull_number, - body: message, - comment_id: topLevelCommentId - }); - // replace COMMENT_TAG with COMMENT_REPLY_TAG in topLevelComment - const newBody = topLevelComment.body.replace(_commenter_js__WEBPACK_IMPORTED_MODULE_2__/* .COMMENT_TAG */ .Rs, _commenter_js__WEBPACK_IMPORTED_MODULE_2__/* .COMMENT_REPLY_TAG */ .aD); - await octokit.pulls.updateReviewComment({ - owner: repo.owner, - repo: repo.repo, - comment_id: topLevelCommentId, - body: newBody - }); - } - catch (error) { - _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to reply to the top-level comment`); - try { - await octokit.pulls.createReplyForReviewComment({ - owner: repo.owner, - repo: repo.repo, - pull_number, - body: `Could not post the reply to the top-level comment due to the following error: ${error}`, - comment_id: topLevelCommentId - }); - } - catch (error) { - _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to reply to the top-level comment`); - } - } + await commenter.review_comment_reply(pull_number, topLevelComment, reply); } else { _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Failed to find the top-level comment to reply to`); diff --git a/src/commenter.ts b/src/commenter.ts index 8c4a8051..dba0a630 100644 --- a/src/commenter.ts +++ b/src/commenter.ts @@ -140,6 +140,59 @@ ${COMMENT_TAG}` } } + async review_comment_reply( + pull_number: number, + top_level_comment: any, + message: string + ) { + const reply = `${COMMENT_GREETING} + +${message} + +${COMMENT_REPLY_TAG} +` + try { + // Post the reply to the user comment + await octokit.pulls.createReplyForReviewComment({ + owner: repo.owner, + repo: repo.repo, + pull_number, + body: reply, + comment_id: top_level_comment.id + }) + } catch (error) { + core.warning(`Failed to reply to the top-level comment ${error}`) + try { + await octokit.pulls.createReplyForReviewComment({ + owner: repo.owner, + repo: repo.repo, + pull_number, + body: `Could not post the reply to the top-level comment due to the following error: ${error}`, + comment_id: top_level_comment.id + }) + } catch (e) { + core.warning(`Failed to reply to the top-level comment ${e}`) + } + } + try { + if (top_level_comment.body.includes(COMMENT_TAG)) { + // replace COMMENT_TAG with COMMENT_REPLY_TAG in topLevelComment + const newBody = top_level_comment.body.replace( + COMMENT_TAG, + COMMENT_REPLY_TAG + ) + await octokit.pulls.updateReviewComment({ + owner: repo.owner, + repo: repo.repo, + comment_id: top_level_comment.id, + body: newBody + }) + } + } catch (error) { + core.warning(`Failed to update the top-level comment ${error}`) + } + } + async get_comments_at_line(pull_number: number, path: string, line: number) { const comments = await this.list_review_comments(pull_number) return comments.filter( diff --git a/src/review-comment.ts b/src/review-comment.ts index 86753227..a815c987 100644 --- a/src/review-comment.ts +++ b/src/review-comment.ts @@ -4,7 +4,6 @@ import {Octokit} from '@octokit/action' import {Bot} from './bot.js' import { Commenter, - COMMENT_GREETING, COMMENT_REPLY_TAG, COMMENT_TAG, SUMMARIZE_TAG @@ -175,48 +174,12 @@ export const handleReviewComment = async (bot: Bot, prompts: Prompts) => { next_comment_ids ) - const message = `${COMMENT_GREETING} - -${reply} - -${COMMENT_REPLY_TAG} -` if (topLevelComment) { - const topLevelCommentId = topLevelComment.id - try { - // Post the reply to the user comment - await octokit.pulls.createReplyForReviewComment({ - owner: repo.owner, - repo: repo.repo, - pull_number, - body: message, - comment_id: topLevelCommentId - }) - // replace COMMENT_TAG with COMMENT_REPLY_TAG in topLevelComment - const newBody = topLevelComment.body.replace( - COMMENT_TAG, - COMMENT_REPLY_TAG - ) - await octokit.pulls.updateReviewComment({ - owner: repo.owner, - repo: repo.repo, - comment_id: topLevelCommentId, - body: newBody - }) - } catch (error) { - core.warning(`Failed to reply to the top-level comment`) - try { - await octokit.pulls.createReplyForReviewComment({ - owner: repo.owner, - repo: repo.repo, - pull_number, - body: `Could not post the reply to the top-level comment due to the following error: ${error}`, - comment_id: topLevelCommentId - }) - } catch (error) { - core.warning(`Failed to reply to the top-level comment`) - } - } + await commenter.review_comment_reply( + pull_number, + topLevelComment, + reply + ) } else { core.warning(`Failed to find the top-level comment to reply to`) } From 2b957fa104499c2edb1ab06cb6bd5d17fa625559 Mon Sep 17 00:00:00 2001 From: Harjot Gill Date: Mon, 20 Mar 2023 14:27:33 -0700 Subject: [PATCH 2/2] refactor code --- dist/index.js | 28 +++++++++++++++++----------- src/main.ts | 4 +--- src/options.ts | 9 +++++++++ src/review-comment.ts | 13 ++++++++----- src/review.ts | 7 +++---- 5 files changed, 38 insertions(+), 23 deletions(-) diff --git a/dist/index.js b/dist/index.js index 1a93674d..e3aaa68d 100644 --- a/dist/index.js +++ b/dist/index.js @@ -27511,7 +27511,7 @@ async function run() { await (0,_review_js__WEBPACK_IMPORTED_MODULE_4__/* .codeReview */ .z)(bot, options, prompts); } else if (process.env.GITHUB_EVENT_NAME === 'pull_request_review_comment') { - await (0,_review_comment_js__WEBPACK_IMPORTED_MODULE_3__/* .handleReviewComment */ .V)(bot, prompts); + await (0,_review_comment_js__WEBPACK_IMPORTED_MODULE_3__/* .handleReviewComment */ .V)(bot, options, prompts); } else { _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning('Skipped: this action only works on push event'); @@ -27528,11 +27528,9 @@ async function run() { } process .on('unhandledRejection', (reason, p) => { - console.error(reason, 'Unhandled Rejection at Promise', p); _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Unhandled Rejection at Promise: ${reason}, promise is ${p}`); }) .on('uncaughtException', (e) => { - console.error(e, 'Uncaught Exception thrown'); _actions_core__WEBPACK_IMPORTED_MODULE_0__.warning(`Uncaught Exception thrown: ${e}, backtrace: ${e.stack}`); }); await run(); @@ -29189,6 +29187,7 @@ class Options { openai_retries; openai_timeout_ms; openai_concurrency_limit; + max_tokens_for_extra_content; constructor(debug, max_files = '60', review_comment_lgtm = false, path_filters = null, system_message = '', openai_model = 'gpt-3.5-turbo', openai_model_temperature = '0.0', openai_retries = '3', openai_timeout_ms = '60000', openai_concurrency_limit = '4') { this.debug = debug; this.max_files = parseInt(max_files); @@ -29200,6 +29199,15 @@ class Options { this.openai_retries = parseInt(openai_retries); this.openai_timeout_ms = parseInt(openai_timeout_ms); this.openai_concurrency_limit = parseInt(openai_concurrency_limit); + if (this.openai_model === 'gpt-4') { + this.max_tokens_for_extra_content = 5000; + } + else if (this.openai_model === 'gpt-3.5-turbo') { + this.max_tokens_for_extra_content = 2500; + } + else { + this.max_tokens_for_extra_content = 1000; + } } check_path(path) { const ok = this.path_filters.check(path); @@ -29277,8 +29285,7 @@ const octokit = new _octokit_action__WEBPACK_IMPORTED_MODULE_5__/* .Octokit */ . const context = _actions_github__WEBPACK_IMPORTED_MODULE_1__.context; const repo = context.repo; const ASK_BOT = '@openai'; -const MAX_TOKENS_FOR_EXTRA_CONTENT = 2500; -const handleReviewComment = async (bot, prompts) => { +const handleReviewComment = async (bot, options, prompts) => { const commenter = new _commenter_js__WEBPACK_IMPORTED_MODULE_2__/* .Commenter */ .Es(); const inputs = new _options_js__WEBPACK_IMPORTED_MODULE_3__/* .Inputs */ .kq(); if (context.eventName !== 'pull_request_review_comment') { @@ -29372,7 +29379,7 @@ const handleReviewComment = async (bot, prompts) => { if (file_content.length > 0) { inputs.file_content = file_content; const file_content_tokens = _tokenizer_js__WEBPACK_IMPORTED_MODULE_4__/* .get_token_count */ .u(file_content); - if (file_content_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_content_tokens < options.max_tokens_for_extra_content) { const [file_content_resp, file_content_ids] = await bot.chat(prompts.render_comment_file(inputs), next_comment_ids); if (file_content_resp) { next_comment_ids = file_content_ids; @@ -29386,7 +29393,7 @@ const handleReviewComment = async (bot, prompts) => { inputs.diff = file_diff; } const file_diff_tokens = _tokenizer_js__WEBPACK_IMPORTED_MODULE_4__/* .get_token_count */ .u(file_diff); - if (file_diff_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_diff_tokens < options.max_tokens_for_extra_content) { const [file_diff_resp, file_diff_ids] = await bot.chat(prompts.render_comment_file_diff(inputs), next_comment_ids); if (file_diff_resp) { next_comment_ids = file_diff_ids; @@ -29584,7 +29591,6 @@ const token = core.getInput('token') const octokit = new dist_node/* Octokit */.v({ auth: `token ${token}` }); const context = github.context; const repo = context.repo; -const MAX_TOKENS_FOR_EXTRA_CONTENT = 2500; const codeReview = async (bot, options, prompts) => { const commenter = new lib_commenter/* Commenter */.Es(); const openai_concurrency_limit = pLimit(options.openai_concurrency_limit); @@ -29691,7 +29697,7 @@ const codeReview = async (bot, options, prompts) => { if (file_diff.length > 0) { ins.file_diff = file_diff; const file_diff_tokens = tokenizer/* get_token_count */.u(file_diff); - if (file_diff_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_diff_tokens < options.max_tokens_for_extra_content) { // summarize diff try { const [summarize_resp] = await bot.chat(prompts.render_summarize_file_diff(ins), summarize_begin_ids); @@ -29764,7 +29770,7 @@ Tips: if (file_content.length > 0) { ins.file_content = file_content; const file_content_tokens = tokenizer/* get_token_count */.u(file_content); - if (file_content_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_content_tokens < options.max_tokens_for_extra_content) { try { // review file const [resp, review_file_ids] = await bot.chat(prompts.render_review_file(ins), next_review_ids); @@ -29786,7 +29792,7 @@ Tips: if (file_diff.length > 0) { ins.file_diff = file_diff; const file_diff_tokens = tokenizer/* get_token_count */.u(file_diff); - if (file_diff_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_diff_tokens < options.max_tokens_for_extra_content) { try { // review diff const [resp, review_diff_ids] = await bot.chat(prompts.render_review_file_diff(ins), next_review_ids); diff --git a/src/main.ts b/src/main.ts index e50fa962..765c74ab 100644 --- a/src/main.ts +++ b/src/main.ts @@ -54,7 +54,7 @@ async function run(): Promise { } else if ( process.env.GITHUB_EVENT_NAME === 'pull_request_review_comment' ) { - await handleReviewComment(bot, prompts) + await handleReviewComment(bot, options, prompts) } else { core.warning('Skipped: this action only works on push event') } @@ -69,11 +69,9 @@ async function run(): Promise { process .on('unhandledRejection', (reason, p) => { - console.error(reason, 'Unhandled Rejection at Promise', p) core.warning(`Unhandled Rejection at Promise: ${reason}, promise is ${p}`) }) .on('uncaughtException', (e: any) => { - console.error(e, 'Uncaught Exception thrown') core.warning(`Uncaught Exception thrown: ${e}, backtrace: ${e.stack}`) }) diff --git a/src/options.ts b/src/options.ts index fa8d4c61..111c0d2e 100644 --- a/src/options.ts +++ b/src/options.ts @@ -202,6 +202,7 @@ export class Options { openai_retries: number openai_timeout_ms: number openai_concurrency_limit: number + max_tokens_for_extra_content: number constructor( debug: boolean, @@ -225,6 +226,14 @@ export class Options { this.openai_retries = parseInt(openai_retries) this.openai_timeout_ms = parseInt(openai_timeout_ms) this.openai_concurrency_limit = parseInt(openai_concurrency_limit) + + if (this.openai_model === 'gpt-4') { + this.max_tokens_for_extra_content = 5000 + } else if (this.openai_model === 'gpt-3.5-turbo') { + this.max_tokens_for_extra_content = 2500 + } else { + this.max_tokens_for_extra_content = 1000 + } } check_path(path: string): boolean { diff --git a/src/review-comment.ts b/src/review-comment.ts index a815c987..a911b862 100644 --- a/src/review-comment.ts +++ b/src/review-comment.ts @@ -8,7 +8,7 @@ import { COMMENT_TAG, SUMMARIZE_TAG } from './commenter.js' -import {Inputs, Prompts} from './options.js' +import {Inputs, Options, Prompts} from './options.js' import * as tokenizer from './tokenizer.js' const token = core.getInput('token') @@ -19,9 +19,12 @@ const octokit = new Octokit({auth: `token ${token}`}) const context = github.context const repo = context.repo const ASK_BOT = '@openai' -const MAX_TOKENS_FOR_EXTRA_CONTENT = 2500 -export const handleReviewComment = async (bot: Bot, prompts: Prompts) => { +export const handleReviewComment = async ( + bot: Bot, + options: Options, + prompts: Prompts +) => { const commenter: Commenter = new Commenter() const inputs: Inputs = new Inputs() @@ -140,7 +143,7 @@ export const handleReviewComment = async (bot: Bot, prompts: Prompts) => { if (file_content.length > 0) { inputs.file_content = file_content const file_content_tokens = tokenizer.get_token_count(file_content) - if (file_content_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_content_tokens < options.max_tokens_for_extra_content) { const [file_content_resp, file_content_ids] = await bot.chat( prompts.render_comment_file(inputs), next_comment_ids @@ -158,7 +161,7 @@ export const handleReviewComment = async (bot: Bot, prompts: Prompts) => { inputs.diff = file_diff } const file_diff_tokens = tokenizer.get_token_count(file_diff) - if (file_diff_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_diff_tokens < options.max_tokens_for_extra_content) { const [file_diff_resp, file_diff_ids] = await bot.chat( prompts.render_comment_file_diff(inputs), next_comment_ids diff --git a/src/review.ts b/src/review.ts index 50396c7c..cc52f241 100644 --- a/src/review.ts +++ b/src/review.ts @@ -13,7 +13,6 @@ const token = core.getInput('token') const octokit = new Octokit({auth: `token ${token}`}) const context = github.context const repo = context.repo -const MAX_TOKENS_FOR_EXTRA_CONTENT = 2500 export const codeReview = async ( bot: Bot, @@ -171,7 +170,7 @@ export const codeReview = async ( if (file_diff.length > 0) { ins.file_diff = file_diff const file_diff_tokens = tokenizer.get_token_count(file_diff) - if (file_diff_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_diff_tokens < options.max_tokens_for_extra_content) { // summarize diff try { const [summarize_resp] = await bot.chat( @@ -270,7 +269,7 @@ Tips: if (file_content.length > 0) { ins.file_content = file_content const file_content_tokens = tokenizer.get_token_count(file_content) - if (file_content_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_content_tokens < options.max_tokens_for_extra_content) { try { // review file const [resp, review_file_ids] = await bot.chat( @@ -295,7 +294,7 @@ Tips: if (file_diff.length > 0) { ins.file_diff = file_diff const file_diff_tokens = tokenizer.get_token_count(file_diff) - if (file_diff_tokens < MAX_TOKENS_FOR_EXTRA_CONTENT) { + if (file_diff_tokens < options.max_tokens_for_extra_content) { try { // review diff const [resp, review_diff_ids] = await bot.chat(