Detect command error in test (#689)

- Detect if there are any error in commands so that we can assert that in test. - Add a flag to enable full command result collection. - Reorganizing some code around
microsoft · Feb 8, 2025 · b02279b · b02279b
1 parent 4c420c0
commit b02279b
Show file tree

Hide file tree

Showing 17 changed files with 155 additions and 125 deletions.
diff --git a/ts/packages/api/src/webDispatcher.ts b/ts/packages/api/src/webDispatcher.ts
@@ -68,11 +68,11 @@ export async function createWebDispatcher(): Promise<WebDispatcher> {
         }
         console.log(dispatcher.getPrompt(), text);
 
-        const metrics = await dispatcher.processCommand(text, id, images);
+        const result = await dispatcher.processCommand(text, id, images);
 
         updateSettingSummary();
 
-        return metrics;
+        return result;
     }
 
     const patchedDispatcher = {

diff --git a/ts/packages/cli/src/commands/test/translate.ts b/ts/packages/cli/src/commands/test/translate.ts
@@ -260,6 +260,7 @@ export default class TestTranslateCommand extends Command {
                 translation: { history: { enabled: false } },
                 explainer: { enabled: false },
                 cache: { enabled: false },
+                collectCommandResult: true,
             });
             while (requests.length > 0) {
                 const request = requests.shift()!;

diff --git a/ts/packages/defaultAgentProvider/test/translateTestCommon.ts b/ts/packages/defaultAgentProvider/test/translateTestCommon.ts
@@ -46,6 +46,7 @@ export async function defineTranslateTest(name: string, dataFiles: string[]) {
                         translation: { history: { enabled: false } },
                         explainer: { enabled: false },
                         cache: { enabled: false },
+                        collectCommandResult: true,
                     }),
                 );
             }
