From 626312d495c3bfaf67fd09107edba95df9a30ad1 Mon Sep 17 00:00:00 2001 From: Simon Larsen Date: Thu, 27 Jun 2024 14:33:25 +0100 Subject: [PATCH] refactor: Refactor code into smaller functions/methods --- .../CopilotActions/CopilotActionsBase.ts | 25 +--- .../CopilotActions/FixGrammarAndSpelling.ts | 2 +- .../Service/CopilotActions/ImproveComments.ts | 140 ++++++++++-------- .../CopilotActions/ImproveVariableNames.ts | 2 +- .../Service/CopilotActions/ImroveReadme.ts | 24 +-- Copilot/Service/CopilotActions/Index.ts | 10 +- .../Service/CopilotActions/RefactorCode.ts | 2 +- .../Service/CopilotActions/WriteUnitTests.ts | 29 +--- 8 files changed, 96 insertions(+), 138 deletions(-) diff --git a/Copilot/Service/CopilotActions/CopilotActionsBase.ts b/Copilot/Service/CopilotActions/CopilotActionsBase.ts index da43023e48..e9628fd6f9 100644 --- a/Copilot/Service/CopilotActions/CopilotActionsBase.ts +++ b/Copilot/Service/CopilotActions/CopilotActionsBase.ts @@ -51,7 +51,9 @@ export default class CopilotActionBase { if ( !this.acceptFileExtentions.find((item: string) => { - return item.includes(LocalFile.getFileExtension(data.input.currentFilePath)); + return item.includes( + LocalFile.getFileExtension(data.input.currentFilePath), + ); }) ) { throw new NotAcceptedFileExtentionForCopilotAction( @@ -101,9 +103,7 @@ If you have any feedback or suggestions, please let us know. We would love to h return `OneUptime Copilot: ${this.copilotActionType} on ${data.input.currentFilePath}`; } - public async onExecutionStep( - data: CopilotProcess, - ): Promise { + public async onExecutionStep(data: CopilotProcess): Promise { return Promise.resolve(data); } @@ -111,12 +111,6 @@ If you have any feedback or suggestions, please let us know. We would love to h return true; // by default the action is completed } - public async filterNoOperation( - data: CopilotProcess, - ): Promise { - return Promise.resolve(data); - } - public async getNextFilePath(_data: CopilotProcess): Promise { return null; } @@ -134,16 +128,12 @@ If you have any feedback or suggestions, please let us know. We would love to h data.result.files = {}; } - let isActionComplete: boolean = false; + let isActionComplete: boolean = false; while (!isActionComplete) { - data = await this.onExecutionStep(data); - data = await this.filterNoOperation(data); - isActionComplete = await this.isActionComplete(data); - } return await this.onAfterExecute(data); @@ -159,7 +149,6 @@ If you have any feedback or suggestions, please let us know. We would love to h } return prompt; - } public async getPrompt( @@ -168,7 +157,9 @@ If you have any feedback or suggestions, please let us know. We would love to h throw new NotImplementedException(); } - public async askCopilot(prompt: CopilotActionPrompt): Promise { + public async askCopilot( + prompt: CopilotActionPrompt, + ): Promise { return await LLM.getResponse(prompt); } } diff --git a/Copilot/Service/CopilotActions/FixGrammarAndSpelling.ts b/Copilot/Service/CopilotActions/FixGrammarAndSpelling.ts index 25e5830d1e..d4987f5dff 100644 --- a/Copilot/Service/CopilotActions/FixGrammarAndSpelling.ts +++ b/Copilot/Service/CopilotActions/FixGrammarAndSpelling.ts @@ -52,7 +52,7 @@ export default class FixGrammarAndSpelling extends CopilotActionBase { }; } - protected override async getPrompt( + public override async getPrompt( _data: CopilotProcess, ): Promise { const prompt: string = `Please fix grammar and spelling in this file. diff --git a/Copilot/Service/CopilotActions/ImproveComments.ts b/Copilot/Service/CopilotActions/ImproveComments.ts index 3578290974..89318278da 100644 --- a/Copilot/Service/CopilotActions/ImproveComments.ts +++ b/Copilot/Service/CopilotActions/ImproveComments.ts @@ -1,16 +1,16 @@ import CopilotActionType from "Common/Types/Copilot/CopilotActionType"; import CopilotActionBase, { CopilotActionPrompt, - CopilotActionRunResult, CopilotProcess, } from "./CopilotActionsBase"; import CodeRepositoryUtil from "../../Utils/CodeRepository"; import ServiceLanguage from "Common/Types/ServiceCatalog/ServiceLanguage"; +import { CopilotPromptResult } from "../LLM/LLMBase"; +import CodeRepositoryFile from "CommonServer/Utils/CodeRepository/CodeRepositoryFile"; export default class ImproveComments extends CopilotActionBase { + public isRequirementsMet: boolean = false; - public isRequirementsMet: boolean = false - public constructor() { super({ copilotActionType: CopilotActionType.IMPROVE_COMMENTS, @@ -22,47 +22,74 @@ export default class ImproveComments extends CopilotActionBase { return Promise.resolve(this.isRequirementsMet); } - public override async onExecutionStep(data: CopilotProcess): Promise { - - // ask copilot again if the requirements are met. - - - - - const oldCode: string = data.input.files[data.input.currentFilePath]?.fileContent as string; - const newCode: string = data.result.files[data.input.currentFilePath]?.fileContent as string; - - const validationPrompt = await this.getValidationPrompt({oldCode, newCode}); - - return data; - } - - public override async filterNoOperation( + public override async onExecutionStep( data: CopilotProcess, ): Promise { - const finalResult: CopilotActionRunResult = { - files: {}, - }; + // Action Prompt - for (const filePath in data.result.files) { - if (data.result.files[filePath]?.fileContent.includes("--all-good--")) { - continue; - } + const actionPrompt: CopilotActionPrompt = await this.getPrompt(data); + const copilotResult: CopilotPromptResult = + await this.askCopilot(actionPrompt); - finalResult.files[filePath] = data.result.files[filePath]!; + const newContent = await this.cleanup(copilotResult.output as string); + + if (await this.isFileAlreadyWellCommented(newContent)) { + this.isRequirementsMet = true; + return data; } - return { - ...data, - result: finalResult, - }; + // ask copilot again if the requirements are met. + + const oldCode: string = data.input.files[data.input.currentFilePath] + ?.fileContent as string; + const newCode: string = newContent; + + const validationPrompt = await this.getValidationPrompt({ + oldCode, + newCode, + }); + + const validationResponse = await this.askCopilot(validationPrompt); + + const didPassValidation = await this.didPassValidation(validationResponse); + + if (didPassValidation) { + // add to result. + data.result.files[data.input.currentFilePath] = { + ...data.input.files[data.input.currentFilePath], + fileContent: newContent, + } as CodeRepositoryFile; + + this.isRequirementsMet = true; + return data; + } + + // TODO: if the validation is not passed then ask copilot to improve the comments again. + + return data; + } + + public async didPassValidation(data: CopilotPromptResult): Promise { + const validationResponse = data.output as string; + if (validationResponse === "--no--") { + return true; + } + + return false; + } + + public async isFileAlreadyWellCommented(content: string): Promise { + if (content.includes("--all-good--")) { + return true; + } + + return false; } public async getValidationPrompt(data: { oldCode: string; newCode: string; }): Promise { - const oldCode: string = data.oldCode; const newCode: string = data.newCode; @@ -82,21 +109,23 @@ export default class ImproveComments extends CopilotActionBase { Was anything changed in the code except comments? If yes, please reply with the following text: --yes-- - If the code was not changed except comments, please reply with the following text: + If the code was NOT changed EXCEPT comments, please reply with the following text: --no-- `, - systemPrompt: await this.getSystemPrompt() + systemPrompt: await this.getSystemPrompt(), }; - return prompt; + return prompt; } - protected override async getPrompt( + public override async getPrompt( data: CopilotProcess, ): Promise { - - const fileLanguage: ServiceLanguage = data.input.files[data.input.currentFilePath]?.fileLanguage as ServiceLanguage; - const code: string = data.input.files[data.input.currentFilePath]?.fileContent as string; + const fileLanguage: ServiceLanguage = data.input.files[ + data.input.currentFilePath + ]?.fileLanguage as ServiceLanguage; + const code: string = data.input.files[data.input.currentFilePath] + ?.fileContent as string; const prompt: string = `Please improve the comments in this code. Please only comment code that is hard to understand. @@ -125,8 +154,7 @@ export default class ImproveComments extends CopilotActionBase { return systemPrompt; } - - public async cleanup(data: CopilotProcess): Promise { + public async cleanup(code: string): Promise { // this code contains text as well. The code is in betwen ``` and ```. Please extract the code and return it. // for example code can be in the format of // ```python @@ -137,30 +165,16 @@ export default class ImproveComments extends CopilotActionBase { // the code can be in multiple lines as well. - const actionResult: CopilotActionRunResult = data.result; + let extractedCode: string = code; // this is the code in the file - for (const filePath in actionResult.files) { - // check all the files which were modified by the copilot action - - const file: CodeRepositoryFile | undefined = actionResult.files[filePath]; - - if (!file) { - continue; - } - - const extractedCode: string = file.fileContent; // this is the code in the file - - if (!extractedCode.includes("```")) { - actionResult.files[filePath]!.fileContent = extractedCode; - } - - actionResult.files[filePath]!.fileContent = - extractedCode.match(/```.*\n([\s\S]*?)```/)?.[1] ?? ""; + if (!extractedCode.includes("```")) { + return extractedCode; } - return { - input: data.input, - result: actionResult, - }; + extractedCode = + extractedCode.match(/```.*\n([\s\S]*?)```/)?.[1] ?? ""; + + + return extractedCode; } } diff --git a/Copilot/Service/CopilotActions/ImproveVariableNames.ts b/Copilot/Service/CopilotActions/ImproveVariableNames.ts index 2de603c8cd..69320ac6da 100644 --- a/Copilot/Service/CopilotActions/ImproveVariableNames.ts +++ b/Copilot/Service/CopilotActions/ImproveVariableNames.ts @@ -32,7 +32,7 @@ export default class ImproveVariableNames extends CopilotActionBase { return { ...data, result: finalResult }; } - protected override async getPrompt( + public override async getPrompt( _data: CopilotProcess, ): Promise { const prompt: string = `Please improve this readme. diff --git a/Copilot/Service/CopilotActions/ImroveReadme.ts b/Copilot/Service/CopilotActions/ImroveReadme.ts index 790e6ca0ee..b8dee39a8e 100644 --- a/Copilot/Service/CopilotActions/ImroveReadme.ts +++ b/Copilot/Service/CopilotActions/ImroveReadme.ts @@ -1,7 +1,6 @@ import CopilotActionType from "Common/Types/Copilot/CopilotActionType"; import CopilotActionBase, { CopilotActionPrompt, - CopilotActionRunResult, CopilotProcess, } from "./CopilotActionsBase"; import CodeRepositoryUtil from "../../Utils/CodeRepository"; @@ -14,28 +13,7 @@ export default class ImproveReadme extends CopilotActionBase { }); } - public override async filterNoOperation( - data: CopilotProcess, - ): Promise { - const finalResult: CopilotActionRunResult = { - files: {}, - }; - - for (const filePath in data.result.files) { - if (data.result.files[filePath]?.fileContent.includes("--all-good--")) { - continue; - } - - finalResult.files[filePath] = data.result.files[filePath]!; - } - - return { - ...data, - result: finalResult, - }; - } - - protected override async getPrompt( + public override async getPrompt( _data: CopilotProcess, ): Promise { const prompt: string = `Please improve this readme. diff --git a/Copilot/Service/CopilotActions/Index.ts b/Copilot/Service/CopilotActions/Index.ts index 4831a83a86..ee0212df62 100644 --- a/Copilot/Service/CopilotActions/Index.ts +++ b/Copilot/Service/CopilotActions/Index.ts @@ -41,7 +41,7 @@ export default class CopilotActionService { public static async execute(data: { serviceRepository: ServiceRepository; copilotActionType: CopilotActionType; - vars: CopilotActionVars; + input: CopilotActionVars; }): Promise { await CodeRepositoryUtil.switchToMainBranch(); @@ -140,8 +140,8 @@ export default class CopilotActionService { pullRequest = await CodeRepositoryUtil.createPullRequest({ branchName: branchName, serviceRepository: data.serviceRepository, - title: await action.getPullRequestTitle({ vars: data.input }), - body: await action.getPullRequestBody({ vars: data.input }), + title: await action.getPullRequestTitle(processResult), + body: await action.getPullRequestBody(processResult), }); // switch to main branch. @@ -166,7 +166,7 @@ export default class CopilotActionService { } const fileCommitHash: string | undefined = - data.input.serviceFiles[data.input.filePath]?.gitCommitHash; + data.input.files[data.input.currentFilePath]?.gitCommitHash; if (!fileCommitHash) { throw new BadDataException("File commit hash not found"); @@ -175,7 +175,7 @@ export default class CopilotActionService { await CopilotActionService.addCopilotAction({ serviceCatalogId: data.serviceRepository.serviceCatalog!.id!, serviceRepositoryId: data.serviceRepository.id!, - filePath: data.input.filePath, + filePath: data.input.currentFilePath, commitHash: fileCommitHash, copilotActionType: data.copilotActionType, pullRequest: pullRequest, diff --git a/Copilot/Service/CopilotActions/RefactorCode.ts b/Copilot/Service/CopilotActions/RefactorCode.ts index 7486fac9f9..a248117a4b 100644 --- a/Copilot/Service/CopilotActions/RefactorCode.ts +++ b/Copilot/Service/CopilotActions/RefactorCode.ts @@ -35,7 +35,7 @@ export default class RefactorCode extends CopilotActionBase { }; } - protected override async getPrompt( + public override async getPrompt( _data: CopilotProcess, ): Promise { const prompt: string = `Please refactor this code into smaller functions/methods if its not refactored properly. diff --git a/Copilot/Service/CopilotActions/WriteUnitTests.ts b/Copilot/Service/CopilotActions/WriteUnitTests.ts index 3f45dbd7f2..120465a115 100644 --- a/Copilot/Service/CopilotActions/WriteUnitTests.ts +++ b/Copilot/Service/CopilotActions/WriteUnitTests.ts @@ -1,9 +1,5 @@ import CopilotActionType from "Common/Types/Copilot/CopilotActionType"; -import CopilotActionBase, { - CopilotActionPrompt, - CopilotActionRunResult, - CopilotProcess, -} from "./CopilotActionsBase"; +import CopilotActionBase, { CopilotActionPrompt } from "./CopilotActionsBase"; import CodeRepositoryUtil from "../../Utils/CodeRepository"; export default class WriteUnitTests extends CopilotActionBase { @@ -14,28 +10,7 @@ export default class WriteUnitTests extends CopilotActionBase { }); } - public override async filterNoOperation( - data: CopilotProcess, - ): Promise { - const finalResult: CopilotActionRunResult = { - files: {}, - }; - - for (const filePath in data.result.files) { - if (data.result.files[filePath]?.fileContent.includes("--all-good--")) { - continue; - } - - finalResult.files[filePath] = data.result.files[filePath]!; - } - - return { - ...data, - result: finalResult, - }; - } - - protected override async getPrompt(): Promise { + public override async getPrompt(): Promise { const prompt: string = `Write unit tests for this file. Here is the code. This is in {{fileLanguage}}: