feat(copilot/chat): make /review a first-class chat command#320029
Draft
andysharman wants to merge 1 commit into
Draft
feat(copilot/chat): make /review a first-class chat command#320029andysharman wants to merge 1 commit into
andysharman wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR reroutes the /review intent for chat (Panel) to a new narrative-driven flow that calls githubReview() directly, integrates with the VS Code Comments panel, and adds chat controls to navigate/reveal review findings.
Changes:
- Extended
IReviewServiceto support suppressing auto-reveal when adding comments and to expose the currently active review thread. - Implemented Panel chat
/reviewexecution inreviewIntent.ts(scope parsing, pre-clear behavior, streaming comment adds, summary rendering, cancellation/rollback behavior). - Added new chat commands for “Next Comment” and “Reveal Comment” plus updated command wiring to open chat with
/review ...queries.
Show a summary per file
| File | Description |
|---|---|
| extensions/copilot/src/platform/test/node/simulationWorkspaceServices.ts | Updates test double to match IReviewService API additions. |
| extensions/copilot/src/platform/review/vscode/reviewServiceImpl.ts | Adds suppressible auto-reveal and exposes active thread for chat navigation. |
| extensions/copilot/src/platform/review/common/reviewService.ts | Extends IReviewService API (opts + getActiveThread()). |
| extensions/copilot/src/extension/review/node/test/doReview.spec.ts | Aligns tests with updated review service API. |
| extensions/copilot/src/extension/review/node/githubReviewAgent.ts | Minor signature formatting change (trailing comma). |
| extensions/copilot/src/extension/intents/node/test/reviewIntent.spec.ts | Adds coverage for Panel /review narrative + Comments-panel integration behaviors. |
| extensions/copilot/src/extension/intents/node/reviewIntent.ts | Implements Panel /review flow, summary markdown, and scope parsing. |
| extensions/copilot/src/extension/inlineChat/vscode-node/inlineChatCommands.ts | Rewires review commands to open chat queries; adds chat navigation/reveal commands. |
| extensions/copilot/package.json | Removes when gating for review slash-command contribution. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 3
Comment on lines
+342
to
+391
| function buildFindingsByFileSummary(included: readonly ReviewComment[], excluded: readonly ReviewComment[], workspaceService: IWorkspaceService): MarkdownString { | ||
| type GroupEntry = { comment: ReviewComment; isExcluded: boolean }; | ||
| const groups = new Map<string, { relPath: string; entries: GroupEntry[] }>(); | ||
| const addEntry = (comment: ReviewComment, isExcluded: boolean): void => { | ||
| const key = comment.uri.toString(); | ||
| let group = groups.get(key); | ||
| if (!group) { | ||
| group = { relPath: workspaceService.asRelativePath(comment.uri), entries: [] }; | ||
| groups.set(key, group); | ||
| } | ||
| group.entries.push({ comment, isExcluded }); | ||
| }; | ||
| for (const c of included) { | ||
| addEntry(c, false); | ||
| } | ||
| for (const c of excluded) { | ||
| addEntry(c, true); | ||
| } | ||
| const orderedGroups = [...groups.values()].sort((a, b) => | ||
| a.relPath.localeCompare(b.relPath, undefined, { sensitivity: 'base' }) | ||
| ); | ||
| const lines: string[] = []; | ||
| lines.push(`**${l10n.t('Findings by file')}**`); | ||
| lines.push(''); | ||
| for (const group of orderedGroups) { | ||
| const entries = group.entries.slice().sort((a, b) => { | ||
| const lineDiff = a.comment.range.start.line - b.comment.range.start.line; | ||
| if (lineDiff !== 0) { | ||
| return lineDiff; | ||
| } | ||
| return a.comment.range.start.character - b.comment.range.start.character; | ||
| }); | ||
| const headingLabel = entries.length === 1 | ||
| ? l10n.t('{0} (1 finding)', group.relPath) | ||
| : l10n.t('{0} ({1} findings)', group.relPath, entries.length); | ||
| lines.push(`### ${headingLabel}`); | ||
| lines.push(''); | ||
| for (const entry of entries) { | ||
| const { line, excerpt } = buildFindingExcerpt(entry.comment, workspaceService); | ||
| const basename = path.basename(entry.comment.uri.fsPath); | ||
| const suffix = entry.isExcluded ? ' ' + l10n.t('(filtered)') : ''; | ||
| const ch = entry.comment.range.start.character; | ||
| const args = encodeURIComponent(JSON.stringify([entry.comment.uri.toString(), line, ch])); | ||
| lines.push(`- [${basename}:${line}](command:github.copilot.chat.review.revealComment?${args}): ${excerpt}${suffix}`); | ||
| } | ||
| lines.push(''); | ||
| } | ||
| const md = new MarkdownString(lines.join('\n')); | ||
| md.isTrusted = { enabledCommands: ['github.copilot.chat.review.revealComment'] }; | ||
| return md; |
Comment on lines
+221
to
+276
| let result: FeedbackResult; | ||
| try { | ||
| result = await githubReview( | ||
| this.logService, this.gitExtensionService, this.authService, | ||
| this.capiClientService, this.domainService, this.fetcherService, | ||
| this.envService, this.ignoreService, this.workspaceService, | ||
| this.customInstructionsService, group, editor, progress, token | ||
| ); | ||
| if (result.type === 'cancelled' || token.isCancellationRequested) { | ||
| if (!reviewComplete.isSettled) { | ||
| reviewComplete.complete(l10n.t('Review cancelled')); | ||
| } | ||
| if (addedComments.length) { | ||
| this.reviewService.removeReviewComments(addedComments); | ||
| } | ||
| outputStream.markdown(l10n.t('Review cancelled.')); | ||
| return; | ||
| } | ||
| if (result.type === 'error') { | ||
| if (!reviewComplete.isSettled) { | ||
| reviewComplete.complete(l10n.t('Review failed')); | ||
| } | ||
| outputStream.markdown(l10n.t('Review failed: {0}', result.reason)); | ||
| return; | ||
| } | ||
| const excluded = result.excludedComments ?? []; | ||
| if (excluded.length) { | ||
| safeAdd(excluded); | ||
| } | ||
| const filesWithComments = new Set([...result.comments, ...excluded].map(c => c.uri.toString())).size; | ||
| if (result.comments.length === 0) { | ||
| if (!reviewComplete.isSettled) { | ||
| reviewComplete.complete(l10n.t('No issues found')); | ||
| } | ||
| outputStream.markdown(l10n.t('No issues found.')); | ||
| return; | ||
| } | ||
| if (!reviewComplete.isSettled) { | ||
| reviewComplete.complete(l10n.t('Review complete: {0} findings across {1} files', result.comments.length, filesWithComments)); | ||
| } | ||
| outputStream.markdown(l10n.t('Review complete: {0} findings across {1} files.', result.comments.length, filesWithComments)); | ||
| if (excluded.length) { | ||
| outputStream.markdown('\n\n' + l10n.t('{0} comments filtered by policy.', excluded.length)); | ||
| } | ||
| if (result.comments.length + excluded.length >= 5) { | ||
| outputStream.markdown(buildFindingsByFileSummary(result.comments, excluded, this.workspaceService)); | ||
| } | ||
| outputStream.button({ command: 'github.copilot.chat.review.nextFromChat', title: l10n.t('Next Comment') }); | ||
| } finally { | ||
| // Defensive: always settle the task-bound progress so the spinner stops, | ||
| // even if `githubReview` threw before any explicit branch ran. | ||
| if (!reviewComplete.isSettled) { | ||
| reviewComplete.complete(l10n.t('Review failed')); | ||
| } | ||
| reviewStore.dispose(); | ||
| } |
Comment on lines
+294
to
+296
| disposables.add(vscode.commands.registerCommand('github.copilot.chat.review.nextFromChat', () => | ||
| goToNextReview(reviewService.getActiveThread(), +1) | ||
| )); |
Unhides the /review slash command and adds a chat-driven entry point that complements the existing SCM gutter buttons. The chat path: * accepts scope arguments: staged, unstaged, all, selection, file * streams findings into the Comments panel with a task-bound spinner * renders a per-file findings summary with clickable links * provides a 'Next Comment' button for keyboard-free navigation * routes the SCM gutter buttons through /review so the chat thread is the canonical entry point and preserves the user's current chat mode (agent/ask) Existing non-chat flows (selection review via Ctrl+I, per-file SCM context menu, codeReview.run) continue to use the legacy direct-to- Comments-panel path with their original auto-jump-to-first-finding behaviour, gated by an opts.suppressAutoReveal flag on IReviewService.addReviewComments.
andysharman
force-pushed
the
andysharman/ccr-progress-ux
branch
from
6c8b3fd to
ab1a8aa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unhides the /review slash command and adds a chat-driven entry point from the existing SCM gutter buttons. The chat path:
Existing non-chat flows (selection review via Ctrl+I, per-file SCM context menu, codeReview.run) continue to use the legacy direct-to- Comments-panel path with their original auto-jump-to-first-finding behaviour, gated by an opts.suppressAutoReveal flag on IReviewService.addReviewComments.