diff --git a/src/extension/prompt/node/repoInfoTelemetry.ts b/src/extension/prompt/node/repoInfoTelemetry.ts index 7ecd031e8f..3d118318ba 100644 --- a/src/extension/prompt/node/repoInfoTelemetry.ts +++ b/src/extension/prompt/node/repoInfoTelemetry.ts @@ -8,10 +8,35 @@ import { IFileSystemService } from '../../../platform/filesystem/common/fileSyst import { IGitDiffService } from '../../../platform/git/common/gitDiffService'; import { IGitExtensionService } from '../../../platform/git/common/gitExtensionService'; import { getOrderedRepoInfosFromContext, IGitService, normalizeFetchUrl } from '../../../platform/git/common/gitService'; +import { Change } from '../../../platform/git/vscode/git'; import { ILogService } from '../../../platform/log/common/logService'; import { ITelemetryService } from '../../../platform/telemetry/common/telemetry'; import { IWorkspaceFileIndex } from '../../../platform/workspaceChunkSearch/node/workspaceFileIndex'; +// Create a mapping for the git status enum to put the actual status string in telemetry +// The enum is a const enum and part of the public git extension API, so the order should stay stable +const STATUS_TO_STRING: Record = { + 0: 'INDEX_MODIFIED', + 1: 'INDEX_ADDED', + 2: 'INDEX_DELETED', + 3: 'INDEX_RENAMED', + 4: 'INDEX_COPIED', + 5: 'MODIFIED', + 6: 'DELETED', + 7: 'UNTRACKED', + 8: 'IGNORED', + 9: 'INTENT_TO_ADD', + 10: 'INTENT_TO_RENAME', + 11: 'TYPE_CHANGED', + 12: 'ADDED_BY_US', + 13: 'ADDED_BY_THEM', + 14: 'DELETED_BY_US', + 15: 'DELETED_BY_THEM', + 16: 'BOTH_ADDED', + 17: 'BOTH_DELETED', + 18: 'BOTH_MODIFIED', +}; + // Max telemetry payload size is 1MB, we add shared properties in further code and JSON structure overhead to that // so check our diff JSON size against 900KB to be conservative with space const MAX_DIFFS_JSON_SIZE = 900 * 1024; @@ -192,7 +217,29 @@ export class RepoInfoTelemetry { diffSizeBytes: 0, // Will be updated }; - const changes = await this._gitService.diffWith(repoContext.rootUri, upstreamCommit); + // Combine our diff against the upstream commit with untracked changes, and working tree changes + // A change like a new untracked file could end up in either the untracked or working tree changes and won't be in the diffWith. + const diffChanges = await this._gitService.diffWith(repoContext.rootUri, upstreamCommit) ?? []; + + const changeMap = new Map(); + + // Prority to the diffWith changes, then working tree changes, then untracked changes. + for (const change of diffChanges) { + changeMap.set(change.uri.toString(), change); + } + for (const change of repository.state.workingTreeChanges) { + if (!changeMap.has(change.uri.toString())) { + changeMap.set(change.uri.toString(), change); + } + } + for (const change of repository.state.untrackedChanges) { + if (!changeMap.has(change.uri.toString())) { + changeMap.set(change.uri.toString(), change); + } + } + + const changes = Array.from(changeMap.values()); + if (!changes || changes.length === 0) { return { properties: { ...baseProperties, diffsJSON: undefined, result: 'noChanges' }, @@ -217,12 +264,12 @@ export class RepoInfoTelemetry { }; } - const diffs = (await this._gitDiffService.getChangeDiffs(repoContext.rootUri, changes)).map(diff => { + const diffs = (await this._gitDiffService.getWorkingTreeDiffsFromRef(repoContext.rootUri, changes, upstreamCommit)).map(diff => { return { uri: diff.uri.toString(), originalUri: diff.originalUri.toString(), renameUri: diff.renameUri?.toString(), - status: diff.status, + status: STATUS_TO_STRING[diff.status] ?? `UNKNOWN_${diff.status}`, diff: diff.diff, }; }); diff --git a/src/extension/prompt/node/test/repoInfoTelemetry.spec.ts b/src/extension/prompt/node/test/repoInfoTelemetry.spec.ts index 37f4a83622..1387117900 100644 --- a/src/extension/prompt/node/test/repoInfoTelemetry.spec.ts +++ b/src/extension/prompt/node/test/repoInfoTelemetry.spec.ts @@ -733,7 +733,7 @@ suite('RepoInfoTelemetry', () => { }] as any); // Mock git diff service to trigger file change during processing - vi.spyOn(gitDiffService, 'getChangeDiffs').mockImplementation(async () => { + vi.spyOn(gitDiffService, 'getWorkingTreeDiffsFromRef').mockImplementation(async () => { // Simulate file change during diff processing mockWatcher.triggerChange(URI.file('/test/repo/file.ts') as any); return [{ @@ -846,7 +846,7 @@ suite('RepoInfoTelemetry', () => { // Create a diff that exceeds 900KB when serialized to JSON const largeDiff = 'x'.repeat(901 * 1024); - vi.spyOn(gitDiffService, 'getChangeDiffs').mockResolvedValue([{ + vi.spyOn(gitDiffService, 'getWorkingTreeDiffsFromRef').mockResolvedValue([{ uri: URI.file('/test/repo/file.ts'), originalUri: URI.file('/test/repo/file.ts'), renameUri: undefined, @@ -891,7 +891,7 @@ suite('RepoInfoTelemetry', () => { // Create a diff that is within limits const normalDiff = 'some normal diff content'; - vi.spyOn(gitDiffService, 'getChangeDiffs').mockResolvedValue([{ + vi.spyOn(gitDiffService, 'getWorkingTreeDiffsFromRef').mockResolvedValue([{ uri: URI.file('/test/repo/file.ts'), originalUri: URI.file('/test/repo/file.ts'), renameUri: undefined, @@ -950,7 +950,7 @@ suite('RepoInfoTelemetry', () => { } ] as any); - vi.spyOn(gitDiffService, 'getChangeDiffs').mockResolvedValue([ + vi.spyOn(gitDiffService, 'getWorkingTreeDiffsFromRef').mockResolvedValue([ { uri: URI.file('/test/repo/file1.ts'), originalUri: URI.file('/test/repo/file1.ts'), @@ -995,9 +995,9 @@ suite('RepoInfoTelemetry', () => { const diffs = JSON.parse(call[1].diffsJSON); assert.strictEqual(diffs.length, 3); - assert.strictEqual(diffs[0].status, Status.MODIFIED); - assert.strictEqual(diffs[1].status, Status.INDEX_ADDED); - assert.strictEqual(diffs[2].status, Status.DELETED); + assert.strictEqual(diffs[0].status, 'MODIFIED'); + assert.strictEqual(diffs[1].status, 'INDEX_ADDED'); + assert.strictEqual(diffs[2].status, 'DELETED'); }); test('should handle renamed files in diff', async () => { @@ -1012,7 +1012,7 @@ suite('RepoInfoTelemetry', () => { status: Status.INDEX_RENAMED }] as any); - vi.spyOn(gitDiffService, 'getChangeDiffs').mockResolvedValue([{ + vi.spyOn(gitDiffService, 'getWorkingTreeDiffsFromRef').mockResolvedValue([{ uri: URI.file('/test/repo/newname.ts'), originalUri: URI.file('/test/repo/oldname.ts'), renameUri: URI.file('/test/repo/newname.ts'), @@ -1041,10 +1041,128 @@ suite('RepoInfoTelemetry', () => { const diffs = JSON.parse(call[1].diffsJSON); assert.strictEqual(diffs.length, 1); - assert.strictEqual(diffs[0].status, Status.INDEX_RENAMED); + assert.strictEqual(diffs[0].status, 'INDEX_RENAMED'); assert.ok(diffs[0].renameUri); }); + test('should include untracked files from both workingTreeChanges and untrackedChanges', async () => { + setupInternalUser(); + mockGitServiceWithRepository(); + + // Mock git extension with untracked files in both workingTreeChanges and untrackedChanges + const mockRepo = { + getMergeBase: vi.fn(), + getBranchBase: vi.fn(), + state: { + HEAD: { + upstream: { + commit: 'abc123', + remote: 'origin', + }, + }, + remotes: [{ + name: 'origin', + fetchUrl: 'https://github.com/microsoft/vscode.git', + pushUrl: 'https://github.com/microsoft/vscode.git', + isReadOnly: false, + }], + workingTreeChanges: [{ + uri: URI.file('/test/repo/filea.txt'), + originalUri: URI.file('/test/repo/filea.txt'), + renameUri: undefined, + status: Status.UNTRACKED + }], + untrackedChanges: [{ + uri: URI.file('/test/repo/fileb.txt'), + originalUri: URI.file('/test/repo/fileb.txt'), + renameUri: undefined, + status: Status.UNTRACKED + }], + }, + }; + + mockRepo.getMergeBase.mockImplementation(async (ref1: string, ref2: string) => { + if (ref1 === 'HEAD' && ref2 === '@{upstream}') { + return 'abc123'; + } + return undefined; + }); + + mockRepo.getBranchBase.mockResolvedValue(undefined); + + const mockApi = { + getRepository: () => mockRepo, + }; + vi.spyOn(gitExtensionService, 'getExtensionApi').mockReturnValue(mockApi as any); + + // Mock diffWith to return one modified file + vi.spyOn(gitService, 'diffWith').mockResolvedValue([{ + uri: URI.file('/test/repo/modified.ts'), + originalUri: URI.file('/test/repo/modified.ts'), + renameUri: undefined, + status: Status.MODIFIED + }] as any); + + // Mock diff service to return all three files + vi.spyOn(gitDiffService, 'getWorkingTreeDiffsFromRef').mockResolvedValue([ + { + uri: URI.file('/test/repo/modified.ts'), + originalUri: URI.file('/test/repo/modified.ts'), + renameUri: undefined, + status: Status.MODIFIED, + diff: 'modified content' + }, + { + uri: URI.file('/test/repo/filea.txt'), + originalUri: URI.file('/test/repo/filea.txt'), + renameUri: undefined, + status: Status.UNTRACKED, + diff: 'new file a' + }, + { + uri: URI.file('/test/repo/fileb.txt'), + originalUri: URI.file('/test/repo/fileb.txt'), + renameUri: undefined, + status: Status.UNTRACKED, + diff: 'new file b' + } + ]); + + const repoTelemetry = new RepoInfoTelemetry( + 'test-message-id', + telemetryService, + gitService, + gitDiffService, + gitExtensionService, + copilotTokenStore, + logService, + fileSystemService, + workspaceFileIndex + ); + + await repoTelemetry.sendBeginTelemetryIfNeeded(); + + // Assert: success with all three files in telemetry + assert.strictEqual((telemetryService.sendInternalMSFTTelemetryEvent as any).mock.calls.length, 1); + const call = (telemetryService.sendInternalMSFTTelemetryEvent as any).mock.calls[0]; + assert.strictEqual(call[1].result, 'success'); + + const diffs = JSON.parse(call[1].diffsJSON); + assert.strictEqual(diffs.length, 3, 'Should include 1 modified file + 2 untracked files'); + + // Verify all three files are present + const uris = diffs.map((d: any) => d.uri); + assert.ok(uris.includes('file:///test/repo/modified.ts'), 'Should include modified file'); + assert.ok(uris.includes('file:///test/repo/filea.txt'), 'Should include filea.txt from workingTreeChanges'); + assert.ok(uris.includes('file:///test/repo/fileb.txt'), 'Should include fileb.txt from untrackedChanges'); + + // Verify statuses + const fileaEntry = diffs.find((d: any) => d.uri === 'file:///test/repo/filea.txt'); + const filebEntry = diffs.find((d: any) => d.uri === 'file:///test/repo/fileb.txt'); + assert.strictEqual(fileaEntry.status, 'UNTRACKED'); + assert.strictEqual(filebEntry.status, 'UNTRACKED'); + }); + // ======================================== // Measurements Tests // ======================================== @@ -1205,7 +1323,7 @@ suite('RepoInfoTelemetry', () => { // Create a diff that exceeds 900KB when serialized to JSON const largeDiff = 'x'.repeat(901 * 1024); - vi.spyOn(gitDiffService, 'getChangeDiffs').mockResolvedValue([{ + vi.spyOn(gitDiffService, 'getWorkingTreeDiffsFromRef').mockResolvedValue([{ uri: URI.file('/test/repo/file.ts'), originalUri: URI.file('/test/repo/file.ts'), renameUri: undefined, @@ -1313,7 +1431,7 @@ suite('RepoInfoTelemetry', () => { uri: 'file:///test/repo/file.ts', originalUri: 'file:///test/repo/file.ts', renameUri: undefined, - status: Status.MODIFIED, + status: 'MODIFIED', diff: testDiff }]); const expectedSize = Buffer.byteLength(expectedDiffsJSON, 'utf8'); @@ -1364,7 +1482,7 @@ suite('RepoInfoTelemetry', () => { }] as any); // Mock diff service to throw error - vi.spyOn(gitDiffService, 'getChangeDiffs').mockRejectedValue(new Error('Diff processing error')); + vi.spyOn(gitDiffService, 'getWorkingTreeDiffsFromRef').mockRejectedValue(new Error('Diff processing error')); const repoTelemetry = new RepoInfoTelemetry( 'test-message-id', @@ -1445,6 +1563,8 @@ suite('RepoInfoTelemetry', () => { pushUrl: remoteUrl, isReadOnly: false, }], + workingTreeChanges: [], + untrackedChanges: [], }, }; @@ -1478,8 +1598,8 @@ suite('RepoInfoTelemetry', () => { diffs.length > 0 ? changes as any : [] ); - // Mock getChangeDiffs to return Diff objects (Change + diff property) - vi.spyOn(gitDiffService, 'getChangeDiffs').mockResolvedValue( + // Mock getWorkingTreeDiffsFromRef to return Diff objects (Change + diff property) + vi.spyOn(gitDiffService, 'getWorkingTreeDiffsFromRef').mockResolvedValue( diffs.map(d => ({ uri: URI.file(d.uri || '/test/repo/file.ts'), originalUri: URI.file(d.originalUri || d.uri || '/test/repo/file.ts'), diff --git a/src/extension/prompt/vscode-node/gitDiffService.ts b/src/extension/prompt/vscode-node/gitDiffService.ts index 339719a060..4d01592d5e 100644 --- a/src/extension/prompt/vscode-node/gitDiffService.ts +++ b/src/extension/prompt/vscode-node/gitDiffService.ts @@ -30,6 +30,46 @@ export class GitDiffService implements IGitDiffService { return repositoryOrUri; } + // Get the diff between the current state of the repository and the specified ref for each of the provided changes + async getWorkingTreeDiffsFromRef(repositoryOrUri: Repository | Uri, changes: Change[], ref: string): Promise { + this._logService.debug(`[GitDiffService] Getting working tree diffs from ref ${ref} for ${changes.length} file(s)`); + + const repository = await this._resolveRepository(repositoryOrUri); + if (!repository) { + this._logService.debug(`[GitDiffService] Repository not found for uri: ${repositoryOrUri.toString()}`); + return []; + } + + const diffs: Diff[] = []; + for (const change of changes) { + if (await this._ignoreService.isCopilotIgnored(change.uri)) { + this._logService.debug(`[GitDiffService] Ignoring change due to content exclusion rule based on uri: ${change.uri.toString()}`); + continue; + } + + let diff: string; + if (change.status === 7 /* UNTRACKED */) { + // For untracked files, generate a patch showing all content as additions + diff = await this._getUntrackedChangePatch(repository, change.uri); + } else { + // For all other changes, get diff from ref to current working tree state + diff = await repository.diffWith(ref, change.uri.fsPath); + } + + diffs.push({ + originalUri: change.originalUri, + renameUri: change.renameUri, + status: change.status, + uri: change.uri, + diff + }); + } + + this._logService.debug(`[GitDiffService] Working tree diffs from ref (after context exclusion): ${diffs.length} file(s)`); + + return diffs; + } + async getChangeDiffs(repositoryOrUri: Repository | Uri, changes: Change[]): Promise { this._logService.debug(`[GitDiffService] Changes (before context exclusion): ${changes.length} file(s)`); diff --git a/src/platform/git/common/gitDiffService.ts b/src/platform/git/common/gitDiffService.ts index 8487c06cda..14477f3092 100644 --- a/src/platform/git/common/gitDiffService.ts +++ b/src/platform/git/common/gitDiffService.ts @@ -13,6 +13,7 @@ export interface IGitDiffService { readonly _serviceBrand: undefined; getChangeDiffs(repository: Repository | Uri, changes: Change[]): Promise; + getWorkingTreeDiffsFromRef(repository: Repository | Uri, changes: Change[], ref: string): Promise; } export interface Diff extends Change { diff --git a/src/platform/git/common/nullGitDiffService.ts b/src/platform/git/common/nullGitDiffService.ts index 5fed800b10..4fbcdbec39 100644 --- a/src/platform/git/common/nullGitDiffService.ts +++ b/src/platform/git/common/nullGitDiffService.ts @@ -11,4 +11,8 @@ export class NullGitDiffService implements IGitDiffService { async getChangeDiffs(): Promise { return []; } + + async getWorkingTreeDiffsFromRef(): Promise { + return []; + } }