Merge PR #6742: perf(coding-agent): lazily construct computer schema (@usr-bin-roygbiv)
This commit is contained in:
@@ -13,6 +13,10 @@
|
||||
- Fixed deferred CLI model roles resolving ambiguous bare model IDs to a preferred but unauthenticated provider instead of the authenticated provider selected by the eager path ([#6727](https://github.com/can1357/oh-my-pi/issues/6727)).
|
||||
- Fixed Windows sessions crashing with an unhandled `EPIPE: broken pipe, write` when an LSP server closed its stdin between filesystem mutations; LSP writes now observe asynchronous `FileSink.write()` failures and route them through the existing request/notification failure path.
|
||||
|
||||
### Changed
|
||||
|
||||
- Reduced default startup resident memory by constructing the default-off ComputerTool ArkType schema only on first parameter access, then reusing it across tool instances without changing validation or tool behavior ([#6742](https://github.com/can1357/oh-my-pi/pull/6742) by [@usr-bin-roygbiv](https://github.com/usr-bin-roygbiv)).
|
||||
|
||||
## [17.1.4] - 2026-07-26
|
||||
|
||||
### Added
|
||||
|
||||
@@ -14,8 +14,8 @@ import type {
|
||||
DesktopDisplay,
|
||||
DesktopSessionOptions,
|
||||
} from "@oh-my-pi/pi-natives";
|
||||
import { prompt, sanitizeText } from "@oh-my-pi/pi-utils";
|
||||
import { type } from "arktype";
|
||||
import { once, prompt, sanitizeText } from "@oh-my-pi/pi-utils";
|
||||
import { type Type, type } from "arktype";
|
||||
import computerDescription from "../prompts/tools/computer.md" with { type: "text" };
|
||||
import { truncateForPrompt } from "./approval";
|
||||
import { type ComputerController, ComputerSupervisor, registerComputerController } from "./computer/supervisor";
|
||||
@@ -57,47 +57,73 @@ function captureOptions(session: ToolSession, coordinateSafeImageSizing: boolean
|
||||
const INT32_MIN = -2_147_483_648;
|
||||
const INT32_MAX = 2_147_483_647;
|
||||
|
||||
const coordinateSchema = type("0 <= number.integer <= 2147483647");
|
||||
const scrollDeltaSchema = type("-2147483648 <= number.integer <= 2147483647");
|
||||
type ComputerSchemaPoint = {
|
||||
x: number;
|
||||
y: number;
|
||||
};
|
||||
|
||||
const pointSchema = type({
|
||||
x: coordinateSchema.describe("x pixel coordinate"),
|
||||
y: coordinateSchema.describe("y pixel coordinate"),
|
||||
"+": "reject",
|
||||
type ComputerSchemaAction = {
|
||||
type: ComputerAction["type"];
|
||||
x?: number;
|
||||
y?: number;
|
||||
button?: "left" | "right" | "wheel" | "back" | "forward";
|
||||
path?: ComputerSchemaPoint[];
|
||||
keys?: string[] | null;
|
||||
scroll_x?: number;
|
||||
scroll_y?: number;
|
||||
text?: string;
|
||||
};
|
||||
|
||||
export type ComputerParams = {
|
||||
actions?: ComputerSchemaAction[];
|
||||
};
|
||||
|
||||
type IsSameType<Left, Right> = [Left] extends [Right] ? ([Right] extends [Left] ? true : false) : false;
|
||||
type ComputerSchema<Schema extends Type = Type<ComputerParams>> =
|
||||
IsSameType<ComputerParams, Schema["infer"]> extends true ? Schema : never;
|
||||
|
||||
const getComputerSchema: () => ComputerSchema = once(() => {
|
||||
const coordinateSchema = type("0 <= number.integer <= 2147483647");
|
||||
const scrollDeltaSchema = type("-2147483648 <= number.integer <= 2147483647");
|
||||
|
||||
const pointSchema = type({
|
||||
x: coordinateSchema.describe("x pixel coordinate"),
|
||||
y: coordinateSchema.describe("y pixel coordinate"),
|
||||
"+": "reject",
|
||||
});
|
||||
|
||||
const computerActionSchema = type({
|
||||
type: type(
|
||||
"'click' | 'double_click' | 'drag' | 'keypress' | 'move' | 'screenshot' | 'scroll' | 'type' | 'wait'",
|
||||
).describe("action kind"),
|
||||
"x?": coordinateSchema.describe(
|
||||
"x pixel coordinate in the most recent screenshot (click, double_click, move, scroll)",
|
||||
),
|
||||
"y?": coordinateSchema.describe(
|
||||
"y pixel coordinate in the most recent screenshot (click, double_click, move, scroll)",
|
||||
),
|
||||
"button?": type("'left' | 'right' | 'wheel' | 'back' | 'forward'").describe("mouse button; required for click"),
|
||||
"path?": pointSchema.array().atLeastLength(2).describe("waypoints from press to release; required for drag"),
|
||||
"keys?": type("string[] | null").describe(
|
||||
"key names (e.g. CTRL, SHIFT, ENTER, A); required chord for keypress, optional held modifiers for pointer actions",
|
||||
),
|
||||
"scroll_x?": scrollDeltaSchema.describe("horizontal scroll delta in pixels; required for scroll"),
|
||||
"scroll_y?": scrollDeltaSchema.describe(
|
||||
"vertical scroll delta in pixels, positive scrolls content down; required for scroll",
|
||||
),
|
||||
"text?": type("string").describe("literal text to type; required for type"),
|
||||
"+": "reject",
|
||||
});
|
||||
|
||||
const computerSchema = type({
|
||||
"actions?": computerActionSchema
|
||||
.array()
|
||||
.describe("ordered actions executed as one batch; omit or pass [] to just capture a screenshot"),
|
||||
"+": "reject",
|
||||
});
|
||||
return computerSchema satisfies ComputerSchema<typeof computerSchema>;
|
||||
});
|
||||
|
||||
const computerActionSchema = type({
|
||||
type: type(
|
||||
"'click' | 'double_click' | 'drag' | 'keypress' | 'move' | 'screenshot' | 'scroll' | 'type' | 'wait'",
|
||||
).describe("action kind"),
|
||||
"x?": coordinateSchema.describe(
|
||||
"x pixel coordinate in the most recent screenshot (click, double_click, move, scroll)",
|
||||
),
|
||||
"y?": coordinateSchema.describe(
|
||||
"y pixel coordinate in the most recent screenshot (click, double_click, move, scroll)",
|
||||
),
|
||||
"button?": type("'left' | 'right' | 'wheel' | 'back' | 'forward'").describe("mouse button; required for click"),
|
||||
"path?": pointSchema.array().atLeastLength(2).describe("waypoints from press to release; required for drag"),
|
||||
"keys?": type("string[] | null").describe(
|
||||
"key names (e.g. CTRL, SHIFT, ENTER, A); required chord for keypress, optional held modifiers for pointer actions",
|
||||
),
|
||||
"scroll_x?": scrollDeltaSchema.describe("horizontal scroll delta in pixels; required for scroll"),
|
||||
"scroll_y?": scrollDeltaSchema.describe(
|
||||
"vertical scroll delta in pixels, positive scrolls content down; required for scroll",
|
||||
),
|
||||
"text?": type("string").describe("literal text to type; required for type"),
|
||||
"+": "reject",
|
||||
});
|
||||
|
||||
const computerSchema = type({
|
||||
"actions?": computerActionSchema
|
||||
.array()
|
||||
.describe("ordered actions executed as one batch; omit or pass [] to just capture a screenshot"),
|
||||
"+": "reject",
|
||||
});
|
||||
|
||||
export type ComputerParams = typeof computerSchema.infer;
|
||||
|
||||
export interface ComputerToolDetails {
|
||||
width: number;
|
||||
height: number;
|
||||
@@ -374,14 +400,16 @@ function approvalActionSummary(actions: unknown): string[] {
|
||||
return truncateForPrompt(lines.join("\n"), 2_000).split("\n");
|
||||
}
|
||||
|
||||
export class ComputerTool implements AgentTool<typeof computerSchema, ComputerToolDetails> {
|
||||
export class ComputerTool implements AgentTool<ComputerSchema, ComputerToolDetails> {
|
||||
readonly name = "computer";
|
||||
readonly native = { type: "computer" } as const;
|
||||
readonly label = "Computer";
|
||||
readonly loadMode = "essential" as const;
|
||||
readonly concurrency = "exclusive" as const;
|
||||
readonly summary = "Capture and control the host desktop through native OS APIs";
|
||||
readonly parameters = computerSchema;
|
||||
get parameters(): ComputerSchema {
|
||||
return getComputerSchema();
|
||||
}
|
||||
readonly strict = false;
|
||||
readonly approval = computerApproval;
|
||||
readonly formatApprovalDetails = (args: unknown): string[] => {
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
|
||||
const preloadPath = path.join(import.meta.dir, "fixtures", "computer-schema-construction-preload.ts");
|
||||
const probePath = path.join(import.meta.dir, "fixtures", "computer-schema-construction-probe.ts");
|
||||
|
||||
test("computer schema is constructed once on first parameters access", async () => {
|
||||
const proc = Bun.spawn([process.execPath, "--preload", preloadPath, probePath], {
|
||||
cwd: path.join(import.meta.dir, "../../.."),
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
});
|
||||
const [stdout, stderr, exitCode] = await Promise.all([
|
||||
new Response(proc.stdout).text(),
|
||||
new Response(proc.stderr).text(),
|
||||
proc.exited,
|
||||
]);
|
||||
|
||||
expect(exitCode, stderr).toBe(0);
|
||||
expect(JSON.parse(stdout)).toEqual({
|
||||
counts: {
|
||||
afterModuleImport: 0,
|
||||
afterDefaultOffFactory: 0,
|
||||
afterToolConstruction: 0,
|
||||
afterFirstParametersAccess: 1,
|
||||
afterRepeatedParametersAccess: 1,
|
||||
afterSecondToolParametersAccess: 1,
|
||||
afterValidation: 1,
|
||||
},
|
||||
disabledToolCount: 0,
|
||||
schema: {
|
||||
callable: true,
|
||||
repeatedIdentity: true,
|
||||
crossToolIdentity: true,
|
||||
validAccepted: true,
|
||||
validOutput: {
|
||||
actions: [
|
||||
{ type: "click", x: 1, y: 2, button: "left", keys: null },
|
||||
{ type: "double_click", x: 3, y: 4 },
|
||||
{
|
||||
type: "drag",
|
||||
path: [
|
||||
{ x: 0, y: 0 },
|
||||
{ x: 9, y: 9 },
|
||||
],
|
||||
},
|
||||
{ type: "keypress", keys: ["CTRL", "A"] },
|
||||
{ type: "move", x: 5, y: 6 },
|
||||
{ type: "screenshot" },
|
||||
{ type: "scroll", x: 7, y: 8, scroll_x: -10, scroll_y: 20 },
|
||||
{ type: "type", text: "hello" },
|
||||
{ type: "wait" },
|
||||
],
|
||||
},
|
||||
invalidRejected: [true, true, true, true, true, true],
|
||||
},
|
||||
});
|
||||
}, 20_000);
|
||||
@@ -0,0 +1,19 @@
|
||||
import { spyOn } from "bun:test";
|
||||
import * as arktype from "arktype";
|
||||
|
||||
declare global {
|
||||
var __computerCoordinateSchemaConstructionCount: number;
|
||||
}
|
||||
|
||||
const coordinateDefinition = "0 <= number.integer <= 2147483647";
|
||||
globalThis.__computerCoordinateSchemaConstructionCount = 0;
|
||||
const originalType = arktype.type;
|
||||
const countedType = ((...args: unknown[]) => {
|
||||
if (args[0] === coordinateDefinition) {
|
||||
globalThis.__computerCoordinateSchemaConstructionCount += 1;
|
||||
}
|
||||
return Reflect.apply(originalType, undefined, args);
|
||||
}) as typeof originalType;
|
||||
Object.assign(countedType, originalType);
|
||||
const typeSpy = spyOn(arktype, "type").mockImplementation(countedType);
|
||||
Object.assign(typeSpy, originalType);
|
||||
@@ -0,0 +1,101 @@
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { BUILTIN_TOOLS, ComputerTool, createTools, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
|
||||
import { type as arkType } from "arktype";
|
||||
|
||||
declare global {
|
||||
var __computerCoordinateSchemaConstructionCount: number;
|
||||
}
|
||||
|
||||
const count = () => globalThis.__computerCoordinateSchemaConstructionCount;
|
||||
const toolSession = (settings: Settings): ToolSession =>
|
||||
({
|
||||
cwd: ".",
|
||||
hasUI: false,
|
||||
settings,
|
||||
getSessionFile: () => null,
|
||||
getSessionSpawns: () => null,
|
||||
}) as ToolSession;
|
||||
|
||||
const counts = {
|
||||
afterModuleImport: count(),
|
||||
afterDefaultOffFactory: -1,
|
||||
afterToolConstruction: -1,
|
||||
afterFirstParametersAccess: -1,
|
||||
afterRepeatedParametersAccess: -1,
|
||||
afterSecondToolParametersAccess: -1,
|
||||
afterValidation: -1,
|
||||
};
|
||||
|
||||
const disabledTools = await createTools(toolSession(Settings.isolated()), ["computer"]);
|
||||
counts.afterDefaultOffFactory = count();
|
||||
|
||||
const firstTool = await BUILTIN_TOOLS.computer(toolSession(Settings.isolated()));
|
||||
const secondTool = await BUILTIN_TOOLS.computer(toolSession(Settings.isolated()));
|
||||
if (!(firstTool instanceof ComputerTool) || !(secondTool instanceof ComputerTool)) {
|
||||
throw new Error("Expected the built-in computer factory to construct ComputerTool instances");
|
||||
}
|
||||
counts.afterToolConstruction = count();
|
||||
|
||||
const firstSchema = firstTool.parameters;
|
||||
counts.afterFirstParametersAccess = count();
|
||||
const repeatedSchema = firstTool.parameters;
|
||||
counts.afterRepeatedParametersAccess = count();
|
||||
const secondToolSchema = secondTool.parameters;
|
||||
counts.afterSecondToolParametersAccess = count();
|
||||
|
||||
const validInput = {
|
||||
actions: [
|
||||
{ type: "click", x: 1, y: 2, button: "left", keys: null },
|
||||
{ type: "double_click", x: 3, y: 4 },
|
||||
{
|
||||
type: "drag",
|
||||
path: [
|
||||
{ x: 0, y: 0 },
|
||||
{ x: 9, y: 9 },
|
||||
],
|
||||
},
|
||||
{ type: "keypress", keys: ["CTRL", "A"] },
|
||||
{ type: "move", x: 5, y: 6 },
|
||||
{ type: "screenshot" },
|
||||
{ type: "scroll", x: 7, y: 8, scroll_x: -10, scroll_y: 20 },
|
||||
{ type: "type", text: "hello" },
|
||||
{ type: "wait" },
|
||||
],
|
||||
};
|
||||
const validOutput = firstSchema(validInput);
|
||||
const invalidOutputs = [
|
||||
firstSchema({ actions: [{ type: "click", x: -1, y: 2, button: "left" }] }),
|
||||
firstSchema({ actions: [{ type: "move", x: 0.5, y: 0 }] }),
|
||||
firstSchema({ actions: [{ type: "scroll", x: 0, y: 0, scroll_x: 2 ** 31, scroll_y: 0 }] }),
|
||||
firstSchema({ actions: [{ type: "drag", path: [{ x: 0, y: 0 }] }] }),
|
||||
firstSchema({
|
||||
actions: [
|
||||
{
|
||||
type: "drag",
|
||||
path: [
|
||||
{ x: 0, y: 0, label: "unexpected" },
|
||||
{ x: 1, y: 1 },
|
||||
],
|
||||
},
|
||||
],
|
||||
}),
|
||||
firstSchema({ actions: [], unexpected: true }),
|
||||
];
|
||||
counts.afterValidation = count();
|
||||
|
||||
await Promise.all([firstTool.close(), secondTool.close()]);
|
||||
|
||||
process.stdout.write(
|
||||
JSON.stringify({
|
||||
counts,
|
||||
disabledToolCount: disabledTools.length,
|
||||
schema: {
|
||||
callable: typeof firstSchema === "function",
|
||||
repeatedIdentity: firstSchema === repeatedSchema,
|
||||
crossToolIdentity: firstSchema === secondToolSchema,
|
||||
validAccepted: !(validOutput instanceof arkType.errors),
|
||||
validOutput,
|
||||
invalidRejected: invalidOutputs.map(output => output instanceof arkType.errors),
|
||||
},
|
||||
}),
|
||||
);
|
||||
Reference in New Issue
Block a user