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..315a74e 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,6 +1,8 @@ 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; @@ -90,7 +92,7 @@ export const processGitFilepath = (filepath: string) => { }; export interface EnclosingContext { - enclosingContext: Node | null; + enclosingContext: Node | SyntaxNode | null; } export interface AbstractParser { @@ -98,8 +100,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([ @@ -107,11 +112,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/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..2a9d77b 100644 --- a/src/context/language/python-parser.ts +++ b/src/context/language/python-parser.ts @@ -1,15 +1,144 @@ 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; +} + export class PythonParser implements AbstractParser { - findEnclosingContext( + private async getParser(): Promise { + console.log("๐Ÿ” Getting Python parser..."); + return await initializeParser(); + } + + async findEnclosingContext( file: string, lineStart: number, lineEnd: number - ): EnclosingContext { - // TODO: Implement this method for Python - return null; + ): Promise { + console.log(`๐Ÿ” Searching for context for range: ${lineStart} - ${lineEnd}`); + + const parser = await this.getParser(); + const tree = parser.parse(file); + + // 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 + }; + + // 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); + } + } + + // 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((largest, current) => { + const largestSize = largest.endPosition.row - largest.startPosition.row; + const currentSize = current.endPosition.row - current.startPosition.row; + return currentSize > largestSize ? current : largest; + }); + 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: bestNode + }; } - 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 }> { + console.log("๐Ÿงช Starting Python parser dryRun"); + 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) { + 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 7d44aa1..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( @@ -185,58 +193,68 @@ 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 scopeRangeNodeMap = new Map(); const expandStrategy: diff.Hunk[] = []; + const order: number[] = []; - patches.forEach((p) => { - p.hunks.forEach((hunk) => { - hunks.push(hunk); - }); - }); - - hunks.forEach((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; - const largestEnclosingFunction = parser.findEnclosingContext( + + // Expand the search range significantly above and below the changed lines + const contextRange = 0; // 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 - ).enclosingContext; + ); + + 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) || []; - 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()) { @@ -248,7 +266,7 @@ const diffContextPerHunk = (file: PRFile, parser: AbstractParser) => { scopeStategy.forEach(([rangeKey, hunk]) => { const context = buildingScopeString( updatedFile, - scopeRangeNodeMap.get(rangeKey), + scopeRangeNodeMap.get(rangeKey) as Node | SyntaxNode, hunk ).join("\n"); contexts.push(context); @@ -260,26 +278,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/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 diff --git a/src/prompts.ts b/src/prompts.ts index 42a907e..4befe88 100644 --- a/src/prompts.ts +++ b/src/prompts.ts @@ -119,12 +119,18 @@ export const buildSuggestionPrompt = (file: PRFile) => { return `## ${file.filename}\n\n${patchWithLines}`; }; -export const buildPatchPrompt = (file: PRFile) => { +export const buildPatchPrompt = async (file: PRFile) => { + 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); } + + return result; }; export const getReviewPrompt = (diff: string): ChatCompletionMessageParam[] => { @@ -143,12 +149,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..9815b29 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 { parsePatch } from "diff"; export const reviewDiff = async (messages: ChatCompletionMessageParam[]) => { const message = await generateChatCompletion({ @@ -36,11 +37,28 @@ 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 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; }; @@ -101,22 +119,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 +147,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 +158,7 @@ const processWithinLimitFiles = ( processGroups.push(currentGroup); currentGroup = [file]; } - }); + } if (currentGroup.length > 0) { processGroups.push(currentGroup); } @@ -159,9 +180,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 +191,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 +208,7 @@ const processOutsideLimitFiles = ( exceedingLimits.push(file); } }); - const withinLimitsGroup = processWithinLimitFiles( + const withinLimitsGroup = await processWithinLimitFiles( withinLimits, patchBuilder, convoBuilder @@ -336,34 +357,70 @@ export const reviewChanges = async ( convoBuilder: (diff: string) => ChatCompletionMessageParam[], responseBuilder: (responses: string[]) => Promise ) => { - const patchBuilder = buildPatchPrompt; + 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)); - filteredFiles.map((file) => { - file.patchTokenLength = getTokenLength(patchBuilder(file)); + console.log("After filtering:", { + originalCount: files.length, + filteredCount: filteredFiles.length, + filtered: filteredFiles.map(f => f.filename) }); - // further subdivide if necessary, maybe group files by common extension? + + // 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) => { + const patch = await patchBuilder(file); + file.patchTokenLength = getTokenLength(patch); + console.log(`Token length for ${file.filename}: ${file.patchTokenLength}`); + })); + + // 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((file) => { + // Change forEach to for...of for proper async handling + for (const file of filteredFiles) { + const patch = await buildPatchPrompt(file); const patchWithPromptWithinLimit = isConversationWithinLimit( - constructPrompt([file], patchBuilder, convoBuilder) + 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}`); - const withinLimitsPatchGroups = processWithinLimitFiles( + const withinLimitsPatchGroups = await processWithinLimitFiles( patchesWithinModelLimit, patchBuilder, convoBuilder ); - const exceedingLimitsPatchGroups = processOutsideLimitFiles( + const exceedingLimitsPatchGroups = await processOutsideLimitFiles( patchesOutsideModelLimit, patchBuilder, convoBuilder @@ -372,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( @@ -432,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; }