mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
feat(coding-agent): prefer strict sampling for built-in tools by default
This commit is contained in:
parent
caf6dfe731
commit
fcff255b00
8 changed files with 118 additions and 54 deletions
|
|
@ -2,6 +2,10 @@
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
### Changed
|
||||
|
||||
- Enabled strict-prefer JSON-schema sampling by default for built-in `read`, `bash`, `powershell`, `edit`, and `write` tools, without requiring `PI_EXPERIMENTAL`. Extensions can re-register tool definitions with `constrainedSampling: false`.
|
||||
|
||||
## [0.85.1] - 2026-09-05
|
||||
|
||||
### New Features
|
||||
|
|
|
|||
|
|
@ -1,9 +1,3 @@
|
|||
const PREFER_STRICT_TOOL_SAMPLING = { type: "json_schema", strict: "prefer" } as const;
|
||||
|
||||
export function areExperimentalFeaturesEnabled(): boolean {
|
||||
return process.env.PI_EXPERIMENTAL === "1";
|
||||
}
|
||||
|
||||
export function getExperimentalToolSampling() {
|
||||
return areExperimentalFeaturesEnabled() ? PREFER_STRICT_TOOL_SAMPLING : undefined;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,7 +12,6 @@ import {
|
|||
trackDetachedChildPid,
|
||||
untrackDetachedChildPid,
|
||||
} from "../../utils/shell.ts";
|
||||
import { getExperimentalToolSampling } from "../experimental.ts";
|
||||
import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts";
|
||||
import { OutputAccumulator } from "./output-accumulator.ts";
|
||||
import { BASH_UPDATE_THROTTLE_MS, createShellRenderers } from "./renderers/bash.ts";
|
||||
|
|
@ -236,7 +235,7 @@ export function createShellToolDefinition(
|
|||
promptSnippet: config.promptSnippet,
|
||||
promptGuidelines: exposeSessionEnvironment && config.promptGuidelines ? [...config.promptGuidelines] : undefined,
|
||||
parameters: bashSchema,
|
||||
constrainedSampling: getExperimentalToolSampling(),
|
||||
constrainedSampling: { type: "json_schema", strict: "prefer" },
|
||||
async execute(
|
||||
_toolCallId,
|
||||
{ command, timeout }: { command: string; timeout?: number },
|
||||
|
|
|
|||
|
|
@ -3,7 +3,6 @@ import { constants } from "fs";
|
|||
import { access as fsAccess, readFile as fsReadFile, writeFile as fsWriteFile } from "fs/promises";
|
||||
import { type Static, Type } from "typebox";
|
||||
import { splitBom } from "../../utils/text.ts";
|
||||
import { getExperimentalToolSampling } from "../experimental.ts";
|
||||
import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts";
|
||||
import {
|
||||
applyEditsToNormalizedContent,
|
||||
|
|
@ -154,7 +153,7 @@ export function createEditToolDefinition(
|
|||
promptSnippet: editToolSystemPromptContribution.snippet,
|
||||
promptGuidelines: [...editToolSystemPromptContribution.guidelines],
|
||||
parameters: editSchema,
|
||||
constrainedSampling: getExperimentalToolSampling(),
|
||||
constrainedSampling: { type: "json_schema", strict: "prefer" },
|
||||
renderShell: "self",
|
||||
prepareArguments: prepareEditArguments,
|
||||
async execute(_toolCallId, input: EditToolInput, signal?: AbortSignal, _onUpdate?, ctx?: ExtensionContext) {
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@ import { access as fsAccess, readFile as fsReadFile } from "fs/promises";
|
|||
import { type Static, Type } from "typebox";
|
||||
import { processImage } from "../../utils/image-process.ts";
|
||||
import { detectSupportedImageMimeTypeFromFile } from "../../utils/mime.ts";
|
||||
import { getExperimentalToolSampling } from "../experimental.ts";
|
||||
import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts";
|
||||
import { resolveReadPathAsync } from "./path-utils.ts";
|
||||
import { readRenderers } from "./renderers/read.ts";
|
||||
|
|
@ -75,7 +74,7 @@ export function createReadToolDefinition(
|
|||
promptSnippet: readToolSystemPromptContribution.snippet,
|
||||
promptGuidelines: [...readToolSystemPromptContribution.guidelines],
|
||||
parameters: readSchema,
|
||||
constrainedSampling: getExperimentalToolSampling(),
|
||||
constrainedSampling: { type: "json_schema", strict: "prefer" },
|
||||
async execute(
|
||||
_toolCallId,
|
||||
{ path, offset, limit }: { path: string; offset?: number; limit?: number },
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ import type { AgentTool } from "@earendil-works/pi-agent-core";
|
|||
import { mkdir as fsMkdir, writeFile as fsWriteFile } from "fs/promises";
|
||||
import { dirname } from "path";
|
||||
import { type Static, Type } from "typebox";
|
||||
import { getExperimentalToolSampling } from "../experimental.ts";
|
||||
import type { ExtensionContext, ToolDefinition } from "../extensions/types.ts";
|
||||
import { withFileMutationQueue } from "./file-mutation-queue.ts";
|
||||
import { resolveToCwd } from "./path-utils.ts";
|
||||
|
|
@ -55,7 +54,7 @@ export function createWriteToolDefinition(
|
|||
promptSnippet: writeToolSystemPromptContribution.snippet,
|
||||
promptGuidelines: [...writeToolSystemPromptContribution.guidelines],
|
||||
parameters: writeSchema,
|
||||
constrainedSampling: getExperimentalToolSampling(),
|
||||
constrainedSampling: { type: "json_schema", strict: "prefer" },
|
||||
async execute(
|
||||
_toolCallId,
|
||||
{ path, content }: { path: string; content: string },
|
||||
|
|
|
|||
110
packages/coding-agent/test/builtin-tool-strict-mode.test.ts
Normal file
110
packages/coding-agent/test/builtin-tool-strict-mode.test.ts
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { getModel } from "@earendil-works/pi-ai/compat";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { DefaultResourceLoader } from "../src/core/resource-loader.ts";
|
||||
import { createAgentSession } from "../src/core/sdk.ts";
|
||||
import { SessionManager } from "../src/core/session-manager.ts";
|
||||
import { SettingsManager } from "../src/core/settings-manager.ts";
|
||||
import { createAllToolDefinitions, createAllTools } from "../src/core/tools/index.ts";
|
||||
import { wrapToolDefinition } from "../src/core/tools/tool-definition-wrapper.ts";
|
||||
|
||||
const strictToolNames = ["read", "bash", "powershell", "edit", "write"] as const;
|
||||
|
||||
describe("strict built-in tools", () => {
|
||||
afterEach(() => vi.unstubAllEnvs());
|
||||
|
||||
it.each([undefined, "0", "1"])("prefers strict sampling with PI_EXPERIMENTAL=%s", (experimental) => {
|
||||
vi.stubEnv("PI_EXPERIMENTAL", experimental);
|
||||
const definitions = createAllToolDefinitions(process.cwd());
|
||||
const tools = createAllTools(process.cwd());
|
||||
for (const name of strictToolNames) {
|
||||
expect(definitions[name].constrainedSampling).toEqual({ type: "json_schema", strict: "prefer" });
|
||||
expect(tools[name].constrainedSampling).toEqual(definitions[name].constrainedSampling);
|
||||
}
|
||||
for (const name of ["grep", "find", "ls"] as const) {
|
||||
expect(definitions[name].constrainedSampling).toBeUndefined();
|
||||
}
|
||||
// Strictness is a provider-side conversion, not a change to the execution schema.
|
||||
expect(definitions.read.parameters.required).toEqual(["path"]);
|
||||
expect(definitions.bash.parameters.required).toEqual(["command"]);
|
||||
});
|
||||
|
||||
it("preserves explicit opt-outs when wrapping definitions for execution", () => {
|
||||
const definitions = createAllToolDefinitions(process.cwd());
|
||||
for (const name of strictToolNames) {
|
||||
const definition = definitions[name];
|
||||
const override = { ...definition, constrainedSampling: false as const };
|
||||
expect(wrapToolDefinition(override).constrainedSampling).toBe(false);
|
||||
expect(override.execute).toBe(definition.execute);
|
||||
expect(override.prepareArguments).toBe(definition.prepareArguments);
|
||||
expect(override.renderCall).toBe(definition.renderCall);
|
||||
expect(override.renderResult).toBe(definition.renderResult);
|
||||
expect(override.promptGuidelines).toBe(definition.promptGuidelines);
|
||||
expect(definition.constrainedSampling).toEqual({ type: "json_schema", strict: "prefer" });
|
||||
}
|
||||
});
|
||||
|
||||
it.each([{ activeTools: [] }, { activeTools: ["read"] }, { activeTools: [...strictToolNames] }])(
|
||||
"allows extensions to re-register tools without strict sampling: $activeTools",
|
||||
async ({ activeTools }) => {
|
||||
const cwd = mkdtempSync(join(tmpdir(), "pi-non-strict-tools-"));
|
||||
const agentDir = join(cwd, "agent");
|
||||
const settingsManager = SettingsManager.inMemory({ defaultTools: activeTools });
|
||||
const resourceLoader = new DefaultResourceLoader({
|
||||
cwd,
|
||||
agentDir,
|
||||
settingsManager,
|
||||
noExtensions: true,
|
||||
noSkills: true,
|
||||
noPromptTemplates: true,
|
||||
noThemes: true,
|
||||
extensionFactories: [
|
||||
(pi) => {
|
||||
pi.on("session_start", () => {
|
||||
const definitions = createAllToolDefinitions(cwd);
|
||||
for (const name of strictToolNames) {
|
||||
pi.registerTool({ ...definitions[name], constrainedSampling: false });
|
||||
}
|
||||
pi.setActiveTools(activeTools);
|
||||
});
|
||||
},
|
||||
],
|
||||
});
|
||||
try {
|
||||
await resourceLoader.reload();
|
||||
const { session } = await createAgentSession({
|
||||
cwd,
|
||||
agentDir,
|
||||
model: getModel("anthropic", "claude-sonnet-4-5"),
|
||||
settingsManager,
|
||||
sessionManager: SessionManager.inMemory(cwd),
|
||||
resourceLoader,
|
||||
});
|
||||
try {
|
||||
const originalPrompt = session.systemPrompt;
|
||||
await session.bindExtensions({});
|
||||
expect(session.getActiveToolNames()).toEqual(activeTools);
|
||||
expect(session.systemPrompt).toBe(originalPrompt);
|
||||
for (const name of strictToolNames) {
|
||||
expect(session.getToolDefinition(name)?.constrainedSampling).toBe(false);
|
||||
}
|
||||
for (const tool of session.agent.state.tools) {
|
||||
expect(tool.constrainedSampling).toBe(false);
|
||||
}
|
||||
if (activeTools.includes("read")) {
|
||||
writeFileSync(join(cwd, "sample.txt"), "still works");
|
||||
const read = session.agent.state.tools.find((tool) => tool.name === "read")!;
|
||||
const result = await read.execute("read-test", { path: "sample.txt" });
|
||||
expect(result.content).toEqual([{ type: "text", text: "still works" }]);
|
||||
}
|
||||
} finally {
|
||||
session.dispose();
|
||||
}
|
||||
} finally {
|
||||
rmSync(cwd, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
@ -1,40 +0,0 @@
|
|||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
createBashToolDefinition,
|
||||
createEditToolDefinition,
|
||||
createPowerShellToolDefinition,
|
||||
createReadToolDefinition,
|
||||
createWriteToolDefinition,
|
||||
} from "../src/core/tools/index.ts";
|
||||
|
||||
function createBuiltInTools() {
|
||||
return [
|
||||
createReadToolDefinition(process.cwd()),
|
||||
createBashToolDefinition(process.cwd()),
|
||||
createPowerShellToolDefinition(process.cwd()),
|
||||
createEditToolDefinition(process.cwd()),
|
||||
createWriteToolDefinition(process.cwd()),
|
||||
];
|
||||
}
|
||||
|
||||
describe("experimental strict built-in tools", () => {
|
||||
const originalPiExperimental = process.env.PI_EXPERIMENTAL;
|
||||
|
||||
afterEach(() => {
|
||||
if (originalPiExperimental === undefined) delete process.env.PI_EXPERIMENTAL;
|
||||
else process.env.PI_EXPERIMENTAL = originalPiExperimental;
|
||||
});
|
||||
|
||||
it("only enables strict-prefer sampling in experimental mode", () => {
|
||||
delete process.env.PI_EXPERIMENTAL;
|
||||
const normalTools = createBuiltInTools();
|
||||
process.env.PI_EXPERIMENTAL = "1";
|
||||
const experimentalTools = createBuiltInTools();
|
||||
|
||||
for (const [index, tool] of experimentalTools.entries()) {
|
||||
expect(tool.constrainedSampling).toEqual({ type: "json_schema", strict: "prefer" });
|
||||
expect(tool.parameters).toEqual(normalTools[index]?.parameters);
|
||||
expect(normalTools[index]?.constrainedSampling).toBeUndefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue