Skip to content

Commit

Permalink
Move from chat variable values array to a single value (#211897)
Browse files Browse the repository at this point in the history
* Move from chat variable values array to a single value

* Update
  • Loading branch information
roblourens authored May 3, 2024
1 parent f8b1159 commit a1f2ea3
Show file tree
Hide file tree
Showing 17 changed files with 57 additions and 120 deletions.
2 changes: 1 addition & 1 deletion src/vs/workbench/api/browser/mainThreadChatAgents2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
kind: CompletionItemKind.Text,
detail: v.detail,
documentation: v.documentation,
command: { id: AddDynamicVariableAction.ID, title: '', arguments: [{ widget, range: rangeAfterInsert, variableData: revive(v.values), command: v.command } satisfies IAddDynamicVariableContext] }
command: { id: AddDynamicVariableAction.ID, title: '', arguments: [{ widget, range: rangeAfterInsert, variableData: revive(v.value) as any, command: v.command } satisfies IAddDynamicVariableContext] }
} satisfies CompletionItem;
});

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/api/browser/mainThreadChatVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ export class MainThreadChatVariables implements MainThreadChatVariablesShape {
const registration = this._chatVariablesService.registerVariable(data, async (messageText, _arg, model, progress, token) => {
const varRequestId = `${model.sessionId}-${handle}`;
this._pendingProgress.set(varRequestId, progress);
const result = revive<IChatRequestVariableValue[]>(await this._proxy.$resolveVariable(handle, varRequestId, messageText, token));
const result = revive<IChatRequestVariableValue>(await this._proxy.$resolveVariable(handle, varRequestId, messageText, token));

this._pendingProgress.delete(varRequestId);
return result;
return result as any; // 'revive' type signature doesn't like this type for some reason
});
this._variables.set(handle, registration);
}
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,7 @@ export interface MainThreadChatAgentsShape2 extends IDisposable {
export interface IChatAgentCompletionItem {
insertText?: string;
label: string | languages.CompletionItemLabel;
values: IChatRequestVariableValueDto[];
value: IChatRequestVariableValueDto;
detail?: string;
documentation?: string | IMarkdownString;
command?: ICommandDto;
Expand Down Expand Up @@ -1273,7 +1273,7 @@ export interface MainThreadChatVariablesShape extends IDisposable {
export type IChatRequestVariableValueDto = Dto<IChatRequestVariableValue>;

export interface ExtHostChatVariablesShape {
$resolveVariable(handle: number, requestId: string, messageText: string, token: CancellationToken): Promise<IChatRequestVariableValueDto[] | undefined>;
$resolveVariable(handle: number, requestId: string, messageText: string, token: CancellationToken): Promise<IChatRequestVariableValueDto | undefined>;
}

export interface MainThreadInlineChatShape extends IDisposable {
Expand Down
12 changes: 7 additions & 5 deletions src/vs/workbench/api/common/extHostChatVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class ExtHostChatVariables implements ExtHostChatVariablesShape {
this._proxy = mainContext.getProxy(MainContext.MainThreadChatVariables);
}

async $resolveVariable(handle: number, requestId: string, messageText: string, token: CancellationToken): Promise<IChatRequestVariableValue[] | undefined> {
async $resolveVariable(handle: number, requestId: string, messageText: string, token: CancellationToken): Promise<IChatRequestVariableValue | undefined> {
const item = this._resolver.get(handle);
if (!item) {
return undefined;
Expand All @@ -35,13 +35,15 @@ export class ExtHostChatVariables implements ExtHostChatVariablesShape {
checkProposedApiEnabled(item.extension, 'chatParticipantAdditions');
const stream = new ChatVariableResolverResponseStream(requestId, this._proxy);
const value = await item.resolver.resolve2(item.data.name, { prompt: messageText }, stream.apiObject, token);
if (value) {
return value.map(typeConvert.ChatVariable.from);

// Temp, ignoring other returned values to convert the array into a single value
if (value && value[0]) {
return value[0].value;
}
} else {
const value = await item.resolver.resolve(item.data.name, { prompt: messageText }, token);
if (value) {
return value.map(typeConvert.ChatVariable.from);
if (value && value[0]) {
return value[0].value;
}
}
} catch (err) {
Expand Down
59 changes: 3 additions & 56 deletions src/vs/workbench/api/common/extHostTypeConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Mimes } from 'vs/base/common/mime';
import { cloneAndChange } from 'vs/base/common/objects';
import { IPrefixTreeNode, WellDefinedPrefixTree } from 'vs/base/common/prefixTree';
import { basename } from 'vs/base/common/resources';
import { ThemeIcon } from 'vs/base/common/themables';
import { isDefined, isEmptyObject, isNumber, isString, isUndefinedOrNull } from 'vs/base/common/types';
import { URI, UriComponents, isUriComponents } from 'vs/base/common/uri';
import { IURITransformer } from 'vs/base/common/uriIpc';
Expand All @@ -40,7 +41,6 @@ import { IViewBadge } from 'vs/workbench/common/views';
import { ChatAgentLocation, IChatAgentRequest, IChatAgentResult } from 'vs/workbench/contrib/chat/common/chatAgents';
import { IChatRequestVariableEntry } from 'vs/workbench/contrib/chat/common/chatModel';
import { IChatAgentDetection, IChatAgentMarkdownContentWithVulnerability, IChatCommandButton, IChatContentInlineReference, IChatContentReference, IChatFollowup, IChatMarkdownContent, IChatProgressMessage, IChatTextEdit, IChatTreeData, IChatUserActionEvent, IChatWarningMessage } from 'vs/workbench/contrib/chat/common/chatService';
import { IChatRequestVariableValue } from 'vs/workbench/contrib/chat/common/chatVariables';
import * as chatProvider from 'vs/workbench/contrib/chat/common/languageModels';
import { DebugTreeItemCollapsibleState, IDebugVisualizationTreeItem } from 'vs/workbench/contrib/debug/common/debug';
import { IInlineChatCommandFollowup, IInlineChatFollowup, IInlineChatReplyFollowup, InlineChatResponseFeedbackKind } from 'vs/workbench/contrib/inlineChat/common/inlineChat';
Expand All @@ -54,7 +54,6 @@ import { ACTIVE_GROUP, SIDE_GROUP } from 'vs/workbench/services/editor/common/ed
import { Dto } from 'vs/workbench/services/extensions/common/proxyIdentifier';
import type * as vscode from 'vscode';
import * as types from './extHostTypes';
import { ThemeIcon } from 'vs/base/common/themables';

export namespace Command {

Expand Down Expand Up @@ -2286,58 +2285,6 @@ export namespace LanguageModelMessage {
}
}

export namespace ChatVariable {
export function objectTo(variableObject: Record<string, IChatRequestVariableValue[]>): Record<string, vscode.ChatVariableValue[]> {
const result: Record<string, vscode.ChatVariableValue[]> = {};
for (const key of Object.keys(variableObject)) {
result[key] = variableObject[key].map(ChatVariable.to);
}

return result;
}

export function to(variable: IChatRequestVariableValue): vscode.ChatVariableValue {
return {
level: ChatVariableLevel.to(variable.level),
kind: variable.kind,
value: isUriComponents(variable.value) ? URI.revive(variable.value) : variable.value,
description: variable.description
};
}

export function from(variable: vscode.ChatVariableValue): IChatRequestVariableValue {
return {
level: ChatVariableLevel.from(variable.level),
kind: variable.kind,
value: variable.value,
description: variable.description
};
}
}

export namespace ChatVariableLevel {


export function to(level: 'short' | 'medium' | 'full'): vscode.ChatVariableLevel {
switch (level) {
case 'short': return types.ChatVariableLevel.Short;
case 'medium': return types.ChatVariableLevel.Medium;
case 'full':
default:
return types.ChatVariableLevel.Full;
}
}
export function from(level: vscode.ChatVariableLevel): 'short' | 'medium' | 'full' {
switch (level) {
case types.ChatVariableLevel.Short: return 'short';
case types.ChatVariableLevel.Medium: return 'medium';
case types.ChatVariableLevel.Full:
default:
return 'full';
}
}
}

export namespace InteractiveEditorResponseFeedbackKind {

export function to(kind: InlineChatResponseFeedbackKind): vscode.InteractiveEditorResponseFeedbackKind {
Expand Down Expand Up @@ -2628,7 +2575,7 @@ export namespace ChatLocation {

export namespace ChatAgentValueReference {
export function to(request: IChatRequestVariableEntry): vscode.ChatValueReference {
const value = request.values[0]?.value;
const value = request.value;
if (!value) {
throw new Error('Invalid value reference');
}
Expand All @@ -2645,7 +2592,7 @@ export namespace ChatAgentCompletionItem {
export function from(item: vscode.ChatCompletionItem, commandsConverter: CommandsConverter, disposables: DisposableStore): extHostProtocol.IChatAgentCompletionItem {
return {
label: item.label,
values: item.values.map(ChatVariable.from),
value: item.values[0].value,
insertText: item.insertText,
detail: item.detail,
documentation: item.documentation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ export class ChatMarkdownDecorationsRenderer {

result += `[${text}](${agentRefUrl}?${encodeURIComponent(part.agent.id)})`;
} else {
const uri = part instanceof ChatRequestDynamicVariablePart && part.data.map(d => d.value).find((d): d is URI => d instanceof URI)
|| undefined;
const uri = part instanceof ChatRequestDynamicVariablePart && part.data instanceof URI ?
part.data :
undefined;
const title = uri ? encodeURIComponent(this.labelService.getUriLabel(uri, { relative: true })) :
part instanceof ChatRequestAgentPart ? part.agent.id :
'';
Expand Down
16 changes: 9 additions & 7 deletions src/vs/workbench/contrib/chat/browser/chatVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,20 @@ export class ChatVariablesService implements IChatVariablesService {
}
progress(item);
};
jobs.push(data.resolver(prompt.text, part.variableArg, model, variableProgressCallback, token).then(values => {
resolvedVariables[i] = { name: part.variableName, range: part.range, values: values ?? [], references };
jobs.push(data.resolver(prompt.text, part.variableArg, model, variableProgressCallback, token).then(value => {
if (value) {
resolvedVariables[i] = { name: part.variableName, range: part.range, value, references };
}
}).catch(onUnexpectedExternalError));
}
} else if (part instanceof ChatRequestDynamicVariablePart) {
resolvedVariables[i] = { name: part.referenceText, range: part.range, values: part.data };
resolvedVariables[i] = { name: part.referenceText, range: part.range, value: part.data };
}
});

await Promise.allSettled(jobs);

resolvedVariables = coalesce(resolvedVariables);
resolvedVariables = coalesce<IChatRequestVariableEntry>(resolvedVariables);

// "reverse", high index first so that replacement is simple
resolvedVariables.sort((a, b) => b.range!.start - a.range!.start);
Expand All @@ -68,13 +70,13 @@ export class ChatVariablesService implements IChatVariablesService {
};
}

async resolveVariable(variableName: string, promptText: string, model: IChatModel, progress: (part: IChatVariableResolverProgress) => void, token: CancellationToken): Promise<IChatRequestVariableValue[]> {
async resolveVariable(variableName: string, promptText: string, model: IChatModel, progress: (part: IChatVariableResolverProgress) => void, token: CancellationToken): Promise<IChatRequestVariableValue | undefined> {
const data = this._resolver.get(variableName.toLowerCase());
if (!data) {
return Promise.resolve([]);
return undefined;
}

return (await data.resolver(promptText, undefined, model, progress, token)) ?? [];
return (await data.resolver(promptText, undefined, model, progress, token));
}

hasVariable(name: string): boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ export class ChatDynamicVariableModel extends Disposable implements IChatWidgetC
}

private getHoverForReference(ref: IDynamicVariable): string | IMarkdownString {
const value = ref.data[0];
if (URI.isUri(value.value)) {
return new MarkdownString(this.labelService.getUriLabel(value.value, { relative: true }));
const value = ref.data;
if (URI.isUri(value)) {
return new MarkdownString(this.labelService.getUriLabel(value, { relative: true }));
} else {
return value.value.toString();
return (value as any).toString();
}
}
}
Expand Down Expand Up @@ -209,7 +209,7 @@ export class SelectAndInsertFileAction extends Action2 {

context.widget.getContrib<ChatDynamicVariableModel>(ChatDynamicVariableModel.ID)?.addReference({
range: { startLineNumber: range.startLineNumber, startColumn: range.startColumn, endLineNumber: range.endLineNumber, endColumn: range.startColumn + text.length },
data: [{ level: 'full', value: resource }]
data: resource
});
}
}
Expand All @@ -218,7 +218,7 @@ registerAction2(SelectAndInsertFileAction);
export interface IAddDynamicVariableContext {
widget: IChatWidget;
range: IRange;
variableData: IChatRequestVariableValue[];
variableData: IChatRequestVariableValue;
command?: Command;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ChatHistoryVariables extends Disposable {
return undefined;
}

return [{ level: 'full', value: response.response.asString() }];
return response.response.asString();
}));
}
}
Expand Down
6 changes: 1 addition & 5 deletions src/vs/workbench/contrib/chat/common/chatModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,10 @@ import { ChatRequestTextPart, IParsedChatRequest, getPromptText, reviveParsedCha
import { IChatAgentMarkdownContentWithVulnerability, IChatCommandButton, IChatContentInlineReference, IChatContentReference, IChatFollowup, IChatMarkdownContent, IChatProgress, IChatProgressMessage, IChatResponseProgressFileTreeData, IChatTextEdit, IChatTreeData, IChatUsedContext, IChatWarningMessage, InteractiveSessionVoteDirection, isIUsedContext } from 'vs/workbench/contrib/chat/common/chatService';
import { IChatRequestVariableValue } from 'vs/workbench/contrib/chat/common/chatVariables';

export interface IChatPromptVariableData {
variables: { name: string; range: IOffsetRange; values: IChatRequestVariableValue[] }[];
}

export interface IChatRequestVariableEntry {
name: string;
range?: IOffsetRange;
values: IChatRequestVariableValue[];
value: IChatRequestVariableValue;
references?: IChatContentReference[];
}

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/chat/common/chatParserTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export class ChatRequestSlashCommandPart implements IParsedChatRequestPart {
export class ChatRequestDynamicVariablePart implements IParsedChatRequestPart {
static readonly Kind = 'dynamic';
readonly kind = ChatRequestDynamicVariablePart.Kind;
constructor(readonly range: OffsetRange, readonly editorRange: IRange, readonly text: string, readonly data: IChatRequestVariableValue[]) { }
constructor(readonly range: OffsetRange, readonly editorRange: IRange, readonly text: string, readonly data: IChatRequestVariableValue) { }

get referenceText(): string {
return this.text.replace(chatVariableLeader, '');
Expand Down Expand Up @@ -183,7 +183,7 @@ export function reviveParsedChatRequest(serialized: IParsedChatRequest): IParsed
new OffsetRange(part.range.start, part.range.endExclusive),
part.editorRange,
(part as ChatRequestDynamicVariablePart).text,
revive((part as ChatRequestDynamicVariablePart).data)
revive((part as ChatRequestDynamicVariablePart).data) as any
);
} else {
throw new Error(`Unknown chat request part: ${part.kind}`);
Expand Down
9 changes: 7 additions & 2 deletions src/vs/workbench/contrib/chat/common/chatServiceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { Action } from 'vs/base/common/actions';
import { coalesce } from 'vs/base/common/arrays';
import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
import { ErrorNoTelemetry } from 'vs/base/common/errors';
import { Emitter, Event } from 'vs/base/common/event';
Expand Down Expand Up @@ -544,8 +545,12 @@ export class ChatService extends Disposable implements IChatService {
if (implicitVariablesEnabled) {
const implicitVariables = agent.defaultImplicitVariables;
if (implicitVariables) {
const resolvedImplicitVariables = await Promise.all(implicitVariables.map(async v => ({ name: v, values: await this.chatVariablesService.resolveVariable(v, parsedRequest.text, model, progressCallback, token) } satisfies IChatRequestVariableEntry)));
updatedVariableData.variables.push(...resolvedImplicitVariables);
const resolvedImplicitVariables = await Promise.all(implicitVariables.map(async v => {
const value = await this.chatVariablesService.resolveVariable(v, parsedRequest.text, model, progressCallback, token);
return value ? { name: v, value } satisfies IChatRequestVariableEntry :
undefined;
}));
updatedVariableData.variables.push(...coalesce(resolvedImplicitVariables));
}
}

Expand Down
15 changes: 5 additions & 10 deletions src/vs/workbench/contrib/chat/common/chatVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { CancellationToken } from 'vs/base/common/cancellation';
import { IDisposable } from 'vs/base/common/lifecycle';
import { URI } from 'vs/base/common/uri';
import { IRange } from 'vs/editor/common/core/range';
import { Location } from 'vs/editor/common/languages';
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { IChatModel, IChatRequestVariableData } from 'vs/workbench/contrib/chat/common/chatModel';
import { IParsedChatRequest } from 'vs/workbench/contrib/chat/common/chatParserTypes';
Expand All @@ -19,20 +20,14 @@ export interface IChatVariableData {
canTakeArgument?: boolean;
}

export interface IChatRequestVariableValue {
level: 'short' | 'medium' | 'full';
kind?: string;
value: string | URI;
description?: string;
}
export type IChatRequestVariableValue = string | URI | Location | unknown;

export type IChatVariableResolverProgress =
| IChatContentReference
| IChatProgressMessage;

export interface IChatVariableResolver {
// TODO should we spec "zoom level"
(messageText: string, arg: string | undefined, model: IChatModel, progress: (part: IChatVariableResolverProgress) => void, token: CancellationToken): Promise<IChatRequestVariableValue[] | undefined>;
(messageText: string, arg: string | undefined, model: IChatModel, progress: (part: IChatVariableResolverProgress) => void, token: CancellationToken): Promise<IChatRequestVariableValue | undefined>;
}

export const IChatVariablesService = createDecorator<IChatVariablesService>('IChatVariablesService');
Expand All @@ -49,10 +44,10 @@ export interface IChatVariablesService {
* Resolves all variables that occur in `prompt`
*/
resolveVariables(prompt: IParsedChatRequest, model: IChatModel, progress: (part: IChatVariableResolverProgress) => void, token: CancellationToken): Promise<IChatRequestVariableData>;
resolveVariable(variableName: string, promptText: string, model: IChatModel, progress: (part: IChatVariableResolverProgress) => void, token: CancellationToken): Promise<IChatRequestVariableValue[]>;
resolveVariable(variableName: string, promptText: string, model: IChatModel, progress: (part: IChatVariableResolverProgress) => void, token: CancellationToken): Promise<IChatRequestVariableValue | undefined>;
}

export interface IDynamicVariable {
range: IRange;
data: IChatRequestVariableValue[];
data: IChatRequestVariableValue;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ suite('ChatVariables', function () {

test('ChatVariables - resolveVariables', async function () {

const v1 = service.registerVariable({ name: 'foo', description: 'bar' }, async () => ([{ level: 'full', value: 'farboo' }]));
const v2 = service.registerVariable({ name: 'far', description: 'boo' }, async () => ([{ level: 'full', value: 'farboo' }]));
const v1 = service.registerVariable({ name: 'foo', description: 'bar' }, async () => 'farboo');
const v2 = service.registerVariable({ name: 'far', description: 'boo' }, async () => 'farboo');

const parser = instantiationService.createInstance(ChatRequestParser);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class MockChatVariablesService implements IChatVariablesService {
};
}

resolveVariable(variableName: string, promptText: string, model: IChatModel, progress: (part: IChatVariableResolverProgress) => void, token: CancellationToken): Promise<IChatRequestVariableValue[]> {
resolveVariable(variableName: string, promptText: string, model: IChatModel, progress: (part: IChatVariableResolverProgress) => void, token: CancellationToken): Promise<IChatRequestVariableValue> {
throw new Error('Method not implemented.');
}
}
Loading

0 comments on commit a1f2ea3

Please sign in to comment.