Skip to content

cli plan widget providing feedback improvements#313604

Merged
justschen merged 4 commits into
mainfrom
justin/xerneas
May 1, 2026
Merged

cli plan widget providing feedback improvements#313604
justschen merged 4 commits into
mainfrom
justin/xerneas

Conversation

@justschen
Copy link
Copy Markdown
Collaborator

new feedback providing flow for the plan widget:

  • clicking on "edit or provide feedback" will open the editor AND put it in the feedback mode.
  • in agents, the modal opens and you can provide feedback. closing it, you'll see it in this feedback mode where you can review and also provide additional feedback
  • second screenshot is just a visualization of inline + submitted feedback!
Screenshot 2026-04-30 at 2 39 49 PM (1) Screenshot 2026-04-30 at 2 39 56 PM (1)

Copilot AI review requested due to automatic review settings April 30, 2026 22:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the chat plan review (“plan widget”) feedback flow to make reviewing/editing the plan file and providing feedback a single cohesive experience, including support for inline editor-anchored comments and improved transcript rendering.

Changes:

  • Replaces the legacy “Provide Feedback” footer entry with a single “Review / Edit or Provide Feedback” affordance that opens the plan file and enters feedback mode.
  • Adds an inline-comments list (with per-item remove + “Clear All”) and includes inline-comment counts in the “Submit Feedback” action.
  • Updates transcript/progress rendering to avoid collapsing feedback into an unreadable single-line wall of text.
Show a summary per file
File Description
src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatPlanReviewPart.test.ts Updates/extends unit tests to cover new review/feedback behavior and inline-comments list behavior.
src/vs/workbench/contrib/chat/common/chatService/chatService.ts Extends IChatPlanReviewResult with structured fields for overall vs inline feedback display.
src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts Renders structured feedback fields in the plan review progress message (overall inline + inline-comments markdown).
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatPlanReview.css Adjusts scroll/geometry handling and adds styling for the inline-comments list + header actions.
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatPlanReviewPart.ts Implements the new review/feedback mode, inline-comments list management, and structured feedback submission.
src/vs/workbench/contrib/chat/browser/planReviewFeedback/planReviewFeedbackEditorContribution.ts Removes editor-surface “submit” flow; keeps editor overlay focused on capturing inline comments and improves activation behavior.
src/vs/workbench/contrib/chat/browser/planReviewFeedback/planReviewFeedbackEditorActions.ts Removes the “Submit Feedback” editor action and adjusts menu grouping/labels accordingly.

Copilot's findings

Comments suppressed due to low confidence (3)

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatPlanReviewPart.ts:742

  • exitFeedbackMode is marked async but contains no await. This adds an unnecessary Promise-returning API surface and can make call sites look asynchronous when they aren't. Consider removing async/Promise<void> and keeping it synchronous.
	private async exitFeedbackMode(): Promise<void> {
		if (!this._isFeedbackMode) {
			return;
		}

		// "Back" is non-destructive: inline comments and the draft textarea
		// are preserved so the user can resume by clicking Review again.
		// Per-row \u00d7 buttons and the Clear All button handle deletion
		// explicitly.
		this._isFeedbackMode = false;
		if (this._feedbackSection) {
			dom.hide(this._feedbackSection);
		}
		this.domNode.classList.remove('chat-plan-review-feedback-mode');
		this.renderCurrentActionButtons();
		this._messageScrollable.scanDomNode();
		this._onDidChangeHeight.fire();
	}

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatPlanReviewPart.ts:780

  • feedbackInlineMarkdown is built via string interpolation that inserts item.text directly into a markdown bullet. If the comment text contains markdown control characters/newlines, it can break the formatting in both the transcript and the message sent to the agent. Escape user-provided text (and ideally the filename) with escapeMarkdownSyntaxTokens (or otherwise sanitize) before embedding it into markdown.
			const heading = fileName
				? localize('chat.planReview.inlineCommentsHeading', "Inline comments on `{0}`:", fileName)
				: localize('chat.planReview.inlineCommentsHeadingNoFile', "Inline comments:");
			const bullets = editorFeedbackItems.map(item => {
				const location = item.column > 1
					? localize('chat.planReview.inlineCommentLocation', "Line {0}, Column {1}", item.line, item.column)
					: localize('chat.planReview.inlineCommentLocationLine', "Line {0}", item.line);
				return `- **${location}:** ${item.text}`;
			});
			feedbackInlineMarkdown = [heading, ...bullets].join('\n');

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatPlanReviewPart.ts:451

  • The confirmation prompt text uses the "comment(s)" pattern, which reads awkwardly and isn't localized for singular vs plural. Prefer separate singular/plural strings (e.g. conditional on items.length) or rephrase to a neutral plural ("Clear {0} inline comments?") to avoid "(s)".
		const result = await this._dialogService.confirm({
			type: Severity.Warning,
			message: localize('chat.planReview.clearAllConfirm', 'Clear {0} inline comment(s)?', items.length),
			detail: localize('chat.planReview.clearAllDetail', 'These comments will be removed from the plan file and not sent to the agent.'),
			primaryButton: localize('chat.planReview.clearAllConfirmPrimary', 'Clear All'),
		});
  • Files reviewed: 7/7 changed files
  • Comments generated: 2

@justschen justschen marked this pull request as ready for review May 1, 2026 09:19
@justschen justschen enabled auto-merge (squash) May 1, 2026 09:19
@justschen justschen merged commit ad9b3ef into main May 1, 2026
26 checks passed
@justschen justschen deleted the justin/xerneas branch May 1, 2026 11:02
@vs-code-engineering vs-code-engineering Bot added this to the 1.119.0 milestone May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants