Skip to content

Commit 3b41744

Browse files
roblourensroblourens
authored
Fix using dynamic variables with chat input history navigation (#199133)
* Fix using dynamic variables with chat input history navigation and reloading the window Instead of just saving string history items, saving and restoring a string input plus * Don't deepClone variable values that contain URIs * Remove test code
1 parent 8b85934 commit 3b41744

File tree

7 files changed

+111
-39
lines changed

7 files changed

+111
-39
lines changed

‎src/vs/workbench/contrib/chat/browser/chatEditor.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { EditorPane } from 'vs/workbench/browser/parts/editor/editorPane';
1818
import { IEditorOpenContext } from 'vs/workbench/common/editor';
1919
import { Memento } from 'vs/workbench/common/memento';
2020
import { ChatEditorInput } from 'vs/workbench/contrib/chat/browser/chatEditorInput';
21-
import { IViewState, ChatWidget } from 'vs/workbench/contrib/chat/browser/chatWidget';
21+
import { IChatViewState, ChatWidget } from 'vs/workbench/contrib/chat/browser/chatWidget';
2222
import { IChatModel, ISerializableChatData } from 'vs/workbench/contrib/chat/common/chatModel';
2323
import { clearChatEditor } from 'vs/workbench/contrib/chat/browser/actions/chatClear';
2424

@@ -35,7 +35,7 @@ export class ChatEditor extends EditorPane {
3535
}
3636

3737
private _memento: Memento | undefined;
38-
private _viewState: IViewState | undefined;
38+
private _viewState: IChatViewState | undefined;
3939

4040
constructor(
4141
@ITelemetryService telemetryService: ITelemetryService,
@@ -99,7 +99,7 @@ export class ChatEditor extends EditorPane {
9999

100100
private updateModel(model: IChatModel): void {
101101
this._memento = new Memento('interactive-session-editor-' + model.sessionId, this.storageService);
102-
this._viewState = this._memento.getMemento(StorageScope.WORKSPACE, StorageTarget.MACHINE) as IViewState;
102+
this._viewState = this._memento.getMemento(StorageScope.WORKSPACE, StorageTarget.MACHINE) as IChatViewState;
103103
this.widget.setModel(model, { ...this._viewState });
104104
}
105105

‎src/vs/workbench/contrib/chat/browser/chatInputPart.ts‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
5151
static readonly INPUT_SCHEME = 'chatSessionInput';
5252
private static _counter = 0;
5353

54+
private _onDidLoadInputState = this._register(new Emitter<any>());
55+
readonly onDidLoadInputState = this._onDidLoadInputState.event;
56+
5457
private _onDidChangeHeight = this._register(new Emitter<void>());
5558
readonly onDidChangeHeight = this._onDidChangeHeight.event;
5659

@@ -78,7 +81,7 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
7881
return this._inputEditor;
7982
}
8083

81-
private history: HistoryNavigator<string>;
84+
private history: HistoryNavigator<{ text: string; state?: any }>;
8285
private historyNavigationBackwardsEnablement!: IContextKey<boolean>;
8386
private historyNavigationForewardsEnablement!: IContextKey<boolean>;
8487
private inputModel: ITextModel | undefined;
@@ -124,7 +127,7 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
124127

125128
setState(providerId: string, inputValue: string | undefined): void {
126129
this.providerId = providerId;
127-
const history = this.historyService.getHistory(providerId).map(h => typeof h === 'object' ? (h as any).text : h);
130+
const history = this.historyService.getHistory(providerId);
128131
this.history = new HistoryNavigator(history, 50);
129132

130133
if (typeof inputValue === 'string') {
@@ -147,10 +150,11 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
147150
private navigateHistory(previous: boolean): void {
148151
const historyInput = (previous ?
149152
(this.history.previous() ?? this.history.first()) : this.history.next())
150-
?? '';
153+
?? { text: '' };
151154

152-
aria.status(historyInput);
153-
this.setValue(historyInput);
155+
aria.status(historyInput.text);
156+
this.setValue(historyInput.text);
157+
this._onDidLoadInputState.fire(historyInput.state);
154158
if (previous) {
155159
this._inputEditor.setPosition({ lineNumber: 1, column: 1 });
156160
} else {
@@ -181,9 +185,9 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
181185
* Reset the input and update history.
182186
* @param userQuery If provided, this will be added to the history. Followups and programmatic queries should not be passed.
183187
*/
184-
async acceptInput(userQuery?: string): Promise<void> {
188+
async acceptInput(userQuery?: string, inputState?: any): Promise<void> {
185189
if (userQuery) {
186-
this.history.add(userQuery);
190+
this.history.add({ text: userQuery, state: inputState });
187191
}
188192

189193
if (this.accessibilityService.isScreenReaderOptimized() && isMacintosh) {

‎src/vs/workbench/contrib/chat/browser/chatViewPane.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,15 @@ import { Memento } from 'vs/workbench/common/memento';
2222
import { SIDE_BAR_FOREGROUND } from 'vs/workbench/common/theme';
2323
import { IViewDescriptorService } from 'vs/workbench/common/views';
2424
import { IChatViewPane } from 'vs/workbench/contrib/chat/browser/chat';
25-
import { IViewState, ChatWidget } from 'vs/workbench/contrib/chat/browser/chatWidget';
25+
import { IChatViewState, ChatWidget } from 'vs/workbench/contrib/chat/browser/chatWidget';
2626
import { IChatModel } from 'vs/workbench/contrib/chat/common/chatModel';
2727
import { IChatService } from 'vs/workbench/contrib/chat/common/chatService';
2828

2929
export interface IChatViewOptions {
3030
readonly providerId: string;
3131
}
3232

33-
interface IViewPaneState extends IViewState {
33+
interface IViewPaneState extends IChatViewState {
3434
sessionId?: string;
3535
}
3636

@@ -43,7 +43,7 @@ export class ChatViewPane extends ViewPane implements IChatViewPane {
4343

4444
private modelDisposables = this._register(new DisposableStore());
4545
private memento: Memento;
46-
private viewState: IViewPaneState;
46+
private readonly viewState: IViewPaneState;
4747
private didProviderRegistrationFail = false;
4848

4949
constructor(
@@ -199,6 +199,7 @@ export class ChatViewPane extends ViewPane implements IChatViewPane {
199199

200200
const widgetViewState = this._widget.getViewState();
201201
this.viewState.inputValue = widgetViewState.inputValue;
202+
this.viewState.inputState = widgetViewState.inputState;
202203
this.memento.saveMemento();
203204
}
204205

‎src/vs/workbench/contrib/chat/browser/chatWidget.ts‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ function revealLastElement(list: WorkbenchObjectTree<any>) {
3939
list.scrollTop = list.scrollHeight - list.renderHeight;
4040
}
4141

42-
export interface IViewState {
42+
export type IChatInputState = Record<string, any>;
43+
export interface IChatViewState {
4344
inputValue?: string;
44-
// renderData
45+
inputState?: IChatInputState;
4546
}
4647

4748
export interface IChatWidgetStyles {
@@ -53,6 +54,16 @@ export interface IChatWidgetStyles {
5354

5455
export interface IChatWidgetContrib extends IDisposable {
5556
readonly id: string;
57+
58+
/**
59+
* A piece of state which is related to the input editor of the chat widget
60+
*/
61+
getInputState?(): any;
62+
63+
/**
64+
* Called with the result of getInputState when navigating input history.
65+
*/
66+
setInputState?(s: any): void;
5667
}
5768

5869
export class ChatWidget extends Disposable implements IChatWidget {
@@ -390,6 +401,13 @@ export class ChatWidget extends Disposable implements IChatWidget {
390401
}));
391402
this.inputPart.render(container, '', this);
392403

404+
this._register(this.inputPart.onDidLoadInputState(state => {
405+
this.contribs.forEach(c => {
406+
if (c.setInputState && state[c.id]) {
407+
c.setInputState(state[c.id]);
408+
}
409+
});
410+
}));
393411
this._register(this.inputPart.onDidFocus(() => this._onDidFocus.fire()));
394412
this._register(this.inputPart.onDidAcceptFollowup(e => {
395413
if (!this.viewModel) {
@@ -423,7 +441,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
423441
this.container.style.setProperty('--vscode-interactive-session-foreground', this.editorOptions.configuration.foreground?.toString() ?? '');
424442
}
425443

426-
setModel(model: IChatModel, viewState: IViewState): void {
444+
setModel(model: IChatModel, viewState: IChatViewState): void {
427445
if (!this.container) {
428446
throw new Error('Call render() before setModel()');
429447
}
@@ -447,6 +465,11 @@ export class ChatWidget extends Disposable implements IChatWidget {
447465
this.onDidChangeItems();
448466
}));
449467
this.inputPart.setState(model.providerId, viewState.inputValue);
468+
this.contribs.forEach(c => {
469+
if (c.setInputState && viewState.inputState?.[c.id]) {
470+
c.setInputState(viewState.inputState?.[c.id]);
471+
}
472+
});
450473

451474
if (this.tree) {
452475
this.onDidChangeItems();
@@ -497,6 +520,16 @@ export class ChatWidget extends Disposable implements IChatWidget {
497520
this._acceptInput({ prefix });
498521
}
499522

523+
private collectInputState(): IChatInputState {
524+
const inputState: IChatInputState = {};
525+
this.contribs.forEach(c => {
526+
if (c.getInputState) {
527+
inputState[c.id] = c.getInputState();
528+
}
529+
});
530+
return inputState;
531+
}
532+
500533
private async _acceptInput(opts: { query: string } | { prefix: string } | undefined): Promise<void> {
501534
if (this.viewModel) {
502535
this._onDidAcceptInput.fire();
@@ -510,7 +543,8 @@ export class ChatWidget extends Disposable implements IChatWidget {
510543
const result = await this.chatService.sendRequest(this.viewModel.sessionId, input);
511544

512545
if (result) {
513-
this.inputPart.acceptInput(isUserQuery ? input : undefined);
546+
const inputState = this.collectInputState();
547+
this.inputPart.acceptInput(isUserQuery ? input : undefined, isUserQuery ? inputState : undefined);
514548
result.responseCompletePromise.then(async () => {
515549
const responses = this.viewModel?.getItems().filter(isResponseVM);
516550
const lastResponse = responses?.[responses.length - 1];
@@ -676,9 +710,9 @@ export class ChatWidget extends Disposable implements IChatWidget {
676710
this.inputPart.saveState();
677711
}
678712

679-
getViewState(): IViewState {
713+
getViewState(): IChatViewState {
680714
this.inputPart.saveState();
681-
return { inputValue: this.getInput() };
715+
return { inputValue: this.getInput(), inputState: this.collectInputState() };
682716
}
683717
}
684718

‎src/vs/workbench/contrib/chat/browser/contrib/chatDynamicVariables.ts‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { coalesce } from 'vs/base/common/arrays';
67
import { IMarkdownString, MarkdownString } from 'vs/base/common/htmlContent';
78
import { Disposable } from 'vs/base/common/lifecycle';
89
import { basename } from 'vs/base/common/resources';
@@ -24,7 +25,7 @@ export const dynamicVariableDecorationType = 'chat-dynamic-variable';
2425
export class ChatDynamicVariableModel extends Disposable implements IChatWidgetContrib {
2526
public static readonly ID = 'chatDynamicVariableModel';
2627

27-
private readonly _variables: IDynamicVariable[] = [];
28+
private _variables: IDynamicVariable[] = [];
2829
get variables(): ReadonlyArray<IDynamicVariable> {
2930
return [...this._variables];
3031
}
@@ -35,37 +36,59 @@ export class ChatDynamicVariableModel extends Disposable implements IChatWidgetC
3536

3637
constructor(
3738
private readonly widget: IChatWidget,
38-
@ILabelService private readonly labelService: ILabelService
39+
@ILabelService private readonly labelService: ILabelService,
40+
@ILogService private readonly logService: ILogService,
3941
) {
4042
super();
4143
this._register(widget.inputEditor.onDidChangeModelContent(e => {
4244
e.changes.forEach(c => {
43-
this._variables.forEach((ref, i) => {
45+
// Don't mutate entries in _variables, since they will be returned from the getter
46+
this._variables = coalesce(this._variables.map(ref => {
4447
if (Range.areIntersecting(ref.range, c.range)) {
4548
// The reference text was changed, it's broken
46-
this._variables.splice(i, 1);
49+
return null;
4750
} else if (Range.compareRangesUsingStarts(ref.range, c.range) > 0) {
4851
const delta = c.text.length - c.rangeLength;
49-
ref.range = {
50-
startLineNumber: ref.range.startLineNumber,
51-
startColumn: ref.range.startColumn + delta,
52-
endLineNumber: ref.range.endLineNumber,
53-
endColumn: ref.range.endColumn + delta
52+
return {
53+
...ref,
54+
range: {
55+
startLineNumber: ref.range.startLineNumber,
56+
startColumn: ref.range.startColumn + delta,
57+
endLineNumber: ref.range.endLineNumber,
58+
endColumn: ref.range.endColumn + delta
59+
}
5460
};
5561
}
56-
});
62+
63+
return ref;
64+
}));
5765
});
5866

59-
this.updateReferences();
67+
this.updateDecorations();
6068
}));
6169
}
6270

71+
getInputState(): any {
72+
return this.variables;
73+
}
74+
75+
setInputState(s: any): void {
76+
if (!Array.isArray(s)) {
77+
// Something went wrong
78+
this.logService.warn('ChatDynamicVariableModel.setInputState called with invalid state: ' + JSON.stringify(s));
79+
return;
80+
}
81+
82+
this._variables = s;
83+
this.updateDecorations();
84+
}
85+
6386
addReference(ref: IDynamicVariable): void {
6487
this._variables.push(ref);
65-
this.updateReferences();
88+
this.updateDecorations();
6689
}
6790

68-
private updateReferences(): void {
91+
private updateDecorations(): void {
6992
this.widget.inputEditor.setDecorationsByType('chat', dynamicVariableDecorationType, this._variables.map(r => (<IDecorationOptions>{
7093
range: r.range,
7194
hoverMessage: this.getHoverForReference(r)

‎src/vs/workbench/contrib/chat/common/chatVariables.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,5 @@ export interface IChatVariableResolveResult {
5252

5353
export interface IDynamicVariable {
5454
range: IRange;
55-
// data: any; // File details for a file, something else for a different type of thing, is it typed?
5655
data: IChatRequestVariableValue[];
5756
}

‎src/vs/workbench/contrib/chat/common/chatWidgetHistoryService.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,24 @@ import { createDecorator } from 'vs/platform/instantiation/common/instantiation'
88
import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
99
import { Memento } from 'vs/workbench/common/memento';
1010

11+
export interface IChatHistoryEntry {
12+
text: string;
13+
state?: any;
14+
}
15+
1116
export const IChatWidgetHistoryService = createDecorator<IChatWidgetHistoryService>('IChatWidgetHistoryService');
1217
export interface IChatWidgetHistoryService {
1318
_serviceBrand: undefined;
1419

1520
readonly onDidClearHistory: Event<void>;
1621

1722
clearHistory(): void;
18-
getHistory(providerId: string): string[];
19-
saveHistory(providerId: string, history: string[]): void;
23+
getHistory(providerId: string): IChatHistoryEntry[];
24+
saveHistory(providerId: string, history: IChatHistoryEntry[]): void;
2025
}
2126

2227
interface IChatHistory {
23-
history: { [providerId: string]: string[] };
28+
history: { [providerId: string]: IChatHistoryEntry[] };
2429
}
2530

2631
export class ChatWidgetHistoryService implements IChatWidgetHistoryService {
@@ -36,14 +41,20 @@ export class ChatWidgetHistoryService implements IChatWidgetHistoryService {
3641
@IStorageService storageService: IStorageService
3742
) {
3843
this.memento = new Memento('interactive-session', storageService);
39-
this.viewState = this.memento.getMemento(StorageScope.WORKSPACE, StorageTarget.MACHINE) as IChatHistory;
44+
const loadedState = this.memento.getMemento(StorageScope.WORKSPACE, StorageTarget.MACHINE) as IChatHistory;
45+
for (const provider in loadedState.history) {
46+
// Migration from old format
47+
loadedState.history[provider] = loadedState.history[provider].map(entry => typeof entry === 'string' ? { text: entry } : entry);
48+
}
49+
50+
this.viewState = loadedState;
4051
}
4152

42-
getHistory(providerId: string): string[] {
53+
getHistory(providerId: string): IChatHistoryEntry[] {
4354
return this.viewState.history?.[providerId] ?? [];
4455
}
4556

46-
saveHistory(providerId: string, history: string[]): void {
57+
saveHistory(providerId: string, history: IChatHistoryEntry[]): void {
4758
if (!this.viewState.history) {
4859
this.viewState.history = {};
4960
}

0 commit comments

Comments
 (0)