Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Try to clean up the chat model initialization code #179340

Merged
merged 3 commits into from Apr 6, 2023
Merged

Try to clean up the chat model initialization code #179340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af11ae4
Try to clean up the chat model initialization code
roblourens Apr 6, 2023
4bec2b7
Clean up when model initialization fail
roblourens Apr 6, 2023
b7d9277
Fix test
roblourens Apr 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion src/vs/workbench/contrib/interactiveSession/browser/interactiveSession.ts
View file
Open in desktop
Expand Up @@ -15,5 +15,4 @@ export interface IInteractiveSessionWidget {

acceptInput(): void;
getSlashCommands(): Promise<IInteractiveSlashCommand[] | undefined>;
waitForViewModel(): Promise<IInteractiveSessionViewModel | undefined>;
}
5 changes: 0 additions & 5 deletions src/vs/workbench/contrib/interactiveSession/browser/interactiveSessionSidebar.ts
View file
Open in desktop
Expand Up @@ -16,7 +16,6 @@ import { IThemeService } from 'vs/platform/theme/common/themeService';
import { IViewPaneOptions, ViewPane } from 'vs/workbench/browser/parts/views/viewPane';
import { IViewDescriptorService } from 'vs/workbench/common/views';
import { InteractiveSessionWidget } from 'vs/workbench/contrib/interactiveSession/browser/interactiveSessionWidget';
import { IInteractiveSessionViewModel } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionViewModel';

