From 83381131806f38110cec75a8df208948bfb91264 Mon Sep 17 00:00:00 2001 From: Ramin Tadayon Date: Wed, 10 Sep 2025 14:14:45 -0700 Subject: [PATCH] Stores hunks and safety state on webview host, uses for ai operations and safety checks --- .../apps/plus/composer/components/app.ts | 18 +---- .../apps/plus/composer/stateProvider.ts | 2 - src/webviews/plus/composer/composerWebview.ts | 77 +++++++++++++------ src/webviews/plus/composer/mockData.ts | 16 +--- src/webviews/plus/composer/protocol.ts | 43 ++--------- src/webviews/plus/composer/utils.ts | 45 ++++------- 6 files changed, 81 insertions(+), 120 deletions(-) diff --git a/src/webviews/apps/plus/composer/components/app.ts b/src/webviews/apps/plus/composer/components/app.ts index 26dd71ed431f8..2ea6289ede40a 100644 --- a/src/webviews/apps/plus/composer/components/app.ts +++ b/src/webviews/apps/plus/composer/components/app.ts @@ -27,7 +27,7 @@ import { OpenOnboardingCommand, ReloadComposerCommand, } from '../../../../plus/composer/protocol'; -import { createCombinedDiffForCommit, updateHunkAssignments } from '../../../../plus/composer/utils'; +import { updateHunkAssignments } from '../../../../plus/composer/utils'; import type { RepoButtonGroupClickEvent } from '../../../shared/components/repo-button-group'; import { focusableBaseStyles } from '../../../shared/components/styles/lit/a11y.css'; import { boxSizingBase } from '../../../shared/components/styles/lit/base.css'; @@ -1271,9 +1271,7 @@ export class ComposerApp extends LitElement { private finishAndCommit() { this._ipc.sendCommand(FinishAndCommitCommand, { commits: this.state.commits, - hunks: this.hunksWithAssignments, baseCommit: this.state.baseCommit, - safetyState: this.state.safetyState, }); } @@ -1288,7 +1286,6 @@ export class ComposerApp extends LitElement { private handleReloadComposer() { this.resetHistory(); this._ipc.sendCommand(ReloadComposerCommand, { - repoPath: this.state.safetyState.repoPath, mode: this.state.mode, }); } @@ -1480,11 +1477,8 @@ export class ComposerApp extends LitElement { this.saveToHistory(); this._ipc.sendCommand(GenerateCommitsCommand, { - hunks: hunksToGenerate, - // In preview mode, send empty commits array to overwrite existing commits - // In interactive mode, send existing commits to preserve them + hunkIndices: hunksToGenerate.map(hunk => hunk.index), commits: this.isPreviewMode ? [] : this.state.commits, - hunkMap: this.state.hunkMap, baseCommit: this.state.baseCommit, customInstructions: customInstructions || undefined, }); @@ -1499,15 +1493,9 @@ export class ComposerApp extends LitElement { return; } - // Create combined diff for the commit - const { patch } = createCombinedDiffForCommit(commit, this.hunksWithAssignments); - if (!patch) { - return; - } - this._ipc.sendCommand(GenerateCommitMessageCommand, { commitId: commitId, - diff: patch, + commitHunkIndices: commit.hunkIndices, overwriteExistingMessage: commit.message.trim() !== '', }); } diff --git a/src/webviews/apps/plus/composer/stateProvider.ts b/src/webviews/apps/plus/composer/stateProvider.ts index 6e45c2d34a55a..6f37a2b8d6f8a 100644 --- a/src/webviews/apps/plus/composer/stateProvider.ts +++ b/src/webviews/apps/plus/composer/stateProvider.ts @@ -136,9 +136,7 @@ export class ComposerStateProvider implements StateProvider { ...this._state, hunks: msg.params.hunks, commits: msg.params.commits, - hunkMap: msg.params.hunkMap, baseCommit: msg.params.baseCommit, - safetyState: msg.params.safetyState, loadingError: msg.params.loadingError, hasChanges: msg.params.hasChanges, safetyError: null, // Clear any existing safety errors diff --git a/src/webviews/plus/composer/composerWebview.ts b/src/webviews/plus/composer/composerWebview.ts index 00a89ad1f4d60..dd8c486181660 100644 --- a/src/webviews/plus/composer/composerWebview.ts +++ b/src/webviews/plus/composer/composerWebview.ts @@ -25,7 +25,9 @@ import type { ComposerContext, ComposerGenerateCommitMessageEventData, ComposerGenerateCommitsEventData, + ComposerHunk, ComposerLoadedErrorData, + ComposerSafetyState, ComposerTelemetryEvent, FinishAndCommitParams, GenerateCommitMessageParams, @@ -78,6 +80,7 @@ import { import type { ComposerWebviewShowingArgs } from './registration'; import { convertToComposerDiffInfo, + createCombinedDiffForCommit, createHunksFromDiffs, createSafetyState, getWorkingTreeDiffs, @@ -98,6 +101,10 @@ export class ComposerWebviewProvider implements WebviewProvider ({ - index: hunk.index, - fileName: hunk.fileName, - diffHeader: hunk.diffHeader || `diff --git a/${hunk.fileName} b/${hunk.fileName}`, - hunkHeader: hunk.hunkHeader, - content: hunk.content, - source: hunk.source, - })); + const hunks = []; + for (const index of params.hunkIndices) { + hunks.push({ ...this._hunks.find(m => m.index === index)!, assigned: true }); + } const existingCommits = params.commits.map(commit => ({ id: commit.id, @@ -808,7 +818,7 @@ export class ComposerWebviewProvider implements WebviewProvider ({ index: m.index, hunkHeader: m.hunkHeader })), { source: 'composer', correlationId: this.host.instanceId }, { cancellation: this._generateCommitsCancellation.token, @@ -942,9 +952,28 @@ export class ComposerWebviewProvider implements WebviewProvider params.commitHunkIndices.includes(h.index)), + ); + if (!patch) { + this._context.operations.generateCommitMessage.errorCount++; + this._context.errors.operation.count++; + // Send error notification for failure (not cancellation) + this.sendTelemetryEvent('composer/action/generateCommitMessage/failed', { + ...eventData, + 'failure.reason': 'error', + 'failure.error.message': 'Failed to create diff for commit', + }); + await this.host.notify(DidErrorAIOperationNotification, { + operation: 'generate commit message', + error: 'Failed to create diff for commit', + }); + } + // Call the AI service to generate commit message const result = await this.container.ai.generateCommitMessage( - params.diff, + patch, { source: 'composer', correlationId: this.host.instanceId }, { cancellation: this._generateCommitMessageCancellation.token, @@ -1032,7 +1061,7 @@ export class ComposerWebviewProvider implements WebviewProvider + const commitHunkIndices = params.commits.flatMap(c => c.hunkIndices); + const hunks: ComposerHunk[] = []; + for (const hunk of commitHunkIndices) { + hunks.push({ ...this._hunks.find(m => m.index === hunk)!, assigned: true }); + } + + const hunksBeingCommitted = hunks.filter(hunk => params.commits.some(c => c.hunkIndices.includes(hunk.index)), ); // Validate repository safety state before proceeding - const validation = await validateSafetyState(repo, params.safetyState, hunksBeingCommitted); + const validation = await validateSafetyState(repo, this._safetyState, hunksBeingCommitted); if (!validation.isValid) { // Clear loading state and show safety error await this.host.notify(DidFinishCommittingNotification, undefined); @@ -1073,8 +1107,7 @@ export class ComposerWebviewProvider implements WebviewProvider = { hunks: [], commits: [], - hunkMap: [], baseCommit: { sha: '', message: '', repoName: '', branchName: '', }, - safetyState: { - repoPath: '', - headSha: '', - hashes: { - staged: null, - unstaged: null, - unified: null, - }, - }, selectedCommitId: null, selectedCommitIds: new Set(), selectedUnassignedSection: null, @@ -404,7 +390,6 @@ export const OnRedoCommand = new IpcCommand(ipcScope, 'onRedo'); export const OnResetCommand = new IpcCommand(ipcScope, 'onReset'); // Notifications sent from host to webview -export const DidChangeNotification = new IpcNotification(ipcScope, 'didChange'); export const DidStartGeneratingNotification = new IpcNotification(ipcScope, 'didStartGenerating'); export const DidStartGeneratingCommitMessageNotification = new IpcNotification<{ commitId: string }>( ipcScope, @@ -450,9 +435,8 @@ export const DidChangeAiModelNotification = new IpcNotification commit.hunkIndices.includes(hunk.index)); @@ -82,22 +82,17 @@ export function getFileChanges(hunks: ComposerHunk[]): { additions: number; dele ); } -export function createCombinedDiffForCommit( - commit: ComposerCommit, - hunks: ComposerHunk[], -): { patch: string; filePatches: Map } { - // Get hunks for this commit - const commitHunks = commit.hunkIndices - .map(index => hunks.find(hunk => hunk.index === index)) - .filter((hunk): hunk is ComposerHunk => hunk !== undefined); - - if (commitHunks.length === 0) { +export function createCombinedDiffForCommit(hunks: ComposerHunk[]): { + patch: string; + filePatches: Map; +} { + if (hunks.length === 0) { return { patch: '', filePatches: new Map() }; } // Group hunks by file (diffHeader) const filePatches = new Map(); - for (const hunk of commitHunks) { + for (const hunk of hunks) { const diffHeader = hunk.diffHeader || `diff --git a/${hunk.fileName} b/${hunk.fileName}`; let array = filePatches.get(diffHeader); @@ -137,7 +132,7 @@ export function convertToComposerDiffInfo( hunks: ComposerHunk[], ): Array<{ message: string; explanation?: string; filePatches: Map; patch: string }> { return commits.map(commit => { - const { patch, filePatches } = createCombinedDiffForCommit(commit, hunks); + const { patch, filePatches } = createCombinedDiffForCommit(getHunksForCommit(commit, hunks)); return { message: commit.message, @@ -174,34 +169,27 @@ export function generateComposerMarkdown( return markdown; } -export function createHunksFromDiffs( - stagedDiffContent?: string, - unstagedDiffContent?: string, -): { hunkMap: ComposerHunkMap[]; hunks: ComposerHunk[] } { - const allHunkMaps: ComposerHunkMap[] = []; +export function createHunksFromDiffs(stagedDiffContent?: string, unstagedDiffContent?: string): ComposerHunk[] { const allHunks: ComposerHunk[] = []; let count = 0; - let hunkMap: ComposerHunkMap[] = []; let hunks: ComposerHunk[] = []; if (stagedDiffContent) { const stagedDiff = parseGitDiff(stagedDiffContent); - ({ hunkMap, hunks, count } = convertDiffToComposerHunks(stagedDiff, 'staged', count)); + ({ hunks, count } = convertDiffToComposerHunks(stagedDiff, 'staged', count)); - allHunkMaps.push(...hunkMap); allHunks.push(...hunks); } if (unstagedDiffContent) { const unstagedDiff = parseGitDiff(unstagedDiffContent); - ({ hunkMap, hunks, count } = convertDiffToComposerHunks(unstagedDiff, 'unstaged', count)); + ({ hunks, count } = convertDiffToComposerHunks(unstagedDiff, 'unstaged', count)); - allHunkMaps.push(...hunkMap); allHunks.push(...hunks); } - return { hunkMap: allHunkMaps, hunks: allHunks }; + return allHunks; } /** Converts @type {ParsedGitDiff} output to @type {ComposerHunk}'s */ @@ -209,8 +197,7 @@ function convertDiffToComposerHunks( diff: ParsedGitDiff, source: 'staged' | 'unstaged', startingCount: number, -): { hunkMap: ComposerHunkMap[]; hunks: ComposerHunk[]; count: number } { - const hunkMap: ComposerHunkMap[] = []; +): { hunks: ComposerHunk[]; count: number } { const hunks: ComposerHunk[] = []; let counter = startingCount; @@ -238,8 +225,6 @@ function convertDiffToComposerHunks( content = file.header.split('\n').slice(1).join('\n'); // Skip the diff --git line } - hunkMap.push({ index: hunkIndex, hunkHeader: hunkHeader }); - const composerHunk: ComposerHunk = { index: hunkIndex, fileName: file.path, @@ -260,8 +245,6 @@ function convertDiffToComposerHunks( for (const hunk of file.hunks) { const hunkIndex = ++counter; - hunkMap.push({ index: hunkIndex, hunkHeader: hunk.header }); - // Calculate additions and deletions from the hunk content const { additions, deletions } = calculateHunkStats(hunk.content); @@ -284,7 +267,7 @@ function convertDiffToComposerHunks( } } - return { hunkMap: hunkMap, hunks: hunks, count: counter }; + return { hunks: hunks, count: counter }; } function calculateHunkStats(content: string): { additions: number; deletions: number } {