Skip to content

Commit

Permalink
more appropriate name for isHidden, move remove-on-send logic into …
Browse files Browse the repository at this point in the history
…chatService
  • Loading branch information
jrieken committed Jan 9, 2025
1 parent 30e6417 commit 29bd016
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 35 deletions.
10 changes: 2 additions & 8 deletions src/vs/workbench/contrib/chat/browser/chatWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
// Re-render once content references are loaded
(isResponseVM(element) ? `_${element.contentReferences.length}` : '') +
// Re-render if element becomes hidden due to undo/redo
`_${element.isHidden ? '1' : '0'}` +
`_${element.shouldBeRemovedOnSend ? '1' : '0'}` +
// Rerender request if we got new content references in the response
// since this may change how we render the corresponding attachments in the request
(isRequestVM(element) && element.contentReferences ? `_${element.contentReferences?.length}` : '');
Expand Down Expand Up @@ -1002,13 +1002,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
`${query.prefix} ${editorValue}`;
const isUserQuery = !query || 'prefix' in query;

const requests = this.viewModel.model.getRequests();
for (let i = requests.length - 1; i >= 0; i -= 1) {
const request = requests[i];
if (request.isHidden) {
this.chatService.removeRequest(this.viewModel.sessionId, request.id);
}
}


let attachedContext = this.inputPart.getAttachedAndImplicitContext(this.viewModel.sessionId);
let workingSet: URI[] | undefined;
Expand Down
33 changes: 19 additions & 14 deletions src/vs/workbench/contrib/chat/common/chatModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export interface IChatRequestModel {
readonly workingSet?: URI[];
readonly isCompleteAddedRequest: boolean;
readonly response?: IChatResponseModel;
isHidden: boolean;
shouldBeRemovedOnSend: boolean;
}

