From a5a795d55de5cea8b403c9e459a064248631abf5 Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 30 Jan 2026 23:11:22 +0000 Subject: [PATCH] Fix issue #36: Improve thread matching logic for PR comments --- src/format-check/scripts/format-check.test.ts | 56 ++++++++++++++++++- src/format-check/scripts/format-check.ts | 18 +++++- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/format-check/scripts/format-check.test.ts b/src/format-check/scripts/format-check.test.ts index 9b4e2a9..ab525c3 100644 --- a/src/format-check/scripts/format-check.test.ts +++ b/src/format-check/scripts/format-check.test.ts @@ -30,7 +30,7 @@ import * as gi from 'azure-devops-node-api/interfaces/GitInterfaces'; import { beforeEach, describe, expect, it } from '@jest/globals'; import { PullRequestService } from './services/pull-request-service'; import { PullRequestFileChange } from './types/pull-request-file-change'; -import { getChangedFilesInPR, runFormatCheck } from "./format-check"; +import { getChangedFilesInPR, runFormatCheck, updatePullRequestThreads } from "./format-check"; import { Settings } from './types/settings'; import { BaseGitApiService } from './services/base-git-api-service'; import { IGitApi } from 'azure-devops-node-api/GitApi'; @@ -353,4 +353,58 @@ describe('runFormatCheck', () => { } } }); + + it('should find existing thread by file path and line number when content does not match exactly', async () => { + // Test the thread matching logic directly + const existingThreads = [ + { + id: 1, + comments: [ + { + content: '[DotNetFormatTask][Automated] IMPORTS: Fix imports ordering. on line 1, position 1', + } + ], + threadContext: { + filePath: '/src/somefile.ts' + } + } + ]; + + const reports = [ + { + DocumentId: { + Id: randomUUID(), + ProjectId: { + Id: randomUUID() + } + }, + FileName: "somefile.ts", + FilePath: "/src/somefile.ts", + FileChanges: [ + { + CharNumber: 1, + DiagnosticId: "IMPORTS", + LineNumber: 1, + FormatDescription: "Fix imports ordering" + } + ], + commitId: '', + changeType: gi.VersionControlChangeType.None + } + ]; + + // Mock the necessary functions + const mockPullRequestService = { + getThreads: jest.fn().mockReturnValue(Promise.resolve(existingThreads)), + updateThread: jest.fn(), + createThread: jest.fn() + }; + + // Test the updatePullRequestThreads function directly + await updatePullRequestThreads(mockPullRequestService as any, reports as any); + + // Verify that the existing thread was updated instead of creating a new one + expect(mockPullRequestService.updateThread).toHaveBeenCalled(); + expect(mockPullRequestService.createThread).not.toHaveBeenCalled(); + }); }); \ No newline at end of file diff --git a/src/format-check/scripts/format-check.ts b/src/format-check/scripts/format-check.ts index 1959537..bbbea1e 100644 --- a/src/format-check/scripts/format-check.ts +++ b/src/format-check/scripts/format-check.ts @@ -214,7 +214,23 @@ async function updatePullRequestThreads( for (const change of report.FileChanges) { const content = `${commentPreamble} ${change.DiagnosticId}: ${change.FormatDescription} on line ${change.LineNumber}, position ${change.CharNumber}`; activeIssuesContent.push(content); // Keep track of active issues - const existingThread = existingThreads.find(thread => thread.comments?.some(comment => comment.content === content)); + + // First try to find existing thread by exact content match + let existingThread = existingThreads.find(thread => thread.comments?.some(comment => comment.content === content)); + + // If no match found by content, try to find by file path and line number + if (!existingThread) { + existingThread = existingThreads.find(thread => { + return thread.threadContext?.filePath === report.FilePath && + thread.comments?.some(comment => { + // Check if the comment is about the same issue (same line and diagnostic) + const commentContent = comment.content || ''; + return commentContent.includes(change.DiagnosticId) && + commentContent.includes(`line ${change.LineNumber}`) && + commentContent.includes(commentPreamble); + }); + }); + } const comment = { content: content,