feat(evals): add pi bench harness (integrations/pi-sdk) - #2808
Draft
miguelg719 wants to merge 4 commits into
Draft
feat(evals): add pi bench harness (integrations/pi-sdk)#2808miguelg719 wants to merge 4 commits into
miguelg719 wants to merge 4 commits into
Conversation
|
miguelg719
marked this pull request as draft
August 24, 2026 18:19
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
7 issues found across 24 files
Confidence score: 3/5
packages/evals/framework/harnesses/piAdapter.tscan discard structured MCPdetailswhen text is also returned, leaving verifiers with incomplete trajectory data; prefer non-emptydetailsbeforejoined.packages/evals/framework/piToolAdapter.tscan throw when a handle snippet returns a function orSymbol, becauseJSON.stringifyproducesundefined; fall back toString(value)before clipping.packages/integrations/pi-sdk/package.jsonrequires Node 22.19 while the repository advertises Node 22.18, which may block installation in engine-strict environments; align the SDK requirement or supported Node range.- The tests in
packages/evals/tests/framework/piToolAdapter.test.tsandpackages/integrations/pi-sdk/tests/session.test.tsleave tool ordering, timeout diagnostics, and abort forwarding under-verified, whilepackages/evals/framework/benchHarness.tsis not reflected in the evals README; strengthen the assertions and document thepiharness.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/evals/framework/benchHarness.ts">
<violation number="1" location="packages/evals/framework/benchHarness.ts:299">
P3: The new registry entry makes `pi` a supported harness, but the checked-in evals README still advertises only `stagehand`, `claude_code`, and `codex`. Update the harness option documentation to include `pi` so users can discover the new CLI mode.</violation>
</file>
<file name="packages/evals/tests/framework/piToolAdapter.test.ts">
<violation number="1" location="packages/evals/tests/framework/piToolAdapter.test.ts:41">
P2: This test does not actually verify the supported-surface order: it checks only `stagehand_facade` and that `browse_cli` is absent. Assert the complete array so removing or reordering a pi tool surface cannot silently pass.</violation>
<violation number="2" location="packages/evals/tests/framework/piToolAdapter.test.ts:172">
P2: This timeout test does not protect the diagnostic contract because it accepts any message containing `timed out`. Assert `timed out after 20ms` so regressions that hide the configured limit are caught.</violation>
</file>
<file name="packages/evals/framework/piToolAdapter.ts">
<violation number="1" location="packages/evals/framework/piToolAdapter.ts:352">
P2: When a handle snippet returns a function or `Symbol`, `JSON.stringify` returns `undefined`, so `clip(text, 500)` throws instead of returning a tool result. Fall back to `String(value)` when JSON serialization is undefined.</violation>
</file>
<file name="packages/integrations/pi-sdk/tests/session.test.ts">
<violation number="1" location="packages/integrations/pi-sdk/tests/session.test.ts:263">
P2: This test does not verify abort forwarding during an active prompt: it passes even if the `abort` listener or `forwardAbort` is removed. Add an in-flight prompt that aborts the caller signal, then assert the session aborts and the forwarder is detached.
(Based on your team's feedback about adding focused tests for new behavior and lifecycle edge cases.)</violation>
</file>
<file name="packages/integrations/pi-sdk/package.json">
<violation number="1" location="packages/integrations/pi-sdk/package.json:35">
P2: When the harness runs on the repository's advertised Node 22.18 minimum, its required Pi SDK declares Node 22.19 as the minimum, producing an engine mismatch and potentially failing installation in engine-strict environments. Raise this package's engine minimum to `>=22.19.0` or pin a Pi SDK release that supports 22.18.</violation>
</file>
<file name="packages/evals/framework/harnesses/piAdapter.ts">
<violation number="1" location="packages/evals/framework/harnesses/piAdapter.ts:147">
P2: When a bridged MCP tool returns text plus structured `details`, this line discards the structured result and leaves the trajectory with only a text modality. Prefer non-empty `details` before `joined` so verifier consumers retain the JSON result shape.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Runner as Evals Runner
participant PiR as piRunner.ts
participant PiTA as piToolAdapter.ts
participant SDK as pi-sdk (Integration)
participant Agent as PI Agent SDK
participant Target as Tool Target (MCP/Handles)
Note over Runner,Target: NEW: Evaluation Flow with pi Harness
Runner->>PiR: runPiAgent(plan, model)
PiR->>PiTA: NEW: preparePiToolAdapter()
PiTA->>PiTA: Resolve Tool Surface & Profile
PiTA->>PiTA: Start AgentToolRuntime
PiTA->>PiTA: NEW: buildPiMountConfig()
alt via: "mcp"
PiTA-->>PiR: MCP Server Specs
else via: "handles"
PiTA-->>PiR: Custom Tool Definitions (in-process)
end
PiR->>SDK: NEW: runPiSession(prompt, tools, mcpServers)
opt Has MCP Servers
SDK->>Target: NEW: connectPiMcpServers()
Target-->>SDK: Tool list (prefixed mcp__)
end
SDK->>Agent: createSession(customTools)
Agent-->>SDK: AgentSession object
loop Agent Step (Turn budget: EVAL_PI_MAX_TURNS)
Agent->>SDK: Emit AgentSessionEvent
SDK->>SDK: NEW: Redact secrets (sk-*, bb_*)
SDK->>SDK: NEW: Line-buffer stderr logs
alt Tool Call: MCP
Agent->>SDK: tool_execution_start (mcp__*)
SDK->>Target: Call stdio MCP tool
Target-->>SDK: result
else Tool Call: Handles
Agent->>SDK: tool_execution_start (run_browser_code)
SDK->>PiTA: NEW: executeCodeExposureSnippet()
PiTA->>Target: Execute snippet against Browser Handles
Target-->>PiTA: result
PiTA-->>SDK: result text
end
opt After observed tool call
PiTA->>PiTA: NEW: recordObservation() (screenshots/URL)
end
SDK-->>Agent: tool_execution_end
end
Agent-->>SDK: turn_end / stopReason
SDK-->>PiR: PiSessionResult (Events, Usage, FinalAnswer)
PiR->>PiR: NEW: parsePiResult() (look for EVAL_RESULT)
PiR->>PiR: CHANGED: Map stopReason (aborted/length -> sdk_error)
PiR->>PiR: NEW: piAdapter.fromHarnessResult()
Note right of PiR: Normalizes PI Events + Usage + ProbeEvidence<br/>into Trajectory format
PiR->>PiTA: NEW: cleanup()
PiTA->>PiTA: Stop runtime & rm temp dirs
PiR-->>Runner: TaskResult (Trajectory + Metrics)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Comment on lines
+41
to
+42
| expect(PI_TOOL_SURFACES[0]).toBe("stagehand_facade"); | ||
| expect(PI_TOOL_SURFACES).not.toContain("browse_cli"); |
Contributor
There was a problem hiding this comment.
P2: This test does not actually verify the supported-surface order: it checks only stagehand_facade and that browse_cli is absent. Assert the complete array so removing or reordering a pi tool surface cannot silently pass.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/tests/framework/piToolAdapter.test.ts, line 41:
<comment>This test does not actually verify the supported-surface order: it checks only `stagehand_facade` and that `browse_cli` is absent. Assert the complete array so removing or reordering a pi tool surface cannot silently pass.</comment>
<file context>
@@ -0,0 +1,277 @@
+
+describe("pi tool adapter", () => {
+ it("publishes the supported surfaces in default order", () => {
+ expect(PI_TOOL_SURFACES[0]).toBe("stagehand_facade");
+ expect(PI_TOOL_SURFACES).not.toContain("browse_cli");
+ });
</file context>
Suggested change
| expect(PI_TOOL_SURFACES[0]).toBe("stagehand_facade"); | |
| expect(PI_TOOL_SURFACES).not.toContain("browse_cli"); | |
| expect(PI_TOOL_SURFACES).toEqual([ | |
| "stagehand_facade", | |
| "playwright_mcp", | |
| "chrome_devtools_mcp", | |
| "stagehand_code", | |
| "playwright_code", | |
| "cdp_code", | |
| ]); |
| undefined, | ||
| {} as never, | ||
| ), | ||
| ).rejects.toThrow(/timed out/); |
Contributor
There was a problem hiding this comment.
P2: This timeout test does not protect the diagnostic contract because it accepts any message containing timed out. Assert timed out after 20ms so regressions that hide the configured limit are caught.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/tests/framework/piToolAdapter.test.ts, line 172:
<comment>This timeout test does not protect the diagnostic contract because it accepts any message containing `timed out`. Assert `timed out after 20ms` so regressions that hide the configured limit are caught.</comment>
<file context>
@@ -0,0 +1,277 @@
+ undefined,
+ {} as never,
+ ),
+ ).rejects.toThrow(/timed out/);
+ } finally {
+ if (previous === undefined) delete process.env.EVAL_PI_RUN_TOOL_TIMEOUT_MS;
</file context>
| if (value === undefined) return "undefined"; | ||
| if (typeof value === "string") return value; | ||
| try { | ||
| return JSON.stringify(value); |
Contributor
There was a problem hiding this comment.
P2: When a handle snippet returns a function or Symbol, JSON.stringify returns undefined, so clip(text, 500) throws instead of returning a tool result. Fall back to String(value) when JSON serialization is undefined.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/piToolAdapter.ts, line 352:
<comment>When a handle snippet returns a function or `Symbol`, `JSON.stringify` returns `undefined`, so `clip(text, 500)` throws instead of returning a tool result. Fall back to `String(value)` when JSON serialization is undefined.</comment>
<file context>
@@ -0,0 +1,364 @@
+ if (value === undefined) return "undefined";
+ if (typeof value === "string") return value;
+ try {
+ return JSON.stringify(value);
+ } catch {
+ return String(value);
</file context>
| expect(fake.disposeCount).toBe(0); | ||
| }); | ||
|
|
||
| it("forwards an already-aborted signal", async () => { |
Contributor
There was a problem hiding this comment.
P2: This test does not verify abort forwarding during an active prompt: it passes even if the abort listener or forwardAbort is removed. Add an in-flight prompt that aborts the caller signal, then assert the session aborts and the forwarder is detached.
(Based on your team's feedback about adding focused tests for new behavior and lifecycle edge cases.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/pi-sdk/tests/session.test.ts, line 263:
<comment>This test does not verify abort forwarding during an active prompt: it passes even if the `abort` listener or `forwardAbort` is removed. Add an in-flight prompt that aborts the caller signal, then assert the session aborts and the forwarder is detached.
(Based on your team's feedback about adding focused tests for new behavior and lifecycle edge cases.) </comment>
<file context>
@@ -0,0 +1,326 @@
+ expect(fake.disposeCount).toBe(0);
+ });
+
+ it("forwards an already-aborted signal", async () => {
+ const fake = scriptedSdk([]);
+ const controller = new AbortController();
</file context>
| "vitest": "catalog:" | ||
| }, | ||
| "engines": { | ||
| "node": ">=22.18.0" |
Contributor
There was a problem hiding this comment.
P2: When the harness runs on the repository's advertised Node 22.18 minimum, its required Pi SDK declares Node 22.19 as the minimum, producing an engine mismatch and potentially failing installation in engine-strict environments. Raise this package's engine minimum to >=22.19.0 or pin a Pi SDK release that supports 22.18.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integrations/pi-sdk/package.json, line 35:
<comment>When the harness runs on the repository's advertised Node 22.18 minimum, its required Pi SDK declares Node 22.19 as the minimum, producing an engine mismatch and potentially failing installation in engine-strict environments. Raise this package's engine minimum to `>=22.19.0` or pin a Pi SDK release that supports 22.18.</comment>
<file context>
@@ -0,0 +1,37 @@
+ "vitest": "catalog:"
+ },
+ "engines": {
+ "node": ">=22.18.0"
+ }
+}
</file context>
Suggested change
| "node": ">=22.18.0" | |
| "node": ">=22.19.0" |
| } | ||
| const joined = text.join("\n"); | ||
| return { | ||
| result: joined || (value.details !== undefined ? value.details : value), |
Contributor
There was a problem hiding this comment.
P2: When a bridged MCP tool returns text plus structured details, this line discards the structured result and leaves the trajectory with only a text modality. Prefer non-empty details before joined so verifier consumers retain the JSON result shape.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/harnesses/piAdapter.ts, line 147:
<comment>When a bridged MCP tool returns text plus structured `details`, this line discards the structured result and leaves the trajectory with only a text modality. Prefer non-empty `details` before `joined` so verifier consumers retain the JSON result shape.</comment>
<file context>
@@ -0,0 +1,155 @@
+ }
+ const joined = text.join("\n");
+ return {
+ result: joined || (value.details !== undefined ? value.details : value),
+ text: joined,
+ images,
</file context>
Suggested change
| result: joined || (value.details !== undefined ? value.details : value), | |
| result: | |
| value.details !== undefined && | |
| (!isRecord(value.details) || Object.keys(value.details).length > 0) | |
| ? value.details | |
| : joined || value, |
| ["claude_code", claudeCodeHarness], | ||
| ["codex", codexHarness], | ||
| ["mastra", mastraHarness], | ||
| ["pi", piHarness], |
Contributor
There was a problem hiding this comment.
P3: The new registry entry makes pi a supported harness, but the checked-in evals README still advertises only stagehand, claude_code, and codex. Update the harness option documentation to include pi so users can discover the new CLI mode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/benchHarness.ts, line 299:
<comment>The new registry entry makes `pi` a supported harness, but the checked-in evals README still advertises only `stagehand`, `claude_code`, and `codex`. Update the harness option documentation to include `pi` so users can discover the new CLI mode.</comment>
<file context>
@@ -281,11 +283,20 @@ export const mastraHarness = defineExternalHarness({
["claude_code", claudeCodeHarness],
["codex", codexHarness],
["mastra", mastraHarness],
+ ["pi", piHarness],
]);
</file context>
miguelg719
force-pushed
the
harness/wave-mastra
branch
from
August 24, 2026 18:34
7964829 to
c0a2370
Compare
Adds @browserbasehq/stagehand-integrations-pi-sdk (in-process pi-coding-agent session layer: hermetic ResourceLoader, custom-tool allowlist, turn budget, abort forwarding, token/cost extraction, stdio MCP -> pi tool bridge) and wires a "pi" bench harness with defineExternalHarness + runExternalHarnessTask. - piToolAdapter: stagehand_facade (default), playwright_mcp, chrome_devtools_mcp, stagehand_code, playwright_code, cdp_code; mcp mounts bridged in-process, handles mounts hosted as the harness run tool inside pi - piAdapter: pi AgentSession events -> verifier Trajectory - piRunner: marker result contract, harness_* metrics via the skeleton, pi_turns - registry: "pi" registered after codex; EVAL_PI_MODELS derives automatically - tests: pi-sdk session/mcp, adapter, tool adapter, runner, registry; existing hardcoded harness-list assertions made registry-derived
…ering, stop reasons, and run-tool aborts - pi-sdk session: sanitize every event summary (message + detail) so logPiEvent/buildPiTranscript never leak tool args/results or provider errorMessage fields; treat pi stopReason 'aborted' (without an external signal) and 'length' as sdk_error instead of completed - pi-sdk mcp: line-buffer MCP stderr via attachPiMcpStderrLogger, sanitize complete lines (CRLF-aware), flush the trailing partial line once on end/close - evals piToolAdapter: honor the AbortSignal pi hands the run tool (reject before start when already aborted, race abort alongside the timeout), and attach a logging catch to abandoned snippets so late rejections never surface as unhandled
miguelg719
force-pushed
the
harness/wave-pi
branch
from
August 24, 2026 18:37
77e7cb7 to
48370f3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
--harness pito evals, backed by a new private@browserbasehq/stagehand-integrations-pi-sdkpackage.packages/integrations/pi-sdk—runPiSessionover the programmatic@earendil-works/pi-coding-agentSDK (createAgentSession, hermetic tool allowlist so bash/edit/write are never exposed), in-process MCP bridge for facade tools, streamedAgentSessionEvents → tool-call/result/usage, abort forwarding, redaction, status/stopReason normalization.packages/evals/framework/piToolAdapter.ts—via:"mcp"mounts (stagehand_facade first, playwright_mcp, chrome_devtools_mcp);via:"handles"run-tool via the in-process bridge.packages/evals/framework/harnesses/piAdapter.ts— events →NormalizedToolCall[].defineExternalHarness.Testing
evals run b:webvoyager --harness pi --tool stagehand_facade -l 1 -t 1 -e browserbase): 10 Stagehand tool calls via the facade, final answer produced.Stacked on the mastra PR. Implemented with Codex (gpt-5.6) under supervision; review findings (redaction, stderr buffering, stop reasons, run-tool aborts) fixed in-branch.
Summary by cubic
Adds a new
--harness pibench harness backed by a private@browserbasehq/stagehand-integrations-pi-sdk, enabling evals to run the PI coding agent with Stagehand tools while keeping tool exposure hermetic and logs redacted. Behavior change: PI stop reasons “aborted” and “length” now map to sdk_error; harness guidance strings derive from the registry dynamically.piharness with default modelopenai/gpt-5.4-mini; supportsstagehand_facade(default),playwright_mcp,chrome_devtools_mcp,stagehand_code,playwright_code,cdp_code.@browserbasehq/stagehand-integrations-pi-sdk:mcp__server__tool), line-buffers stderr and redacts secrets; flushes trailing partial lines.piRunner: plugs into the external harness skeleton (marker contract), addspi_turnsmetric.piAdapter: converts PI events into verifier trajectories (reasoning, images, probe evidence).piToolAdapter: mounts MCP or in-process handles; records step observations; cleans runtimes and temp dirs; avoids unhandled rejections on late failures.Rollout
evals run <task> --harness pi --tool stagehand_facade(or another supported surface).EVAL_PI_MAX_TURNS(orAGENT_EVAL_MAX_STEPS) to cap turns;EVAL_PI_THINKINGfor thinking level.EVAL_PI_RUN_TOOL_TIMEOUT_MS,EVAL_AGENT_MOUNT_CLEANUP_TIMEOUT_MS,EVAL_CAPTURE_EVIDENCE_TIMEOUT_MS.mcpServersin the mount; tools appear asmcp__server__tooland emit redacted, line-buffered stderr logs.Written for commit 48370f3. Summary will update on new commits.