Fix up for the GitDiffService #2116

IanMatthewHuff · 2025-11-20T19:28:42Z

The repoInfoTelemetry service was being used by the science team for some model development. This telemetry uses the gitDiffService to provide diffs and there were some git diff patches that were not able to be applied coming specifically from new untracked files.

These new untracked files are actually handled a bit differently as the main git services doesn't pick up these changes. Instead a synthetic patch is created via _getUntrackedChangePatch. This function had some issue with generating an actual synthetic patch that could be applied without fixup.

Fixes:

For a totally empty file, correctly omit range / lines
For the range header correctly use lines and not byte length
Add 'new file mode' header
Correctly add 'no newline' footer when needed
Add unit tests for this function

Note: This fix is specifically for the usage of this code path for repoInfoTelemetry, but this existing function was used already by gitCommitMessageService and scmChanges tool. In both those changes the git diffs look to be just handled as a string and passed to an LLM, so improving the accuracy here should not cause any issues. But that is why I avoided added in extra (possibly risky) work for checking file permissions or calculating the SHA1 hash (which are not required to apply the patch) into this work.

Manually tested with pulling the telemetry locally and creating and applying a diff patch for:

New untracked file with content and no end new line
New untracked file with content and an ending new line
New fully empty untracked file
New file with just a new line

Patch applied and created the file as it existed before in all four cases.

IanMatthewHuff · 2025-11-20T19:30:05Z

src/extension/prompt/vscode-node/test/gitDiffService.spec.ts

