From b4c84e09e241ad53ad3bdb6d8711d8e415599381 Mon Sep 17 00:00:00 2001 From: Tomas Sebestik Date: Tue, 27 Jun 2023 10:04:04 +0200 Subject: [PATCH] ci(danger): Change commit message default rules - maximum length of commit message summary 72 characters (before 50) - drop rule for commit message summary to start with capital letter - AI generated commit message only for poor messages - updated version of pre-commit hook 'conventional-precommit-linter' - update prompt AI generated commit message --- .gitlab/dangerjs/aiGenerateGitMessage.js | 224 +++++++++++++-------- .gitlab/dangerjs/mrCommitsCommitMessage.js | 48 ++--- .gitlab/dangerjs/mrCommitsConstants.js | 16 ++ .pre-commit-config.yaml | 2 +- 4 files changed, 178 insertions(+), 112 deletions(-) create mode 100644 .gitlab/dangerjs/mrCommitsConstants.js diff --git a/.gitlab/dangerjs/aiGenerateGitMessage.js b/.gitlab/dangerjs/aiGenerateGitMessage.js index b2ded5b860..1335f7ec70 100644 --- a/.gitlab/dangerjs/aiGenerateGitMessage.js +++ b/.gitlab/dangerjs/aiGenerateGitMessage.js @@ -1,120 +1,170 @@ -const { OpenAI } = require("langchain/llms/openai"); -const { - ChatPromptTemplate, - SystemMessagePromptTemplate, -} = require("langchain/prompts"); +const { minimumSummaryChars } = require("./mrCommitsConstants.js"); +const { maximumSummaryChars } = require("./mrCommitsConstants.js"); +const { maximumBodyLineChars } = require("./mrCommitsConstants.js"); +const { allowedTypes } = require("./mrCommitsConstants.js"); +const { gptStandardModelTokens } = require("./mrCommitsConstants.js"); + +const { ChatPromptTemplate } = require("langchain/prompts"); +const { SystemMessagePromptTemplate } = require("langchain/prompts"); const { LLMChain } = require("langchain/chains"); const { ChatOpenAI } = require("langchain/chat_models/openai"); const openAiTokenCount = require("openai-gpt-token-counter"); -const mrModifiedFiles = danger.git.modified_files; -const mrCommits = danger.gitlab.commits; - module.exports = async function () { - let mrDiff = await getMrGitDiff(mrModifiedFiles); - const mrCommitMessages = getCommitMessages(mrCommits); - - // Init output message let outputDangerMessage = `\n\nPerhaps you could use an AI-generated suggestion for your commit message. Here is one `; - // Setup LLM prompt - const inputPrompt = `You are a helpful assistant that creates suggestions for single git commit message, that user can use to describe all the changes in their merge request. - Use git diff: {mrDiff} and users current commit messages: {mrCommitMessages} to get the changes made in the commit. - - Output should be git commit message following the conventional commit format. - - Output only git commit message in desired format, without comments and other text. - - Do not include lines with JIRA tickets mentions (e.g. "Closes JIRA-123") to the output. - - Avoid including temporary commit messages (e.g. "Cleanup", "Merged" or "wip: Test") to the output. - - Avoid using vague terms (e.g. "some checks", "add new ones", "few changes" ) in the output. - - - [EXAMPLE OUTPUT] - feat(scope): add support for component XXX - - - adds support for wifi6 - - adds validations for logging script - - - [EXAMPLE OUTPUT] - feat(scope): add support for component XXX - - - adds support for wifi6 - - adds validations for logging script - - Closes https://github.com/espressif/esp-idf/issues/1234 - - `; - - // Count input tokens for LLM - const mrCommitMessagesTokens = openAiTokenCount(mrCommitMessages.join(" ")); - const gitDiffTokens = openAiTokenCount(mrDiff); - const promptTokens = openAiTokenCount(inputPrompt); - - const inputLlmTokens = - mrCommitMessagesTokens + gitDiffTokens + promptTokens; - - console.log(`Input tokens for LLM: ${inputLlmTokens}`); - - if (inputLlmTokens < 4096) { - outputDangerMessage += `(based on your MR git-diff and your current commit messages):\n\n`; - } else { - outputDangerMessage += `(based only on your current commit messages, git-diff of this MR is too big (${inputLlmTokens} tokens) for the AI model):\n\n`; - mrDiff = ""; - } - - // Call LLM - const generatedCommitMessage = await createAiGitMessage( + let mrDiff = await getMrGitDiff(danger.git.modified_files); + const mrCommitMessages = getCommitMessages(danger.gitlab.commits); + const inputPrompt = getInputPrompt(); + const inputLlmTokens = getInputLlmTokens( inputPrompt, mrDiff, mrCommitMessages ); + console.log(`Input tokens for LLM: ${inputLlmTokens}`); - outputDangerMessage += "```\n" + generatedCommitMessage + "\n```\n"; // Add the generated git message, format to the markdown code block - outputDangerMessage += "\n**NOTE: AI-generated suggestions may not always be correct, please review the suggestion before using it.**" // Add disclaimer + if (inputLlmTokens >= gptStandardModelTokens) { + mrDiff = ""; // If the input mrDiff is larger than 16k model, don't use mrDiff, use only current commit messages + outputDangerMessage += `(based only on your current commit messages, git-diff of this MR is too big (${inputLlmTokens} tokens) for the AI models):\n\n`; + } else { + outputDangerMessage += `(based on your MR git-diff and your current commit messages):\n\n`; + } + + // Generate AI commit message + let generatedCommitMessage = ""; + try { + const rawCommitMessage = await createAiGitMessage( + inputPrompt, + mrDiff, + mrCommitMessages + ); + generatedCommitMessage = postProcessCommitMessage(rawCommitMessage); + } catch (error) { + console.error("Error in generating AI commit message: ", error); + outputDangerMessage += + "\nCould not generate commit message due to an error.\n"; + } + + // Append closing statements ("Closes https://github.com/espressif/esp-idf/issues/XXX") to the generated commit message + let closingStatements = extractClosingStatements(mrCommitMessages); + if (closingStatements.length > 0) { + generatedCommitMessage += "\n\n" + closingStatements; + } + + // Add the generated git message, format to the markdown code block + outputDangerMessage += `\n\`\`\`\n${generatedCommitMessage}\n\`\`\`\n`; + outputDangerMessage += + "\n**NOTE: AI-generated suggestions may not always be correct, please review the suggestion before using it.**"; // Add disclaimer return outputDangerMessage; }; async function getMrGitDiff(mrModifiedFiles) { - let mrDiff = ""; - for (const file of mrModifiedFiles) { - const fileDiff = await danger.git.diffForFile(file); - mrDiff += fileDiff.diff.trim(); - } - - return mrDiff; + const fileDiffs = await Promise.all( + mrModifiedFiles.map((file) => danger.git.diffForFile(file)) + ); + return fileDiffs.map((fileDiff) => fileDiff.diff.trim()).join(" "); } function getCommitMessages(mrCommits) { - let mrCommitMessages = []; - for (const commit of mrCommits) { - mrCommitMessages.push(commit.message); - } + return mrCommits.map((commit) => commit.message); +} - return mrCommitMessages; +function getInputPrompt() { + return `You are a helpful assistant that creates suggestions for single git commit message, that user can use to describe all the changes in their merge request. + Use git diff: {mrDiff} and users current commit messages: {mrCommitMessages} to get the changes made in the commit. + + Output should be git commit message following the conventional commit format. + + Output only git commit message in desired format, without comments and other text. + + Do not include the closing statements ("Closes https://....") in the output. + + Here are the strict rules you must follow: + + - Avoid mentioning any JIRA tickets (e.g., "Closes JIRA-123"). + - Be specific. Don't use vague terms (e.g., "some checks", "add new ones", "few changes"). + - The commit message structure should be: <(scope/component)>: + - Types allowed: ${allowedTypes.join(", ")} + - If 'scope/component' is used, it must start with a lowercase letter. + - The 'summary' must NOT end with a period. + - The 'summary' must be between ${minimumSummaryChars} and ${maximumSummaryChars} characters long. + + If a 'body' of commit message is used: + + - Each line must be no longer than ${maximumBodyLineChars} characters. + - It must be separated from the 'summary' by a blank line. + + Examples of correct commit messages: + + - With scope and body: + fix(freertos): Fix startup timeout issue + + This is a text of commit message body... + - adds support for wifi6 + - adds validations for logging script + + - Without scope and body: + ci: added target test job for ESP32-Wifi6`; +} + +function getInputLlmTokens(inputPrompt, mrDiff, mrCommitMessages) { + const mrCommitMessagesTokens = openAiTokenCount(mrCommitMessages.join(" ")); + const gitDiffTokens = openAiTokenCount(mrDiff); + const promptTokens = openAiTokenCount(inputPrompt); + return mrCommitMessagesTokens + gitDiffTokens + promptTokens; } async function createAiGitMessage(inputPrompt, mrDiff, mrCommitMessages) { - const chat = new ChatOpenAI({ - engine: "gpt-3.5-turbo", - temperature: 0, - }); - + const chat = new ChatOpenAI({ engine: "gpt-3.5-turbo", temperature: 0 }); const chatPrompt = ChatPromptTemplate.fromPromptMessages([ SystemMessagePromptTemplate.fromTemplate(inputPrompt), ]); - - const chain = new LLMChain({ - prompt: chatPrompt, - llm: chat, - }); + const chain = new LLMChain({ prompt: chatPrompt, llm: chat }); const response = await chain.call({ mrDiff: mrDiff, mrCommitMessages: mrCommitMessages, }); - return response.text; } + +function postProcessCommitMessage(rawCommitMessage) { + // Split the result into lines + let lines = rawCommitMessage.split("\n"); + + // Format each line + for (let i = 0; i < lines.length; i++) { + let line = lines[i].trim(); + + // If the line is longer than maximumBodyLineChars, split it into multiple lines + if (line.length > maximumBodyLineChars) { + let newLines = []; + while (line.length > maximumBodyLineChars) { + let lastSpaceIndex = line.lastIndexOf( + " ", + maximumBodyLineChars + ); + newLines.push(line.substring(0, lastSpaceIndex)); + line = line.substring(lastSpaceIndex + 1); + } + newLines.push(line); + lines[i] = newLines.join("\n"); + } + } + + // Join the lines back into a single string with a newline between each one + return lines.join("\n"); +} + +function extractClosingStatements(mrCommitMessages) { + let closingStatements = []; + mrCommitMessages.forEach((message) => { + const lines = message.split("\n"); + lines.forEach((line) => { + if (line.startsWith("Closes")) { + closingStatements.push(line); + } + }); + }); + return closingStatements.join("\n"); +} diff --git a/.gitlab/dangerjs/mrCommitsCommitMessage.js b/.gitlab/dangerjs/mrCommitsCommitMessage.js index 6173e9e537..a8aee53fb5 100644 --- a/.gitlab/dangerjs/mrCommitsCommitMessage.js +++ b/.gitlab/dangerjs/mrCommitsCommitMessage.js @@ -1,33 +1,25 @@ +const { minimumSummaryChars } = require("./mrCommitsConstants.js"); +const { maximumSummaryChars } = require("./mrCommitsConstants.js"); +const { maximumBodyLineChars } = require("./mrCommitsConstants.js"); +const { allowedTypes } = require("./mrCommitsConstants.js"); + /** - * Check that commit messages are based on the Espressif ESP-IDF project's internal rules for git commit messages. + * Check that commit messages are based on the Espressif ESP-IDF project's rules for git commit messages. * * @dangerjs WARN */ - module.exports = async function () { const mrCommits = danger.gitlab.commits; const lint = require("@commitlint/lint").default; - const allowedTypes = [ - "change", - "ci", - "docs", - "feat", - "fix", - "refactor", - "remove", - "revert", - ]; const lintingRules = { // rule definition: [(0-1 = off/on), (always/never = must be/mustn't be), (value)] - "body-max-line-length": [1, "always", 100], // Max length of the body line + "body-max-line-length": [1, "always", maximumBodyLineChars], // Max length of the body line "footer-leading-blank": [1, "always"], // Always have a blank line before the footer section - "footer-max-line-length": [1, "always", 100], // Max length of the footer line - "subject-max-length": [1, "always", 50], // Max length of the "Summary" - "subject-min-length": [1, "always", 20], // Min length of the "Summary" + "footer-max-line-length": [1, "always", maximumBodyLineChars], // Max length of the footer line + "subject-max-length": [1, "always", maximumSummaryChars], // Max length of the "Summary" + "subject-min-length": [1, "always", minimumSummaryChars], // Min length of the "Summary" "scope-case": [1, "always", "lower-case"], // "scope/component" must start with lower-case - // "scope-empty": [1, "never"], // "scope/component" is mandatory - "subject-case": [1, "always", ["sentence-case"]], // "Summary" must start with upper-case "subject-full-stop": [1, "never", "."], // "Summary" must not end with a full stop (period) "subject-empty": [1, "never"], // "Summary" is mandatory "type-case": [1, "always", "lower-case"], // "type/action" must start with lower-case @@ -36,6 +28,9 @@ module.exports = async function () { "body-leading-blank": [1, "always"], // Always have a blank line before the body section }; + // Switcher for AI suggestions (for poor messages) + let generateAISuggestion = false; + // Search for the messages in each commit let issuesAllCommitMessages = []; @@ -90,11 +85,13 @@ module.exports = async function () { break; case "subject-empty": issuesSingleCommitMessage.push(`- *summary* looks empty`); + generateAISuggestion = true; break; case "subject-min-length": issuesSingleCommitMessage.push( `- *summary* looks too short` ); + generateAISuggestion = true; break; case "subject-case": issuesSingleCommitMessage.push( @@ -131,8 +128,9 @@ module.exports = async function () { if (issuesAllCommitMessages.length) { issuesAllCommitMessages.sort(); const basicTips = [ - `- correct format of commit message should be: \`(): \`, for example \`fix(esp32): Fixed startup timeout issue\``, - `- sufficiently descriptive message summary should be between 20 to 50 characters and start with upper case letter`, + `- correct format of commit message should be: \`(): \`, for example \`fix(esp32): Fixed startup timeout issue\``, + `- allowed types are: \`${allowedTypes}\``, + `- sufficiently descriptive message summary should be between ${minimumSummaryChars} to ${maximumSummaryChars} characters and start with upper case letter`, `- avoid Jira references in commit messages (unavailable/irrelevant for our customers)`, `- follow this [commit messages guide](${process.env.DANGER_GITLAB_HOST}/espressif/esp-idf/-/wikis/dev-proc/Commit-messages)`, ]; @@ -146,10 +144,12 @@ module.exports = async function () { \n*** `; - // Create AI generated suggestion for git commit message based of gitDiff and current commit messages - const AImessageSuggestion = - await require("./aiGenerateGitMessage.js")(); - dangerMessage += AImessageSuggestion; + if (generateAISuggestion) { + // Create AI generated suggestion for git commit message based of gitDiff and current commit messages + const AImessageSuggestion = + await require("./aiGenerateGitMessage.js")(); + dangerMessage += AImessageSuggestion; + } warn(dangerMessage); } diff --git a/.gitlab/dangerjs/mrCommitsConstants.js b/.gitlab/dangerjs/mrCommitsConstants.js new file mode 100644 index 0000000000..373722465e --- /dev/null +++ b/.gitlab/dangerjs/mrCommitsConstants.js @@ -0,0 +1,16 @@ +module.exports = { + gptStandardModelTokens: 4096, + minimumSummaryChars: 20, + maximumSummaryChars: 72, + maximumBodyLineChars: 100, + allowedTypes: [ + "change", + "ci", + "docs", + "feat", + "fix", + "refactor", + "remove", + "revert", + ], +}; diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3272455d4a..88dd63d604 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -169,7 +169,7 @@ repos: - id: check-copyright args: ['--ignore', 'tools/ci/check_copyright_ignore.txt', '--config', 'tools/ci/check_copyright_config.yaml'] - repo: https://github.com/espressif/conventional-precommit-linter - rev: v1.0.0 + rev: v1.1.0 hooks: - id: conventional-precommit-linter stages: [commit-msg]