Try out comment-outside-the-diff

alexr00 · alexr00 · commit 3435f6146f0e · 2026-03-09T11:49:13.000+01:00
Fixes #3305
diff --git a/src/github/credentials.ts b/src/github/credentials.ts
@@ -576,7 +576,8 @@ const link = (url: string, token: string) =>
 		headers: {
 			...headers,
 			authorization: token ? `Bearer ${token}` : '',
-			Accept: 'application/vnd.github.merge-info-preview'
+			Accept: 'application/vnd.github.merge-info-preview',
+			'GraphQL-Features': 'graphql_pr_comment_positioning'
 		},
 	})).concat(
 		createHttpLink({
diff --git a/src/github/graphql.ts b/src/github/graphql.ts
@@ -273,6 +273,12 @@ export interface ReviewThread {
 			}
 		}]
 	};
+	positioning?: {
+		__typename: string;
+		startLine?: number;
+		endLine?: number;
+		line?: number;
+	}
 }
 
 export interface TimelineEventsResponse {
diff --git a/src/github/pullRequestModel.ts b/src/github/pullRequestModel.ts
@@ -742,6 +742,26 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 		const pendingReviewId = await this.getPendingReviewId();
 
 		const { mutate, schema } = await this.githubRepository.ensure();
+
+		let linePositioning: any;
+		let multilinePositioning: any;
+		if (startLine === endLine && startLine !== undefined) {
+			linePositioning = {
+				line: startLine,
+				path: commentPath,
+				commitOid: this.head?.sha
+			};
+		} else if (startLine !== undefined && endLine !== undefined) {
+			multilinePositioning = {
+				startLine,
+				endLine,
+				startPath: commentPath,
+				endPath: commentPath,
+				startCommitOid: this.head?.sha,
+				endCommitOid: this.head?.sha
+			};
+		}
+
 		const { data } = await mutate<AddReviewThreadResponse>({
 			mutation: schema.AddReviewThread,
 			variables: {
@@ -753,7 +773,9 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 					startLine: startLine === endLine ? undefined : startLine,
 					line: (endLine === undefined) ? 0 : endLine,
 					side,
-					subjectType: (startLine === undefined || endLine === undefined) ? SubjectType.FILE : SubjectType.LINE
+					subjectType: (startLine === undefined || endLine === undefined) ? SubjectType.FILE : SubjectType.LINE,
+					linePositioning,
+					multilinePositioning
 				}
 			}
 		}, { mutation: schema.LegacyAddReviewThread, deleteProps: ['subjectType'] });
@@ -772,7 +794,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 		}
 
 		const thread = data.addPullRequestReviewThread.thread;
-		const newThread = parseGraphQLReviewThread(thread, this.githubRepository);
+		const newThread = parseGraphQLReviewThread(thread, this.githubRepository, this.head?.sha);
 		if (!this._reviewThreadsCache) {
 			this._reviewThreadsCache = [];
 		}
@@ -1453,7 +1475,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 	}
 
 	private setReviewThreadCacheFromRaw(raw: ReviewThread[]): IReviewThread[] {
-		const reviewThreads: IReviewThread[] = raw.map(thread => parseGraphQLReviewThread(thread, this.githubRepository));
+		const reviewThreads: IReviewThread[] = raw.map(thread => parseGraphQLReviewThread(thread, this.githubRepository, this.head?.sha));
 		const oldReviewThreads = this._reviewThreadsCache ?? [];
 		this._reviewThreadsCache = reviewThreads;
 		this.diffThreads(oldReviewThreads, reviewThreads);
@@ -2105,7 +2127,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 
 			const index = this._reviewThreadsCache?.findIndex(thread => thread.id === threadId) ?? -1;
 			if (index > -1) {
-				const thread = parseGraphQLReviewThread(data.resolveReviewThread.thread, this.githubRepository);
+				const thread = parseGraphQLReviewThread(data.resolveReviewThread.thread, this.githubRepository, this.head?.sha);
 				this._reviewThreadsCache?.splice(index, 1, thread);
 				this._onDidChangeReviewThreads.fire({ added: [], changed: [thread], removed: [] });
 			}
@@ -2148,7 +2170,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 
 			const index = this._reviewThreadsCache?.findIndex(thread => thread.id === threadId) ?? -1;
 			if (index > -1) {
-				const thread = parseGraphQLReviewThread(data.unresolveReviewThread.thread, this.githubRepository);
+				const thread = parseGraphQLReviewThread(data.unresolveReviewThread.thread, this.githubRepository, this.head?.sha);
 				this._reviewThreadsCache?.splice(index, 1, thread);
 				this._onDidChangeReviewThreads.fire({ added: [], changed: [thread], removed: [] });
 			}
diff --git a/src/github/queriesShared.gql b/src/github/queriesShared.gql
@@ -576,6 +576,16 @@ query PullRequestComments($owner: String!, $name: String!, $number: Int!, $after
 							...ReviewComment
 						}
 					}
