From beeb3fd00060dd7032092a90aeaae6a2f39d3e30 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Tue, 9 Jun 2026 17:57:15 +0200 Subject: [PATCH] Fix resolving review threads from the webview --- src/common/comment.ts | 5 +++++ src/common/timelineEvent.ts | 8 +++++++- src/github/pullRequestModel.ts | 13 ++++++++++--- src/github/utils.ts | 7 ++++--- webviews/components/timeline.tsx | 28 ++++++++++++++++++++-------- 5 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/common/comment.ts b/src/common/comment.ts index 718c64d747..3f9559d973 100644 --- a/src/common/comment.ts +++ b/src/common/comment.ts @@ -66,6 +66,11 @@ export interface IComment { reactions?: Reaction[]; isResolved?: boolean; isOutdated?: boolean; + /** + * The GraphQL node ID of the review thread this comment belongs to, if any. + * Populated when the comment is parsed from a review thread. + */ + threadId?: string; } const COPILOT_AUTHOR = { diff --git a/src/common/timelineEvent.ts b/src/common/timelineEvent.ts index e56efbe293..01b586407c 100644 --- a/src/common/timelineEvent.ts +++ b/src/common/timelineEvent.ts @@ -61,7 +61,13 @@ export type ReviewStateValue = 'COMMENTED' | 'APPROVED' | 'CHANGES_REQUESTED' | export interface ReviewEvent { id: number; - reviewThread?: ReviewResolveInfo + /** + * Map of review thread GraphQL node id to its resolve info, for all + * review threads belonging to this review. A single review can have + * comments on multiple threads, so resolve actions must look up the + * thread by the comment's `threadId`. + */ + reviewThreads?: { [threadId: string]: ReviewResolveInfo }; event: EventType.Reviewed; comments: IComment[]; submittedAt: string; diff --git a/src/github/pullRequestModel.ts b/src/github/pullRequestModel.ts index 2e975eed52..eae30ca56d 100644 --- a/src/github/pullRequestModel.ts +++ b/src/github/pullRequestModel.ts @@ -95,7 +95,7 @@ import Logger from '../common/logger'; import { Remote } from '../common/remote'; import { DEFAULT_MERGE_METHOD, PR_SETTINGS_NAMESPACE } from '../common/settingKeys'; import { ITelemetry } from '../common/telemetry'; -import { ClosedEvent, EventType, ReviewEvent, TimelineEvent } from '../common/timelineEvent'; +import { ClosedEvent, EventType, ReviewEvent, ReviewResolveInfo, TimelineEvent } from '../common/timelineEvent'; import { resolvePath, Schemes, toGitHubCommitUri, toPRUri, toReviewUri } from '../common/uri'; import { formatError, isDescendant } from '../common/utils'; import { InMemFileChangeModel, RemoteFileChangeModel } from '../view/fileChangeModel'; @@ -980,13 +980,20 @@ export class PullRequestModel extends IssueModel implements IPullRe return; } const prReviewThreadEvent = reviewEventsById[thread.prReviewDatabaseId]; - prReviewThreadEvent.reviewThread = { + const resolveInfo: ReviewResolveInfo = { threadId: thread.id, canResolve: thread.viewerCanResolve, canUnresolve: thread.viewerCanUnresolve, isResolved: thread.isResolved }; - + // A single review can have comments on multiple threads. Track each + // thread's resolve info by its id so the webview can resolve the + // correct thread when the user clicks resolve/unresolve on any of + // the comment groups within the review. + if (!prReviewThreadEvent.reviewThreads) { + prReviewThreadEvent.reviewThreads = {}; + } + prReviewThreadEvent.reviewThreads[thread.id] = resolveInfo; }); const pendingReview = reviewEvents.filter(r => r.state?.toLowerCase() === 'pending')[0]; diff --git a/src/github/utils.ts b/src/github/utils.ts index 5b69998881..c6c51ad4f7 100644 --- a/src/github/utils.ts +++ b/src/github/utils.ts @@ -596,12 +596,12 @@ export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRep originalEndLine: thread.originalLine, diffSide: thread.diffSide, isOutdated: thread.isOutdated, - comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, thread.isOutdated, githubRepository)), + comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, thread.isOutdated, githubRepository, thread.id)), subjectType: thread.subjectType ?? SubjectType.LINE }; } -export function parseGraphQLComment(comment: GraphQL.ReviewComment, isResolved: boolean, isOutdated: boolean, githubRepository: GitHubRepository): IComment { +export function parseGraphQLComment(comment: GraphQL.ReviewComment, isResolved: boolean, isOutdated: boolean, githubRepository: GitHubRepository, threadId?: string): IComment { const specialAuthor = COPILOT_ACCOUNTS[comment.author?.login ?? '']; const c: IComment = { id: comment.databaseId, @@ -626,7 +626,8 @@ export function parseGraphQLComment(comment: GraphQL.ReviewComment, isResolved: inReplyToId: comment.replyTo && comment.replyTo.databaseId, reactions: parseGraphQLReaction(comment.reactionGroups), isResolved, - isOutdated + isOutdated, + threadId }; const diffHunks = parseCommentDiffHunk(c); diff --git a/webviews/components/timeline.tsx b/webviews/components/timeline.tsx index d8277651d9..4925b421ce 100644 --- a/webviews/components/timeline.tsx +++ b/webviews/components/timeline.tsx @@ -211,11 +211,19 @@ const NewCommitsSinceReviewEventView = () => { const positionKey = (comment: IComment) => comment.position !== null ? `pos:${comment.position}` : `ori:${comment.originalPosition}`; -const groupCommentsByPath = (comments: IComment[]) => - groupBy(comments, comment => comment.path + ':' + positionKey(comment)); +// Group review comments so that each rendered CommentThread contains exactly +// one review thread's comments. Prefer grouping by the thread's GraphQL id +// (set on each IComment when parsed from a review thread) so that resolve / +// unresolve actions target the right thread. Fall back to path+position for +// comments without a threadId (e.g. pending review comments not yet posted). +const groupCommentsByThread = (comments: IComment[]) => + groupBy(comments, comment => + comment.threadId + ? `thread:${comment.threadId}` + : comment.path + ':' + positionKey(comment)); const ReviewEventView = (event: ReviewEvent) => { - const comments = groupCommentsByPath(event.comments); + const comments = groupCommentsByThread(event.comments); const reviewIsPending = event.state === 'PENDING'; return ( @@ -243,17 +251,21 @@ function CommentThread({ thread, event }: { thread: IComment[]; event: ReviewEve setResolved(!!comment.isResolved); setRevealed(!comment.isResolved); }, [comment.isResolved]); + // Look up this group's own review thread info. A single ReviewEvent can + // have comments on multiple threads, so resolve actions must target the + // thread that owns this group's comments - looked up by `comment.threadId`. + const reviewThread = comment.threadId ? event.reviewThreads?.[comment.threadId] : undefined; const resolvePermission = - event.reviewThread && - ((event.reviewThread.canResolve && !event.reviewThread.isResolved) || - (event.reviewThread.canUnresolve && event.reviewThread.isResolved)); + reviewThread && + ((reviewThread.canResolve && !reviewThread.isResolved) || + (reviewThread.canUnresolve && reviewThread.isResolved)); const toggleResolve = () => { - if (event.reviewThread) { + if (reviewThread) { const newResolved = !resolved; setRevealed(!newResolved); setResolved(newResolved); - toggleResolveComment(event.reviewThread.threadId, thread, newResolved); + toggleResolveComment(reviewThread.threadId, thread, newResolved); } };