export interface IInteractiveSessionViewOptions {
readonly providerId: string;
Expand Down Expand Up @@ -59,10 +58,6 @@ export class InteractiveSessionViewPane extends ViewPane {
this.view.acceptInput(query);
}

waitForViewModel(): Promise<IInteractiveSessionViewModel | undefined> {
return this.view.waitForViewModel();
}

async clear(): Promise<void> {
await this.view.clear();
}
Expand Down
68 changes: 19 additions & 49 deletions src/vs/workbench/contrib/interactiveSession/browser/interactiveSessionWidget.ts
View file
Open in desktop
Expand Up @@ -29,7 +29,6 @@ import { InteractiveSessionEditorOptions } from 'vs/workbench/contrib/interactiv
import { CONTEXT_INTERACTIVE_REQUEST_IN_PROGRESS, CONTEXT_IN_INTERACTIVE_SESSION } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionContextKeys';
import { IInteractiveSessionReplyFollowup, IInteractiveSessionService, IInteractiveSlashCommand } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionService';
import { IInteractiveSessionViewModel, InteractiveSessionViewModel, isRequestVM, isResponseVM, isWelcomeVM } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionViewModel';
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';

export const IInteractiveSessionWidgetService = createDecorator<IInteractiveSessionWidgetService>('interactiveSessionWidgetService');

Expand Down Expand Up @@ -79,8 +78,6 @@ export class InteractiveSessionWidget extends Disposable implements IInteractive

private previousTreeScrollHeight: number = 0;

private currentViewModelPromise: Promise<IInteractiveSessionViewModel | undefined> | undefined;

private viewModelDisposables = new DisposableStore();
private _viewModel: InteractiveSessionViewModel | undefined;
private set viewModel(viewModel: InteractiveSessionViewModel | undefined) {
Expand All @@ -95,7 +92,6 @@ export class InteractiveSessionWidget extends Disposable implements IInteractive
this.viewModelDisposables.add(viewModel);
}

this.currentViewModelPromise = undefined;
this.slashCommandsPromise = undefined;
this.lastSlashCommands = undefined;
this.getSlashCommands().then(() => {
Expand Down Expand Up @@ -124,7 +120,6 @@ export class InteractiveSessionWidget extends Disposable implements IInteractive
@IStorageService storageService: IStorageService,
@IContextKeyService private readonly contextKeyService: IContextKeyService,
@IInstantiationService private readonly instantiationService: IInstantiationService,
@IExtensionService private readonly extensionService: IExtensionService,
@IInteractiveSessionService private readonly interactiveSessionService: IInteractiveSessionService,
@IInteractiveSessionWidgetService interactiveSessionWidgetService: IInteractiveSessionWidgetService,
@IContextMenuService private readonly contextMenuService: IContextMenuService,
Expand Down Expand Up @@ -340,49 +335,28 @@ export class InteractiveSessionWidget extends Disposable implements IInteractive
this.container.style.setProperty('--vscode-interactive-result-editor-background-color', this.editorOptions.configuration.resultEditor.backgroundColor?.toString() ?? '');
}

private async initializeSessionModel(initial = false) {
if (this.currentViewModelPromise) {
await this.currentViewModelPromise;
return;
private initializeSessionModel(initial = false) {
const model = this.interactiveSessionService.startSession(this.providerId, initial, CancellationToken.None);
if (!model) {
throw new Error('Failed to start session');
}

const doInitializeSessionModel = async () => {
await this.extensionService.whenInstalledExtensionsRegistered();
const model = await this.interactiveSessionService.startSession(this.providerId, initial, CancellationToken.None);
if (!model) {
throw new Error('Failed to start session');
}

if (this.viewModel) {
// Oops, created two. TODO this could be better
return;
}
this.viewModel = this.instantiationService.createInstance(InteractiveSessionViewModel, model);
this.viewModelDisposables.add(this.viewModel.onDidChange(() => {
this.slashCommandsPromise = undefined;
this.onDidChangeItems();
}));
this.viewModelDisposables.add(this.viewModel.onDidDisposeModel(() => {
this.viewModel = undefined;
this.onDidChangeItems();
}));

this.viewModel = this.instantiationService.createInstance(InteractiveSessionViewModel, model);
this.viewModelDisposables.add(this.viewModel.onDidChange(() => {
this.slashCommandsPromise = undefined;
this.onDidChangeItems();
}));
this.viewModelDisposables.add(this.viewModel.onDidDisposeModel(() => {
this.viewModel = undefined;
this.onDidChangeItems();
}));

if (this.tree) {
this.onDidChangeItems();
}
};
this.currentViewModelPromise = doInitializeSessionModel()
.then(() => this.viewModel);
await this.currentViewModelPromise;
if (this.tree) {
this.onDidChangeItems();
}
}

async acceptInput(query?: string | IInteractiveSessionReplyFollowup): Promise<void> {
if (!this.viewModel) {
// This currently shouldn't happen anymore, but leaving this here to make sure we don't get stuck without a viewmodel
await this.initializeSessionModel();
}

if (this.viewModel) {
const editorValue = this.inputPart.inputEditor.getValue();

Expand All @@ -395,10 +369,10 @@ export class InteractiveSessionWidget extends Disposable implements IInteractive
}

const input = query ?? editorValue;
const result = this.interactiveSessionService.sendRequest(this.viewModel.sessionId, input);
const result = await this.interactiveSessionService.sendRequest(this.viewModel.sessionId, input);
if (result) {
this.requestInProgress.set(true);
result.completePromise.finally(() => {
result.requestCompletePromise.finally(() => {
this.requestInProgress.set(false);
});

Expand All @@ -408,10 +382,6 @@ export class InteractiveSessionWidget extends Disposable implements IInteractive
}
}

async waitForViewModel(): Promise<IInteractiveSessionViewModel | undefined> {
return this.currentViewModelPromise;
}

focusLastMessage(): void {
if (!this.viewModel) {
return;
Expand All @@ -430,7 +400,7 @@ export class InteractiveSessionWidget extends Disposable implements IInteractive
async clear(): Promise<void> {
if (this.viewModel) {
this.interactiveSessionService.clearSession(this.viewModel.sessionId);
await this.initializeSessionModel();
this.initializeSessionModel();
this.focusInput();
}
}
Expand Down
68 changes: 54 additions & 14 deletions src/vs/workbench/contrib/interactiveSession/common/interactiveSessionModel.ts
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@
import { Emitter, Event } from 'vs/base/common/event';
import { IMarkdownString, MarkdownString } from 'vs/base/common/htmlContent';
import { Disposable } from 'vs/base/common/lifecycle';
import { URI } from 'vs/base/common/uri';
import { URI, UriComponents } from 'vs/base/common/uri';
import { ILogService } from 'vs/platform/log/common/log';
import { IInteractiveProgress, IInteractiveResponse, IInteractiveResponseErrorDetails, IInteractiveSession, IInteractiveSessionFollowup, IInteractiveSessionReplyFollowup, InteractiveSessionVoteDirection } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionService';

Expand Down Expand Up @@ -173,6 +173,10 @@ export interface ISerializableInteractiveSessionRequestData {

export interface ISerializableInteractiveSessionData {
requests: ISerializableInteractiveSessionRequestData[];
requesterUsername: string;
responderUsername: string;
requesterAvatarIconUri: UriComponents | undefined;
responderAvatarIconUri: UriComponents | undefined;
providerId: string;
providerState: any;
}
Expand All @@ -194,27 +198,42 @@ export interface IInteractiveSessionClearEvent {
}

export class InteractiveSessionModel extends Disposable implements IInteractiveSessionModel {
private static nextId = 0;

private readonly _onDidDispose = this._register(new Emitter<void>());
readonly onDidDispose = this._onDidDispose.event;

private readonly _onDidChange = this._register(new Emitter<IInteractiveSessionChangeEvent>());
readonly onDidChange = this._onDidChange.event;

private _requests: InteractiveRequestModel[];

private _session: IInteractiveSession | undefined;
get session(): IInteractiveSession | undefined {
return this._session;
}

private _welcomeMessage: InteractiveSessionWelcomeMessageModel | undefined;
get welcomeMessage(): InteractiveSessionWelcomeMessageModel | undefined {
return this._welcomeMessage;
}

private _providerState: any;
get providerState(): any {
return this._providerState;
}

private _sessionId = InteractiveSessionModel.nextId++;
get sessionId(): number {
return this.session.id;
return this._sessionId;
}

get inputPlaceholder(): string | undefined {
return this.session.inputPlaceholder;
return this._session?.inputPlaceholder;
}

constructor(
public readonly session: IInteractiveSession,
public readonly providerId: string,
public readonly welcomeMessage: InteractiveWelcomeMessageModel | undefined,
initialData: ISerializableInteractiveSessionData | undefined,
@ILogService private readonly logService: ILogService
) {
Expand All @@ -231,14 +250,19 @@ export class InteractiveSessionModel extends Disposable implements IInteractiveS
}

return requests.map((raw: ISerializableInteractiveSessionRequestData) => {
const request = new InteractiveRequestModel(raw.message, this.session.requesterUsername, this.session.requesterAvatarIconUri);
const request = new InteractiveRequestModel(raw.message, obj.requesterUsername, obj.requesterAvatarIconUri && URI.revive(obj.requesterAvatarIconUri));
if (raw.response || raw.responseErrorDetails) {
request.response = new InteractiveResponseModel(new MarkdownString(raw.response), this.session.responderUsername, this.providerId, this.session.responderAvatarIconUri, true, raw.isCanceled, raw.vote, raw.providerResponseId, raw.responseErrorDetails, raw.followups);
request.response = new InteractiveResponseModel(new MarkdownString(raw.response), obj.responderUsername, this.providerId, obj.responderAvatarIconUri && URI.revive(obj.responderAvatarIconUri), true, raw.isCanceled, raw.vote, raw.providerResponseId, raw.responseErrorDetails, raw.followups);
}
return request;
});
}

initialize(session: IInteractiveSession, welcomeMessage: InteractiveSessionWelcomeMessageModel | undefined): void {
this._session = session;
this._welcomeMessage = welcomeMessage;
}

acceptNewProviderState(providerState: any): void {
this._providerState = providerState;
}
Expand All @@ -254,20 +278,28 @@ export class InteractiveSessionModel extends Disposable implements IInteractiveS
}

addRequest(message: string | IInteractiveSessionReplyFollowup): InteractiveRequestModel {
const request = new InteractiveRequestModel(message, this.session.requesterUsername, this.session.requesterAvatarIconUri);
if (!this._session) {
throw new Error('addRequest: No session');
}

const request = new InteractiveRequestModel(message, this._session.requesterUsername, this._session.requesterAvatarIconUri);

// TODO this is suspicious, maybe the request should know that it is "in progress" instead of having a fake response model.
// But the response already knows that it is "in progress" and so does a map in the session service.
request.response = new InteractiveResponseModel(new MarkdownString(''), this.session.responderUsername, this.providerId, this.session.responderAvatarIconUri);
request.response = new InteractiveResponseModel(new MarkdownString(''), this._session.responderUsername, this.providerId, this._session.responderAvatarIconUri);

this._requests.push(request);
this._onDidChange.fire({ kind: 'addRequest', request });
return request;
}

acceptResponseProgress(request: InteractiveRequestModel, progress: IInteractiveProgress): void {
if (!this._session) {
throw new Error('acceptResponseProgress: No session');
}

if (!request.response) {
request.response = new InteractiveResponseModel(new MarkdownString(''), this.session.responderUsername, this.providerId, this.session.responderAvatarIconUri);
request.response = new InteractiveResponseModel(new MarkdownString(''), this._session.responderUsername, this.providerId, this._session.responderAvatarIconUri);
}

if ('content' in progress) {
Expand All @@ -284,8 +316,12 @@ export class InteractiveSessionModel extends Disposable implements IInteractiveS
}

completeResponse(request: InteractiveRequestModel, rawResponse: IInteractiveResponse): void {
if (!this._session) {
throw new Error('completeResponse: No session');
}

if (!request.response) {
request.response = new InteractiveResponseModel(new MarkdownString(''), this.session.responderUsername, this.providerId, this.session.responderAvatarIconUri);
request.response = new InteractiveResponseModel(new MarkdownString(''), this._session.responderUsername, this.providerId, this._session.responderAvatarIconUri);
}

request.response.complete(rawResponse.errorDetails);
Expand All @@ -307,6 +343,10 @@ export class InteractiveSessionModel extends Disposable implements IInteractiveS

toJSON(): ISerializableInteractiveSessionData {
return {
requesterUsername: this._session!.requesterUsername,
requesterAvatarIconUri: this._session!.requesterAvatarIconUri,
responderUsername: this._session!.responderUsername,
responderAvatarIconUri: this._session!.responderAvatarIconUri,
requests: this._requests.map((r): ISerializableInteractiveSessionRequestData => {
return {
providerResponseId: r.response?.providerResponseId,
Expand All @@ -324,7 +364,7 @@ export class InteractiveSessionModel extends Disposable implements IInteractiveS
}

override dispose() {
this.session.dispose?.();
this._session?.dispose?.();
this._requests.forEach(r => r.response?.dispose());
this._onDidDispose.fire();
super.dispose();
Expand All @@ -341,7 +381,7 @@ export interface IInteractiveSessionWelcomeMessageModel {

}

export class InteractiveWelcomeMessageModel implements IInteractiveSessionWelcomeMessageModel {
export class InteractiveSessionWelcomeMessageModel implements IInteractiveSessionWelcomeMessageModel {
private static nextId = 0;

private _id: string;
Expand All @@ -350,6 +390,6 @@ export class InteractiveWelcomeMessageModel implements IInteractiveSessionWelcom
}

constructor(public readonly content: IInteractiveWelcomeMessageContent[], public readonly username: string, public readonly avatarIconUri?: URI) {
this._id = 'welcome_' + InteractiveWelcomeMessageModel.nextId++;
this._id = 'welcome_' + InteractiveSessionWelcomeMessageModel.nextId++;
}
}
7 changes: 4 additions & 3 deletions src/vs/workbench/contrib/interactiveSession/common/interactiveSessionService.ts
View file
Open in desktop
Expand Up @@ -12,7 +12,7 @@ import { createDecorator } from 'vs/platform/instantiation/common/instantiation'
import { InteractiveSessionModel } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionModel';

export interface IInteractiveSession {
id: number;
id: number; // TODO This is only used by the provider internally, we can probably get rid of it
requesterUsername: string;
requesterAvatarIconUri?: URI;
responderUsername: string;
Expand Down Expand Up @@ -146,12 +146,13 @@ export const IInteractiveSessionService = createDecorator<IInteractiveSessionSer
export interface IInteractiveSessionService {
_serviceBrand: undefined;
registerProvider(provider: IInteractiveProvider): IDisposable;
startSession(providerId: string, allowRestoringSession: boolean, token: CancellationToken): Promise<InteractiveSessionModel | undefined>;
startSession(providerId: string, allowRestoringSession: boolean, token: CancellationToken): InteractiveSessionModel | undefined;
waitForSessionInitialization(sessionId: number): Promise<InteractiveSessionModel | undefined>;

/**
* Returns whether the request was accepted.
*/
sendRequest(sessionId: number, message: string | IInteractiveSessionReplyFollowup): { completePromise: Promise<void> } | undefined;
sendRequest(sessionId: number, message: string | IInteractiveSessionReplyFollowup): Promise<{ requestCompletePromise: Promise<void> } | undefined>;
cancelCurrentRequestForSession(sessionId: number): void;
getSlashCommands(sessionId: number, token: CancellationToken): Promise<IInteractiveSlashCommand[] | undefined>;
clearSession(sessionId: number): void;
Expand Down