From 629ea48932bdbaa7b9e94d0cfcc1d151d03592bf Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Fri, 3 Jul 2026 14:39:55 +0200 Subject: [PATCH] Fixes and improves agentFeedback fixtures (#324208) --- .../agentFeedbackEditorInputContribution.ts | 7 +- .../agentFeedbackEditorWidgetContribution.ts | 2 +- .../media/agentFeedbackEditorInput.css | 3 +- .../agentFeedbackEditorWidget.fixture.ts | 116 ++++++++- .../agentFeedbackInputWidget.fixture.ts | 237 ++++++++++++++++++ 5 files changed, 360 insertions(+), 5 deletions(-) create mode 100644 src/vs/sessions/contrib/agentFeedback/test/browser/agentFeedbackInputWidget.fixture.ts diff --git a/src/vs/sessions/contrib/agentFeedback/browser/agentFeedbackEditorInputContribution.ts b/src/vs/sessions/contrib/agentFeedback/browser/agentFeedbackEditorInputContribution.ts index 0ed2a08d2b3..fdaf7d3923f 100644 --- a/src/vs/sessions/contrib/agentFeedback/browser/agentFeedbackEditorInputContribution.ts +++ b/src/vs/sessions/contrib/agentFeedback/browser/agentFeedbackEditorInputContribution.ts @@ -36,7 +36,12 @@ const addFeedbackAtCurrentLineActionId = 'agentFeedbackEditor.action.addAtCurren const agentFeedbackHoverGlyphClassName = 'agent-feedback-glyph'; const hasAgentFeedbackSessionForEditor = new RawContextKey('agentFeedbackEditor.hasSession', false); -class AgentFeedbackInputWidget extends Disposable implements IOverlayWidget { +/** + * The inline "Add Feedback" input shown in the editor when the user selects a + * range to comment on. Exported so it can be rendered in a component fixture; + * it only depends on {@link ICodeEditor} for its layout geometry. + */ +export class AgentFeedbackInputWidget extends Disposable implements IOverlayWidget { private static readonly _ID = 'agentFeedback.inputWidget'; private static readonly _MIN_WIDTH = 150; diff --git a/src/vs/sessions/contrib/agentFeedback/browser/agentFeedbackEditorWidgetContribution.ts b/src/vs/sessions/contrib/agentFeedback/browser/agentFeedbackEditorWidgetContribution.ts index eb95cdd399b..6ff2f8c18a7 100644 --- a/src/vs/sessions/contrib/agentFeedback/browser/agentFeedbackEditorWidgetContribution.ts +++ b/src/vs/sessions/contrib/agentFeedback/browser/agentFeedbackEditorWidgetContribution.ts @@ -1031,7 +1031,7 @@ export class AgentFeedbackEditorWidget extends Disposable implements IOverlayWid * Groups feedback items and creates combined widgets for nearby items. * Widgets start collapsed and expand when navigated to. */ -class AgentFeedbackEditorWidgetContribution extends Disposable implements IEditorContribution { +export class AgentFeedbackEditorWidgetContribution extends Disposable implements IEditorContribution { static readonly ID = 'agentFeedback.editorWidgetContribution'; diff --git a/src/vs/sessions/contrib/agentFeedback/browser/media/agentFeedbackEditorInput.css b/src/vs/sessions/contrib/agentFeedback/browser/media/agentFeedbackEditorInput.css index 693cc4bed55..aced779633f 100644 --- a/src/vs/sessions/contrib/agentFeedback/browser/media/agentFeedbackEditorInput.css +++ b/src/vs/sessions/contrib/agentFeedback/browser/media/agentFeedbackEditorInput.css @@ -79,7 +79,8 @@ flex-shrink: 0; } -.agent-feedback-input-widget .agent-feedback-input-actions .action-bar .action-item .action-label { +.agent-feedback-input-widget .agent-feedback-input-actions .monaco-action-bar .action-item .action-label { + box-sizing: content-box; width: 16px; height: 16px; } diff --git a/src/vs/sessions/contrib/agentFeedback/test/browser/agentFeedbackEditorWidget.fixture.ts b/src/vs/sessions/contrib/agentFeedback/test/browser/agentFeedbackEditorWidget.fixture.ts index 34bb397cfd4..0f4133c06f0 100644 --- a/src/vs/sessions/contrib/agentFeedback/test/browser/agentFeedbackEditorWidget.fixture.ts +++ b/src/vs/sessions/contrib/agentFeedback/test/browser/agentFeedbackEditorWidget.fixture.ts @@ -12,12 +12,14 @@ import { mock } from '../../../../../base/test/common/mock.js'; import { CodeEditorWidget, ICodeEditorWidgetOptions } from '../../../../../editor/browser/widget/codeEditor/codeEditorWidget.js'; import { IRange } from '../../../../../editor/common/core/range.js'; import { TokenizationRegistry } from '../../../../../editor/common/languages.js'; -import { AgentFeedbackKind, IAgentFeedback, IAgentFeedbackService } from '../../browser/agentFeedbackService.js'; -import { AgentFeedbackEditorWidget } from '../../browser/agentFeedbackEditorWidgetContribution.js'; +import { AgentFeedbackKind, AgentFeedbackState, IAgentFeedback, IAgentFeedbackService } from '../../browser/agentFeedbackService.js'; +import { AgentFeedbackEditorWidget, AgentFeedbackEditorWidgetContribution } from '../../browser/agentFeedbackEditorWidgetContribution.js'; import { ComponentFixtureContext, createEditorServices, createTextModel, defineComponentFixture, defineThemedFixtureGroup } from '../../../../../workbench/test/browser/componentFixtures/fixtureUtils.js'; import { ICodeReviewService, ICodeReviewSuggestion } from '../../../codeReview/browser/codeReviewService.js'; import { createMockCodeReviewService } from '../../../../../workbench/test/browser/componentFixtures/sessions/mockCodeReviewService.js'; import { ISessionEditorComment, SessionEditorCommentSource } from '../../browser/sessionEditorComments.js'; +import { ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; +import { ISession } from '../../../../services/sessions/common/session.js'; const sessionResource = URI.parse('vscode-agent-session://fixture/session-1'); const fileResource = URI.parse('inmemory://model/agent-feedback-widget.ts'); @@ -40,6 +42,20 @@ const sampleCode = [ '}', ].join('\n'); +const longSampleCode = Array.from({ length: 100 }, (_, i) => { + const line = i + 1; + if (line % 6 === 1) { + return `function fn${line}() {`; + } + if (line % 6 === 0) { + return '}'; + } + if (line % 6 === 5) { + return `\treturn value${line};`; + } + return `\tconst value${line} = ${line} + compute${line}();`; +}).join('\n'); + interface IFixtureOptions { readonly expanded?: boolean; readonly focusedCommentId?: string; @@ -221,6 +237,92 @@ function renderWidget(context: ComponentFixtureContext, options: IFixtureOptions } } +/** + * Renders the agent feedback widgets the same way production does: by + * instantiating the real {@link AgentFeedbackEditorWidgetContribution} and + * feeding it comments through the services. This exercises the production + * grouping (far-apart comments become separate widgets) and scroll handling + * (widgets follow their anchor line as the editor scrolls), which a directly + * constructed {@link AgentFeedbackEditorWidget} does not. + */ +function renderViaContribution(context: ComponentFixtureContext, code: string, comments: readonly ISessionEditorComment[]): void { + const scopedDisposables = context.disposableStore.add(new DisposableStore()); + context.container.style.width = '760px'; + context.container.style.height = '420px'; + context.container.style.border = '1px solid var(--vscode-editorWidget-border)'; + context.container.style.background = 'var(--vscode-editor-background)'; + + ensureTokenColorMap(); + + const feedback: readonly IAgentFeedback[] = comments.map(comment => ({ + id: comment.sourceId, + text: comment.text, + resourceUri: comment.resourceUri, + range: comment.range, + sessionResource: comment.sessionResource, + suggestion: comment.suggestion, + kind: comment.kind, + replies: comment.replies, + state: comment.state ?? AgentFeedbackState.Accepted, + })); + + const agentFeedbackService = new class extends mock() { + override readonly onDidChangeFeedback = Event.None; + override readonly onDidChangeNavigation = Event.None; + + override getSessionForFile(resourceUri: URI): ISession | undefined { + // eslint-disable-next-line local/code-no-dangerous-type-assertions + return resourceUri.toString() === fileResource.toString() ? { resource: sessionResource } as ISession : undefined; + } + + override getFeedback(resource: URI): readonly IAgentFeedback[] { + return resource.toString() === sessionResource.toString() ? feedback : []; + } + + override getNavigationBearing() { + return { activeIdx: -1, totalCount: feedback.length }; + } + }(); + + const sessionsManagementService = new class extends mock() { + override getSession(): ISession | undefined { + return undefined; + } + }(); + + const codeReviewService = createMockCodeReviewService(); + const instantiationService = createEditorServices(scopedDisposables, { + colorTheme: context.theme, + additionalServices: reg => { + reg.defineInstance(IAgentFeedbackService, agentFeedbackService); + reg.defineInstance(ISessionsManagementService, sessionsManagementService); + reg.defineInstance(ICodeReviewService, codeReviewService); + reg.define(IMarkdownRendererService, MarkdownRendererService); + }, + }); + const model = scopedDisposables.add(createTextModel(instantiationService, code, fileResource, 'typescript')); + + const editor = scopedDisposables.add(instantiationService.createInstance( + CodeEditorWidget, + context.container, + { + automaticLayout: true, + lineNumbers: 'on', + minimap: { enabled: false }, + scrollBeyondLastLine: false, + fontSize: 13, + lineHeight: 20, + }, + { contributions: [] } + )); + + editor.setModel(model); + + // The contribution builds, groups, positions and keeps the widgets in sync + // with editor scroll — exactly as in production. + scopedDisposables.add(instantiationService.createInstance(AgentFeedbackEditorWidgetContribution, editor)); +} + const singleFeedback = [ createFeedbackComment('f-1', 'Prefer a clearer variable name on this line.', 2), ]; @@ -279,6 +381,11 @@ const allSourcesMixed = [ createPRReviewComment('pr-2', 'This logic duplicates what we have in utils.ts — consider reusing.', 8, 9), ]; +const longFileFeedback = [ + createFeedbackComment('lf-1', 'Consider validating this input before using it.', 5), + createFeedbackComment('lf-2', 'This computation is duplicated further down — extract a helper.', 20), +]; + export default defineThemedFixtureGroup({ path: 'sessions/agentFeedback/' }, { CollapsedSingleComment: defineComponentFixture({ labels: { kind: 'screenshot' }, @@ -392,4 +499,9 @@ export default defineThemedFixtureGroup({ path: 'sessions/agentFeedback/' }, { hidden: true, }), }), + + LongFileTwoComments: defineComponentFixture({ + labels: { kind: 'screenshot' }, + render: context => renderViaContribution(context, longSampleCode, longFileFeedback), + }), }); diff --git a/src/vs/sessions/contrib/agentFeedback/test/browser/agentFeedbackInputWidget.fixture.ts b/src/vs/sessions/contrib/agentFeedback/test/browser/agentFeedbackInputWidget.fixture.ts new file mode 100644 index 00000000000..7dc65cc1653 --- /dev/null +++ b/src/vs/sessions/contrib/agentFeedback/test/browser/agentFeedbackInputWidget.fixture.ts @@ -0,0 +1,237 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Color } from '../../../../../base/common/color.js'; +import { Event } from '../../../../../base/common/event.js'; +import { DisposableStore } from '../../../../../base/common/lifecycle.js'; +import { observableValue } from '../../../../../base/common/observable.js'; +import { isEqual } from '../../../../../base/common/resources.js'; +import { URI } from '../../../../../base/common/uri.js'; +import { mock } from '../../../../../base/test/common/mock.js'; +import { ICodeEditor } from '../../../../../editor/browser/editorBrowser.js'; +import { CodeEditorWidget } from '../../../../../editor/browser/widget/codeEditor/codeEditorWidget.js'; +import { EditorLayoutInfo } from '../../../../../editor/common/config/editorOptions.js'; +import { Position } from '../../../../../editor/common/core/position.js'; +import { TokenizationRegistry } from '../../../../../editor/common/languages.js'; +import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; +import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js'; +import { AgentFeedbackEditorInputContribution, AgentFeedbackInputWidget } from '../../browser/agentFeedbackEditorInputContribution.js'; +import { IAgentFeedbackService } from '../../browser/agentFeedbackService.js'; +import { ISession, ISessionFileChange } from '../../../../services/sessions/common/session.js'; +import { ComponentFixtureContext, createEditorServices, createTextModel, defineComponentFixture, defineThemedFixtureGroup } from '../../../../../workbench/test/browser/componentFixtures/fixtureUtils.js'; +import '../../../../../base/browser/ui/codicons/codiconStyles.js'; +import '../../browser/media/agentFeedbackEditorInput.css'; + +const sessionResource = URI.parse('vscode-agent-session://fixture/session-1'); +const fileResource = URI.parse('inmemory://model/agent-feedback-input.ts'); + +const sampleCode = [ + 'function alpha() {', + '\tconst first = 1;', + '\treturn first;', + '}', + '', + 'function beta() {', + '\tconst second = 2;', + '\tconst third = second + 1;', + '\treturn third;', + '}', +].join('\n'); + +function ensureTokenColorMap(): void { + if (TokenizationRegistry.getColorMap()?.length) { + return; + } + TokenizationRegistry.setColorMap([ + Color.fromHex('#000000'), + Color.fromHex('#d4d4d4'), + Color.fromHex('#9cdcfe'), + Color.fromHex('#ce9178'), + Color.fromHex('#b5cea8'), + Color.fromHex('#569cd6'), + Color.fromHex('#dcdcaa'), + ]); +} + +interface IInputFixtureOptions { + /** Initial text in the input. Empty renders the placeholder state. */ + readonly text?: string; + /** Placeholder to show — "Add Feedback" (has changes) vs "Add Comment". */ + readonly placeholder?: string; +} + +/** + * A minimal {@link ICodeEditor} stand-in for the standalone variants. The input + * widget only reads layout geometry ({@link ICodeEditor.getLayoutInfo}) and asks + * the editor to re-layout itself ({@link ICodeEditor.layoutOverlayWidget}), so a + * mock that provides just those two is enough to render it on its own. + */ +function createFakeEditor(): ICodeEditor { + return new class extends mock() { + override getLayoutInfo(): EditorLayoutInfo { + // Only `width` and `contentLeft` are read (to clamp the input width). + // eslint-disable-next-line local/code-no-dangerous-type-assertions + return { width: 520, contentLeft: 64 } as EditorLayoutInfo; + } + override layoutOverlayWidget(): void { } + }(); +} + +/** Renders the input widget on its own — the widget's own DOM/CSS in isolation. */ +function renderInputWidget(context: ComponentFixtureContext, options: IInputFixtureOptions): void { + // The widget is `position: absolute`, so give it a positioned host with + // room, and let it flow statically so it is fully captured (not clipped). + context.container.style.position = 'relative'; + context.container.style.width = '520px'; + context.container.style.padding = '24px'; + context.container.style.background = 'var(--vscode-editor-background)'; + + const widget = context.disposableStore.add(new AgentFeedbackInputWidget(createFakeEditor())); + const domNode = widget.getDomNode(); + domNode.style.position = 'static'; + // When absolutely positioned (as in the editor) the widget shrinks to its + // content. Flowing it statically would instead stretch the flex container to + // the host width, so pin it to its content width to preserve the real layout. + domNode.style.width = 'fit-content'; + domNode.style.animation = 'none'; + context.container.appendChild(domNode); + + if (options.placeholder) { + widget.setPlaceholder(options.placeholder); + } + if (options.text) { + widget.inputElement.value = options.text; + } + + // Reveal (it starts hidden) and let it size itself + enable/disable actions + // exactly as the contribution does after mounting it. + widget.show(); + widget.updateActionEnabled(); + widget.autoSize(); +} + +/** A session whose feedback scopes {@link fileResource}, for the mock service. */ +function createFixtureSession(): ISession { + const changes = observableValue('agentFeedbackFixtureChanges', []); + return new class extends mock() { + override readonly resource = sessionResource; + override readonly changes = changes; + }(); +} + +/** + * Renders the input widget the way production does: by instantiating the real + * {@link AgentFeedbackEditorInputContribution} on a real editor and letting it + * create, show and position the widget. The contribution requires chat to be + * enabled and the file to be owned by a session, so both are stubbed here, then + * its public {@link AgentFeedbackEditorInputContribution.showAtCurrentLine} entry + * point (also used by the "add feedback at current line" command) is invoked to + * summon the box — exercising the real placement instead of re-implementing it. + */ +function renderInEditor(context: ComponentFixtureContext): Promise { + const scopedDisposables = context.disposableStore.add(new DisposableStore()); + context.container.style.width = '760px'; + context.container.style.height = '260px'; + context.container.style.border = '1px solid var(--vscode-editorWidget-border)'; + context.container.style.background = 'var(--vscode-editor-background)'; + + ensureTokenColorMap(); + + const session = createFixtureSession(); + const agentFeedbackService = new class extends mock() { + override readonly onDidChangeFeedback = Event.None; + override readonly onDidChangeNavigation = Event.None; + override getSessionForFile(resourceUri: URI): ISession | undefined { + return isEqual(resourceUri, fileResource) ? session : undefined; + } + override getFeedback() { + return []; + } + }(); + + // The contribution only offers the input when chat is enabled. The fixtures' + // default MockContextKeyService reports every rule as unmatched, so provide a + // variant that reports the chat-enabled gate as satisfied. + const contextKeyService = new class extends MockContextKeyService { + override contextMatchesRules(): boolean { return true; } + }(); + + const instantiationService = createEditorServices(scopedDisposables, { + colorTheme: context.theme, + additionalServices: reg => { + reg.defineInstance(IAgentFeedbackService, agentFeedbackService); + reg.defineInstance(IContextKeyService, contextKeyService); + }, + }); + + const model = scopedDisposables.add(createTextModel(instantiationService, sampleCode, fileResource, 'typescript')); + const editor = scopedDisposables.add(instantiationService.createInstance( + CodeEditorWidget, + context.container, + { + automaticLayout: false, + lineNumbers: 'on', + minimap: { enabled: false }, + scrollBeyondLastLine: false, + fontSize: 13, + lineHeight: 20, + }, + { contributions: [] }, + )); + editor.setModel(model); + // Lay out synchronously so the contribution positions/sizes the widget + // against real geometry (automaticLayout would settle asynchronously, after + // the box has already been placed against a zero-size editor). + editor.layout({ width: 760, height: 260 }); + + const contribution = scopedDisposables.add(instantiationService.createInstance(AgentFeedbackEditorInputContribution, editor)); + + // Put the cursor on the line to comment on and let the contribution create, + // show and position the input exactly as the command does in production. + editor.setPosition(new Position(7, 1)); + contribution.showAtCurrentLine(false); + + // Let the DOM settle, then trigger a layout change so the contribution + // re-measures and re-positions the (now measurable) widget against real + // geometry — this is the same reposition path it runs on editor resize. + return new Promise(resolve => { + // this is fine in fixtures + // eslint-disable-next-line no-restricted-globals + requestAnimationFrame(() => { + editor.layout({ width: 759, height: 260 }); + editor.layout({ width: 760, height: 260 }); + resolve(); + }); + }); +} + +export default defineThemedFixtureGroup({ path: 'sessions/agentFeedback/' }, { + Empty: defineComponentFixture({ + labels: { kind: 'screenshot' }, + render: context => renderInputWidget(context, {}), + }), + + AddComment: defineComponentFixture({ + labels: { kind: 'screenshot' }, + render: context => renderInputWidget(context, { placeholder: 'Add Comment' }), + }), + + WithText: defineComponentFixture({ + labels: { kind: 'screenshot' }, + render: context => renderInputWidget(context, { text: 'Prefer a clearer variable name on this line.' }), + }), + + MultilineText: defineComponentFixture({ + labels: { kind: 'screenshot' }, + render: context => renderInputWidget(context, { + text: 'This branch needs a stronger explanation.\nAlso consider extracting it into a helper so the intent is explicit.', + }), + }), + + InEditor: defineComponentFixture({ + labels: { kind: 'screenshot' }, + render: context => renderInEditor(context), + }), +});