From b4bd0343654b67b1ae9523e24f38721b1f927c14 Mon Sep 17 00:00:00 2001 From: Manon Faour Date: Mon, 31 Jul 2023 14:32:13 +0100 Subject: [PATCH 1/5] feat: move git commands in common folder out of review --- src/common/git/getChangedFileLines.ts | 38 +++++++++++++++++++ src/common/git/getChangedFilesNames.ts | 34 +++++++++++++++++ src/common/git/getFilesWithChanges.ts | 23 +++++++++++ src/common/types/File.ts | 5 +++ src/common/types/index.ts | 1 + src/index.ts | 6 ++- src/review/index.ts | 7 ++-- .../prompt/constructPrompt/constructPrompt.ts | 36 ++++-------------- .../prompt/fileLines/getChangedLines.ts | 27 ------------- .../prompt/filesNames/getFileNames.test.ts | 31 --------------- src/review/prompt/filesNames/getFileNames.ts | 33 ---------------- .../prompt/filesNames/getFileNamesFromGit.ts | 23 ----------- src/review/prompt/gitCommands.ts | 24 ------------ src/review/prompt/makeSlimmedFile.ts | 31 +++++++-------- 14 files changed, 130 insertions(+), 189 deletions(-) create mode 100644 src/common/git/getChangedFileLines.ts create mode 100644 src/common/git/getChangedFilesNames.ts create mode 100644 src/common/git/getFilesWithChanges.ts create mode 100644 src/common/types/File.ts create mode 100644 src/common/types/index.ts delete mode 100644 src/review/prompt/fileLines/getChangedLines.ts delete mode 100644 src/review/prompt/filesNames/getFileNames.test.ts delete mode 100644 src/review/prompt/filesNames/getFileNames.ts delete mode 100644 src/review/prompt/filesNames/getFileNamesFromGit.ts delete mode 100644 src/review/prompt/gitCommands.ts diff --git a/src/common/git/getChangedFileLines.ts b/src/common/git/getChangedFileLines.ts new file mode 100644 index 00000000..b8b1dc37 --- /dev/null +++ b/src/common/git/getChangedFileLines.ts @@ -0,0 +1,38 @@ +import { exec } from "child_process"; + +import { getGitHubEnvVariables } from "../../config"; + +export const getChangesFileLinesCommand = ( + isCi: boolean, + fileName: string +): string => { + if (isCi) { + const { githubSha, baseSha } = getGitHubEnvVariables(); + return `git diff -U0 --diff-filter=AMT ${baseSha} ${githubSha} ${fileName}`; + } + return `git diff -U0 --diff-filter=AMT --cached ${fileName}`; +}; + +export const getChangedFileLines = async ( + isCi: boolean, + fileName: string +): Promise => { + const commandString = getChangesFileLinesCommand(isCi, fileName); + + return new Promise((resolve, reject) => { + exec(commandString, (error, stdout, stderr) => { + if (error) { + reject(new Error(`Failed to execute command. Error: ${error.message}`)); + } else if (stderr) { + reject(new Error(`Command execution error: ${stderr}`)); + } else { + const changedLines = stdout + .split("\n") + .filter((line) => line.startsWith("+") || line.startsWith("-")) + .filter((line) => !line.startsWith("---") && !line.startsWith("+++")) + .join("\n"); + resolve(changedLines); + } + }); + }); +}; diff --git a/src/common/git/getChangedFilesNames.ts b/src/common/git/getChangedFilesNames.ts new file mode 100644 index 00000000..83174782 --- /dev/null +++ b/src/common/git/getChangedFilesNames.ts @@ -0,0 +1,34 @@ +import { exec } from "child_process"; +import { join } from "path"; + +import { getGitHubEnvVariables } from "../../config"; + +export const getChangedFilesNamesCommand = (isCi: boolean): string => { + if (isCi) { + const { githubSha, baseSha } = getGitHubEnvVariables(); + return `git diff --name-only --diff-filter=AMT ${baseSha} ${githubSha}`; + } + return "git diff --name-only --diff-filter=AMT --cached"; +}; + +export const getChangedFilesNames = async ( + isCi: boolean +): Promise => { + const commandString = getChangedFilesNamesCommand(isCi); + + return new Promise((resolve, reject) => { + exec(commandString, (error, stdout, stderr) => { + if (error) { + reject(new Error(`Failed to execute command. Error: ${error.message}`)); + } else if (stderr) { + reject(new Error(`Command execution error: ${stderr}`)); + } else { + const files = stdout + .split("\n") + .filter((fileName) => fileName.trim() !== "") + .map((fileName) => join(process.cwd(), fileName.trim())); + resolve(files); + } + }); + }); +}; diff --git a/src/common/git/getFilesWithChanges.ts b/src/common/git/getFilesWithChanges.ts new file mode 100644 index 00000000..a9c28b36 --- /dev/null +++ b/src/common/git/getFilesWithChanges.ts @@ -0,0 +1,23 @@ +import { readFile } from "fs/promises"; +import { getChangedFileLines } from "./getChangedFileLines"; +import { getChangedFilesNames } from "./getChangedFilesNames"; +import { File } from "../types/File"; + +export const getFilesWithChanges = async (isCi: boolean): Promise => { + try { + const fileNames = await getChangedFilesNames(isCi); + + const files = await Promise.all( + fileNames.map(async (fileName) => { + const fileContent = await readFile(fileName, "utf8"); + const changedLines = await getChangedFileLines(isCi, fileName); + + return { fileName, fileContent, changedLines }; + }) + ); + + return files; + } catch (error) { + throw new Error(`Failed to get files with changes: ${error}`); + } +}; diff --git a/src/common/types/File.ts b/src/common/types/File.ts new file mode 100644 index 00000000..796ffb08 --- /dev/null +++ b/src/common/types/File.ts @@ -0,0 +1,5 @@ +export interface File { + fileName: string; + fileContent: string; + changedLines: string; +} diff --git a/src/common/types/index.ts b/src/common/types/index.ts new file mode 100644 index 00000000..514abf1a --- /dev/null +++ b/src/common/types/index.ts @@ -0,0 +1 @@ +export { File } from "./File"; diff --git a/src/index.ts b/src/index.ts index c01555c3..8b38f102 100755 --- a/src/index.ts +++ b/src/index.ts @@ -13,7 +13,11 @@ const main = async () => { break; case "review": const { review } = await import("./review"); - await review(argv); + const { getFilesWithChanges } = await import( + "./common/git/getFilesWithChanges" + ); + const files = await getFilesWithChanges(argv.ci); + await review(argv, files); break; case "test": const { test } = await import("./test"); diff --git a/src/review/index.ts b/src/review/index.ts index 45b103bd..88a74ba3 100644 --- a/src/review/index.ts +++ b/src/review/index.ts @@ -4,7 +4,7 @@ import { commentPerFile } from "../common/ci/commentPerFile"; import { signOff } from "./constants"; import { askAI } from "./llm/askAI"; import { constructPromptsArray } from "./prompt/constructPrompt/constructPrompt"; -import { getFileNames } from "./prompt/filesNames/getFileNames"; +import { File } from "../common/types"; interface ReviewArgs { [x: string]: unknown; @@ -14,7 +14,7 @@ interface ReviewArgs { $0: string; } -export const review = async (yargs: ReviewArgs) => { +export const review = async (yargs: ReviewArgs, files: File[]) => { const isCi = yargs.ci; const shouldCommentPerFile = yargs.commentPerFile; @@ -22,8 +22,7 @@ export const review = async (yargs: ReviewArgs) => { const maxPromptLength = getMaxPromptLength(modelName); - const fileNames = await getFileNames(isCi); - const prompts = await constructPromptsArray(fileNames, maxPromptLength, isCi); + const prompts = await constructPromptsArray(files, maxPromptLength); const { markdownReport: response, feedbacks } = await askAI( prompts, diff --git a/src/review/prompt/constructPrompt/constructPrompt.ts b/src/review/prompt/constructPrompt/constructPrompt.ts index 8ccd2c0e..c721ba32 100644 --- a/src/review/prompt/constructPrompt/constructPrompt.ts +++ b/src/review/prompt/constructPrompt/constructPrompt.ts @@ -1,15 +1,14 @@ -import { readFile } from "fs/promises"; import { makeSlimmedFile } from "../makeSlimmedFile"; import { instructionPrompt } from "../prompts"; +import { File } from "../../../common/types"; import { ReviewFile } from "../types"; const getSizeOfReviewFile = (file: ReviewFile): number => file.fileName.length + file.fileContent.length; const splitFilesIntoBatches = async ( - files: ReviewFile[], - maxBatchSize: number, - isCi: boolean + files: File[], + maxBatchSize: number ): Promise => { const batches: ReviewFile[][] = []; let currentBatch: ReviewFile[] = []; @@ -20,7 +19,7 @@ const splitFilesIntoBatches = async ( console.warn( `File ${file.fileName} is larger than the max prompt length, consider using a model with a larger context window. Attempting to slim the file...` ); - const slimmedFile = await makeSlimmedFile(file, maxBatchSize, isCi); + const slimmedFile = await makeSlimmedFile(file, maxBatchSize); currentBatch.push(slimmedFile); } else if (currentBatchSize + currentFileSize > maxBatchSize) { batches.push(currentBatch); @@ -40,33 +39,14 @@ const splitFilesIntoBatches = async ( return batches; }; -const readFiles = async (fileNames: string[]): Promise => { - const files: ReviewFile[] = []; - - for (const fileName of fileNames) { - try { - const fileContent = await readFile(fileName, "utf8"); - files.push({ fileName, fileContent }); - } catch (error) { - console.error(`Failed to process file ${fileName}: ${error}`); - } - } - - return files; -}; - export const constructPromptsArray = async ( - fileNames: string[], - maxPromptLength: number, - isCi: boolean + files: File[], + maxPromptLength: number ): Promise => { - const filesToReview = await readFiles(fileNames); - const maxPromptPayloadLength = maxPromptLength - instructionPrompt.length; const promptPayloads = await splitFilesIntoBatches( - filesToReview, - maxPromptPayloadLength, - isCi + files, + maxPromptPayloadLength ); const prompts = promptPayloads.map((payload) => { diff --git a/src/review/prompt/fileLines/getChangedLines.ts b/src/review/prompt/fileLines/getChangedLines.ts deleted file mode 100644 index 8b33d4f3..00000000 --- a/src/review/prompt/fileLines/getChangedLines.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { exec } from "child_process"; -import { gitChangedFileLinesCommand } from "../gitCommands"; -import { ReviewFile } from "../types"; - -export const getChangedLines = async ( - file: ReviewFile, - isCi: boolean -): Promise => { - const gitCommand = await gitChangedFileLinesCommand(isCi, file.fileName); - - return new Promise((resolve, reject) => { - exec(gitCommand, (error, stdout, stderr) => { - if (error) { - reject(new Error(error.message)); - } else if (stderr) { - reject(new Error(stderr)); - } else { - const changedLines = stdout - .split("\n") - .filter((line) => line.startsWith("+") || line.startsWith("-")) - .filter((line) => !line.startsWith("---") && !line.startsWith("+++")) - .join("\n"); - resolve(changedLines); - } - }); - }); -}; diff --git a/src/review/prompt/filesNames/getFileNames.test.ts b/src/review/prompt/filesNames/getFileNames.test.ts deleted file mode 100644 index 097da9c5..00000000 --- a/src/review/prompt/filesNames/getFileNames.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { readdir } from "fs/promises"; -import { join } from "path"; -import { getFileNames } from "./getFileNames"; -import * as getFileNamesFromGitModule from "./getFileNamesFromGit"; - -jest.mock("./getFileNamesFromGit"); - -describe("getFileNames unit test", () => { - afterEach(() => { - jest.restoreAllMocks(); - }); - - test("returns only supported files", async () => { - const testDir = join(__dirname, "../../../testFiles"); - - const mockedGetFileNamesFromGit = jest.fn().mockImplementation(() => { - return readdir(testDir).then((files) => { - return files.map((file) => join(testDir, file)); - }); - }); - - (getFileNamesFromGitModule as any).getFileNamesFromGit = - mockedGetFileNamesFromGit; - - const result = await getFileNames(false); - - expect(result).toEqual([ - join(__dirname, "../../../testFiles", "longFile.tsx"), - ]); - }); -}); diff --git a/src/review/prompt/filesNames/getFileNames.ts b/src/review/prompt/filesNames/getFileNames.ts deleted file mode 100644 index 024e53a9..00000000 --- a/src/review/prompt/filesNames/getFileNames.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { extname } from "path"; -import { excludedKeywords, supportedFiles } from "../../constants"; -import { getFileNamesFromGit } from "./getFileNamesFromGit"; - -const filterFiles = (fileNames: string[]): string[] => { - const filteredFileNames = fileNames.filter((fileName) => { - const ext = extname(fileName); - return ( - supportedFiles.has(ext) && - ![...excludedKeywords].some((keyword) => fileName.includes(keyword)) - ); - }); - - return filteredFileNames; -}; - -export const getFileNames = async (isCi: boolean): Promise => { - console.info("Getting files..."); - try { - const fileNames = await getFileNamesFromGit(isCi); - - const filteredFileNames = filterFiles(fileNames); - - if (filteredFileNames.length === 0) { - process.exit(0); - } - - return filteredFileNames; - } catch (error) { - console.error(`Failed to get files: ${error}`); - throw error; - } -}; diff --git a/src/review/prompt/filesNames/getFileNamesFromGit.ts b/src/review/prompt/filesNames/getFileNamesFromGit.ts deleted file mode 100644 index da779e5c..00000000 --- a/src/review/prompt/filesNames/getFileNamesFromGit.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { exec } from "child_process"; -import { join } from "path"; -import { gitChangedFileNamesCommand } from "../gitCommands"; - -export const getFileNamesFromGit = async (isCi: boolean): Promise => { - const commandString = await gitChangedFileNamesCommand(isCi); - - return new Promise((resolve, reject) => { - exec(commandString, (error, stdout, stderr) => { - if (error) { - reject(new Error(error.message)); - } else if (stderr) { - reject(new Error(stderr)); - } else { - const files = stdout - .split("\n") - .filter((fileName) => fileName.trim() !== "") - .map((fileName) => join(process.cwd(), fileName.trim())); - resolve(files); - } - }); - }); -}; diff --git a/src/review/prompt/gitCommands.ts b/src/review/prompt/gitCommands.ts deleted file mode 100644 index b2594949..00000000 --- a/src/review/prompt/gitCommands.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { getGitHubEnvVariables } from "../../config"; - -export const gitChangedFileNamesCommand = async ( - isCi: boolean -): Promise => { - if (isCi) { - const { githubSha, baseSha } = getGitHubEnvVariables(); - return `git diff --name-only --diff-filter=AMT ${baseSha} ${githubSha}`; - } else { - return "git diff --name-only --diff-filter=AMT --cached"; - } -}; - -export const gitChangedFileLinesCommand = async ( - isCi: boolean, - fileName: string -): Promise => { - if (isCi) { - const { githubSha, baseSha } = getGitHubEnvVariables(); - return `git diff -U0 --diff-filter=AMT ${baseSha} ${githubSha} ${fileName}`; - } else { - return `git diff -U0 --diff-filter=AMT --cached ${fileName}`; - } -}; diff --git a/src/review/prompt/makeSlimmedFile.ts b/src/review/prompt/makeSlimmedFile.ts index bf978ee6..6bff3491 100644 --- a/src/review/prompt/makeSlimmedFile.ts +++ b/src/review/prompt/makeSlimmedFile.ts @@ -1,18 +1,16 @@ import { OpenAIEmbeddings } from "langchain/embeddings/openai"; import { RecursiveCharacterTextSplitter } from "langchain/text_splitter"; import { MemoryVectorStore } from "langchain/vectorstores/memory"; -import { getChangedLines } from "./fileLines/getChangedLines"; import { getLanguageOfFile } from "./getLanguageOfFile"; import { slimmedContextPrompt } from "./prompts"; import { ReviewFile } from "./types"; +import { File } from "../../common/types"; export const makeSlimmedFile = async ( - file: ReviewFile, - maxBatchSize: number, - isCi: boolean + file: File, + maxBatchSize: number ): Promise => { - let changedLines: string; - changedLines = await getChangedLines(file, isCi); + let changedLines: string = file.changedLines; if (changedLines.length > maxBatchSize) { console.error( @@ -24,18 +22,15 @@ export const makeSlimmedFile = async ( const fileLanguage = getLanguageOfFile(file.fileName); - let splitter: RecursiveCharacterTextSplitter; - if (!fileLanguage) { - splitter = new RecursiveCharacterTextSplitter({ - chunkSize: 100, - chunkOverlap: 0, - }); - } else { - splitter = RecursiveCharacterTextSplitter.fromLanguage(fileLanguage, { - chunkSize: 100, - chunkOverlap: 0, - }); - } + const splitter = fileLanguage + ? RecursiveCharacterTextSplitter.fromLanguage(fileLanguage, { + chunkSize: 100, + chunkOverlap: 0, + }) + : new RecursiveCharacterTextSplitter({ + chunkSize: 100, + chunkOverlap: 0, + }); const doc = await splitter.createDocuments([changedLines]); From 7b102d2e4f191da08eaab7aa1f454393324dd58d Mon Sep 17 00:00:00 2001 From: Manon Faour Date: Mon, 31 Jul 2023 14:32:26 +0100 Subject: [PATCH 2/5] test: adapt tests to new input types --- src/test/load/loadTestCodeSnippets.ts | 13 +++++-------- src/test/run/runTest.ts | 12 ++++++++---- src/test/types.ts | 4 +++- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/test/load/loadTestCodeSnippets.ts b/src/test/load/loadTestCodeSnippets.ts index 2f4e3aa8..dd5b2651 100644 --- a/src/test/load/loadTestCodeSnippets.ts +++ b/src/test/load/loadTestCodeSnippets.ts @@ -42,12 +42,13 @@ const loadOrGenerateCodeSnippet = async ( // Try to load the snippet from the cache, using the hashed description as the key const hashedDescription = generateHash(testCase.description); + const fileName = path.join(snippetCacheDir, `${hashedDescription}.ts`); try { - readFileSync(path.join(snippetCacheDir, `${hashedDescription}.ts`), "utf8"); + const fileContent = readFileSync(fileName, "utf8"); return { ...testCase, - snippet: path.join(snippetCacheDir, `${hashedDescription}.ts`), + snippet: { fileName, fileContent, changedLines: fileContent }, }; } catch (error) { console.info( @@ -57,15 +58,11 @@ const loadOrGenerateCodeSnippet = async ( const snippet = await generateCodeSnippet(testCase, model); // Save the snippet to the cache - writeFileSync( - path.join(snippetCacheDir, `${hashedDescription}.ts`), - snippet, - "utf8" - ); + writeFileSync(fileName, snippet, "utf8"); return { ...testCase, - snippet: path.join(snippetCacheDir, `${hashedDescription}.ts`), + snippet: { fileName, fileContent: snippet, changedLines: snippet }, }; } }; diff --git a/src/test/run/runTest.ts b/src/test/run/runTest.ts index 09d59895..bbe452e3 100644 --- a/src/test/run/runTest.ts +++ b/src/test/run/runTest.ts @@ -22,7 +22,7 @@ const runTest = async ( modelName: string, maxPromptLength: number, vectorStore: MemoryVectorStore, - ci: boolean, + ci: boolean ): Promise => { if (!testCase.snippet) { throw new Error(`Test case ${testCase.name} does not have a snippet.`); @@ -33,10 +33,14 @@ const runTest = async ( // First step: run the review on the code snippet. const prompts = await constructPromptsArray( [testCase.snippet], - maxPromptLength, - ci, + maxPromptLength + ); + + const { markdownReport: reviewResponse } = await askAI( + prompts, + modelName, + false ); - const {markdownReport: reviewResponse} = await askAI(prompts, modelName, false); const similarityResponse = await vectorStore.similaritySearchWithScore( reviewResponse, diff --git a/src/test/types.ts b/src/test/types.ts index efd4ee5a..163bab7b 100644 --- a/src/test/types.ts +++ b/src/test/types.ts @@ -1,6 +1,8 @@ +import { File } from "../common/types"; + export interface TestCase { name: string; description: string; hash?: string; - snippet?: string; + snippet?: File; } From 96475b339729f0e56a5ad7f2e273cb4a17120305 Mon Sep 17 00:00:00 2001 From: Manon Faour Date: Mon, 31 Jul 2023 15:39:51 +0100 Subject: [PATCH 3/5] fix: do not send changedLines in prompt --- .../constructPrompt/constructPrompt.test.ts | 18 +++++++++++------- .../prompt/constructPrompt/constructPrompt.ts | 12 ++++++++++-- src/review/prompt/makeSlimmedFile.ts | 4 ++-- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/review/prompt/constructPrompt/constructPrompt.test.ts b/src/review/prompt/constructPrompt/constructPrompt.test.ts index d764c763..e084d8cb 100644 --- a/src/review/prompt/constructPrompt/constructPrompt.test.ts +++ b/src/review/prompt/constructPrompt/constructPrompt.test.ts @@ -1,3 +1,4 @@ +import { readFile } from "fs/promises"; import { join } from "path"; import { constructPromptsArray } from "./constructPrompt"; @@ -5,19 +6,22 @@ describe("When a file is longer than the max prompt length", () => { jest.setTimeout(30000); test("constructPromptsArray splits up the file into prompts", async () => { const testFilePath = join(__dirname, "../../../testFiles/longFile.tsx"); - const fileNames = [testFilePath]; + const testFileContent = await readFile(testFilePath, "utf8"); + const files = [ + { + fileName: testFilePath, + fileContent: testFileContent, + changedLines: testFileContent, + }, + ]; const maxPromptLength = 2000; const stringifyMargin = 1.2; - const isCi = false; - const result = await constructPromptsArray( - fileNames, - maxPromptLength, - isCi - ); + const result = await constructPromptsArray(files, maxPromptLength); expect(result).toBeInstanceOf(Array); expect(result.length).toBeGreaterThan(0); + console.log(result[0]); for (const prompt of result) { expect(prompt.length).toBeLessThanOrEqual( diff --git a/src/review/prompt/constructPrompt/constructPrompt.ts b/src/review/prompt/constructPrompt/constructPrompt.ts index c721ba32..467719c3 100644 --- a/src/review/prompt/constructPrompt/constructPrompt.ts +++ b/src/review/prompt/constructPrompt/constructPrompt.ts @@ -23,10 +23,18 @@ const splitFilesIntoBatches = async ( currentBatch.push(slimmedFile); } else if (currentBatchSize + currentFileSize > maxBatchSize) { batches.push(currentBatch); - currentBatch = [file]; + currentBatch = [ + { + fileName: file.fileName, + fileContent: file.fileContent, + }, + ]; currentBatchSize = currentFileSize; } else { - currentBatch.push(file); + currentBatch.push({ + fileName: file.fileName, + fileContent: file.fileContent, + }); currentBatchSize += currentFileSize; } } diff --git a/src/review/prompt/makeSlimmedFile.ts b/src/review/prompt/makeSlimmedFile.ts index 6bff3491..c508e406 100644 --- a/src/review/prompt/makeSlimmedFile.ts +++ b/src/review/prompt/makeSlimmedFile.ts @@ -17,7 +17,7 @@ export const makeSlimmedFile = async ( `The changed lines are ${changedLines.length} which is longer than ${maxBatchSize}. Consider using a model with a larger context window. Slicing the changed lines...` ); changedLines = changedLines.slice(0, maxBatchSize); - return { ...file, fileContent: changedLines }; + return { fileName: file.fileName, fileContent: changedLines }; } const fileLanguage = getLanguageOfFile(file.fileName); @@ -61,5 +61,5 @@ export const makeSlimmedFile = async ( .replace("{context}", context); // return the file with the updated prompt - return { ...file, fileContent: prompt }; + return { fileName: file.fileName, fileContent: prompt }; }; From 3aee4f76e3b3bcfd4cc485d40444402a6f738b42 Mon Sep 17 00:00:00 2001 From: Manon Faour Date: Mon, 31 Jul 2023 17:30:22 +0100 Subject: [PATCH 4/5] feat: add back filter files in review --- src/review/index.ts | 5 ++- .../prompt/filterFiles/filterFiles.test.ts | 37 +++++++++++++++++++ src/review/prompt/filterFiles/filterFiles.ts | 15 ++++++++ src/review/prompt/filterFiles/index.ts | 1 + 4 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 src/review/prompt/filterFiles/filterFiles.test.ts create mode 100644 src/review/prompt/filterFiles/filterFiles.ts create mode 100644 src/review/prompt/filterFiles/index.ts diff --git a/src/review/index.ts b/src/review/index.ts index 88a74ba3..ebd89455 100644 --- a/src/review/index.ts +++ b/src/review/index.ts @@ -5,6 +5,7 @@ import { signOff } from "./constants"; import { askAI } from "./llm/askAI"; import { constructPromptsArray } from "./prompt/constructPrompt/constructPrompt"; import { File } from "../common/types"; +import { filterFiles } from "./prompt/filterFiles"; interface ReviewArgs { [x: string]: unknown; @@ -20,9 +21,11 @@ export const review = async (yargs: ReviewArgs, files: File[]) => { const modelName = yargs.model as string; + const filteredFiles = filterFiles(files); + const maxPromptLength = getMaxPromptLength(modelName); - const prompts = await constructPromptsArray(files, maxPromptLength); + const prompts = await constructPromptsArray(filteredFiles, maxPromptLength); const { markdownReport: response, feedbacks } = await askAI( prompts, diff --git a/src/review/prompt/filterFiles/filterFiles.test.ts b/src/review/prompt/filterFiles/filterFiles.test.ts new file mode 100644 index 00000000..12372f06 --- /dev/null +++ b/src/review/prompt/filterFiles/filterFiles.test.ts @@ -0,0 +1,37 @@ +import { readdir, readFile } from "fs/promises"; +import { join } from "path"; +import { File } from "../../../common/types"; +import { filterFiles } from "./filterFiles"; + +describe("filterFiles unit test", () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + test("returns only supported files", async () => { + const testDir = join(__dirname, "../../../testFiles"); + + const testFiles: File[] = []; + + const readDir = await readdir(testDir); + + await Promise.all( + readDir.map(async (file) => { + const fileName = join(testDir, file); + const fileContent = await readFile(fileName, "utf8"); + testFiles.push({ + fileName: fileName, + fileContent: fileContent, + changedLines: fileContent, + }); + }) + ); + + const result = await filterFiles(testFiles); + + expect(result.length).toEqual(1); + expect(result[0].fileName).toBe( + join(__dirname, "../../../testFiles", "longFile.tsx") + ); + }); +}); diff --git a/src/review/prompt/filterFiles/filterFiles.ts b/src/review/prompt/filterFiles/filterFiles.ts new file mode 100644 index 00000000..714a461f --- /dev/null +++ b/src/review/prompt/filterFiles/filterFiles.ts @@ -0,0 +1,15 @@ +import { extname } from "path"; +import { File } from "../../../common/types"; +import { excludedKeywords, supportedFiles } from "../../constants"; + +export const filterFiles = (files: File[]): File[] => { + const filteredFileNames = files.filter((file) => { + const ext = extname(file.fileName); + return ( + supportedFiles.has(ext) && + ![...excludedKeywords].some((keyword) => file.fileName.includes(keyword)) + ); + }); + + return filteredFileNames; +}; diff --git a/src/review/prompt/filterFiles/index.ts b/src/review/prompt/filterFiles/index.ts new file mode 100644 index 00000000..1adf4bd2 --- /dev/null +++ b/src/review/prompt/filterFiles/index.ts @@ -0,0 +1 @@ +export { filterFiles } from "./filterFiles"; From e28d1e57435a4b4687c8a0268eeaf3e884b01acf Mon Sep 17 00:00:00 2001 From: Manon Faour Date: Tue, 1 Aug 2023 10:29:55 +0100 Subject: [PATCH 5/5] fix: remove console.log --- src/review/prompt/constructPrompt/constructPrompt.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/review/prompt/constructPrompt/constructPrompt.test.ts b/src/review/prompt/constructPrompt/constructPrompt.test.ts index e084d8cb..da013f12 100644 --- a/src/review/prompt/constructPrompt/constructPrompt.test.ts +++ b/src/review/prompt/constructPrompt/constructPrompt.test.ts @@ -21,7 +21,6 @@ describe("When a file is longer than the max prompt length", () => { expect(result).toBeInstanceOf(Array); expect(result.length).toBeGreaterThan(0); - console.log(result[0]); for (const prompt of result) { expect(prompt.length).toBeLessThanOrEqual(