Fix duplicate comment threads in diff editor (#8739)

* Initial plan

* Initial plan

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>

* Fix duplicate comment threads in diff editor

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>

* Address CCR comment

* Add a test

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>

Author: Copilot

src/test/view/reviewCommentController.test.ts

@@ -50,6 +50,21 @@ class TestReviewCommentController extends ReviewCommentController {
 	public workspaceFileChangeCommentThreads() {
 		return this._workspaceFileChangeCommentThreads;
 	}
+	public obsoleteFileChangeCommentThreads() {
+		return this._obsoleteFileChangeCommentThreads;
+	}
+	public reviewSchemeFileChangeCommentThreads() {
+		return this._reviewSchemeFileChangeCommentThreads;
+	}
+	public seedWorkspaceThreads(path: string, threads: GHPRCommentThread[]) {
+		this._workspaceFileChangeCommentThreads[path] = threads;
+	}
+	public seedObsoleteThreads(path: string, threads: GHPRCommentThread[]) {
+		this._obsoleteFileChangeCommentThreads[path] = threads;
+	}
+	public callFindMatchingThread(thread: import('../../common/comment').IReviewThread) {
+		return (this as any)._findMatchingThread(thread);
+	}
 }
 
 describe('ReviewCommentController', function () {
@@ -340,4 +355,61 @@ describe('ReviewCommentController', function () {
 			assert.strictEqual(workspaceFileChangeCommentThreads[fileName].length, 1);
 		});
 	});
+
+	describe('_findMatchingThread', function () {
+		it('returns the moved thread when an existing workspace thread becomes outdated', function () {
+			const fileName = 'data/products.json';
+			const uri = vscode.Uri.parse(`${repository.rootUri.toString()}/${fileName}`);
+			const reviewModel = new ReviewModel();
+			const reviewCommentController = new TestReviewCommentController(
+				reviewManager,
+				manager,
+				repository,
+				reviewModel,
+				gitApiImpl,
+				telemetry,
+			);
+
+			const threadA = createGHPRCommentThread('thread-A', uri);
+			const threadB = createGHPRCommentThread('thread-B', uri);
+			const existingObsolete = createGHPRCommentThread('thread-C', uri);
+
+			reviewCommentController.seedWorkspaceThreads(fileName, [threadA, threadB]);
+			reviewCommentController.seedObsoleteThreads(fileName, [existingObsolete]);
+
+			const reviewThread: import('../../common/comment').IReviewThread = {
+				id: 'thread-B',
+				isResolved: false,
+				viewerCanResolve: false,
+				viewerCanUnresolve: false,
+				path: fileName,
+				diffSide: DiffSide.RIGHT,
+				startLine: 1,
+				endLine: 1,
+				originalStartLine: 1,
+				originalEndLine: 1,
+				isOutdated: true,
+				comments: [],
+				subjectType: SubjectType.LINE,
+			};
+
+			const result = reviewCommentController.callFindMatchingThread(reviewThread);
+
+			// The matching thread should be the moved thread, not `undefined` or the wrong one.
+			assert.strictEqual(result.threadMap, reviewCommentController.obsoleteFileChangeCommentThreads());
+			assert.ok(result.index > -1, 'index should point to the moved thread');
+			assert.strictEqual(result.threadMap[fileName][result.index], threadB);
+
+			// Workspace map no longer contains the moved thread.
+			const workspace = reviewCommentController.workspaceFileChangeCommentThreads();
+			assert.strictEqual(workspace[fileName].length, 1);
+			assert.strictEqual(workspace[fileName][0], threadA);
+
+			// Obsolete map now contains the moved thread appended after the pre-existing one.
+			const obsolete = reviewCommentController.obsoleteFileChangeCommentThreads();
+			assert.strictEqual(obsolete[fileName].length, 2);
+			assert.strictEqual(obsolete[fileName][0], existingObsolete);
+			assert.strictEqual(obsolete[fileName][1], threadB);
+		});
+	});
 });

src/view/pullRequestCommentController.ts