export interface IChatTextEditGroupState {
Expand Down Expand Up @@ -207,7 +207,7 @@ export interface IChatResponseModel {
readonly response: IResponse;
readonly isComplete: boolean;
readonly isCanceled: boolean;
readonly isHidden: boolean;
readonly shouldBeRemovedOnSend: boolean;
readonly isCompleteAddedRequest: boolean;
/** A stale response is one that has been persisted and rehydrated, so e.g. Commands that have their arguments stored in the EH are gone. */
readonly isStale: boolean;
Expand All @@ -230,7 +230,7 @@ export class ChatRequestModel implements IChatRequestModel {
return this._session;
}

public isHidden: boolean = false;
public shouldBeRemovedOnSend: boolean = false;

public get username(): string {
return this.session.requesterUsername;
Expand Down Expand Up @@ -506,16 +506,16 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel
return this._session;
}

public get isHidden() {
return this._isHidden;
public get shouldBeRemovedOnSend() {
return this._shouldBeRemovedOnSend;
}

public get isComplete(): boolean {
return this._isComplete;
}

public set isHidden(hidden: boolean) {
this._isHidden = hidden;
public set shouldBeRemovedOnSend(hidden: boolean) {
this._shouldBeRemovedOnSend = hidden;
this._onDidChange.fire();
}

Expand Down Expand Up @@ -605,7 +605,7 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel
private _result?: IChatAgentResult,
followups?: ReadonlyArray<IChatFollowup>,
public readonly isCompleteAddedRequest = false,
private _isHidden: boolean = false,
private _shouldBeRemovedOnSend: boolean = false,
restoredId?: string
) {
super();
Expand Down Expand Up @@ -733,6 +733,8 @@ export interface ISerializableChatRequestData {
/** Is really like "prompt data". This is the message in the format in which the agent gets it + variable values. */
variableData: IChatRequestVariableData;
response: ReadonlyArray<IMarkdownString | IChatResponseProgressFileTreeData | IChatContentInlineReference | IChatAgentMarkdownContentWithVulnerability> | undefined;

/**Old, persisted name for shouldBeRemovedOnSend */
isHidden: boolean;
responseId?: string;
agent?: ISerializableChatAgentData;
Expand Down Expand Up @@ -1084,7 +1086,7 @@ export class ChatModel extends Disposable implements IChatModel {
// Old messages don't have variableData, or have it in the wrong (non-array) shape
const variableData: IChatRequestVariableData = this.reviveVariableData(raw.variableData);
const request = new ChatRequestModel(this, parsedRequest, variableData, raw.timestamp ?? -1, undefined, undefined, undefined, undefined, raw.workingSet?.map((uri) => URI.revive(uri)), undefined, raw.requestId);
request.isHidden = !!raw.isHidden;
request.shouldBeRemovedOnSend = !!raw.isHidden;
if (raw.response || raw.result || (raw as any).responseErrorDetails) {
const agent = (raw.agent && 'metadata' in raw.agent) ? // Check for the new format, ignore entries in the old format
reviveSerializedAgent(raw.agent) : undefined;
Expand All @@ -1094,7 +1096,7 @@ export class ChatModel extends Disposable implements IChatModel {
// eslint-disable-next-line local/code-no-dangerous-type-assertions
{ errorDetails: raw.responseErrorDetails } as IChatAgentResult : raw.result;
request.response = new ChatResponseModel(raw.response ?? [new MarkdownString(raw.response)], this, agent, raw.slashCommand, request.id, true, raw.isCanceled, raw.vote, raw.voteDownReason, result, raw.followups, undefined, undefined, raw.responseId);
request.response.isHidden = !!raw.isHidden;
request.response.shouldBeRemovedOnSend = !!raw.isHidden;
if (raw.usedContext) { // @ulugbekna: if this's a new vscode sessions, doc versions are incorrect anyway?
request.response.applyReference(revive(raw.usedContext));
}
Expand Down Expand Up @@ -1193,11 +1195,14 @@ export class ChatModel extends Disposable implements IChatModel {
}

disableRequests(requestIds: ReadonlyArray<string>) {

const toHide = new Set(requestIds);

this._requests.forEach((request) => {
const isHidden = requestIds.includes(request.id);
request.isHidden = isHidden;
const shouldBeRemovedOnSend = toHide.has(request.id);
request.shouldBeRemovedOnSend = shouldBeRemovedOnSend;
if (request.response) {
request.response.isHidden = isHidden;
request.response.shouldBeRemovedOnSend = shouldBeRemovedOnSend;
}
});

Expand Down Expand Up @@ -1364,7 +1369,7 @@ export class ChatModel extends Disposable implements IChatModel {
})
: undefined,
responseId: r.response?.id,
isHidden: r.isHidden,
isHidden: r.shouldBeRemovedOnSend,
result: r.response?.result,
followups: r.response?.followups,
isCanceled: r.response?.isCanceled,
Expand Down
8 changes: 8 additions & 0 deletions src/vs/workbench/contrib/chat/common/chatServiceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,14 @@ export class ChatService extends Disposable implements IChatService {
return;
}

const requests = model.getRequests();
for (let i = requests.length - 1; i >= 0; i -= 1) {
const request = requests[i];
if (request.shouldBeRemovedOnSend) {
this.removeRequest(sessionId, request.id);
}
}

const location = options?.location ?? model.initialLocation;
const attempt = options?.attempt ?? 0;
const defaultAgent = this.chatAgentService.getDefaultAgent(location)!;
Expand Down
20 changes: 9 additions & 11 deletions src/vs/workbench/contrib/chat/common/chatViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { Emitter, Event } from '../../../../base/common/event.js';
import { IMarkdownString } from '../../../../base/common/htmlContent.js';
import { Disposable } from '../../../../base/common/lifecycle.js';
import { Disposable, dispose } from '../../../../base/common/lifecycle.js';
import * as marked from '../../../../base/common/marked/marked.js';
import { ThemeIcon } from '../../../../base/common/themables.js';
import { URI } from '../../../../base/common/uri.js';
Expand Down Expand Up @@ -74,7 +74,7 @@ export interface IChatRequestViewModel {
readonly contentReferences?: ReadonlyArray<IChatContentReference>;
readonly workingSet?: ReadonlyArray<URI>;
readonly confirmation?: string;
readonly isHidden: boolean;
readonly shouldBeRemovedOnSend: boolean;
readonly isComplete: boolean;
readonly isCompleteAddedRequest: boolean;
readonly slashCommand: IChatAgentCommand | undefined;
Expand Down Expand Up @@ -180,7 +180,7 @@ export interface IChatResponseViewModel {
readonly errorDetails?: IChatResponseErrorDetails;
readonly result?: IChatAgentResult;
readonly contentUpdateTimings?: IChatLiveUpdateData;
readonly isHidden: boolean;
readonly shouldBeRemovedOnSend: boolean;
readonly isCompleteAddedRequest: boolean;
renderData?: IChatResponseRenderData;
currentRenderedHeight: number | undefined;
Expand Down Expand Up @@ -299,14 +299,12 @@ export class ChatViewModel extends Disposable implements IChatViewModel {
}

getItems(): (IChatRequestViewModel | IChatResponseViewModel)[] {
return [...this._items].filter((item) => !item.isHidden);
return this._items.filter((item) => !item.shouldBeRemovedOnSend);
}

override dispose() {
super.dispose();
this._items
.filter((item): item is ChatResponseViewModel => item instanceof ChatResponseViewModel)
.forEach((item: ChatResponseViewModel) => item.dispose());
dispose(this._items.filter((item): item is ChatResponseViewModel => item instanceof ChatResponseViewModel));
}

updateCodeBlockTextModels(model: IChatRequestViewModel | IChatResponseViewModel) {
Expand Down Expand Up @@ -385,8 +383,8 @@ export class ChatRequestViewModel implements IChatRequestViewModel {
return this._model.isCompleteAddedRequest;
}

get isHidden() {
return this._model.isHidden;
get shouldBeRemovedOnSend() {
return this._model.shouldBeRemovedOnSend;
}

get slashCommand(): IChatAgentCommand | undefined {
Expand Down Expand Up @@ -486,8 +484,8 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi
return this._model.isCanceled;
}

get isHidden() {
return this._model.isHidden;
get shouldBeRemovedOnSend() {
return this._model.shouldBeRemovedOnSend;
}

get isCompleteAddedRequest() {
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/chat/test/common/chatModel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ suite('ChatModel', () => {

assert.strictEqual(request1.isCompleteAddedRequest, true);
assert.strictEqual(request1.response!.isCompleteAddedRequest, true);
assert.strictEqual(request1.isHidden, false);
assert.strictEqual(request1.response!.isHidden, false);
assert.strictEqual(request1.shouldBeRemovedOnSend, false);
assert.strictEqual(request1.response!.shouldBeRemovedOnSend, false);
});
});

Expand Down

0 comments on commit 29bd016

Please sign in to comment.