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 all commits
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
52 changes: 49 additions & 3 deletions dist/index.js

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

14 changes: 13 additions & 1 deletion src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,19 @@ 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 diff is a simple change that looks good as it
is or a complex change that needs thorough review.
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)
}

harjotgill marked this conversation as resolved.
Show resolved Hide resolved
render_summarize(inputs: Inputs): string {
Expand Down
64 changes: 60 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,24 @@ ${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 complexityRegex = /\[COMPLEXITY\]:\s*(COMPLEX|SIMPLE)/
const complexityMatch = summarize_resp.match(complexityRegex)

if (complexityMatch) {
const complexity = complexityMatch[1]
const is_complex = complexity === 'COMPLEX' ? true : false

// remove this line from the comment
const summary = summarize_resp.replace(complexityRegex, '').trim()
core.info(`filename: ${filename}, complexity: ${complexity}`)
return [filename, summary, is_complex]
} else {
// Handle the case when the [COMPLEXITY] tag is not found
return [filename, summarize_resp, true]
}
}
} catch (error) {
core.warning(`summarize: error from openai: ${error}`)
Expand All @@ -293,7 +311,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 +438,23 @@ ${
`

if (options.summary_only !== true) {
const files_and_changes_review = files_and_changes.filter(([filename]) => {
const is_complex =
summaries.find(
([summaryFilename]) => summaryFilename === filename
)?.[2] ?? true
return is_complex
})

const reviews_skipped = files_and_changes
.filter(
([filename]) =>
!files_and_changes_review.some(
([reviewFilename]) => reviewFilename === filename
)
)
.map(([filename]) => filename)

// failed reviews array
const reviews_failed: string[] = []
const do_review = async (
Expand Down Expand Up @@ -684,7 +719,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 +750,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