+
+	beforeEach(() => {
+		// Create mock workspace.fs.readFile if it doesn't exist
+		if (!vscode.workspace?.fs?.readFile) {


The product code here uses workspace.fs directly and not the file service, so I added this mock here so that my changes for this test were just in this test file as opposed to trying to mess with the top level vscode test shims.

IanMatthewHuff · 2025-11-20T19:34:28Z

src/extension/prompt/vscode-node/gitDiffService.ts

-
-			// Add original/modified file paths
+			// 100644 is standard file mode for new git files. Saves us from trying to check file permissions and handling
+			// UNIX vs Windows permission differences. Skipping calculating the SHA1 hashes as well since they are not strictly necessary


I put a comment here on this. Did a bit of research into what the diff patch actually contains and what was required to actually apply a patch.

Using 100644 for the file mode simplifies not having to do extra file permission checks for a synthetic patch and handles the basic git new file scenario. Also the SHA1 hash line is skipped as git patch doesn't require it for applying so it's just more calculations that we don't need.

IanMatthewHuff · 2025-11-20T19:35:18Z

src/extension/prompt/vscode-node/gitDiffService.ts

+				}
+
+				// Range header and content
+				patch.push(`@@ -0,0 +1,${lines.length} @@`);


Bug with the old code was here, where it was using the buffer.length and not the lines length for the range header. This would lead to patches that looked incorrect in size.

IanMatthewHuff · 2025-11-20T19:35:42Z

src/extension/prompt/vscode-node/gitDiffService.ts


-		return patch.join('\n');
+		// The patch itself should always end with a newline per git patch standards
+		return patch.join('\n') + '\n';


The patch itself should always end with a newline.

IanMatthewHuff · 2025-11-20T19:36:17Z

src/extension/prompt/vscode-node/gitDiffService.ts

+
+				// Git standard to add this comment if the file does not end with a newline
+				if (!content.endsWith('\n')) {
+					patch.push('\\ No newline at end of file');


This comment was also missing in the old code. Prevents you from getting a warning when applying a patch that doesn't end with a new line.

IanMatthewHuff · 2025-11-20T19:37:14Z

src/extension/prompt/vscode-node/test/gitDiffService.spec.ts

+		readFileSpy.mockRestore();
+	});
+
+	describe('_getUntrackedChangePatch', () => {


GitDiffService didn't have unit tests previously. But while looking into this some of the cases (newline at end, newline not at end, empty file) were a bit quirky, so added some quick tests in for those specific cases.

Copilot

Pull Request Overview

This PR fixes the _getUntrackedChangePatch method in GitDiffService to generate proper git diff patches for untracked files. The existing implementation had issues with range headers using byte length instead of line count, missing standard git patch headers, and incorrect handling of files without trailing newlines. These issues prevented patches from being applied correctly, affecting the repoInfoTelemetry service used by the science team for model development.

Key Changes:

Fixed range header calculation to use line count instead of byte length
Added "new file mode 100644" header and proper handling of files without trailing newlines
Added comprehensive unit tests covering empty files, files with/without trailing newlines, and single blank line files

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/extension/prompt/vscode-node/gitDiffService.ts	Fixed `_getUntrackedChangePatch` to generate standards-compliant git patches with correct headers, line counts, and newline handling
src/extension/prompt/vscode-node/test/gitDiffService.spec.ts	Added comprehensive unit tests for `_getUntrackedChangePatch` covering various edge cases including empty files, files with/without trailing newlines

src/extension/prompt/vscode-node/test/gitDiffService.spec.ts

Copilot · 2025-11-20T19:51:36Z

src/extension/prompt/vscode-node/test/gitDiffService.spec.ts

+			(vscode as any).workspace = {
+				...vscode.workspace,
+				fs: {
+					...vscode.workspace?.fs,
+					readFile: vi.fn()
+				}
+			};


Using 'as any' to override the vscode object is an anti-pattern. According to the project's testing guidelines (CodingGuidelineID: 1000002), you should prefer explicit Mock classes over mutating real instances. Consider creating a MockFileSystemService instead.

Hmm, so I did notice this was a bit odd. My goal here was that I didn't want to mess with the existing gitDiffService since other code paths are using this existing code. And gitDiffService was using workspace.fs directly and not the file system service for file operation. So this test bit here I did so that I could unit test this function without having to change the product code (workspace.fs -> WorkspaceFileService) or make bigger changes to the vscode test shim. If we don't like this for a test file, down to change it to one of the other options.

I did remove the 'as any' for a more strict type here, but I think the core issue might still apply?

src/extension/prompt/vscode-node/test/gitDiffService.spec.ts

src/extension/prompt/vscode-node/gitDiffService.ts

IanMatthewHuff · 2025-11-20T20:08:36Z

@Yoyokrazy @sbatten has context on this change. But you can ping me as well if you need any info.

IanMatthewHuff · 2025-11-26T19:16:47Z

@vritant24 Tagging for context while I'm oof

sbatten · 2025-12-01T18:28:23Z

@lszomoru I'd really appreciate your review

Ian Huff and others added 7 commits November 19, 2025 14:03

initial pass at untracked file changes, testing next

da0f2e0

add final trailing \n

fb198b4

unit tests for function

6219a7c

fix test execution

5e01cbb

tweak some comments and formattting

c01a1eb

another comment tweak after testing

502810e

small change to handling fully empty files

4b50cad

IanMatthewHuff commented Nov 20, 2025

View reviewed changes

IanMatthewHuff marked this pull request as ready for review November 20, 2025 19:47

Copilot AI review requested due to automatic review settings November 20, 2025 19:47

Copilot started reviewing on behalf of IanMatthewHuff November 20, 2025 19:47 View session

Copilot finished reviewing on behalf of IanMatthewHuff November 20, 2025 19:48

vs-code-engineering bot assigned Yoyokrazy Nov 20, 2025

vs-code-engineering bot added the triage-needed label Nov 20, 2025

Copilot AI reviewed Nov 20, 2025

View reviewed changes

copilot PR feedback

ec5ac8e

sbatten assigned sbatten and unassigned Yoyokrazy Nov 24, 2025

sbatten removed triage-needed labels Nov 24, 2025

sbatten requested a review from lszomoru December 1, 2025 18:28

Fix up for the GitDiffService #2116

Are you sure you want to change the base?

Fix up for the GitDiffService #2116

Uh oh!

Conversation

IanMatthewHuff commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

IanMatthewHuff commented Nov 20, 2025

Uh oh!

IanMatthewHuff commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbatten commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

IanMatthewHuff commented Nov 20, 2025 •

edited

Loading

IanMatthewHuff commented Nov 26, 2025 •

edited

Loading