@@ -62,11 +63,15 @@ export async function defineTranslateTest(name: string, dataFiles: string[]) {
                         match,
                     } of requests) {
                         if (history !== undefined) {
-                            await dispatcher.processCommand(
-                                `@history insert ${JSON.stringify(history)}`,
-                            );
+                            const insertResult =
+                                await dispatcher.processCommand(
+                                    `@history insert ${JSON.stringify(history)}`,
+                                );
+                            expect(insertResult?.hasError).toBeFalsy();
                         }
                         const result = await dispatcher.processCommand(request);
+                        expect(result?.hasError).toBeFalsy();
+
                         const actions = result?.actions;
                         expect(actions).toBeDefined();
 

diff --git a/ts/packages/dispatcher/src/command/command.ts b/ts/packages/dispatcher/src/command/command.ts
@@ -3,13 +3,12 @@
 
 import chalk from "chalk";
 import registerDebug from "debug";
-import {
-    RequestId,
-    DispatcherName,
-    makeClientIOMessage,
-} from "../context/interactiveIO.js";
+import { RequestId, makeClientIOMessage } from "../context/interactiveIO.js";
 import { getDefaultExplainerName } from "agent-cache";
-import { CommandHandlerContext } from "../context/commandHandlerContext.js";
+import {
+    CommandHandlerContext,
+    ensureCommandResult,
+} from "../context/commandHandlerContext.js";
 
 import {
     CommandDescriptor,
@@ -21,6 +20,7 @@ import { isCommandDescriptorTable } from "@typeagent/agent-sdk/helpers/command";
 import { parseParams } from "./parameters.js";
 import { getHandlerTableUsage, getUsage } from "./commandHelp.js";
 import { CommandResult } from "../dispatcher.js";
+import { DispatcherName } from "../context/dispatcher/dispatcherUtils.js";
 
 const debugCommand = registerDebug("typeagent:dispatcher:command");
 const debugCommandError = registerDebug("typeagent:dispatcher:command:error");
@@ -288,12 +288,9 @@ function endProcessCommand(
     const metrics = requestId
         ? context.metricsManager?.endCommand(requestId)
         : undefined;
+
     if (metrics) {
-        if (context.commandResult === undefined) {
-            context.commandResult = { metrics };
-        } else {
-            context.commandResult.metrics = metrics;
-        }
+        ensureCommandResult(context).metrics = metrics;
     }
     const result = context.commandResult;
     context.commandResult = undefined;

diff --git a/ts/packages/dispatcher/src/context/appAgentManager.ts b/ts/packages/dispatcher/src/context/appAgentManager.ts
@@ -17,7 +17,7 @@ import { createSessionContext } from "../execute/actionHandlers.js";
 import { AppAgentProvider } from "../agentProvider/agentProvider.js";
 import registerDebug from "debug";
 import { DeepPartialUndefinedAndNull } from "common-utils";
-import { DispatcherName } from "./interactiveIO.js";
+import { DispatcherName } from "./dispatcher/dispatcherUtils.js";
 import {
     ActionSchemaSemanticMap,
     EmbeddingCache,

diff --git a/ts/packages/dispatcher/src/context/commandHandlerContext.ts b/ts/packages/dispatcher/src/context/commandHandlerContext.ts
@@ -20,20 +20,11 @@ import {
     setupAgentCache,
     setupBuiltInCache,
 } from "./session.js";
-import {
-    loadAgentJsonTranslator,
-    TypeAgentTranslator,
-    createTypeAgentTranslatorForSelectedActions,
-} from "../translation/agentTranslators.js";
+import { TypeAgentTranslator } from "../translation/agentTranslators.js";
 import { ActionConfigProvider } from "../translation/actionConfigProvider.js";
 import { getCacheFactory } from "../utils/cacheFactory.js";
 import { createServiceHost } from "./system/handlers/serviceHost/serviceHostCommandHandler.js";
-import {
-    ClientIO,
-    RequestId,
-    DispatcherName,
-    nullClientIO,
-} from "./interactiveIO.js";
+import { ClientIO, RequestId, nullClientIO } from "./interactiveIO.js";
 import { ChatHistory, createChatHistory } from "./chatHistory.js";
 import {
     ensureCacheDir,
@@ -72,6 +63,7 @@ import path from "node:path";
 import { createSchemaInfoProvider } from "../translation/actionSchemaFileCache.js";
 import { createInlineAppAgentProvider } from "./inlineAgentProvider.js";
 import { CommandResult } from "../dispatcher.js";
+import { DispatcherName } from "./dispatcher/dispatcherUtils.js";
 
 const debug = registerDebug("typeagent:dispatcher:init");
 const debugError = registerDebug("typeagent:dispatcher:init:error");
@@ -82,6 +74,24 @@ export interface ClientSettingsProvider {
     set: SetSettingFunction | null;
 }
 
+export function getCommandResult(
+    context: CommandHandlerContext,
+): CommandResult | undefined {
+    if (context.collectCommandResult) {
+        return ensureCommandResult(context);
+    }
+    return undefined;
+}
+
+export function ensureCommandResult(
+    context: CommandHandlerContext,
+): CommandResult {
+    if (context.commandResult === undefined) {
+        context.commandResult = {};
+    }
+    return context.commandResult;
+}
+
 // Command Handler Context definition.
 export type CommandHandlerContext = {
     agents: AppAgentManager;
@@ -96,6 +106,7 @@ export type CommandHandlerContext = {
     explanationAsynchronousMode: boolean;
     dblogging: boolean;
     clientIO: ClientIO;
+    collectCommandResult: boolean;
 
     // Runtime context
     commandLock: Limiter; // Make sure we process one command at a time.
@@ -120,64 +131,6 @@ export type CommandHandlerContext = {
     instanceDirLock: (() => Promise<void>) | undefined;
 };
 
-export function getTranslatorForSchema(
-    context: CommandHandlerContext,
-    translatorName: string,
-) {
-    const translator = context.translatorCache.get(translatorName);
-    if (translator !== undefined) {
-        return translator;
-    }
-    const config = context.session.getConfig().translation;
-    const newTranslator = loadAgentJsonTranslator(
-        translatorName,
-        context.agents,
-        getActiveTranslators(context),
-        config.switch.inline,
-        config.multiple,
-        config.schema.generation.enabled,
-        config.model,
-        !config.schema.optimize.enabled,
-        config.schema.generation.jsonSchema,
-    );
-    context.translatorCache.set(translatorName, newTranslator);
-    return newTranslator;
-}
-
-export async function getTranslatorForSelectedActions(
-    context: CommandHandlerContext,
-    schemaName: string,
-    request: string,
-    numActions: number,
-): Promise<TypeAgentTranslator | undefined> {
-    const actionSchemaFile = context.agents.tryGetActionSchemaFile(schemaName);
-    if (
-        actionSchemaFile === undefined ||
-        actionSchemaFile.actionSchemas.size <= numActions
-    ) {
-        return undefined;
-    }
-    const nearestNeighbors = await context.agents.semanticSearchActionSchema(
-        request,
-        numActions,
-        (name) => name === schemaName,
-    );
-
-    if (nearestNeighbors === undefined) {
-        return undefined;
-    }
-    const config = context.session.getConfig().translation;
-    return createTypeAgentTranslatorForSelectedActions(
-        nearestNeighbors.map((e) => e.item.definition),
-        schemaName,
-        context.agents,
-        getActiveTranslators(context),
-        config.switch.inline,
-        config.multiple,
-        config.model,
-    );
-}
-
 async function getAgentCache(
     session: Session,
     provider: ActionConfigProvider,
@@ -215,6 +168,7 @@ export type DispatcherOptions = SessionOptions & {
     dblogging?: boolean; // default to false
     constructionProvider?: ConstructionProvider;
     agentInstaller?: AppAgentInstaller;
+    collectCommandResult?: boolean; // default to false
 };
 
 async function getSession(instanceDir?: string) {
@@ -419,6 +373,7 @@ export async function initializeCommandHandlerContext(
             batchMode: false,
             instanceDirLock,
             constructionProvider,
+            collectCommandResult: options?.collectCommandResult ?? false,
         };
 
         await addAppAgentProviders(context, options?.appAgentProviders);
@@ -644,9 +599,3 @@ export async function changeContextConfig(
 
     return changed;
 }
-
-function getActiveTranslators(context: CommandHandlerContext) {
-    return Object.fromEntries(
-        context.agents.getActiveSchemas().map((name) => [name, true]),
-    );
-}
diff --git a/ts/packages/dispatcher/src/context/dispatcher/dispatcherUtils.ts b/ts/packages/dispatcher/src/context/dispatcher/dispatcherUtils.ts
@@ -4,6 +4,8 @@
 import { AppAction } from "@typeagent/agent-sdk";
 import { UnknownAction } from "./schema/dispatcherActionSchema.js";
 
+export const DispatcherName = "dispatcher";
+
 export function isUnknownAction(action: AppAction): action is UnknownAction {
     return action.actionName === "unknown";
 }
diff --git a/ts/packages/dispatcher/src/context/dispatcher/handlers/requestCommandHandler.ts b/ts/packages/dispatcher/src/context/dispatcher/handlers/requestCommandHandler.ts
@@ -13,10 +13,7 @@ import {
     toFullActions,
 } from "agent-cache";
 
-import {
-    CommandHandlerContext,
-    getTranslatorForSchema,
-} from "../../commandHandlerContext.js";
+import { CommandHandlerContext } from "../../commandHandlerContext.js";
 
 import { CachedImageWithDetails } from "common-utils";
 import { Logger } from "telemetry";
@@ -34,10 +31,10 @@ import ExifReader from "exifreader";
 import { ProfileNames } from "../../../utils/profileNames.js";
 import { ActionContext, ParsedCommandParams } from "@typeagent/agent-sdk";
 import { CommandHandler } from "@typeagent/agent-sdk/helpers/command";
-import { DispatcherName } from "../../interactiveIO.js";
-import { isUnknownAction } from "../dispatcherUtils.js";
+import { DispatcherName, isUnknownAction } from "../dispatcherUtils.js";
 import {
     getChatHistoryForTranslation,
+    getTranslatorForSchema,
     translateRequest,
 } from "../../../translation/translateRequest.js";
 import { matchRequest } from "../../../translation/matchRequest.js";

diff --git a/ts/packages/dispatcher/src/context/interactiveIO.ts b/ts/packages/dispatcher/src/context/interactiveIO.ts
@@ -2,15 +2,17 @@
 // Licensed under the MIT License.
 
 import { TemplateEditConfig } from "../translation/actionTemplate.js";
-import { CommandHandlerContext } from "./commandHandlerContext.js";
+import {
+    CommandHandlerContext,
+    getCommandResult,
+} from "./commandHandlerContext.js";
 import {
     DisplayContent,
     DisplayAppendMode,
     TypeAgentAction,
 } from "@typeagent/agent-sdk";
 import { RequestMetrics } from "../utils/metrics.js";
 
-export const DispatcherName = "dispatcher";
 export type RequestId = string | undefined;
 
 export enum NotifyCommands {
@@ -88,20 +90,31 @@ export interface ClientIO {
 }
 
 export function makeClientIOMessage(
-    context: CommandHandlerContext | undefined,
+    context: CommandHandlerContext,
     message: DisplayContent,
     requestId: RequestId,
     source: string,
     actionIndex?: number,
 ): IAgentMessage {
+    if (
+        typeof message === "object" &&
+        !Array.isArray(message) &&
+        message.kind === "error"
+    ) {
+        const commandResult = getCommandResult(context);
+        if (commandResult !== undefined) {
+            commandResult.hasError = true;
+        }
+    }
+
     return {
         message,
         requestId,
         source,
         actionIndex,
         metrics:
             requestId !== undefined
-                ? context?.metricsManager?.getMetrics(requestId)
+                ? context.metricsManager?.getMetrics(requestId)
                 : undefined,
     };
 }

diff --git a/ts/packages/dispatcher/src/context/session.ts b/ts/packages/dispatcher/src/context/session.ts
@@ -11,7 +11,7 @@ import ExifReader from "exifreader";
 import { AppAgentState, AppAgentStateOptions } from "./appAgentManager.js";
 import { cloneConfig, mergeConfig } from "./options.js";
 import { TokenCounter, TokenCounterData } from "aiclient";
-import { DispatcherName } from "./interactiveIO.js";
+import { DispatcherName } from "./dispatcher/dispatcherUtils.js";
 import { ConstructionProvider } from "../agentProvider/agentProvider.js";
 import { MultipleActionConfig } from "../translation/multipleActionSchema.js";
 

diff --git a/ts/packages/dispatcher/src/context/system/handlers/notifyCommandHandler.ts b/ts/packages/dispatcher/src/context/system/handlers/notifyCommandHandler.ts
@@ -6,8 +6,9 @@ import {
     CommandHandlerTable,
 } from "@typeagent/agent-sdk/helpers/command";
 import { CommandHandlerContext } from "../../commandHandlerContext.js";
-import { DispatcherName, NotifyCommands } from "../../interactiveIO.js";
+import { NotifyCommands } from "../../interactiveIO.js";
 import { ActionContext } from "@typeagent/agent-sdk";
+import { DispatcherName } from "../../dispatcher/dispatcherUtils.js";
 
 class NotifyInfoCommandHandler implements CommandHandlerNoParams {
     description: string = "Shows the number of notifications available";

diff --git a/ts/packages/dispatcher/src/context/system/handlers/randomCommandHandler.ts b/ts/packages/dispatcher/src/context/system/handlers/randomCommandHandler.ts
@@ -14,7 +14,7 @@ import {
     CommandHandlerTable,
 } from "@typeagent/agent-sdk/helpers/command";
 import { displayStatus } from "@typeagent/agent-sdk/helpers/display";
-import { DispatcherName } from "../../interactiveIO.js";
+import { DispatcherName } from "../../dispatcher/dispatcherUtils.js";
 import { getPackageFilePath } from "../../../utils/getPackageFilePath.js";
 
 export type UserRequestList = {

diff --git a/ts/packages/dispatcher/src/dispatcher.ts b/ts/packages/dispatcher/src/dispatcher.ts
@@ -27,6 +27,7 @@ import { RequestMetrics } from "./utils/metrics.js";
 import { FullAction } from "agent-cache";
 
 export type CommandResult = {
+    hasError?: boolean;
     actions?: FullAction[];
     metrics?: RequestMetrics;
 };