+					positioning {
+						__typename
+						... on MultilineComment  {
+							startLine
+							endLine
+						}
+						... on LineComment  {
+							line
+						}
+					}
 				}
 				pageInfo {
 					hasNextPage
diff --git a/src/github/utils.ts b/src/github/utils.ts
@@ -510,7 +510,29 @@ export function convertGraphQLEventType(text: string) {
 	}
 }
 
-export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRepository: GitHubRepository): IReviewThread {
+export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRepository: GitHubRepository, headCommitOid?: string): IReviewThread {
+	let startLine: number = 0;
+	let endLine: number = 0;
+	if (thread.positioning) {
+		if (thread.positioning.__typename === 'MultilineComment') {
+			startLine = thread.positioning.startLine!;
+			endLine = thread.positioning.endLine!;
+		} else if (thread.positioning.__typename === 'LineComment') {
+			startLine = thread.positioning.line!;
+			endLine = thread.positioning.line!;
+		}
+	} else {
+		startLine = thread.startLine ?? thread.line;
+		endLine = thread.line;
+	}
+
+	// There's a bug with comments outside the diff where they always show as outdated. Try to work around that.
+	let isOutdated = thread.isOutdated;
+	if (isOutdated && headCommitOid && thread.comments.nodes.length > 0 &&
+		thread.comments.nodes.every(comment => comment.commit.oid === headCommitOid)) {
+		isOutdated = false;
+	}
+
 	return {
 		id: thread.id,
 		prReviewDatabaseId: thread.comments.edges && thread.comments.edges.length ?
@@ -520,13 +542,13 @@ export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRep
 		viewerCanResolve: thread.viewerCanResolve,
 		viewerCanUnresolve: thread.viewerCanUnresolve,
 		path: thread.path,
-		startLine: thread.startLine ?? thread.line,
-		endLine: thread.line,
+		startLine,
+		endLine,
 		originalStartLine: thread.originalStartLine ?? thread.originalLine,
 		originalEndLine: thread.originalLine,
 		diffSide: thread.diffSide,
-		isOutdated: thread.isOutdated,
-		comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, thread.isOutdated, githubRepository)),
+		isOutdated,
+		comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, isOutdated, githubRepository)),
 		subjectType: thread.subjectType ?? SubjectType.LINE
 	};
 }
diff --git a/src/view/reviewCommentController.ts b/src/view/reviewCommentController.ts
@@ -31,6 +31,7 @@ import {
 	CommentReactionHandler,
 	createVSCodeCommentThreadForReviewThread,
 	getRepositoryForFile,
+	isEnterprise,
 	isFileInRepo,
 	setReplyAuthor,
 	threadRange,
@@ -522,25 +523,29 @@ export class ReviewCommentController extends CommentControllerBase implements Co
 			const ranges: vscode.Range[] = [];
 
 			if (matchedFile) {
-				const diffHunks = await matchedFile.changeModel.diffHunks();
-				if ((matchedFile.status === GitChangeType.RENAME) && (diffHunks.length === 0)) {
-					Logger.debug('No commenting ranges: File was renamed with no diffs.', ReviewCommentController.ID);
-					return { ranges: [], enableFileComments: true };
-				}
+				if (!isEnterprise(this._folderRepoManager.activePullRequest.remote.authProviderId)) {
+					ranges.push(new vscode.Range(0, 0, document.lineCount - 1, document.lineAt(document.lineCount - 1).text.length - 1));
+				} else {
+					const diffHunks = await matchedFile.changeModel.diffHunks();
+					if ((matchedFile.status === GitChangeType.RENAME) && (diffHunks.length === 0)) {
+						Logger.debug('No commenting ranges: File was renamed with no diffs.', ReviewCommentController.ID);
+						return { ranges: [], enableFileComments: true };
+					}
 
-				const contentDiff = await this.getContentDiff(document.uri, matchedFile.fileName);
+					const contentDiff = await this.getContentDiff(document.uri, matchedFile.fileName);
 
-				for (let i = 0; i < diffHunks.length; i++) {
-					const diffHunk = diffHunks[i];
-					const start = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber, document.lineCount);
-					const end = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber + diffHunk.newLength - 1, document.lineCount);
-					if (start > 0 && end > 0) {
-						ranges.push(new vscode.Range(start - 1, 0, end - 1, 0));
+					for (let i = 0; i < diffHunks.length; i++) {
+						const diffHunk = diffHunks[i];
+						const start = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber, document.lineCount);
+						const end = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber + diffHunk.newLength - 1, document.lineCount);
+						if (start > 0 && end > 0) {
+							ranges.push(new vscode.Range(start - 1, 0, end - 1, 0));
+						}
 					}
-				}
 
