Skip to content

Commit

Permalink
Turn on strict checks when parsing action schema (#562)
Browse files Browse the repository at this point in the history
We already have strict checks on cached schemas. Turns it on when we
first parse as well to have consistent behavior between parsing and
loading from cache.

Improve error message for entry comment error.
Added test for some of the error conditions.
  • Loading branch information
curtisman authored Jan 16, 2025
1 parent d44938c commit bc85471
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 9 deletions.
10 changes: 5 additions & 5 deletions ts/packages/actionSchema/src/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ export function createActionSchemaFile(
): ActionSchemaFile {
if (strict && !entry.exported) {
throw new Error(
`Schema Error: ${schemaName}: Type ${entry.name} must be exported`,
`Schema Error: ${schemaName}: Type '${entry.name}' must be exported`,
);
}

Expand All @@ -208,7 +208,7 @@ export function createActionSchemaFile(
case "type-union":
if (strict && current.comments) {
throw new Error(
`Schema Error: ${schemaName}: entry type comments for '${current.name}' are not supported`,
`Schema Error: ${schemaName}: entry type comments for '${current.name}' are not used for prompts. Remove from the action schema file.\n${current.comments.map((s) => ` - ${s}`).join("\n")}`,
);
}
for (const t of current.type.types) {
Expand All @@ -229,19 +229,19 @@ export function createActionSchemaFile(
// Definition that references another type is the same as a union type with a single type.
if (strict && current.comments) {
throw new Error(
`Schema Error: ${schemaName}: entry type comments for '${current.name} are not supported`,
`Schema Error: ${schemaName}: entry type comments for '${current.name}' are not used for prompts. Remove from the action schema file.\n${current.comments.map((s) => ` - ${s}`).join("\n")}`,
);
}
if (current.type.definition === undefined) {
throw new Error(
`Schema Error: ${schemaName}: unresolved type reference '${current.type.name}' in the entry type union`,
`Schema Error: ${schemaName}: unresolved type reference '${current.type.name}' in the entry type union`,
);
}
pending.push(current.type.definition);
break;
default:
throw new Error(
`Schema Error: ${schemaName}: invalid type ${current.type.type} in action schema type ${current.name}`,
`Schema Error: ${schemaName}: invalid type '${current.type.type}' in action schema type ${current.name}`,
);
}
}
Expand Down
66 changes: 66 additions & 0 deletions ts/packages/actionSchema/test/parse.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { parseActionSchemaSource } from "../src/parser.js";

describe("Action Schema Strict Checks", () => {
it("Error on entry type not exported", async () =>
expect(async () =>
parseActionSchemaSource(
`type SomeAction = { actionName: "someAction" }`,
"test",
"",
"SomeAction",
"",
undefined,
true,
),
).rejects.toThrow(
"Error parsing test: Schema Error: test: Type 'SomeAction' must be exported",
));

it("Error on entry type comment", async () =>
expect(async () =>
parseActionSchemaSource(
`// comments\nexport type AllActions = SomeAction;\ntype SomeAction = { actionName: "someAction" }`,
"test",
"",
"AllActions",
"",
undefined,
true,
),
).rejects.toThrow(
"Error parsing test: Schema Error: test: entry type comments for 'AllActions' are not used for prompts. Remove from the action schema file.",
));

it("Error on duplicate action name", async () =>
expect(async () =>
parseActionSchemaSource(
`export type AllActions = SomeAction | SomeAction2;\ntype SomeAction = { actionName: "someAction" }\ntype SomeAction2 = { actionName: "someAction" }`,
"test",
"",
"AllActions",
"",
undefined,
true,
),
).rejects.toThrow(
"Error parsing test: Schema Error: test: Duplicate action name 'someAction'",
));

it("Error on anonymous types", async () =>
expect(async () =>
parseActionSchemaSource(
`export type AllActions = SomeAction | { actionName: "someAction2" };\ntype SomeAction = { actionName: "someAction" }`,
"test",
"",
"AllActions",
"",
undefined,
true,
),
).rejects.toThrow(
"Error parsing test: Schema Error: test: expected type reference in the entry type union",
));
});
7 changes: 5 additions & 2 deletions ts/packages/agents/test/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
AppAgent,
ParsedCommandParams,
} from "@typeagent/agent-sdk";
import { AddAction } from "./schema.js";
import { TestActions } from "./schema.js";
import { createActionResult } from "@typeagent/agent-sdk/helpers/action";
import {
CommandHandler,
Expand Down Expand Up @@ -44,7 +44,10 @@ export function instantiate(): AppAgent {
};
}

async function executeAction(action: AddAction, context: ActionContext<void>) {
async function executeAction(
action: TestActions,
context: ActionContext<void>,
) {
switch (action.actionName) {
case "add":
const { a, b } = action.parameters;
Expand Down
2 changes: 1 addition & 1 deletion ts/packages/agents/test/src/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
"schema": {
"description": "Math agent with action to do addition",
"schemaFile": "./schema.ts",
"schemaType": "AddAction"
"schemaType": "TestActions"
}
}
3 changes: 2 additions & 1 deletion ts/packages/agents/test/src/schema.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

export type AddAction = {
export type TestActions = AddAction;
type AddAction = {
actionName: "add";
parameters: {
a: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export class ActionSchemaFileCache {
actionConfig.schemaType,
fullPath,
schemaConfig,
true,
);
this.actionSchemaFiles.set(actionConfig.schemaName, parsed);

Expand Down

0 comments on commit bc85471

Please sign in to comment.