From e7ebd7ffb5d439bcd16af69550b0a818bc44d6cb Mon Sep 17 00:00:00 2001 From: Brianleach11 <81440466+Brianleach11@users.noreply.github.com> Date: Wed, 20 Nov 2024 22:29:21 -0500 Subject: [PATCH 1/7] setup and python parser potential solution --- package-lock.json | 71 +++++++++++++++ package.json | 2 + src/app.ts | 3 +- src/constants.ts | 7 +- src/context/language/javascript-parser.ts | 6 +- src/context/language/python-parser.ts | 103 ++++++++++++++++++++-- src/context/review.ts | 7 +- src/env.ts | 5 +- src/logger.ts | 6 ++ 9 files changed, 192 insertions(+), 18 deletions(-) create mode 100644 src/logger.ts diff --git a/package-lock.json b/package-lock.json index f60579a..8c0691c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,6 +20,8 @@ "gpt-tokenizer": "^2.1.2", "groq-sdk": "^0.8.0", "octokit": "^3.1.1", + "tree-sitter-python": "^0.23.4", + "web-tree-sitter": "^0.24.4", "xml2js": "^0.6.2" }, "devDependencies": { @@ -1816,6 +1818,15 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==" }, + "node_modules/node-addon-api": { + "version": "8.2.2", + "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-8.2.2.tgz", + "integrity": "sha512-9emqXAKhVoNrQ792nLI/wpzPpJ/bj/YXxW0CvAau1+RdGBcCRF1Dmz7719zgVsQNrzHl9Tzn3ImZ4qWFarWL0A==", + "license": "MIT", + "engines": { + "node": "^18 || ^20 || >= 21" + } + }, "node_modules/node-domexception": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/node-domexception/-/node-domexception-1.0.0.tgz", @@ -1853,6 +1864,17 @@ } } }, + "node_modules/node-gyp-build": { + "version": "4.8.4", + "resolved": "https://registry.npmjs.org/node-gyp-build/-/node-gyp-build-4.8.4.tgz", + "integrity": "sha512-LA4ZjwlnUblHVgq0oBF3Jl/6h/Nvs5fzBLwdEF4nuxnFdsfajde4WfxtJr3CaiH+F6ewcIB/q4jQ4UzPyid+CQ==", + "license": "MIT", + "bin": { + "node-gyp-build": "bin.js", + "node-gyp-build-optional": "optional.js", + "node-gyp-build-test": "build-test.js" + } + }, "node_modules/object-inspect": { "version": "1.12.3", "resolved": "https://registry.npmjs.org/object-inspect/-/object-inspect-1.12.3.tgz", @@ -2113,6 +2135,25 @@ "resolved": "https://registry.npmjs.org/tr46/-/tr46-0.0.3.tgz", "integrity": "sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw==" }, + "node_modules/tree-sitter-python": { + "version": "0.23.4", + "resolved": "https://registry.npmjs.org/tree-sitter-python/-/tree-sitter-python-0.23.4.tgz", + "integrity": "sha512-MbmUAl7y5UCUWqHscHke7DdRDwQnVNMNKQYQc4Gq2p09j+fgPxaU8JVsuOI/0HD3BSEEe5k9j3xmdtIWbDtDgw==", + "hasInstallScript": true, + "license": "MIT", + "dependencies": { + "node-addon-api": "^8.2.1", + "node-gyp-build": "^4.8.2" + }, + "peerDependencies": { + "tree-sitter": "^0.21.1" + }, + "peerDependenciesMeta": { + "tree-sitter": { + "optional": true + } + } + }, "node_modules/tsx": { "version": "4.19.2", "resolved": "https://registry.npmjs.org/tsx/-/tsx-4.19.2.tgz", @@ -2189,6 +2230,12 @@ "node": ">= 14" } }, + "node_modules/web-tree-sitter": { + "version": "0.24.4", + "resolved": "https://registry.npmjs.org/web-tree-sitter/-/web-tree-sitter-0.24.4.tgz", + "integrity": "sha512-sETP1Sf9OTd4LusrKBNznNgTt3fWoWhJnAFaKPiGSeVKXJbZ72qoMpxddKMdVI5BgXv32OI7tkKQre5PmF9reA==", + "license": "MIT" + }, "node_modules/webidl-conversions": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-3.0.1.tgz", @@ -3482,6 +3529,11 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==" }, + "node-addon-api": { + "version": "8.2.2", + "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-8.2.2.tgz", + "integrity": "sha512-9emqXAKhVoNrQ792nLI/wpzPpJ/bj/YXxW0CvAau1+RdGBcCRF1Dmz7719zgVsQNrzHl9Tzn3ImZ4qWFarWL0A==" + }, "node-domexception": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/node-domexception/-/node-domexception-1.0.0.tgz", @@ -3495,6 +3547,11 @@ "whatwg-url": "^5.0.0" } }, + "node-gyp-build": { + "version": "4.8.4", + "resolved": "https://registry.npmjs.org/node-gyp-build/-/node-gyp-build-4.8.4.tgz", + "integrity": "sha512-LA4ZjwlnUblHVgq0oBF3Jl/6h/Nvs5fzBLwdEF4nuxnFdsfajde4WfxtJr3CaiH+F6ewcIB/q4jQ4UzPyid+CQ==" + }, "object-inspect": { "version": "1.12.3", "resolved": "https://registry.npmjs.org/object-inspect/-/object-inspect-1.12.3.tgz", @@ -3687,6 +3744,15 @@ "resolved": "https://registry.npmjs.org/tr46/-/tr46-0.0.3.tgz", "integrity": "sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw==" }, + "tree-sitter-python": { + "version": "0.23.4", + "resolved": "https://registry.npmjs.org/tree-sitter-python/-/tree-sitter-python-0.23.4.tgz", + "integrity": "sha512-MbmUAl7y5UCUWqHscHke7DdRDwQnVNMNKQYQc4Gq2p09j+fgPxaU8JVsuOI/0HD3BSEEe5k9j3xmdtIWbDtDgw==", + "requires": { + "node-addon-api": "^8.2.1", + "node-gyp-build": "^4.8.2" + } + }, "tsx": { "version": "4.19.2", "resolved": "https://registry.npmjs.org/tsx/-/tsx-4.19.2.tgz", @@ -3740,6 +3806,11 @@ "resolved": "https://registry.npmjs.org/web-streams-polyfill/-/web-streams-polyfill-4.0.0-beta.3.tgz", "integrity": "sha512-QW95TCTaHmsYfHDybGMwO5IJIM93I/6vTRk+daHTWFPhwh+C8Cg7j7XyKrwrj8Ib6vYXe0ocYNrmzY4xAAN6ug==" }, + "web-tree-sitter": { + "version": "0.24.4", + "resolved": "https://registry.npmjs.org/web-tree-sitter/-/web-tree-sitter-0.24.4.tgz", + "integrity": "sha512-sETP1Sf9OTd4LusrKBNznNgTt3fWoWhJnAFaKPiGSeVKXJbZ72qoMpxddKMdVI5BgXv32OI7tkKQre5PmF9reA==" + }, "webidl-conversions": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-3.0.1.tgz", diff --git a/package.json b/package.json index fb08515..d7c2970 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,8 @@ "gpt-tokenizer": "^2.1.2", "groq-sdk": "^0.8.0", "octokit": "^3.1.1", + "tree-sitter-python": "^0.23.4", + "web-tree-sitter": "^0.24.4", "xml2js": "^0.6.2" }, "devDependencies": { diff --git a/src/app.ts b/src/app.ts index 450ae71..a32a953 100644 --- a/src/app.ts +++ b/src/app.ts @@ -7,11 +7,12 @@ import { Review } from "./constants"; import { env } from "./env"; import { processPullRequest } from "./review-agent"; import { applyReview } from "./reviews"; +import * as fs from 'fs'; // This creates a new instance of the Octokit App class. const reviewApp = new App({ appId: env.GITHUB_APP_ID, - privateKey: env.GITHUB_PRIVATE_KEY, + privateKey: fs.readFileSync(env.GITHUB_PRIVATE_KEY, 'utf-8'), webhooks: { secret: env.GITHUB_WEBHOOK_SECRET, }, diff --git a/src/constants.ts b/src/constants.ts index 14c7de1..0c25581 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -98,8 +98,11 @@ export interface AbstractParser { file: string, lineStart: number, lineEnd: number - ): EnclosingContext; - dryRun(file: string): { valid: boolean; error: string }; + ): Promise; + + dryRun( + file: string + ): Promise<{ valid: boolean; error: string }>; } const EXTENSIONS_TO_PARSERS: Map = new Map([ diff --git a/src/context/language/javascript-parser.ts b/src/context/language/javascript-parser.ts index 41cd52a..8dc67fd 100644 --- a/src/context/language/javascript-parser.ts +++ b/src/context/language/javascript-parser.ts @@ -21,11 +21,11 @@ const processNode = ( }; export class JavascriptParser implements AbstractParser { - findEnclosingContext( + async findEnclosingContext( file: string, lineStart: number, lineEnd: number - ): EnclosingContext { + ): Promise { const ast = parser.parse(file, { sourceType: "module", plugins: ["jsx", "typescript"], // To allow JSX and TypeScript @@ -57,7 +57,7 @@ export class JavascriptParser implements AbstractParser { } as EnclosingContext; } - dryRun(file: string): { valid: boolean; error: string } { + async dryRun(file: string): Promise<{ valid: boolean; error: string }> { try { const ast = parser.parse(file, { sourceType: "module", diff --git a/src/context/language/python-parser.ts b/src/context/language/python-parser.ts index 845e90b..fd660b8 100644 --- a/src/context/language/python-parser.ts +++ b/src/context/language/python-parser.ts @@ -1,15 +1,104 @@ import { AbstractParser, EnclosingContext } from "../../constants"; +import Parser = require("web-tree-sitter"); +let parser: Parser; + +// Initialize Tree-sitter parser +async function initializeParser() { + if (!parser) { + await Parser.init(); + parser = new Parser(); + // You'll need to load the WASM file from your node_modules + const Lang = await Parser.Language.load('/Users/bleach/Desktop/School/Headstarter/PR Agent/SecureAgent/node_modules/tree-sitter-python/tree-sitter-python.wasm'); + parser.setLanguage(Lang); + } + return parser; +} + +const processNode = ( + node: Parser.SyntaxNode, + lineStart: number, + lineEnd: number, + largestSize: number, + largestEnclosingContext: Parser.SyntaxNode | null +) => { + const startPosition = node.startPosition; + const endPosition = node.endPosition; + + if (startPosition.row <= lineStart && lineEnd <= endPosition.row) { + const size = endPosition.row - startPosition.row; + if (size > largestSize) { + largestSize = size; + largestEnclosingContext = node; + } + } + return { largestSize, largestEnclosingContext }; +}; + export class PythonParser implements AbstractParser { - findEnclosingContext( + private async getParser(): Promise { + return await initializeParser(); + } + + async findEnclosingContext( file: string, lineStart: number, lineEnd: number - ): EnclosingContext { - // TODO: Implement this method for Python - return null; + ): Promise { + const parser = await this.getParser(); + const tree = parser.parse(file); + let largestEnclosingContext: Parser.SyntaxNode = null; + let largestSize = 0; + + // Traverse the syntax tree to find function and class definitions + const cursor = tree.walk(); + do { + const node = cursor.currentNode; + + // Check for function definitions and class definitions + if ( + node.type === 'function_definition' || + node.type === 'class_definition' + ) { + ({ largestSize, largestEnclosingContext } = processNode( + node, + lineStart, + lineEnd, + largestSize, + largestEnclosingContext + )); + } + } while (cursor.gotoNextSibling() || cursor.gotoParent()); + + return { + enclosingContext: largestEnclosingContext, + } as EnclosingContext; } - dryRun(file: string): { valid: boolean; error: string } { - // TODO: Implement this method for Python - return { valid: false, error: "Not implemented yet" }; + + async dryRun(file: string): Promise<{ valid: boolean; error: string }> { + try { + const parser = await this.getParser(); + const tree = parser.parse(file); + + // Check if there are any ERROR nodes in the syntax tree + let hasError = false; + const cursor = tree.walk(); + + do { + if (cursor.currentNode.type === 'ERROR') { + hasError = true; + break; + } + } while (cursor.gotoNextSibling() || cursor.gotoParent()); + + return { + valid: !hasError, + error: hasError ? "Syntax error in Python code" : "", + }; + } catch (exc) { + return { + valid: false, + error: exc.toString(), + }; + } } } diff --git a/src/context/review.ts b/src/context/review.ts index 7d44aa1..ed1c67a 100644 --- a/src/context/review.ts +++ b/src/context/review.ts @@ -205,7 +205,7 @@ const diffContextPerHunk = (file: PRFile, parser: AbstractParser) => { }); }); - hunks.forEach((hunk, idx) => { + hunks.forEach( async (hunk, idx) => { try { const trimmedHunk = trimHunk(hunk); const insertions = hunk.lines.filter((line) => @@ -213,11 +213,12 @@ const diffContextPerHunk = (file: PRFile, parser: AbstractParser) => { ).length; const lineStart = trimmedHunk.newStart; const lineEnd = lineStart + insertions; - const largestEnclosingFunction = parser.findEnclosingContext( + const largest = await parser.findEnclosingContext( updatedFile, lineStart, lineEnd - ).enclosingContext; + ); + const largestEnclosingFunction = largest.enclosingContext; if (largestEnclosingFunction) { const enclosingRangeKey = `${largestEnclosingFunction.loc.start.line} -> ${largestEnclosingFunction.loc.end.line}`; diff --git a/src/env.ts b/src/env.ts index 0162d2c..c851f78 100644 --- a/src/env.ts +++ b/src/env.ts @@ -1,7 +1,7 @@ import * as dotenv from "dotenv"; import { createPrivateKey } from "crypto"; import chalk from "chalk"; - +import * as fs from 'fs'; dotenv.config(); export const env = { @@ -25,7 +25,8 @@ for (const key in env) { } try { - createPrivateKey(env.GITHUB_PRIVATE_KEY); + const privateKey = fs.readFileSync(env.GITHUB_PRIVATE_KEY, 'utf-8'); + createPrivateKey(privateKey); } catch (error) { console.log( chalk.red( diff --git a/src/logger.ts b/src/logger.ts new file mode 100644 index 0000000..98e5cc4 --- /dev/null +++ b/src/logger.ts @@ -0,0 +1,6 @@ +import { Axiom } from '@axiomhq/js'; + +export const axiom = new Axiom({ + token: process.env.AXIOM_API_KEY, + orgId: process.env.AXIOM_ORG_ID, +}); \ No newline at end of file From a534a66f018b98597087a60c000aafbb35ede080 Mon Sep 17 00:00:00 2001 From: Brianleach11 <81440466+Brianleach11@users.noreply.github.com> Date: Thu, 21 Nov 2024 14:00:02 -0500 Subject: [PATCH 2/7] adding more context --- src/constants.ts | 15 ++++- src/context/language/python-parser.ts | 26 ++++++++ src/context/review.ts | 86 ++++++++++++++------------- src/prompts.ts | 28 +++++++-- src/review-agent.ts | 51 +++++++++------- 5 files changed, 134 insertions(+), 72 deletions(-) diff --git a/src/constants.ts b/src/constants.ts index 0c25581..03f18b6 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,6 +1,7 @@ import { Node } from "@babel/traverse"; import { JavascriptParser } from "./context/language/javascript-parser"; import { ChatCompletionMessageParam } from "groq-sdk/resources/chat/completions"; +import { PythonParser } from "./context/language/python-parser"; export interface PRFile { sha: string; @@ -110,11 +111,19 @@ const EXTENSIONS_TO_PARSERS: Map = new Map([ ["tsx", new JavascriptParser()], ["js", new JavascriptParser()], ["jsx", new JavascriptParser()], + ["py", new PythonParser()], ]); -export const getParserForExtension = (filename: string) => { - const fileExtension = filename.split(".").pop().toLowerCase(); - return EXTENSIONS_TO_PARSERS.get(fileExtension) || null; +export const getParserForExtension = (filename: string): AbstractParser => { + console.log(`๐Ÿ”ง Getting parser for file: ${filename}`); + const extension = filename.split('.').pop()?.toLowerCase(); + console.log(`Extension detected: ${extension}`); + + // Add logging for what parser is being returned + const parser = EXTENSIONS_TO_PARSERS.get(extension); + console.log(`Parser found: ${parser ? parser.constructor.name : 'None'}`); + + return parser; }; export const assignLineNumbers = (contents: string): string => { diff --git a/src/context/language/python-parser.ts b/src/context/language/python-parser.ts index fd660b8..8c70d10 100644 --- a/src/context/language/python-parser.ts +++ b/src/context/language/python-parser.ts @@ -36,6 +36,7 @@ const processNode = ( export class PythonParser implements AbstractParser { private async getParser(): Promise { + console.log("๐Ÿ” Getting Python parser..."); return await initializeParser(); } @@ -44,11 +45,17 @@ export class PythonParser implements AbstractParser { lineStart: number, lineEnd: number ): Promise { + console.log("๐Ÿš€ Starting Python parser findEnclosingContext"); + console.log(`๐Ÿ“„ File content length: ${file.length} characters`); + console.log(`๐ŸŽฏ Target lines: ${lineStart}-${lineEnd}`); + const parser = await this.getParser(); const tree = parser.parse(file); let largestEnclosingContext: Parser.SyntaxNode = null; let largestSize = 0; + console.log(`Searching for context between lines ${lineStart} and ${lineEnd}`); + // Traverse the syntax tree to find function and class definitions const cursor = tree.walk(); do { @@ -59,6 +66,9 @@ export class PythonParser implements AbstractParser { node.type === 'function_definition' || node.type === 'class_definition' ) { + console.log(`Found ${node.type} at lines ${node.startPosition.row}-${node.endPosition.row}`); + + const prevContext = largestEnclosingContext; ({ largestSize, largestEnclosingContext } = processNode( node, lineStart, @@ -66,15 +76,30 @@ export class PythonParser implements AbstractParser { largestSize, largestEnclosingContext )); + + if (largestEnclosingContext !== prevContext) { + console.log(`New largest context found: ${node.type} with size ${largestSize}`); + console.log(`Context text: ${node.text.split('\n')[0]}...`); // Print first line of context + } } } while (cursor.gotoNextSibling() || cursor.gotoParent()); + console.log('Final enclosing context:', + largestEnclosingContext ? { + type: largestEnclosingContext.type, + start: largestEnclosingContext.startPosition.row, + end: largestEnclosingContext.endPosition.row, + text: largestEnclosingContext.text.split('\n')[0] + '...' + } : 'None found' + ); + return { enclosingContext: largestEnclosingContext, } as EnclosingContext; } async dryRun(file: string): Promise<{ valid: boolean; error: string }> { + console.log("๐Ÿงช Starting Python parser dryRun"); try { const parser = await this.getParser(); const tree = parser.parse(file); @@ -95,6 +120,7 @@ export class PythonParser implements AbstractParser { error: hasError ? "Syntax error in Python code" : "", }; } catch (exc) { + console.error("โŒ Python parser dryRun error:", exc); return { valid: false, error: exc.toString(), diff --git a/src/context/review.ts b/src/context/review.ts index ed1c67a..b3bce8f 100644 --- a/src/context/review.ts +++ b/src/context/review.ts @@ -185,59 +185,60 @@ const combineHunks = ( return combinedHunk; }; -const diffContextPerHunk = (file: PRFile, parser: AbstractParser) => { - const updatedFile = diff.applyPatch(file.old_contents, file.patch); - const patches = diff.parsePatch(file.patch); - if (!updatedFile || typeof updatedFile !== "string") { - console.log("APPLYING PATCH ERROR - FALLINGBACK"); - throw "THIS SHOULD NOT HAPPEN!"; - } - - const hunks: diff.Hunk[] = []; - const order: number[] = []; +const diffContextPerHunk = async ( + file: PRFile, + parser: AbstractParser +): Promise => { + console.log(`๐Ÿ“Š Processing hunks for ${file.filename}`); + const updatedFile = file.current_contents; + const patches: PatchInfo[] = diff.parsePatch(file.patch); const scopeRangeHunkMap = new Map(); const scopeRangeNodeMap = new Map(); const expandStrategy: diff.Hunk[] = []; + const order: number[] = []; - patches.forEach((p) => { - p.hunks.forEach((hunk) => { - hunks.push(hunk); - }); - }); - - hunks.forEach( async (hunk, idx) => { + for (const [idx, patch] of patches.entries()) { try { - const trimmedHunk = trimHunk(hunk); - const insertions = hunk.lines.filter((line) => + const currentHunk: diff.Hunk = patch.hunks[0]; + const trimmedHunk = trimHunk(currentHunk); + const insertions = currentHunk.lines.filter((line) => line.startsWith("+") ).length; - const lineStart = trimmedHunk.newStart; - const lineEnd = lineStart + insertions; + + // Expand the search range significantly above and below the changed lines + const contextRange = 50; // Increase this number to look further + const lineStart = Math.max(1, trimmedHunk.newStart - contextRange); + const lineEnd = trimmedHunk.newStart + insertions + contextRange; + + console.log(`๐Ÿ” Searching for context with expanded range: ${lineStart}-${lineEnd}`); + const largest = await parser.findEnclosingContext( updatedFile, lineStart, lineEnd ); + const largestEnclosingFunction = largest.enclosingContext; if (largestEnclosingFunction) { + console.log(`โœ… Found enclosing context: ${largestEnclosingFunction.type} at lines ${largestEnclosingFunction.loc.start.line}-${largestEnclosingFunction.loc.end.line}`); const enclosingRangeKey = `${largestEnclosingFunction.loc.start.line} -> ${largestEnclosingFunction.loc.end.line}`; let existingHunks = scopeRangeHunkMap.get(enclosingRangeKey) || []; - existingHunks.push(hunk); + existingHunks.push(currentHunk); scopeRangeHunkMap.set(enclosingRangeKey, existingHunks); scopeRangeNodeMap.set(enclosingRangeKey, largestEnclosingFunction); } else { + console.log('โŒ No enclosing function found even with expanded range'); throw "No enclosing function."; } order.push(idx); } catch (exc) { - console.log(file.filename); - console.log("NORMAL STRATEGY"); + console.log(`โš ๏ธ Falling back to normal strategy for ${file.filename}`); console.log(exc); - expandStrategy.push(hunk); + expandStrategy.push(patch.hunks[0]); order.push(idx); } - }); + } const scopeStategy: [string, diff.Hunk][] = []; // holds map range key and combined hunk: [[key, hunk]] for (const [range, hunks] of scopeRangeHunkMap.entries()) { @@ -261,26 +262,31 @@ const diffContextPerHunk = (file: PRFile, parser: AbstractParser) => { return contexts; }; -const functionContextPatchStrategy = ( +export const smarterContextPatchStrategy = (file: PRFile) => { + console.log(`๐Ÿš€ smarterContextPatchStrategy for ${file.filename}`); + const parser: AbstractParser = getParserForExtension(file.filename); + console.log(`Parser for ${file.filename}: ${parser ? 'Found' : 'Not found'}`); + if (parser != null) { + console.log('Using functionContextPatchStrategy'); + return functionContextPatchStrategy(file, parser); + } else { + console.log('Falling back to expandedPatchStrategy'); + return expandedPatchStrategy(file); + } +}; + +const functionContextPatchStrategy = async ( file: PRFile, parser: AbstractParser -): string => { +): Promise => { + console.log(`๐Ÿ’ก functionContextPatchStrategy for ${file.filename}`); let res = null; try { - const contextChunks = diffContextPerHunk(file, parser); + const contextChunks = await diffContextPerHunk(file, parser); res = `## ${file.filename}\n\n${contextChunks.join("\n\n")}`; } catch (exc) { - console.log(exc); - res = expandedPatchStrategy(file); + console.log('โŒ Error in functionContextPatchStrategy:', exc); + res = await expandedPatchStrategy(file); } return res; }; - -export const smarterContextPatchStrategy = (file: PRFile) => { - const parser: AbstractParser = getParserForExtension(file.filename); - if (parser != null) { - return functionContextPatchStrategy(file, parser); - } else { - return expandedPatchStrategy(file); - } -}; diff --git a/src/prompts.ts b/src/prompts.ts index 42a907e..f6a10aa 100644 --- a/src/prompts.ts +++ b/src/prompts.ts @@ -119,12 +119,28 @@ export const buildSuggestionPrompt = (file: PRFile) => { return `## ${file.filename}\n\n${patchWithLines}`; }; -export const buildPatchPrompt = (file: PRFile) => { +const patchCache = new Map(); + +export const buildPatchPrompt = async (file: PRFile) => { + const cacheKey = `${file.filename}-${file.sha}`; + + if (patchCache.has(cacheKey)) { + console.log(`๐Ÿ“ฆ Using cached patch for ${file.filename}`); + return patchCache.get(cacheKey); + } + + console.log(`๐Ÿ” buildPatchPrompt called for file: ${file.filename}`); + let result; if (file.old_contents == null) { - return rawPatchStrategy(file); + console.log('โš ๏ธ No old contents, using rawPatchStrategy'); + result = rawPatchStrategy(file); } else { - return smarterContextPatchStrategy(file); + console.log('โœจ Using smarterContextPatchStrategy'); + result = await smarterContextPatchStrategy(file); } + + patchCache.set(cacheKey, result); + return result; }; export const getReviewPrompt = (diff: string): ChatCompletionMessageParam[] => { @@ -143,12 +159,12 @@ export const getXMLReviewPrompt = ( ]; }; -export const constructPrompt = ( +export const constructPrompt = async ( files: PRFile[], - patchBuilder: (file: PRFile) => string, + patchBuilder: (file: PRFile) => Promise, convoBuilder: (diff: string) => ChatCompletionMessageParam[] ) => { - const patches = files.map((file) => patchBuilder(file)); + const patches = await Promise.all(files.map(patchBuilder)); const diff = patches.join("\n"); const convo = convoBuilder(diff); return convo; diff --git a/src/review-agent.ts b/src/review-agent.ts index a634cb9..f90941d 100644 --- a/src/review-agent.ts +++ b/src/review-agent.ts @@ -26,6 +26,7 @@ import { getInlineFixPrompt, } from "./prompts/inline-prompt"; import { getGitFile } from "./reviews"; +import { getParserForExtension } from "./constants"; export const reviewDiff = async (messages: ChatCompletionMessageParam[]) => { const message = await generateChatCompletion({ @@ -36,10 +37,10 @@ export const reviewDiff = async (messages: ChatCompletionMessageParam[]) => { export const reviewFiles = async ( files: PRFile[], - patchBuilder: (file: PRFile) => string, + patchBuilder: (file: PRFile) => Promise, convoBuilder: (diff: string) => ChatCompletionMessageParam[] ) => { - const patches = files.map((file) => patchBuilder(file)); + const patches = await Promise.all(files.map(patchBuilder)); const messages = convoBuilder(patches.join("\n")); const feedback = await reviewDiff(messages); return feedback; @@ -101,22 +102,24 @@ const groupFilesByExtension = (files: PRFile[]): Map => { }; // all of the files here can be processed with the prompt at minimum -const processWithinLimitFiles = ( +const processWithinLimitFiles = async ( files: PRFile[], - patchBuilder: (file: PRFile) => string, + patchBuilder: (file: PRFile) => Promise, convoBuilder: (diff: string) => ChatCompletionMessageParam[] ) => { const processGroups: PRFile[][] = []; + const patches = await Promise.all(files.map(patchBuilder)); const convoWithinModelLimit = isConversationWithinLimit( - constructPrompt(files, patchBuilder, convoBuilder) + await constructPrompt(files, (f) => Promise.resolve(patches[files.indexOf(f)]), convoBuilder) ); console.log(`Within model token limits: ${convoWithinModelLimit}`); if (!convoWithinModelLimit) { const grouped = groupFilesByExtension(files); for (const [extension, filesForExt] of grouped.entries()) { + const extPatches = await Promise.all(filesForExt.map(patchBuilder)); const extGroupWithinModelLimit = isConversationWithinLimit( - constructPrompt(filesForExt, patchBuilder, convoBuilder) + await constructPrompt(filesForExt, (f) => Promise.resolve(extPatches[filesForExt.indexOf(f)]), convoBuilder) ); if (extGroupWithinModelLimit) { processGroups.push(filesForExt); @@ -127,9 +130,10 @@ const processWithinLimitFiles = ( ); let currentGroup: PRFile[] = []; filesForExt.sort((a, b) => a.patchTokenLength - b.patchTokenLength); - filesForExt.forEach((file) => { + for (const file of filesForExt) { + const patch = await patchBuilder(file); const isPotentialGroupWithinLimit = isConversationWithinLimit( - constructPrompt([...currentGroup, file], patchBuilder, convoBuilder) + await constructPrompt([...currentGroup, file], (f) => Promise.resolve(f === file ? patch : patches[files.indexOf(f)]), convoBuilder) ); if (isPotentialGroupWithinLimit) { currentGroup.push(file); @@ -137,7 +141,7 @@ const processWithinLimitFiles = ( processGroups.push(currentGroup); currentGroup = [file]; } - }); + } if (currentGroup.length > 0) { processGroups.push(currentGroup); } @@ -159,9 +163,9 @@ const stripRemovedLines = (originalFile: PRFile) => { return { ...originalFile, patch: strippedPatch }; }; -const processOutsideLimitFiles = ( +const processOutsideLimitFiles = async ( files: PRFile[], - patchBuilder: (file: PRFile) => string, + patchBuilder: (file: PRFile) => Promise, convoBuilder: (diff: string) => ChatCompletionMessageParam[] ) => { const processGroups: PRFile[][] = []; @@ -170,16 +174,16 @@ const processOutsideLimitFiles = ( } files = files.map((file) => stripRemovedLines(file)); const convoWithinModelLimit = isConversationWithinLimit( - constructPrompt(files, patchBuilder, convoBuilder) + await constructPrompt(files, patchBuilder, convoBuilder) ); if (convoWithinModelLimit) { processGroups.push(files); } else { const exceedingLimits: PRFile[] = []; const withinLimits: PRFile[] = []; - files.forEach((file) => { + files.forEach(async (file) => { const isFileConvoWithinLimits = isConversationWithinLimit( - constructPrompt([file], patchBuilder, convoBuilder) + await constructPrompt([file], patchBuilder, convoBuilder) ); if (isFileConvoWithinLimits) { withinLimits.push(file); @@ -187,7 +191,7 @@ const processOutsideLimitFiles = ( exceedingLimits.push(file); } }); - const withinLimitsGroup = processWithinLimitFiles( + const withinLimitsGroup = await processWithinLimitFiles( withinLimits, patchBuilder, convoBuilder @@ -336,19 +340,20 @@ export const reviewChanges = async ( convoBuilder: (diff: string) => ChatCompletionMessageParam[], responseBuilder: (responses: string[]) => Promise ) => { - const patchBuilder = buildPatchPrompt; + const patchBuilder = async (file: PRFile) => await buildPatchPrompt(file); const filteredFiles = files.filter((file) => filterFile(file)); - filteredFiles.map((file) => { - file.patchTokenLength = getTokenLength(patchBuilder(file)); - }); + await Promise.all(filteredFiles.map(async (file) => { + file.patchTokenLength = getTokenLength(await patchBuilder(file)); + })); // further subdivide if necessary, maybe group files by common extension? const patchesWithinModelLimit: PRFile[] = []; // these single file patches are larger than the full model context const patchesOutsideModelLimit: PRFile[] = []; - filteredFiles.forEach((file) => { + filteredFiles.forEach(async (file) => { + const patch = await buildPatchPrompt(file); const patchWithPromptWithinLimit = isConversationWithinLimit( - constructPrompt([file], patchBuilder, convoBuilder) + await constructPrompt([file], () => Promise.resolve(patch), convoBuilder) ); if (patchWithPromptWithinLimit) { patchesWithinModelLimit.push(file); @@ -358,12 +363,12 @@ export const reviewChanges = async ( }); console.log(`files within limits: ${patchesWithinModelLimit.length}`); - const withinLimitsPatchGroups = processWithinLimitFiles( + const withinLimitsPatchGroups = await processWithinLimitFiles( patchesWithinModelLimit, patchBuilder, convoBuilder ); - const exceedingLimitsPatchGroups = processOutsideLimitFiles( + const exceedingLimitsPatchGroups = await processOutsideLimitFiles( patchesOutsideModelLimit, patchBuilder, convoBuilder From c0708de3efa3a78a8afaa44a940d640f39a6e353 Mon Sep 17 00:00:00 2001 From: Brianleach11 <81440466+Brianleach11@users.noreply.github.com> Date: Thu, 21 Nov 2024 16:03:41 -0500 Subject: [PATCH 3/7] Found enclosing function, updated acceptance types from Node to Node | SyntaxNode --- src/constants.ts | 3 +- src/context/language/python-parser.ts | 138 ++++++++++++++------------ src/context/review.ts | 30 ++++-- 3 files changed, 101 insertions(+), 70 deletions(-) diff --git a/src/constants.ts b/src/constants.ts index 03f18b6..315a74e 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -2,6 +2,7 @@ import { Node } from "@babel/traverse"; import { JavascriptParser } from "./context/language/javascript-parser"; import { ChatCompletionMessageParam } from "groq-sdk/resources/chat/completions"; import { PythonParser } from "./context/language/python-parser"; +import { SyntaxNode } from "web-tree-sitter"; export interface PRFile { sha: string; @@ -91,7 +92,7 @@ export const processGitFilepath = (filepath: string) => { }; export interface EnclosingContext { - enclosingContext: Node | null; + enclosingContext: Node | SyntaxNode | null; } export interface AbstractParser { diff --git a/src/context/language/python-parser.ts b/src/context/language/python-parser.ts index 8c70d10..7ba4250 100644 --- a/src/context/language/python-parser.ts +++ b/src/context/language/python-parser.ts @@ -14,26 +14,6 @@ async function initializeParser() { return parser; } -const processNode = ( - node: Parser.SyntaxNode, - lineStart: number, - lineEnd: number, - largestSize: number, - largestEnclosingContext: Parser.SyntaxNode | null -) => { - const startPosition = node.startPosition; - const endPosition = node.endPosition; - - if (startPosition.row <= lineStart && lineEnd <= endPosition.row) { - const size = endPosition.row - startPosition.row; - if (size > largestSize) { - largestSize = size; - largestEnclosingContext = node; - } - } - return { largestSize, largestEnclosingContext }; -}; - export class PythonParser implements AbstractParser { private async getParser(): Promise { console.log("๐Ÿ” Getting Python parser..."); @@ -45,57 +25,91 @@ export class PythonParser implements AbstractParser { lineStart: number, lineEnd: number ): Promise { - console.log("๐Ÿš€ Starting Python parser findEnclosingContext"); - console.log(`๐Ÿ“„ File content length: ${file.length} characters`); - console.log(`๐ŸŽฏ Target lines: ${lineStart}-${lineEnd}`); - + console.log(`๐Ÿ” Searching for context for range: ${lineStart} - ${lineEnd}`); + const parser = await this.getParser(); const tree = parser.parse(file); - let largestEnclosingContext: Parser.SyntaxNode = null; - let largestSize = 0; - console.log(`Searching for context between lines ${lineStart} and ${lineEnd}`); + // Separate tracking for different types of contexts + const contextNodes: { definitions: Parser.SyntaxNode[]; blocks: Parser.SyntaxNode[] } = { + definitions: [], // function_definition, class_definition + blocks: [] // if_statement, with_statement, block, etc + }; - // Traverse the syntax tree to find function and class definitions - const cursor = tree.walk(); - do { - const node = cursor.currentNode; - - // Check for function definitions and class definitions - if ( - node.type === 'function_definition' || - node.type === 'class_definition' - ) { - console.log(`Found ${node.type} at lines ${node.startPosition.row}-${node.endPosition.row}`); - - const prevContext = largestEnclosingContext; - ({ largestSize, largestEnclosingContext } = processNode( - node, - lineStart, - lineEnd, - largestSize, - largestEnclosingContext - )); - - if (largestEnclosingContext !== prevContext) { - console.log(`New largest context found: ${node.type} with size ${largestSize}`); - console.log(`Context text: ${node.text.split('\n')[0]}...`); // Print first line of context + // Helper to check if a node contains our target range + const nodeContainsRange = (node: Parser.SyntaxNode) => { + if (node.endPosition.row - node.startPosition.row < 1) { + return false; + } + + return node.startPosition.row <= lineStart && + node.endPosition.row >= lineEnd && + node.text.trim().length > 0; + }; + + // Recursive function with context type separation + const traverseTree = (node: Parser.SyntaxNode) => { + if (node.endPosition.row - node.startPosition.row < 1) { + return; + } + + if (nodeContainsRange(node)) { + const nodeType = node.type; + const nameNode = node.childForFieldName('name'); + const name = nameNode ? nameNode.text : 'unnamed'; + + if (['function_definition', 'class_definition'].includes(nodeType)) { + console.log(`Found definition context ${nodeType}: ${name} at lines ${node.startPosition.row}-${node.endPosition.row}`); + contextNodes.definitions.push(node); + } else if (['if_statement', 'with_statement', 'block', 'while_statement', 'for_statement', 'try_statement'].includes(nodeType)) { + console.log(`Found block context ${nodeType} at lines ${node.startPosition.row}-${node.endPosition.row}`); + contextNodes.blocks.push(node); } } - } while (cursor.gotoNextSibling() || cursor.gotoParent()); - console.log('Final enclosing context:', - largestEnclosingContext ? { - type: largestEnclosingContext.type, - start: largestEnclosingContext.startPosition.row, - end: largestEnclosingContext.endPosition.row, - text: largestEnclosingContext.text.split('\n')[0] + '...' - } : 'None found' - ); + // Only traverse children if the node might contain our range + if (node.startPosition.row <= lineEnd && node.endPosition.row >= lineStart) { + for (let i = 0; i < node.childCount; i++) { + const child = node.child(i); + if (child) { + traverseTree(child); + } + } + } + }; + + traverseTree(tree.rootNode); + + // Find the smallest containing node, prioritizing definitions + let bestNode = null; + + // First try to find the smallest definition + if (contextNodes.definitions.length > 0) { + bestNode = contextNodes.definitions.reduce((smallest, current) => { + const smallestSize = smallest.endPosition.row - smallest.startPosition.row; + const currentSize = current.endPosition.row - current.startPosition.row; + return currentSize < smallestSize ? current : smallest; + }); + console.log(`Selected definition context: ${bestNode.type} at lines ${bestNode.startPosition.row}-${bestNode.endPosition.row}`); + } + + // If no definitions found, try blocks + if (!bestNode && contextNodes.blocks.length > 0) { + bestNode = contextNodes.blocks.reduce((smallest, current) => { + const smallestSize = smallest.endPosition.row - smallest.startPosition.row; + const currentSize = current.endPosition.row - current.startPosition.row; + return currentSize < smallestSize ? current : smallest; + }); + console.log(`Selected block context: ${bestNode.type} at lines ${bestNode.startPosition.row}-${bestNode.endPosition.row}`); + } + + if (!bestNode) { + console.log('No suitable context found'); + } return { - enclosingContext: largestEnclosingContext, - } as EnclosingContext; + enclosingContext: bestNode + }; } async dryRun(file: string): Promise<{ valid: boolean; error: string }> { diff --git a/src/context/review.ts b/src/context/review.ts index b3bce8f..da9ff86 100644 --- a/src/context/review.ts +++ b/src/context/review.ts @@ -7,6 +7,7 @@ import { import * as diff from "diff"; import { JavascriptParser } from "./language/javascript-parser"; import { Node } from "@babel/traverse"; +import { SyntaxNode } from "web-tree-sitter"; const expandHunk = ( contents: string, @@ -107,13 +108,20 @@ const trimHunk = (hunk: diff.Hunk): diff.Hunk => { const buildingScopeString = ( currentFile: string, - scope: Node, + scope: Node | SyntaxNode, hunk: diff.Hunk ) => { const res: string[] = []; const trimmedHunk = trimHunk(hunk); - const functionStartLine = scope.loc.start.line; - const functionEndLine = scope.loc.end.line; + let functionStartLine: number; + let functionEndLine: number; + if (scope && typeof scope === 'object' && 'loc' in scope) { + functionStartLine = scope.loc.start.line; + functionEndLine = scope.loc.end.line; + } else if (scope && typeof scope === 'object' && 'startPosition' in scope && 'endPosition' in scope) { + functionStartLine = scope.startPosition.row; + functionEndLine = scope.endPosition.row; + } const updatedFileLines = currentFile.split("\n"); // Extract the lines of the function const functionContext = updatedFileLines.slice( @@ -193,7 +201,7 @@ const diffContextPerHunk = async ( const updatedFile = file.current_contents; const patches: PatchInfo[] = diff.parsePatch(file.patch); const scopeRangeHunkMap = new Map(); - const scopeRangeNodeMap = new Map(); + const scopeRangeNodeMap = new Map(); const expandStrategy: diff.Hunk[] = []; const order: number[] = []; @@ -206,7 +214,7 @@ const diffContextPerHunk = async ( ).length; // Expand the search range significantly above and below the changed lines - const contextRange = 50; // Increase this number to look further + const contextRange = 0; // Increase this number to look further const lineStart = Math.max(1, trimmedHunk.newStart - contextRange); const lineEnd = trimmedHunk.newStart + insertions + contextRange; @@ -220,7 +228,15 @@ const diffContextPerHunk = async ( const largestEnclosingFunction = largest.enclosingContext; - if (largestEnclosingFunction) { + if (largestEnclosingFunction && typeof largestEnclosingFunction === 'object' && 'startPosition' in largestEnclosingFunction && 'endPosition' in largestEnclosingFunction) { + console.log(`โœ… Found enclosing context: ${largestEnclosingFunction.type} at lines ${largestEnclosingFunction.startPosition.row}-${largestEnclosingFunction.endPosition.row}`); + const enclosingRangeKey = `${largestEnclosingFunction.startPosition.row} -> ${largestEnclosingFunction.endPosition.row}`; + let existingHunks = scopeRangeHunkMap.get(enclosingRangeKey) || []; + existingHunks.push(currentHunk); + scopeRangeHunkMap.set(enclosingRangeKey, existingHunks); + scopeRangeNodeMap.set(enclosingRangeKey, largestEnclosingFunction); + } + else if (largestEnclosingFunction && typeof largestEnclosingFunction === 'object' && 'loc' in largestEnclosingFunction) { console.log(`โœ… Found enclosing context: ${largestEnclosingFunction.type} at lines ${largestEnclosingFunction.loc.start.line}-${largestEnclosingFunction.loc.end.line}`); const enclosingRangeKey = `${largestEnclosingFunction.loc.start.line} -> ${largestEnclosingFunction.loc.end.line}`; let existingHunks = scopeRangeHunkMap.get(enclosingRangeKey) || []; @@ -250,7 +266,7 @@ const diffContextPerHunk = async ( scopeStategy.forEach(([rangeKey, hunk]) => { const context = buildingScopeString( updatedFile, - scopeRangeNodeMap.get(rangeKey), + scopeRangeNodeMap.get(rangeKey) as Node | SyntaxNode, hunk ).join("\n"); contexts.push(context); From 75d8ce2c9edc11b7fedccc8f4ccab4a0d20e9cfb Mon Sep 17 00:00:00 2001 From: Brianleach11 <81440466+Brianleach11@users.noreply.github.com> Date: Fri, 22 Nov 2024 08:26:59 -0500 Subject: [PATCH 4/7] Find biggest enclosing non-function/class definition as a fallback --- src/context/language/python-parser.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/context/language/python-parser.ts b/src/context/language/python-parser.ts index 7ba4250..2a9d77b 100644 --- a/src/context/language/python-parser.ts +++ b/src/context/language/python-parser.ts @@ -95,10 +95,10 @@ export class PythonParser implements AbstractParser { // If no definitions found, try blocks if (!bestNode && contextNodes.blocks.length > 0) { - bestNode = contextNodes.blocks.reduce((smallest, current) => { - const smallestSize = smallest.endPosition.row - smallest.startPosition.row; + bestNode = contextNodes.blocks.reduce((largest, current) => { + const largestSize = largest.endPosition.row - largest.startPosition.row; const currentSize = current.endPosition.row - current.startPosition.row; - return currentSize < smallestSize ? current : smallest; + return currentSize > largestSize ? current : largest; }); console.log(`Selected block context: ${bestNode.type} at lines ${bestNode.startPosition.row}-${bestNode.endPosition.row}`); } From 4c9b83863ef4545f6e29d9e57701894d7c70bb3b Mon Sep 17 00:00:00 2001 From: Brianleach11 <81440466+Brianleach11@users.noreply.github.com> Date: Fri, 22 Nov 2024 08:50:36 -0500 Subject: [PATCH 5/7] Removed caching and got better results --- src/prompts.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/prompts.ts b/src/prompts.ts index f6a10aa..4befe88 100644 --- a/src/prompts.ts +++ b/src/prompts.ts @@ -119,16 +119,7 @@ export const buildSuggestionPrompt = (file: PRFile) => { return `## ${file.filename}\n\n${patchWithLines}`; }; -const patchCache = new Map(); - export const buildPatchPrompt = async (file: PRFile) => { - const cacheKey = `${file.filename}-${file.sha}`; - - if (patchCache.has(cacheKey)) { - console.log(`๐Ÿ“ฆ Using cached patch for ${file.filename}`); - return patchCache.get(cacheKey); - } - console.log(`๐Ÿ” buildPatchPrompt called for file: ${file.filename}`); let result; if (file.old_contents == null) { @@ -139,7 +130,6 @@ export const buildPatchPrompt = async (file: PRFile) => { result = await smarterContextPatchStrategy(file); } - patchCache.set(cacheKey, result); return result; }; From 993310656de005225d671fa5217f9a93628ba95e Mon Sep 17 00:00:00 2001 From: Brianleach11 <81440466+Brianleach11@users.noreply.github.com> Date: Fri, 22 Nov 2024 09:53:05 -0500 Subject: [PATCH 6/7] Added console logging to help follow steps --- src/review-agent.ts | 66 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/src/review-agent.ts b/src/review-agent.ts index f90941d..184ed7e 100644 --- a/src/review-agent.ts +++ b/src/review-agent.ts @@ -40,8 +40,25 @@ export const reviewFiles = async ( patchBuilder: (file: PRFile) => Promise, convoBuilder: (diff: string) => ChatCompletionMessageParam[] ) => { - const patches = await Promise.all(files.map(patchBuilder)); - const messages = convoBuilder(patches.join("\n")); + console.log("Processing files:", files.map(f => f.filename)); + + const patches = await Promise.all(files.map(async (file) => { + const patch = await patchBuilder(file); + console.log(`Patch for ${file.filename}:`, patch || 'EMPTY'); + if (!patch) { + console.warn(`Empty patch generated for file: ${file.filename}`); + } + return patch; + })); + + const validPatches = patches.filter(Boolean); + if (validPatches.length === 0) { + console.error("No valid patches generated"); + return "No changes to review"; + } + + console.log("Valid patches count:", validPatches.length); + const messages = convoBuilder(validPatches.join("\n")); const feedback = await reviewDiff(messages); return feedback; }; @@ -340,26 +357,61 @@ export const reviewChanges = async ( convoBuilder: (diff: string) => ChatCompletionMessageParam[], responseBuilder: (responses: string[]) => Promise ) => { + console.log("Starting reviewChanges -----------------------------------------"); const patchBuilder = async (file: PRFile) => await buildPatchPrompt(file); + + console.log("Initial files:", { + count: files.length, + files: files.map(f => ({ + filename: f.filename, + status: f.status, + hasPath: !!f.patch + })) + }); + const filteredFiles = files.filter((file) => filterFile(file)); + console.log("After filtering:", { + originalCount: files.length, + filteredCount: filteredFiles.length, + filtered: filteredFiles.map(f => f.filename) + }); + + // Log before token length calculation + console.log("Starting token length calculation for files:", + filteredFiles.map(f => f.filename) + ); + await Promise.all(filteredFiles.map(async (file) => { - file.patchTokenLength = getTokenLength(await patchBuilder(file)); + const patch = await patchBuilder(file); + file.patchTokenLength = getTokenLength(patch); + console.log(`Token length for ${file.filename}: ${file.patchTokenLength}`); })); - // further subdivide if necessary, maybe group files by common extension? + + // Log before model limit separation + console.log("Starting model limit separation"); + const patchesWithinModelLimit: PRFile[] = []; - // these single file patches are larger than the full model context const patchesOutsideModelLimit: PRFile[] = []; - filteredFiles.forEach(async (file) => { + // Change forEach to for...of for proper async handling + for (const file of filteredFiles) { const patch = await buildPatchPrompt(file); const patchWithPromptWithinLimit = isConversationWithinLimit( await constructPrompt([file], () => Promise.resolve(patch), convoBuilder) ); + + console.log(`File ${file.filename} within limit: ${patchWithPromptWithinLimit}`); + if (patchWithPromptWithinLimit) { patchesWithinModelLimit.push(file); } else { patchesOutsideModelLimit.push(file); } + } + + console.log("Final separation results:", { + withinLimit: patchesWithinModelLimit.length, + outsideLimit: patchesOutsideModelLimit.length }); console.log(`files within limits: ${patchesWithinModelLimit.length}`); @@ -377,7 +429,7 @@ export const reviewChanges = async ( console.log( `${patchesOutsideModelLimit.length} files outside limit, skipping them.` ); - + const groups = [...withinLimitsPatchGroups, ...exceedingLimitsPatchGroups]; const feedbacks = await Promise.all( From 6161aa26159bd557b031109644bdd06f4cff5ff5 Mon Sep 17 00:00:00 2001 From: Brianleach11 <81440466+Brianleach11@users.noreply.github.com> Date: Fri, 22 Nov 2024 20:21:19 -0500 Subject: [PATCH 7/7] Fixed inline comment failure --- src/review-agent.ts | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/review-agent.ts b/src/review-agent.ts index 184ed7e..9815b29 100644 --- a/src/review-agent.ts +++ b/src/review-agent.ts @@ -26,7 +26,7 @@ import { getInlineFixPrompt, } from "./prompts/inline-prompt"; import { getGitFile } from "./reviews"; -import { getParserForExtension } from "./constants"; +import { parsePatch } from "diff"; export const reviewDiff = async (messages: ChatCompletionMessageParam[]) => { const message = await generateChatCompletion({ @@ -489,19 +489,50 @@ export const generateInlineComments = async ( throw new Error("No function call found"); } const args = JSON.parse(function_call.arguments); + + // Parse the patch to get the hunks and extract the starting line number + const patches = parsePatch(file.patch); + if (patches.length === 0 || patches[0].hunks.length === 0) { + return null; + } + + const hunk = patches[0].hunks[0]; + const diffStart = hunk.newStart; + + // Adjust the suggestion line numbers to be relative to the diff + const adjustedLineStart = diffStart + (args["lineStart"] % 1000); // Using modulo to keep within reasonable range + const adjustedLineEnd = adjustedLineStart + (args["lineEnd"] - args["lineStart"]); + + // Verify these lines are actually in the changed portion + const changedLines = hunk.lines + .map((line, index) => ({ line, index: diffStart + index })) + .filter(({ line }) => line.startsWith('+')); + + const isInChangedPortion = changedLines.some( + ({ index }) => index >= adjustedLineStart && index <= adjustedLineEnd + ); + + if (!isInChangedPortion) { + console.log(`Skipping comment for ${file.filename} - adjusted lines ${adjustedLineStart}-${adjustedLineEnd} not in changed portion`); + return null; + } + const initialCode = String.raw`${args["code"]}`; const indentedCode = indentCodeFix( file.current_contents, initialCode, - args["lineStart"] + adjustedLineStart ); + const codeFix = { file: suggestion.filename, - line_start: args["lineStart"], - line_end: args["lineEnd"], + line_start: adjustedLineStart, + line_end: adjustedLineEnd, correction: indentedCode, comment: args["comment"], + diff_hunk: hunk.lines.join('\n') }; + if (isCodeSuggestionNew(file.current_contents, codeFix)) { return codeFix; }