-				if (ranges.length === 0) {
-					Logger.debug('No commenting ranges: File has diffs, but they could not be mapped to current lines.', ReviewCommentController.ID);
+					if (ranges.length === 0) {
+						Logger.debug('No commenting ranges: File has diffs, but they could not be mapped to current lines.', ReviewCommentController.ID);
+					}
 				}
 			} else {
 				Logger.debug('No commenting ranges: File does not match any of the files in the review.', ReviewCommentController.ID);
diff --git a/src/view/treeNodes/pullRequestNode.ts b/src/view/treeNodes/pullRequestNode.ts
@@ -22,6 +22,7 @@ import { getIconForeground, getListErrorForeground, getListWarningForeground, ge
 import { DirectoryTreeNode } from './directoryTreeNode';
 import { InMemFileChangeNode, RemoteFileChangeNode } from './fileChangeNode';
 import { TreeNode, TreeNodeParent } from './treeNode';
+import { isEnterprise } from '../../github/utils';
 import { NotificationsManager } from '../../notifications/notificationsManager';
 import { PrsTreeModel } from '../prsTreeModel';
 
@@ -411,7 +412,11 @@ export class PRNode extends TreeNode implements vscode.CommentingRangeProvider2
 				return undefined;
 			}
 
-			return { ranges: getCommentingRanges(await fileChange.changeModel.diffHunks(), params.isBase, PRNode.ID), enableFileComments: true };
+			if (isEnterprise(this.pullRequestModel.remote.authProviderId)) {
+				return { ranges: getCommentingRanges(await fileChange.changeModel.diffHunks(), params.isBase, PRNode.ID), enableFileComments: true };
+			} else {
+				return { ranges: [(new vscode.Range(0, 0, document.lineCount - 1, document.lineAt(document.lineCount).text.length))], enableFileComments: true };
+			}
 		}
 
 		return undefined;

Original file line number	Diff line number	Diff line change
`@@ -273,6 +273,12 @@ export interface ReviewThread {`
`273`	`273`	`}`
`274`	`274`	`}]`
`275`	`275`	`};`
	`276`	`+ positioning?: {`
	`277`	`+ __typename: string;`
	`278`	`+ startLine?: number;`
	`279`	`+ endLine?: number;`
	`280`	`+ line?: number;`
	`281`	`+ }`
`276`	`282`	`}`
`277`	`283`
`278`	`284`	`export interface TimelineEventsResponse {`
Original file line number	Diff line number	Diff line change
`@@ -576,6 +576,16 @@ query PullRequestComments($owner: String!, $name: String!, $number: Int!, $after`
`576`	`576`	`...ReviewComment`
`577`	`577`	`}`
`578`	`578`	`}`
	`579`	`+ positioning {`
	`580`	`+ __typename`
	`581`	`+ ... on MultilineComment {`
	`582`	`+ startLine`
	`583`	`+ endLine`
	`584`	`+ }`
	`585`	`+ ... on LineComment {`
	`586`	`+ line`
	`587`	`+ }`
	`588`	`+ }`
`579`	`589`	`}`
`580`	`590`	`pageInfo {`
`581`	`591`	`hasNextPage`