@@ -230,6 +230,17 @@ export class PullRequestCommentController extends CommentControllerBase implemen
 	private async onDidChangeReviewThreads(e: ReviewThreadChangeEvent): Promise<void> {
 		for (const thread of e.added) {
 			const fileName = thread.path;
+			const key = this.getCommentThreadCacheKey(thread.path, thread.diffSide === DiffSide.LEFT);
+
+			// Defensive: if a comment thread for this review thread id already exists in the cache
+			// (e.g. because addThreadsForEditors already created it from the cache before this event was
+			// processed), update it in place instead of creating a duplicate VS Code comment thread.
+			const existing = this._commentThreadCache[key]?.find(t => t.gitHubThreadId === thread.id);
+			if (existing) {
+				updateThread(this._context, existing, thread, this._githubRepositories);
+				continue;
+			}
+
 			const index = this._pendingCommentThreadAdds.findIndex(t => {
 				const samePath = this._folderRepoManager.gitRelativeRootPath(t.uri.path) === thread.path;
 				const sameLine = (t.range === undefined && thread.subjectType === SubjectType.FILE) || (t.range && t.range.end.line + 1 === thread.endLine);
@@ -270,9 +281,8 @@ export class PullRequestCommentController extends CommentControllerBase implemen
 			}
 
 			if (!newThread) {
-				return;
+				continue;
 			}
-			const key = this.getCommentThreadCacheKey(thread.path, thread.diffSide === DiffSide.LEFT);
 			if (this._commentThreadCache[key]) {
 				this._commentThreadCache[key].push(newThread);
 			} else {

src/view/reviewCommentController.ts

@@ -280,6 +280,17 @@ export class ReviewCommentController extends CommentControllerBase implements Co
 				for (const thread of e.added) {
 					const { path } = thread;
 
+					// Defensive: if a comment thread for this review thread id already exists in any of the
+					// thread maps (e.g. because doInitializeCommentThreads already created it from the cache
+					// before this event was processed), update it in place instead of creating a duplicate
+					// VS Code comment thread.
+					const existingMatch = this._findMatchingThread(thread);
+					if (existingMatch.index > -1) {
+						const matchingThread = existingMatch.threadMap[thread.path][existingMatch.index];
+						updateThread(this._context, matchingThread, thread, githubRepositories);
+						continue;
+					}
+
 					const index = await arrayFindIndexAsync(this._pendingCommentThreadAdds, async t => {
 						const fileName = this._folderRepoManager.gitRelativeRootPath(t.uri.path);
 						if (fileName !== thread.path) {
@@ -358,13 +369,16 @@ export class ReviewCommentController extends CommentControllerBase implements Co
 		let index = threadMap[thread.path]?.findIndex(t => t.gitHubThreadId === thread.id) ?? -1;
 		if ((index === -1) && thread.isOutdated) {
 			// The thread has become outdated and needs to be moved to the obsolete threads.
-			index = this._workspaceFileChangeCommentThreads[thread.path]?.findIndex(t => t.gitHubThreadId === thread.id) ?? -1;
-			if (index > -1) {
-				const matchingThread = this._workspaceFileChangeCommentThreads[thread.path]!.splice(index, 1)[0];
+			const workspaceIndex = this._workspaceFileChangeCommentThreads[thread.path]?.findIndex(t => t.gitHubThreadId === thread.id) ?? -1;
+			if (workspaceIndex > -1) {
+				const matchingThread = this._workspaceFileChangeCommentThreads[thread.path]!.splice(workspaceIndex, 1)[0];
 				if (!this._obsoleteFileChangeCommentThreads[thread.path]) {
 					this._obsoleteFileChangeCommentThreads[thread.path] = [];
 				}
 				this._obsoleteFileChangeCommentThreads[thread.path]!.push(matchingThread);
+				// `threadMap` already references `_obsoleteFileChangeCommentThreads`; the matching
+				// thread is now the last element of the obsolete array for this path.
+				index = this._obsoleteFileChangeCommentThreads[thread.path]!.length - 1;
 			}
 		}
 		return { threadMap, index };