diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatPlanReviewPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatPlanReviewPart.ts index 4c37b189b54..5cf852a8ae5 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatPlanReviewPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatPlanReviewPart.ts @@ -5,6 +5,7 @@ import * as dom from '../../../../../../base/browser/dom.js'; import { StandardKeyboardEvent } from '../../../../../../base/browser/keyboardEvent.js'; +import { status } from '../../../../../../base/browser/ui/aria/aria.js'; import { Button, ButtonWithDropdown, IButton } from '../../../../../../base/browser/ui/button/button.js'; import { DomScrollableElement } from '../../../../../../base/browser/ui/scrollbar/scrollableElement.js'; import { Action, Separator } from '../../../../../../base/common/actions.js'; @@ -19,9 +20,12 @@ import { basename, isEqual } from '../../../../../../base/common/resources.js'; import { ThemeIcon } from '../../../../../../base/common/themables.js'; import { URI } from '../../../../../../base/common/uri.js'; import { generateUuid } from '../../../../../../base/common/uuid.js'; +import { ITextModel } from '../../../../../../editor/common/model.js'; +import { IModelService } from '../../../../../../editor/common/services/model.js'; import { localize } from '../../../../../../nls.js'; import { IContextMenuService } from '../../../../../../platform/contextview/browser/contextView.js'; import { IDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; +import { FileChangeType, IFileService } from '../../../../../../platform/files/common/files.js'; import { IHoverService } from '../../../../../../platform/hover/browser/hover.js'; import { IMarkdownRendererService } from '../../../../../../platform/markdown/browser/markdownRenderer.js'; import { defaultButtonStyles } from '../../../../../../platform/theme/browser/defaultStyles.js'; @@ -52,8 +56,10 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart { private _submitButton: Button | undefined; private _renderedSubmitInlineCount = -1; private readonly _messageContentDisposables = this._register(new MutableDisposable()); + private readonly _planChangeListeners = this._register(new DisposableStore()); private readonly _titleActionsEl: HTMLElement; + private readonly _outdatedBadgeEl: HTMLElement; private readonly _inlineActionsEl: HTMLElement; private readonly _footerButtonsEl: HTMLElement; private readonly _messageEl: HTMLElement; @@ -86,6 +92,8 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart { @IPlanReviewFeedbackService private readonly _planReviewFeedbackService: IPlanReviewFeedbackService, @IAgentEditorCommentsBridge private readonly _agentEditorCommentsBridge: IAgentEditorCommentsBridge, @ITextFileService private readonly _textFileService: ITextFileService, + @IModelService private readonly _modelService: IModelService, + @IFileService private readonly _fileService: IFileService, ) { super(); @@ -137,7 +145,10 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart { const elements = dom.h('.chat-confirmation-widget-container.chat-plan-review-container@container', [ dom.h('.chat-confirmation-widget2.chat-plan-review@root', [ dom.h('.chat-confirmation-widget-title.chat-plan-review-title@title', [ - dom.h('.chat-plan-review-title-label@titleLabel'), + dom.h('.chat-plan-review-title-content', [ + dom.h('.chat-plan-review-title-label@titleLabel'), + dom.h('span.chat-plan-review-outdated@outdatedBadge'), + ]), dom.h('.chat-plan-review-inline-actions@inlineActions'), dom.h('.chat-plan-review-title-actions@titleActions'), ]), @@ -155,6 +166,7 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart { this.domNode.setAttribute('aria-label', localize('chat.planReview.ariaLabel', 'Plan review: {0}', review.title)); this._titleActionsEl = elements.titleActions; + this._outdatedBadgeEl = elements.outdatedBadge; this._inlineActionsEl = elements.inlineActions; this._footerButtonsEl = elements.footerButtons; this._messageEl = elements.message; @@ -162,6 +174,12 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart { // Title label + hover for truncated titles. elements.titleLabel.textContent = review.title; this._register(this._hoverService.setupDelayedHover(elements.titleLabel, { content: review.title })); + this._outdatedBadgeEl.textContent = localize('chat.planReview.outdated', 'Outdated'); + this._outdatedBadgeEl.setAttribute('aria-label', localize('chat.planReview.outdatedAriaLabel', 'Plan summary is outdated')); + if (!review.isOutdated) { + dom.hide(this._outdatedBadgeEl); + } + this.watchPlanChanges(); // Review button — opens the plan file and enters feedback mode. if (review.planUri) { @@ -241,6 +259,43 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart { } } + private watchPlanChanges(): void { + if (!this.review.planUri || this.review.isOutdated) { + return; + } + + const planUri = URI.revive(this.review.planUri); + const modelListener = this._planChangeListeners.add(new MutableDisposable()); + const watchModel = (model: ITextModel) => { + if (isEqual(model.uri, planUri)) { + modelListener.value = model.onDidChangeContent(() => this.markOutdated()); + } + }; + + const model = this._modelService.getModel(planUri); + if (model) { + watchModel(model); + } + this._planChangeListeners.add(this._modelService.onModelAdded(watchModel)); + const watcher = this._planChangeListeners.add(this._fileService.createWatcher(planUri, { recursive: false, excludes: [] })); + this._planChangeListeners.add(watcher.onDidChange(event => { + if (event.contains(planUri, FileChangeType.DELETED) || (!this._modelService.getModel(planUri) && event.contains(planUri, FileChangeType.ADDED, FileChangeType.UPDATED))) { + this.markOutdated(); + } + })); + } + + private markOutdated(): void { + if (this.review.isOutdated) { + return; + } + + this.review.isOutdated = true; + dom.show(this._outdatedBadgeEl); + this._planChangeListeners.clear(); + status(localize('chat.planReview.outdatedAnnouncement', 'Plan summary is outdated')); + } + hasSameContent(other: IChatRendererContent, _followingContent: IChatRendererContent[], _element: ChatTreeItem): boolean { if (other.kind !== 'planReview') { return false; diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatPlanReview.css b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatPlanReview.css index f005e1c6d7f..d49aee9cbeb 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatPlanReview.css +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatPlanReview.css @@ -74,8 +74,16 @@ flex-shrink: 0; } -.interactive-session .chat-plan-review-container .chat-plan-review-title-label { +.interactive-session .chat-plan-review-container .chat-plan-review-title-content { flex: 1; + min-width: 0; + display: flex; + align-items: center; + gap: var(--vscode-spacing-size60); + overflow: hidden; +} + +.interactive-session .chat-plan-review-container .chat-plan-review-title-label { min-width: 0; font-weight: var(--vscode-agents-fontWeight-semiBold); font-size: var(--vscode-chat-font-size-body-s); @@ -84,6 +92,17 @@ text-overflow: ellipsis; } +.interactive-session .chat-plan-review-container .chat-plan-review-outdated { + flex-shrink: 0; + padding: 0 var(--vscode-spacing-size60); + border: var(--vscode-strokeThickness) solid var(--vscode-chat-requestBorder); + border-radius: var(--vscode-cornerRadius-circle); + color: var(--vscode-descriptionForeground); + font-size: var(--vscode-fontSize-label3); + font-weight: var(--vscode-fontWeight-regular); + line-height: 14px; +} + .interactive-session .chat-plan-review-container .chat-plan-review-title-actions { display: flex; align-items: center; diff --git a/src/vs/workbench/contrib/chat/common/chatService/chatService.ts b/src/vs/workbench/contrib/chat/common/chatService/chatService.ts index 19346eb60cc..c8741eb150d 100644 --- a/src/vs/workbench/contrib/chat/common/chatService/chatService.ts +++ b/src/vs/workbench/contrib/chat/common/chatService/chatService.ts @@ -1410,6 +1410,8 @@ export interface IChatPlanReview { data?: IChatPlanReviewResult; /** Whether the widget has been responded to. */ isUsed?: boolean; + /** Whether the backing plan file changed after this summary was generated. */ + isOutdated?: boolean; /** Source attribution. */ source?: ToolDataSource; } diff --git a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatPlanReviewData.ts b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatPlanReviewData.ts index 32ab87e8db6..f19521d1501 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatPlanReviewData.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatPlanReviewData.ts @@ -33,6 +33,7 @@ export class ChatPlanReviewData implements IChatPlanReview { public data?: IChatPlanReviewResult, public isUsed?: boolean, public source?: ToolDataSource, + public isOutdated?: boolean, ) { } /** Dismiss without a user choice (e.g. the response was cancelled). */ @@ -58,6 +59,7 @@ export class ChatPlanReviewData implements IChatPlanReview { resolveId: this.resolveId, data: this.data, isUsed: this.isUsed, + isOutdated: this.isOutdated, source: this.source, }; } diff --git a/src/vs/workbench/contrib/chat/common/model/chatSessionOperationLog.ts b/src/vs/workbench/contrib/chat/common/model/chatSessionOperationLog.ts index 951ca1bde83..d99f3d73b17 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatSessionOperationLog.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatSessionOperationLog.ts @@ -65,6 +65,7 @@ const responsePartSchema = Adapt.v { let lastFeedbackService: IPlanReviewFeedbackService | undefined; let lastEditorService: IEditorService | undefined; let lastTextFileService: ITextFileService | undefined; + let lastModelService: IModelService | undefined; let lastCommentsBridge: AgentEditorCommentsBridge | undefined; + let fileChangesEmitter: Emitter | undefined; function createWidget(review: IChatPlanReview, dialogService?: TestDialogService, onSubmit?: () => void): ChatPlanReviewPart { const instantiationService = workbenchInstantiationService(undefined, store); @@ -95,7 +99,14 @@ suite('ChatPlanReviewPart', () => { lastFeedbackService = feedbackService; lastEditorService = instantiationService.get(IEditorService); lastTextFileService = instantiationService.get(ITextFileService); + lastModelService = instantiationService.get(IModelService); lastCommentsBridge = commentsBridge; + if (fileChangesEmitter) { + sinon.stub(instantiationService.get(IFileService), 'createWatcher').returns({ + onDidChange: fileChangesEmitter.event, + dispose: () => { }, + }); + } if (dialogService) { instantiationService.stub(IDialogService, dialogService); } @@ -120,7 +131,9 @@ suite('ChatPlanReviewPart', () => { lastFeedbackService = undefined; lastEditorService = undefined; lastTextFileService = undefined; + lastModelService = undefined; lastCommentsBridge = undefined; + fileChangesEmitter = undefined; sinon.restore(); }); @@ -141,6 +154,80 @@ suite('ChatPlanReviewPart', () => { assert.strictEqual(label?.textContent, 'My Plan Title'); }); + test('displays the outdated pill only for outdated summaries', () => { + createWidget(createMockReviewWithPlan({ isOutdated: true })); + + const badge = widget.domNode.querySelector('.chat-plan-review-outdated'); + assert.deepStrictEqual({ + text: badge?.textContent, + display: badge?.style.display, + ariaLabel: badge?.getAttribute('aria-label'), + }, { + text: 'Outdated', + display: '', + ariaLabel: 'Plan summary is outdated', + }); + }); + + test('hides the outdated pill for current summaries', () => { + createWidget(createMockReviewWithPlan()); + + assert.strictEqual(widget.domNode.querySelector('.chat-plan-review-outdated')?.style.display, 'none'); + }); + + test('marks the summary outdated when the plan model changes', () => { + const planUri = URI.parse('file:///outdated-plan.md'); + const review = new ChatPlanReviewData( + 'Plan summary', + 'Generated summary', + [{ label: 'Go', default: true }], + true, + planUri.toJSON(), + ); + createWidget(review); + const model = lastModelService!.createModel('# Original plan', null, planUri); + + try { + model.setValue('# Edited plan'); + + assert.deepStrictEqual({ + isOutdated: review.isOutdated, + persistedIsOutdated: review.toJSON().isOutdated, + badgeDisplay: widget.domNode.querySelector('.chat-plan-review-outdated')?.style.display, + summary: review.content, + }, { + isOutdated: true, + persistedIsOutdated: true, + badgeDisplay: '', + summary: 'Generated summary', + }); + } finally { + model.dispose(); + } + }); + + test('marks the summary outdated when an open plan is deleted', () => { + const planUri = URI.parse('file:///deleted-plan.md'); + const review = new ChatPlanReviewData( + 'Plan summary', + 'Generated summary', + [{ label: 'Go', default: true }], + true, + planUri.toJSON(), + ); + fileChangesEmitter = store.add(new Emitter()); + createWidget(review); + const model = lastModelService!.createModel('# Original plan', null, planUri); + + try { + fileChangesEmitter.fire(new FileChangesEvent([{ resource: planUri, type: FileChangeType.DELETED }], false)); + + assert.strictEqual(review.isOutdated, true); + } finally { + model.dispose(); + } + }); + test('renders markdown content in the body', () => { createWidget(createMockReview({ content: '**bold text**' })); diff --git a/src/vs/workbench/contrib/chat/test/common/model/chatSessionOperationLog.test.ts b/src/vs/workbench/contrib/chat/test/common/model/chatSessionOperationLog.test.ts index 3e11b95711d..31602b4b2ef 100644 --- a/src/vs/workbench/contrib/chat/test/common/model/chatSessionOperationLog.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/model/chatSessionOperationLog.test.ts @@ -5,12 +5,33 @@ import assert from 'assert'; import { VSBuffer } from '../../../../../../base/common/buffer.js'; +import { DisposableStore } from '../../../../../../base/common/lifecycle.js'; +import { hasKey } from '../../../../../../base/common/types.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { Range } from '../../../../../../editor/common/core/range.js'; +import { OffsetRange } from '../../../../../../editor/common/core/ranges/offsetRange.js'; +import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; +import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js'; +import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; +import { MockContextKeyService } from '../../../../../../platform/keybinding/test/common/mockKeybindingService.js'; +import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; +import { IStorageService } from '../../../../../../platform/storage/common/storage.js'; +import { IExtensionService } from '../../../../../services/extensions/common/extensions.js'; +import { TestExtensionService, TestStorageService } from '../../../../../test/common/workbenchTestServices.js'; +import { IChatService } from '../../../common/chatService/chatService.js'; +import { ChatAgentLocation } from '../../../common/constants.js'; +import { ChatModel } from '../../../common/model/chatModel.js'; +import { ChatPlanReviewData } from '../../../common/model/chatProgressTypes/chatPlanReviewData.js'; +import { ChatSessionOperationLog } from '../../../common/model/chatSessionOperationLog.js'; import * as Adapt from '../../../common/model/objectMutationLog.js'; import { equals } from '../../../../../../base/common/objects.js'; +import { ChatAgentService, IChatAgentService } from '../../../common/participants/chatAgents.js'; +import { ChatRequestTextPart } from '../../../common/requestParser/chatParserTypes.js'; +import { MockChatService } from '../chatService/mockChatService.js'; suite('ChatSessionOperationLog', () => { - ensureNoDisposablesAreLeakedInTestSuite(); + const testDisposables = ensureNoDisposablesAreLeakedInTestSuite(); // Test data types interface TestItem { @@ -59,6 +80,41 @@ suite('ChatSessionOperationLog', () => { return reader.read(fileContent); } + test('persists plan review changes through the operation log', () => { + const store = testDisposables.add(new DisposableStore()); + const instantiationService = store.add(new TestInstantiationService()); + instantiationService.stub(IStorageService, store.add(new TestStorageService())); + instantiationService.stub(ILogService, new NullLogService()); + instantiationService.stub(IExtensionService, new TestExtensionService()); + instantiationService.stub(IContextKeyService, new MockContextKeyService()); + instantiationService.stub(IChatAgentService, store.add(instantiationService.createInstance(ChatAgentService))); + instantiationService.stub(IConfigurationService, new TestConfigurationService()); + instantiationService.stub(IChatService, new MockChatService()); + + const model = store.add(instantiationService.createInstance(ChatModel, undefined, { initialLocation: ChatAgentLocation.Chat, canUseTools: true })); + const text = 'Create a plan'; + const request = model.addRequest({ + text, + parts: [new ChatRequestTextPart(new OffsetRange(0, text.length), new Range(1, 1, 1, text.length + 1), text)], + }, { variables: [] }, 0); + const review = new ChatPlanReviewData('Plan summary', 'Generated summary', [{ label: 'Approve' }], true); + model.acceptResponseProgress(request, review); + + const writer = new ChatSessionOperationLog(); + const initial = writer.createInitial(model); + review.isOutdated = true; + const update = writer.write(model); + writer.confirmWrite(); + + const reader = new ChatSessionOperationLog(); + const restored = reader.read(VSBuffer.concat([initial, update.data])); + const restoredReview = restored.requests[0].response?.[0]; + if (!restoredReview || !hasKey(restoredReview, { kind: true }) || restoredReview.kind !== 'planReview') { + assert.fail('Expected a restored plan review'); + } + assert.strictEqual(restoredReview.isOutdated, true); + }); + suite('Transform factories', () => { test('key uses strict equality by default', () => { const transform = Adapt.key();