Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

skip review of lower complexity changes #212

Merged
merged 5 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 47 additions & 3 deletions dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 15 additions & 1 deletion src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,21 @@ export class Prompts {
}

render_summarize_file_diff(inputs: Inputs): string {
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
return inputs.render(this.summarize_file_diff)
const prompt = `${this.summarize_file_diff}
harjotgill marked this conversation as resolved.
Show resolved Hide resolved

Below the summary, I would also like you to classify the
complexity of the diff as \`COMPLEX\` or \`SIMPLE\` based
on whether the change is a simple chore such are renaming
a variable or a complex change such as adding a new feature.
Any change that does not change the logic of the code is
considered a simple change.
harjotgill marked this conversation as resolved.
Show resolved Hide resolved

Use the following format to classify the complexity of the
diff and add no additional text:
[COMPLEXITY]: <COMPLEX or SIMPLE>
`
harjotgill marked this conversation as resolved.
Show resolved Hide resolved

return inputs.render(prompt)
}

render_summarize(inputs: Inputs): string {
Expand Down
52 changes: 48 additions & 4 deletions src/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ ${hunks.old_hunk}
filename: string,
file_content: string,
file_diff: string
): Promise<[string, string] | null> => {
): Promise<[string, string, boolean] | null> => {
const ins = inputs.clone()
if (file_diff.length === 0) {
core.warning(`summarize: file_diff is empty, skip ${filename}`)
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -226,6 +226,7 @@ ${hunks.old_hunk}
}

ins.filename = filename

// render prompt based on inputs so far
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
let tokens = tokenizer.get_token_count(
prompts.render_summarize_file_diff(ins)
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -268,7 +269,15 @@ ${hunks.old_hunk}
summaries_failed.push(`${filename} (nothing obtained from openai)`)
return null
} else {
return [filename, summarize_resp]
// parse the comment to look for complexity classification
// Format is : [COMPLEXITY]: <COMPLEX or SIMPLE>
// if the change is complex return true, else false
const complexity = summarize_resp
.split('[COMPLEXITY]: ')[1]
.split('\n')[0]
const is_complex = complexity === 'COMPLEX' ? true : false
core.info(`filename: ${filename}, complexity: ${complexity}`)
return [filename, summarize_resp, is_complex]
}
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
} catch (error) {
core.warning(`summarize: error from openai: ${error}`)
Expand All @@ -293,7 +302,7 @@ ${hunks.old_hunk}

const summaries = (await Promise.all(summaryPromises)).filter(
summary => summary !== null
) as [string, string][]
) as [string, string, boolean][]

if (summaries.length > 0) {
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
// join summaries into one in the batches of 20
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -420,6 +429,20 @@ ${
`

if (options.summary_only !== true) {
let files_and_changes_review = files_and_changes
const reviews_skipped: string[] = []
// filter out files that are less complex and remove them from the review
// loop through summaries and check the complexity flag
for (const [filename, , is_complex] of summaries) {
if (!is_complex) {
// remove the file from the review
files_and_changes_review = files_and_changes_review.filter(
([file]) => file !== filename
)
reviews_skipped.push(filename)
}
}

harjotgill marked this conversation as resolved.
Show resolved Hide resolved
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
// failed reviews array
const reviews_failed: string[] = []
const do_review = async (
Expand Down Expand Up @@ -684,7 +707,12 @@ ${comment_chain}
}

const reviewPromises = []
for (const [filename, file_content, , patches] of files_and_changes) {
for (const [
filename,
file_content,
,
patches
] of files_and_changes_review) {
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
if (options.max_files <= 0 || reviewPromises.length < options.max_files) {
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
reviewPromises.push(
openai_concurrency_limit(async () =>
Expand All @@ -710,6 +738,22 @@ ${

* ${reviews_failed.join('\n* ')}

</details>
`
: ''
}

${
reviews_skipped.length > 0
? `<details>
<summary>Files not reviewed due to simple changes (${
reviews_skipped.length
})</summary>

### Skipped review

* ${reviews_skipped.join('\n* ')}

</details>
`
: ''
harjotgill marked this conversation as resolved.
Show resolved Hide resolved
Expand Down