diff --git a/CHANGELOG.md b/CHANGELOG.md index 87850f1353f63..13bc5f0f69a35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p - Fixes editing search result in Search & Compare view failure ([#4431](https://github.com/gitkraken/vscode-gitlens/issues/4431)) - Fixes search results not paging properly on the _Commit Graph_ when the first page of results is contained within the already loaded commits +- Improves experience for invalid AI rebase responses by implementing conversational retry logic that provides specific feedback to the AI about missing, extra, or duplicate hunks and automatically retries up to 3 times ([#4395](https://github.com/gitkraken/vscode-gitlens/issues/4395)) ## [17.2.1] - 2025-06-26 diff --git a/src/plus/ai/aiProviderService.ts b/src/plus/ai/aiProviderService.ts index 5b82dab9192dd..0f3cd412489c1 100644 --- a/src/plus/ai/aiProviderService.ts +++ b/src/plus/ai/aiProviderService.ts @@ -71,6 +71,51 @@ import type { AIChatMessage, AIProvider, AIRequestResult } from './models/provid import { ensureAccess, getOrgAIConfig, isProviderEnabledByOrg } from './utils/-webview/ai.utils'; import { getLocalPromptTemplate, resolvePrompt } from './utils/-webview/prompt.utils'; +/** + * Removes common leading whitespace from each line in a template string. + * This allows you to write indented template strings but have them trimmed in the result. + * + * @param template The template string to dedent + * @returns The dedented string + * + * @example + * ```typescript + * const str = dedent(` + * Hello + * World + * Test + * `); + * // Result: "Hello\nWorld\nTest" + * ``` + */ +function dedent(template: string): string { + const lines = template.split('\n'); + + // Remove leading and trailing empty lines + while (lines.length > 0 && lines[0].trim() === '') { + lines.shift(); + } + while (lines.length > 0 && lines[lines.length - 1].trim() === '') { + lines.pop(); + } + + if (lines.length === 0) return ''; + + // Find the minimum indentation (excluding empty lines) + const nonEmptyLines = lines.filter(line => line.trim() !== ''); + if (nonEmptyLines.length === 0) return ''; + + const minIndent = Math.min( + ...nonEmptyLines.map(line => { + const match = line.match(/^(\s*)/); + return match ? match[1].length : 0; + }), + ); + + // Remove the common indentation from all lines + return lines.map(line => line.slice(minIndent)).join('\n'); +} + export interface AIResult { readonly id?: string; readonly content: string; @@ -936,6 +981,18 @@ export class AIProviderService implements Disposable { return result === 'cancelled' ? result : result != null ? { ...result } : undefined; } + /** + * Generates a rebase using AI to organize code changes into logical commits. + * + * This method includes automatic retry logic that validates the AI response and + * continues the conversation if the response has issues like: + * - Missing hunks that were in the original diff + * - Extra hunks that weren't in the original diff + * - Duplicate hunks used multiple times + * + * The method will retry up to 3 times, providing specific feedback to the AI + * about what was wrong with the previous response. + */ async generateRebase( repo: Repository, baseRef: string, @@ -985,6 +1042,121 @@ export class AIProviderService implements Disposable { } } + const rq = await this.sendRebaseRequestWithRetry(repo, baseRef, headRef, source, result, options); + + if (rq === 'cancelled') return rq; + + if (rq == null) return undefined; + + return { + ...rq, + ...result, + }; + } + + private async sendRebaseRequestWithRetry( + repo: Repository, + baseRef: string, + headRef: string, + source: Source, + result: Mutable, + options?: { + cancellation?: CancellationToken; + context?: string; + generating?: Deferred; + progress?: ProgressOptions; + generateCommits?: boolean; + }, + ): Promise { + let conversationMessages: AIChatMessage[] = []; + let attempt = 0; + const maxAttempts = 4; + + // First attempt - setup diff and hunk map + const firstAttemptResult = await this.sendRebaseFirstAttempt(repo, baseRef, headRef, source, result, options); + + if (firstAttemptResult === 'cancelled' || firstAttemptResult == null) { + return firstAttemptResult; + } + + conversationMessages = firstAttemptResult.conversationMessages; + let rq = firstAttemptResult.response; + + while (attempt < maxAttempts) { + const validationResult = this.validateRebaseResponse(rq, result.hunkMap, options); + if (validationResult.isValid) { + result.commits = validationResult.commits; + return rq; + } + + Logger.warn( + undefined, + 'AIProviderService', + 'sendRebaseRequestWithRetry', + `Validation failed on attempt ${attempt + 1}: ${validationResult.errorMessage}`, + ); + + // If this was the last attempt, throw the error + if (attempt === maxAttempts - 1) { + throw new Error(validationResult.errorMessage); + } + + // Prepare retry message for conversation + conversationMessages.push( + { role: 'assistant', content: rq.content }, + { role: 'user', content: validationResult.retryPrompt }, + ); + + attempt++; + + // Send retry request + const currentAttempt = attempt; + const retryResult = await this.sendRequest( + 'generate-rebase', + async () => Promise.resolve(conversationMessages), + m => + `Generating ${options?.generateCommits ? 'commits' : 'rebase'} with ${m.name}... (attempt ${ + currentAttempt + 1 + })`, + source, + m => ({ + key: 'ai/generate', + data: { + type: 'rebase', + 'model.id': m.id, + 'model.provider.id': m.provider.id, + 'model.provider.name': m.provider.name, + 'retry.count': currentAttempt, + }, + }), + options, + ); + + if (retryResult === 'cancelled' || retryResult == null) { + return retryResult; + } + + rq = retryResult; + } + + return undefined; + } + + private async sendRebaseFirstAttempt( + repo: Repository, + baseRef: string, + headRef: string, + source: Source, + result: Mutable, + options?: { + cancellation?: CancellationToken; + context?: string; + generating?: Deferred; + progress?: ProgressOptions; + generateCommits?: boolean; + }, + ): Promise<{ response: AIRequestResult; conversationMessages: AIChatMessage[] } | 'cancelled' | undefined> { + let storedPrompt = ''; const rq = await this.sendRequest( 'generate-rebase', async (model, reporting, cancellation, maxInputTokens, retries) => { @@ -1041,6 +1213,9 @@ export class AIProviderService implements Disposable { ); if (cancellation.isCancellationRequested) throw new CancellationError(); + // Store the prompt for later use in conversation messages + storedPrompt = prompt; + const messages: AIChatMessage[] = [{ role: 'user', content: prompt }]; return messages; }, @@ -1063,47 +1238,143 @@ export class AIProviderService implements Disposable { if (rq == null) return undefined; + return { + response: rq, + conversationMessages: [{ role: 'user', content: storedPrompt }], + }; + } + + private validateRebaseResponse( + rq: AIRequestResult, + inputHunkMap: { index: number; hunkHeader: string }[], + options?: { + generateCommits?: boolean; + }, + ): + | { isValid: false; errorMessage: string; retryPrompt: string } + | { isValid: true; commits: AIRebaseResult['commits'] } { + // if it is wrapped in markdown, we need to strip it + const content = rq.content.replace(/^\s*```json\s*/, '').replace(/\s*```$/, ''); + + let commits: AIRebaseResult['commits']; try { - // if it is wrapped in markdown, we need to strip it - const content = rq.content.replace(/^\s*```json\s*/, '').replace(/\s*```$/, ''); // Parse the JSON content from the result - result.commits = JSON.parse(content) as AIRebaseResult['commits']; + commits = JSON.parse(content) as AIRebaseResult['commits']; + } catch { + const errorMessage = `Unable to parse ${options?.generateCommits ? 'commits' : 'rebase'} result`; + const retryPrompt = dedent(` + Your previous response could not be parsed as valid JSON. Please ensure your response is a valid JSON array of commits with the correct structure. + Don't include any preceeding or succeeding text or markup, such as "Here are the commits:" or "Here is a valid JSON array of commits:". + + Here was your previous response: + ${rq.content} - const inputHunkIndices = result.hunkMap.map(h => h.index); - const outputHunkIndices = new Set(result.commits.flatMap(c => c.hunks.map(h => h.hunk))); + Please provide a valid JSON array of commits following this structure: + [ + { + "message": "commit message", + "explanation": "detailed explanation", + "hunks": [{"hunk": 1}, {"hunk": 2}] + } + ] + `); + + return { + isValid: false, + errorMessage: errorMessage, + retryPrompt: retryPrompt, + }; + } - // Find any missing or extra hunks + // Validate the structure and hunk assignments + try { + const inputHunkIndices = inputHunkMap.map(h => h.index); + const allOutputHunks = commits.flatMap(c => c.hunks.map(h => h.hunk)); + const outputHunkIndices = new Map(allOutputHunks.map((hunk, index) => [hunk, index])); const missingHunks = inputHunkIndices.filter(i => !outputHunkIndices.has(i)); - const extraHunks = [...outputHunkIndices].filter(i => !inputHunkIndices.includes(i)); - if (missingHunks.length > 0 || extraHunks.length > 0) { - let hunksMessage = ''; + + if (missingHunks.length > 0 || allOutputHunks.length > inputHunkIndices.length) { + const errorParts: string[] = []; + const retryParts: string[] = []; + if (missingHunks.length > 0) { const pluralize = missingHunks.length > 1 ? 's' : ''; - hunksMessage += ` ${missingHunks.length} missing hunk${pluralize}.`; + errorParts.push(`${missingHunks.length} missing hunk${pluralize}`); + retryParts.push(`You missed hunk${pluralize} ${missingHunks.join(', ')} in your response`); } + const extraHunks = [...outputHunkIndices.keys()].filter(i => !inputHunkIndices.includes(i)); if (extraHunks.length > 0) { const pluralize = extraHunks.length > 1 ? 's' : ''; - hunksMessage += ` ${extraHunks.length} extra hunk${pluralize}.`; + errorParts.push(`${extraHunks.length} extra hunk${pluralize}`); + retryParts.push( + `You included hunk${pluralize} ${extraHunks.join(', ')} which ${ + extraHunks.length > 1 ? 'were' : 'was' + } not in the original diff`, + ); + } + const duplicateHunks = allOutputHunks.filter((hunk, index) => outputHunkIndices.get(hunk)! !== index); + const uniqueDuplicates = [...new Set(duplicateHunks)]; + if (uniqueDuplicates.length > 0) { + const pluralize = uniqueDuplicates.length > 1 ? 's' : ''; + errorParts.push(`${uniqueDuplicates.length} duplicate hunk${pluralize}`); + retryParts.push(`You used hunk${pluralize} ${uniqueDuplicates.join(', ')} multiple times`); } - throw new Error( - `Invalid response in generating ${ - options?.generateCommits ? 'commits' : 'rebase' - } result.${hunksMessage} Try again or select a different AI model.`, - ); - } - } catch (ex) { - debugger; - if (ex?.message?.includes('Invalid response in generating')) { - throw ex; + const errorMessage = `Invalid response in generating ${ + options?.generateCommits ? 'commits' : 'rebase' + } result. ${errorParts.join(', ')}.`; + + const retryPrompt = dedent(` + Your previous response had issues: ${retryParts.join(', ')}. + + Please provide a corrected JSON response that: + 1. Includes ALL hunks from 1 to ${Math.max(...inputHunkIndices)} exactly once + 2. Does not include any hunk numbers outside this range + 3. Does not use any hunk more than once + + Here was your previous response: + ${rq.content} + + Please provide the corrected JSON array of commits. + Don't include any preceeding or succeeding text or markup, such as "Here are the commits:" or "Here is a valid JSON array of commits:". + `); + + return { + isValid: false, + errorMessage: errorMessage, + retryPrompt: retryPrompt, + }; } - throw new Error(`Unable to parse ${options?.generateCommits ? 'commits' : 'rebase'} result`); - } - return { - ...rq, - ...result, - }; + // If validation passes, return the commits + return { isValid: true, commits: commits }; + } catch { + // Handle any errors during hunk validation (e.g., malformed commit structure) + const errorMessage = `Invalid commit structure in ${ + options?.generateCommits ? 'commits' : 'rebase' + } result`; + const retryPrompt = dedent(` + Your previous response has an invalid commit structure. Each commit must have "message", "explanation", and "hunks" properties, where "hunks" is an array of objects with "hunk" numbers. + + Here was your previous response: + ${rq.content} + + Please provide a valid JSON array of commits following this structure: + [ + { + "message": "commit message", + "explanation": "detailed explanation", + "hunks": [{"hunk": 1}, {"hunk": 2}] + } + ] + `); + + return { + isValid: false, + errorMessage: errorMessage, + retryPrompt: retryPrompt, + }; + } } private async sendRequest( @@ -1609,22 +1880,32 @@ export class AIProviderService implements Disposable { const alreadyCompleted = this.container.storage.get(`gk:promo:${userId}:ai:allAccess:dismissed`, false); if (notificationShown || alreadyCompleted) return; - const hasAdvancedOrHigher = subscription.plan && + const hasAdvancedOrHigher = + subscription.plan && (compareSubscriptionPlans(subscription.plan.actual.id, 'advanced') >= 0 || - compareSubscriptionPlans(subscription.plan.effective.id, 'advanced') >= 0); + compareSubscriptionPlans(subscription.plan.effective.id, 'advanced') >= 0); let body = 'All Access Week - now until July 11th!'; - const detail = hasAdvancedOrHigher ? 'Opt in now to get unlimited GitKraken AI until July 11th!' : 'Opt in now to try all Advanced GitLens features with unlimited GitKraken AI for FREE until July 11th!'; + const detail = hasAdvancedOrHigher + ? 'Opt in now to get unlimited GitKraken AI until July 11th!' + : 'Opt in now to try all Advanced GitLens features with unlimited GitKraken AI for FREE until July 11th!'; if (!usingGkProvider) { body += ` ${detail}`; } - const optInButton: MessageItem = usingGkProvider ? { title: 'Opt in for Unlimited AI' } : { title: 'Opt in and Switch to GitKraken AI' }; + const optInButton: MessageItem = usingGkProvider + ? { title: 'Opt in for Unlimited AI' } + : { title: 'Opt in and Switch to GitKraken AI' }; const dismissButton: MessageItem = { title: 'No, Thanks', isCloseAffordance: true }; // Show the notification - const result = await window.showInformationMessage(body, { modal: usingGkProvider, detail: detail }, optInButton, dismissButton); + const result = await window.showInformationMessage( + body, + { modal: usingGkProvider, detail: detail }, + optInButton, + dismissButton, + ); // Mark notification as shown regardless of user action void this.container.storage.store(`gk:promo:${userId}:ai:allAccess:notified`, true); @@ -1647,7 +1928,10 @@ export class AIProviderService implements Disposable { await configuration.updateEffective('ai.model', 'gitkraken'); await configuration.updateEffective(`ai.gitkraken.model`, defaultModel.id); } else { - await configuration.updateEffective('ai.model', `gitkraken:${defaultModel.id}` as SupportedAIModels); + await configuration.updateEffective( + 'ai.model', + `gitkraken:${defaultModel.id}` as SupportedAIModels, + ); } this._onDidChangeModel.fire({ model: defaultModel }); @@ -1656,8 +1940,6 @@ export class AIProviderService implements Disposable { } } - - async function showConfirmAIProviderToS(storage: Storage): Promise { const confirmed = storage.get(`confirm:ai:tos`, false) || storage.getWorkspace(`confirm:ai:tos`, false); if (confirmed) return true; diff --git a/src/plus/ai/prompts.ts b/src/plus/ai/prompts.ts index 5d6dbf177c5bc..1ecdc49d84f7f 100644 --- a/src/plus/ai/prompts.ts +++ b/src/plus/ai/prompts.ts @@ -394,10 +394,11 @@ Your task is to group the hunks in unified_diff into a set of commits, ordered i 1. Only organize the hunks themselves, not individual lines within hunks. 2. Group hunks into logical units that make sense together and can be applied atomically. -3. Ensure each commit is self-contained and only depends on commits that come before it in the new history. -4. Write meaningful commit messages that accurately describe the changes in each commit. -5. Provide a detailed explanation of the changes in each commit. -6. Make sure the new commit history is easy to review and understand. +3. Use each hunk only once. Use all hunks. +4. Ensure each commit is self-contained and only depends on commits that come before it in the new history. +5. Write meaningful commit messages that accurately describe the changes in each commit. +6. Provide a detailed explanation of the changes in each commit. +7. Make sure the new commit history is easy to review and understand. Output your new commit history as a JSON array. Each commit in the array should be an object representing a grouping of hunks forming that commit, with the following properties: - "message": A string containing the commit message. @@ -428,5 +429,7 @@ Remember to base your organization of commits solely on the provided unified_dif \${instructions} -Now, proceed with your analysis and organization of the commits. Output only the JSON array containing the commits, and nothing else.`, +Now, proceed with your analysis and organization of the commits. Output only the JSON array containing the commits, and nothing else. +Do not include any preceeding or succeeding text or markup, such as "Here are the commits:" or "Here is a valid JSON array of commits:". +`, };