evals: run act/extract/observe bench tasks on a v4 Stagehand client - #2570
Conversation
|
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Architecture diagram
sequenceDiagram
participant CLI as CLI Runner
participant Harness as benchHarness
participant Runner as benchRunner
participant Task as Eval Task (act/extract/observe)
participant Init as initStagehand
participant SDK as Stagehand v4 SDK
participant Browser as Browser (local/Browserbase)
participant Braintrust as Braintrust API
Note over CLI,Braintrust: Deterministic Benchmark Flow (act/extract/observe categories)
CLI->>Harness: stagehandHarness(task)
Harness->>Harness: Check task.primaryCategory
alt Category is act/extract/observe
Harness->>Harness: Block agent tasks & --api
Harness->>Init: initStagehand(modelName, environment)
Init->>Init: Resolve provider API key from env
alt environment == "BROWSERBASE"
Init->>Browser: browserbase.launch({apiKey})
else LOCAL
Init->>Browser: localBrowser.launch({headless: false})
end
Browser-->>Init: browser handle
Init->>SDK: Stagehand.create({browser, selfHeal: true, model})
Note over Init,SDK: selfHeal enabled (server default is off)
SDK->>SDK: onLog filter: drop debug lines
Note over SDK: Prevents ~18MB Braintrust payload rejection
alt Creation fails
SDK-->>Init: Error
Init->>Browser: browser.close()
else Success
SDK-->>Init: stagehand instance
Init->>SDK: stagehand.context.activePage()
SDK-->>Init: page handle
Init-->>Harness: {stagehand, page}
end
Harness-->>CLI: ctx with {stagehand, v4Page}
CLI->>Runner: executeBenchTask(ctx)
Runner->>Runner: Forward stagehand from harnessCtx
Runner->>Runner: Forward v4Page as page for task
Runner->>Task: Execute task(stagehand, page)
Task->>SDK: act/extract/observe calls
SDK-->>Task: Results
Task-->>Runner: Complete
Runner-->>CLI: Done
CLI->>Harness: cleanup()
Harness->>SDK: stagehand.close()
Note over Harness,SDK: Tears down RPC client only
Harness->>Browser: stagehand.browser.close()
Note over Harness,Browser: NEW: Prevents leaked Chrome/Browserbase session
else Other category (e.g., combination, agent)
Note over Harness: Falls through to existing v3 flow
end
Note over CLI,Braintrust: Failure Path: init failure after launch
alt Init throws after browser launch
Init->>Browser: browser.close() (in catch block)
Init-->>Harness: Throw error
Harness-->>CLI: Error
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 4/5
- In
packages/evals/initStagehand.ts, the new page-acquisition failure cleanup and Browserbase project-id pass-through paths are untested, so a regression could silently route sessions to the wrong Browserbase project or leave failure handling inconsistent — add targeted unit tests for both the project-id branch and page-acquisition error cleanup behavior.
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/initStagehand.ts">
<violation number="1" location="packages/evals/initStagehand.ts:124">
P3: The new page-acquisition failure cleanup and the Browserbase project-id pass-through are behavior worth locking down with unit tests: a regression in the project-id branch silently lands sessions in the wrong Browserbase project, and one in the page-failure branch re-introduces exactly the leaked-browser/process-per-task bug this change fixes. There's currently no unit test covering initStagehand's launch, cleanup, or activePage failure paths. Consider adding a couple of focused tests (e.g. that projectId is forwarded to browserbase.launch when set, and that a failing activePage() still closes both stagehand and browser) to prevent regressions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // Page acquisition failures need the same cleanup as create failures: the | ||
| // client is up and the browser is running, and stagehand.close() alone | ||
| // tears down the RPC client without closing the browser. | ||
| let page: Page | null; |
There was a problem hiding this comment.
P3: The new page-acquisition failure cleanup and the Browserbase project-id pass-through are behavior worth locking down with unit tests: a regression in the project-id branch silently lands sessions in the wrong Browserbase project, and one in the page-failure branch re-introduces exactly the leaked-browser/process-per-task bug this change fixes. There's currently no unit test covering initStagehand's launch, cleanup, or activePage failure paths. Consider adding a couple of focused tests (e.g. that projectId is forwarded to browserbase.launch when set, and that a failing activePage() still closes both stagehand and browser) to prevent regressions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/initStagehand.ts, line 124:
<comment>The new page-acquisition failure cleanup and the Browserbase project-id pass-through are behavior worth locking down with unit tests: a regression in the project-id branch silently lands sessions in the wrong Browserbase project, and one in the page-failure branch re-introduces exactly the leaked-browser/process-per-task bug this change fixes. There's currently no unit test covering initStagehand's launch, cleanup, or activePage failure paths. Consider adding a couple of focused tests (e.g. that projectId is forwarded to browserbase.launch when set, and that a failing activePage() still closes both stagehand and browser) to prevent regressions.</comment>
<file context>
@@ -109,10 +118,19 @@ export async function initStagehand({
+ // Page acquisition failures need the same cleanup as create failures: the
+ // client is up and the browser is running, and stagehand.close() alone
+ // tears down the RPC client without closing the browser.
+ let page: Page | null;
+ try {
+ page = await stagehand.context.activePage();
</file context>
There was a problem hiding this comment.
Not adding new initializer unit coverage in this PR. The cleanup and Browserbase configuration paths were exercised through the real eval lifecycle, including successful browser cleanup; broader test expansion is deferred while the eval stack is still changing.
## Summary Review follow-up for #2494 (implemented, not just described): this branch does **not** need v3 backcompat for a/e/o bench tasks — if we want a/e/o evals on v3, we switch branches. Only the bench **agent** tier stays on the v3 SDK, imported from the `stagehand-v3` package. That decision deletes the parallel-API layer #2494 introduced: - `defineBenchTask` / `BenchTaskContext` are now **v4-native** (`{ stagehand, page, logger, input, modelName, debugUrl, sessionUrl }`). `defineBenchV4Task`, `BenchV4TaskContext`, and the runtime fail-fast guard are deleted — misuse is a compile error, not a runtime probe. - The agent tier gets an explicit `defineAgentBenchTask` / `AgentBenchTaskContext` carrying the old v3 shape; `buildBenchContext` → `buildAgentBenchContext`. This is why the earlier `defineBenchV4Task → defineBenchTask` rename attempt had to be reverted: agent/combination/experimental occupied the name. - 77 a/e/o tasks: `defineBenchV4Task` → `defineBenchTask` (mechanical, import + call site only). - 47 agent tasks: `defineBenchTask` → `defineAgentBenchTask` (mechanical, bodies untouched). - `tasks/bench/combination` (10) and `tasks/bench/experimental` (13) are v3 a/e/o consumers and are **removed** rather than ported or parked on a fake context; they remain recoverable from history and can be ported as their own PR. Category lists (`args.ts`, `types/evals.ts`, `scripts/test-evals.ts`, docs) updated. `framework/benchTypes.ts`/`benchPlanner.ts`'s separate "combination" task-kind concept is intentionally untouched. - TUI `new` scaffolding emits the right definition per category; guard tests removed, agent- and v4-scaffold tests added. ## Stack position ``` v4-spike → #2494 (tasks port) → this PR → #2570 (harness, restacked on top) ``` - This PR bases on `evals-v4-spike` (#2494's head) because it edits the framework/task layer that only exists there. It cannot base on #2570's branch: `evals-v4-root` is currently rooted on an older `v4-spike` commit, not on #2494, so retargeting would fold #2494's entire diff into this one. - #2570 already implements the dispatch this refactor assumes — `stagehandHarness` selects the v4 client by task category (act/extract/observe) and keeps v3+agent for `bench/agent/*`; there is no `--sdk` switch. Its two commits cherry-pick cleanly onto this branch (typecheck/tests/build verified green). **Ask for #2570:** rebase `evals-v4-root` onto this branch once it merges. Optional cleanup there: with a/e/o guaranteed v4, `BenchHarnessContext`'s dual optional fields (`v3/agent/page` vs `stagehand/v4Page`) and the `v4Page ?? page` fallback can collapse into the two context shapes. - Until #2570 lands, bench a/e/o remains un-runnable (as with #2494 alone); with the guard removed the failure is a plain TypeError rather than a friendly message. Deliberate: the guard would probe a code path that no longer exists once the stack merges. - Any in-flight PR adding agent tasks with the old `defineBenchTask({ v3, agent })` shape becomes a typecheck failure; the fix is the one-line rename to `defineAgentBenchTask`. ## Verification - `pnpm typecheck` clean, `pnpm fmt:check` clean - `pnpm test`: 407/407 (baseline 403; new coverage for agent + v4 scaffolds) - No `defineBenchV4Task` / `BenchV4TaskContext` references remain - #2570's harness commits verified compatible by cherry-pick on top (local branch `evals-v4-stacked`) Net: −1,653 lines. Merge into `evals-v4-spike`. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Make bench `act`/`extract`/`observe` tasks v4-only while keeping agent tasks on `stagehand-v3`. This drops v3 backcompat for a/e/o, simplifies the API, and prevents running tasks against the wrong SDK. - **Refactors** - `defineBenchTask`/`BenchTaskContext` are now v4-native; removed `defineBenchV4Task` and the runtime guard. - Added `defineAgentBenchTask`/`AgentBenchTaskContext`; `buildBenchContext` renamed to `buildAgentBenchContext`. - Migrated tasks: a/e/o (77) to `defineBenchTask`; agent (47) to `defineAgentBenchTask`. - Removed `tasks/bench/combination/*` and `tasks/bench/experimental/*`; updated category lists, defaults, and docs; TUI scaffolding now pins the v4 `defineBenchTask` template for non-agent categories; tests updated. - Cleaned exports/types: `BenchV4TaskContext` deleted; framework barrel now re-exports only task-author-facing types (dropped builder option/result types); applied formatting fixes. - **Migration** - a/e/o tasks: use `defineBenchTask` with v4 context `{ stagehand, page, logger, input, modelName, debugUrl, sessionUrl }`. - Agent tasks: use `defineAgentBenchTask`; if constructing contexts, switch to `buildAgentBenchContext`. - Harness: drop `--sdk v4`; dispatch by category (v4 for act/extract/observe, v3 for `bench/agent/*`). <sup>Written for commit 724d412. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/browserbase/stagehand/pull/2587?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
dab19ae to
171765f
Compare
9b90e74 to
beb70dc
Compare
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Confidence score: 5/5
- In
packages/evals/framework/verifierAdapter.ts, the newEVAL_VERIFIER_MODELoverride logic is only validated for the happy path, so an untested error branch and unasserted default could allow misconfigured env values to slip through and cause confusing verifier behavior at runtime — add targeted tests for the missing error path and default-model fallback.
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/verifierAdapter.ts">
<violation number="1" location="packages/evals/framework/verifierAdapter.ts:38">
P3: The new EVAL_VERIFIER_MODEL override path has one untested edge case plus an unasserted default. The only new test covers the happy path (model + key present). The error branch introduced here — `EVAL_VERIFIER_MODEL` set but `loadApiKeyFromEnv` returning no key, causing the throw — is never exercised, and none of the existing tests assert the constructor options on the env-unset default branch (they pass through `createVerifierEvaluator` and would not catch a regression where the default path wrongly starts requiring an API key). Since this refactor routes all verifier construction through the new function, a quick test asserting the default `{ backend: "verifier" }` options and one asserting the missing-key error would lock in both behaviors and cover the error path end-to-end (e.g. via `verifierError` on the result).</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| const provider = modelName.includes("/") ? modelName.slice(0, modelName.indexOf("/")) : undefined; | ||
| const apiKey = loadApiKeyFromEnv(provider, () => {}); | ||
| if (!apiKey) { |
There was a problem hiding this comment.
P3: The new EVAL_VERIFIER_MODEL override path has one untested edge case plus an unasserted default. The only new test covers the happy path (model + key present). The error branch introduced here — EVAL_VERIFIER_MODEL set but loadApiKeyFromEnv returning no key, causing the throw — is never exercised, and none of the existing tests assert the constructor options on the env-unset default branch (they pass through createVerifierEvaluator and would not catch a regression where the default path wrongly starts requiring an API key). Since this refactor routes all verifier construction through the new function, a quick test asserting the default { backend: "verifier" } options and one asserting the missing-key error would lock in both behaviors and cover the error path end-to-end (e.g. via verifierError on the result).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/verifierAdapter.ts, line 38:
<comment>The new EVAL_VERIFIER_MODEL override path has one untested edge case plus an unasserted default. The only new test covers the happy path (model + key present). The error branch introduced here — `EVAL_VERIFIER_MODEL` set but `loadApiKeyFromEnv` returning no key, causing the throw — is never exercised, and none of the existing tests assert the constructor options on the env-unset default branch (they pass through `createVerifierEvaluator` and would not catch a regression where the default path wrongly starts requiring an API key). Since this refactor routes all verifier construction through the new function, a quick test asserting the default `{ backend: "verifier" }` options and one asserting the missing-key error would lock in both behaviors and cover the error path end-to-end (e.g. via `verifierError` on the result).</comment>
<file context>
@@ -18,6 +20,34 @@ import { RubricCache } from "./rubricCache.js";
+
+ const provider = modelName.includes("/") ? modelName.slice(0, modelName.indexOf("/")) : undefined;
+ const apiKey = loadApiKeyFromEnv(provider, () => {});
+ if (!apiKey) {
+ throw new Error(
+ `${VERIFIER_MODEL_ENV} is set to "${modelName}", but no API key was found for provider "${provider ?? "unknown"}".`,
</file context>
There was a problem hiding this comment.
Not expanding verifier unit coverage in this PR. The existing focused verifier/trajectory suite passes, and the configurable verifier path was also exercised in the real benchmark harness.
95d2930 to
53ab9c1
Compare
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 4/5
- In
packages/evals/framework/verifierAdapter.ts, the new keyless-provider path (bedrock/ollama skipping missing-key errors and omittingmodelClientOptions) is untested, so regressions could silently break provider initialization or error handling in keyless setups—add targeted unit tests for both key-present and key-absent flows ingradeExternalTrajectorycoverage.
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/verifierAdapter.ts">
<violation number="1" location="packages/evals/framework/verifierAdapter.ts:48">
P3: The new keyless-provider behavior (skip the missing-API-key error for bedrock/ollama and omit `modelClientOptions` when no key is present) has no unit test coverage. The existing `gradeExternalTrajectory` test only exercises the keyed groq path and even asserts `modelClientOptions: { apiKey }` is always set, so it would not catch a regression in the keyless branch. Consider adding focused tests for: (1) a bedrock/ollama verifier model with no API key resolving without throwing and without `modelClientOptions`, and (2) a non-keyless provider with no API key still throwing the configured error.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return new V3Evaluator(v3, { | ||
| backend: "verifier", | ||
| modelName: modelName as AvailableModel, | ||
| ...(apiKey ? { modelClientOptions: { apiKey } } : {}), |
There was a problem hiding this comment.
P3: The new keyless-provider behavior (skip the missing-API-key error for bedrock/ollama and omit modelClientOptions when no key is present) has no unit test coverage. The existing gradeExternalTrajectory test only exercises the keyed groq path and even asserts modelClientOptions: { apiKey } is always set, so it would not catch a regression in the keyless branch. Consider adding focused tests for: (1) a bedrock/ollama verifier model with no API key resolving without throwing and without modelClientOptions, and (2) a non-keyless provider with no API key still throwing the configured error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/evals/framework/verifierAdapter.ts, line 48:
<comment>The new keyless-provider behavior (skip the missing-API-key error for bedrock/ollama and omit `modelClientOptions` when no key is present) has no unit test coverage. The existing `gradeExternalTrajectory` test only exercises the keyed groq path and even asserts `modelClientOptions: { apiKey }` is always set, so it would not catch a regression in the keyless branch. Consider adding focused tests for: (1) a bedrock/ollama verifier model with no API key resolving without throwing and without `modelClientOptions`, and (2) a non-keyless provider with no API key still throwing the configured error.</comment>
<file context>
@@ -44,7 +45,7 @@ export function createVerifierEvaluator(v3: V3): V3Evaluator {
backend: "verifier",
modelName: modelName as AvailableModel,
- modelClientOptions: { apiKey },
+ ...(apiKey ? { modelClientOptions: { apiKey } } : {}),
});
}
</file context>
There was a problem hiding this comment.
Not adding new verifier unit cases in this PR while the eval stack is still changing. The keyless branch omits modelClientOptions by construction, the existing focused verifier/trajectory suite passes, and keyed verifier selection was exercised through the real benchmark harness.
3a924d9 to
ce7854f
Compare
|
@shriyatheunicorn shouldn't this pr target #2494 ? |
|
yes indeed; there was some code changing and rebasing w/o my knowing last night - im retargeting it rn |
## Summary Review follow-up for #2494 (implemented, not just described): this branch does **not** need v3 backcompat for a/e/o bench tasks — if we want a/e/o evals on v3, we switch branches. Only the bench **agent** tier stays on the v3 SDK, imported from the `stagehand-v3` package. That decision deletes the parallel-API layer #2494 introduced: - `defineBenchTask` / `BenchTaskContext` are now **v4-native** (`{ stagehand, page, logger, input, modelName, debugUrl, sessionUrl }`). `defineBenchV4Task`, `BenchV4TaskContext`, and the runtime fail-fast guard are deleted — misuse is a compile error, not a runtime probe. - The agent tier gets an explicit `defineAgentBenchTask` / `AgentBenchTaskContext` carrying the old v3 shape; `buildBenchContext` → `buildAgentBenchContext`. This is why the earlier `defineBenchV4Task → defineBenchTask` rename attempt had to be reverted: agent/combination/experimental occupied the name. - 77 a/e/o tasks: `defineBenchV4Task` → `defineBenchTask` (mechanical, import + call site only). - 47 agent tasks: `defineBenchTask` → `defineAgentBenchTask` (mechanical, bodies untouched). - `tasks/bench/combination` (10) and `tasks/bench/experimental` (13) are v3 a/e/o consumers and are **removed** rather than ported or parked on a fake context; they remain recoverable from history and can be ported as their own PR. Category lists (`args.ts`, `types/evals.ts`, `scripts/test-evals.ts`, docs) updated. `framework/benchTypes.ts`/`benchPlanner.ts`'s separate "combination" task-kind concept is intentionally untouched. - TUI `new` scaffolding emits the right definition per category; guard tests removed, agent- and v4-scaffold tests added. ## Stack position ``` v4-spike → #2494 (tasks port) → this PR → #2570 (harness, restacked on top) ``` - This PR bases on `evals-v4-spike` (#2494's head) because it edits the framework/task layer that only exists there. It cannot base on #2570's branch: `evals-v4-root` is currently rooted on an older `v4-spike` commit, not on #2494, so retargeting would fold #2494's entire diff into this one. - #2570 already implements the dispatch this refactor assumes — `stagehandHarness` selects the v4 client by task category (act/extract/observe) and keeps v3+agent for `bench/agent/*`; there is no `--sdk` switch. Its two commits cherry-pick cleanly onto this branch (typecheck/tests/build verified green). **Ask for #2570:** rebase `evals-v4-root` onto this branch once it merges. Optional cleanup there: with a/e/o guaranteed v4, `BenchHarnessContext`'s dual optional fields (`v3/agent/page` vs `stagehand/v4Page`) and the `v4Page ?? page` fallback can collapse into the two context shapes. - Until #2570 lands, bench a/e/o remains un-runnable (as with #2494 alone); with the guard removed the failure is a plain TypeError rather than a friendly message. Deliberate: the guard would probe a code path that no longer exists once the stack merges. - Any in-flight PR adding agent tasks with the old `defineBenchTask({ v3, agent })` shape becomes a typecheck failure; the fix is the one-line rename to `defineAgentBenchTask`. ## Verification - `pnpm typecheck` clean, `pnpm fmt:check` clean - `pnpm test`: 407/407 (baseline 403; new coverage for agent + v4 scaffolds) - No `defineBenchV4Task` / `BenchV4TaskContext` references remain - #2570's harness commits verified compatible by cherry-pick on top (local branch `evals-v4-stacked`) Net: −1,653 lines. Merge into `evals-v4-spike`. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Make bench `act`/`extract`/`observe` tasks v4-only while keeping agent tasks on `stagehand-v3`. This drops v3 backcompat for a/e/o, simplifies the API, and prevents running tasks against the wrong SDK. - **Refactors** - `defineBenchTask`/`BenchTaskContext` are now v4-native; removed `defineBenchV4Task` and the runtime guard. - Added `defineAgentBenchTask`/`AgentBenchTaskContext`; `buildBenchContext` renamed to `buildAgentBenchContext`. - Migrated tasks: a/e/o (77) to `defineBenchTask`; agent (47) to `defineAgentBenchTask`. - Removed `tasks/bench/combination/*` and `tasks/bench/experimental/*`; updated category lists, defaults, and docs; TUI scaffolding now pins the v4 `defineBenchTask` template for non-agent categories; tests updated. - Cleaned exports/types: `BenchV4TaskContext` deleted; framework barrel now re-exports only task-author-facing types (dropped builder option/result types); applied formatting fixes. - **Migration** - a/e/o tasks: use `defineBenchTask` with v4 context `{ stagehand, page, logger, input, modelName, debugUrl, sessionUrl }`. - Agent tasks: use `defineAgentBenchTask`; if constructing contexts, switch to `buildAgentBenchContext`. - Harness: drop `--sdk v4`; dispatch by category (v4 for act/extract/observe, v3 for `bench/agent/*`). <sup>Written for commit 724d412. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/browserbase/stagehand/pull/2587?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
|
retargeted |
ce7854f to
4c51890
Compare
## Summary Review follow-up for #2494 (implemented, not just described): this branch does **not** need v3 backcompat for a/e/o bench tasks — if we want a/e/o evals on v3, we switch branches. Only the bench **agent** tier stays on the v3 SDK, imported from the `stagehand-v3` package. That decision deletes the parallel-API layer #2494 introduced: - `defineBenchTask` / `BenchTaskContext` are now **v4-native** (`{ stagehand, page, logger, input, modelName, debugUrl, sessionUrl }`). `defineBenchV4Task`, `BenchV4TaskContext`, and the runtime fail-fast guard are deleted — misuse is a compile error, not a runtime probe. - The agent tier gets an explicit `defineAgentBenchTask` / `AgentBenchTaskContext` carrying the old v3 shape; `buildBenchContext` → `buildAgentBenchContext`. This is why the earlier `defineBenchV4Task → defineBenchTask` rename attempt had to be reverted: agent/combination/experimental occupied the name. - 77 a/e/o tasks: `defineBenchV4Task` → `defineBenchTask` (mechanical, import + call site only). - 47 agent tasks: `defineBenchTask` → `defineAgentBenchTask` (mechanical, bodies untouched). - `tasks/bench/combination` (10) and `tasks/bench/experimental` (13) are v3 a/e/o consumers and are **removed** rather than ported or parked on a fake context; they remain recoverable from history and can be ported as their own PR. Category lists (`args.ts`, `types/evals.ts`, `scripts/test-evals.ts`, docs) updated. `framework/benchTypes.ts`/`benchPlanner.ts`'s separate "combination" task-kind concept is intentionally untouched. - TUI `new` scaffolding emits the right definition per category; guard tests removed, agent- and v4-scaffold tests added. ## Stack position ``` v4-spike → #2494 (tasks port) → this PR → #2570 (harness, restacked on top) ``` - This PR bases on `evals-v4-spike` (#2494's head) because it edits the framework/task layer that only exists there. It cannot base on #2570's branch: `evals-v4-root` is currently rooted on an older `v4-spike` commit, not on #2494, so retargeting would fold #2494's entire diff into this one. - #2570 already implements the dispatch this refactor assumes — `stagehandHarness` selects the v4 client by task category (act/extract/observe) and keeps v3+agent for `bench/agent/*`; there is no `--sdk` switch. Its two commits cherry-pick cleanly onto this branch (typecheck/tests/build verified green). **Ask for #2570:** rebase `evals-v4-root` onto this branch once it merges. Optional cleanup there: with a/e/o guaranteed v4, `BenchHarnessContext`'s dual optional fields (`v3/agent/page` vs `stagehand/v4Page`) and the `v4Page ?? page` fallback can collapse into the two context shapes. - Until #2570 lands, bench a/e/o remains un-runnable (as with #2494 alone); with the guard removed the failure is a plain TypeError rather than a friendly message. Deliberate: the guard would probe a code path that no longer exists once the stack merges. - Any in-flight PR adding agent tasks with the old `defineBenchTask({ v3, agent })` shape becomes a typecheck failure; the fix is the one-line rename to `defineAgentBenchTask`. ## Verification - `pnpm typecheck` clean, `pnpm fmt:check` clean - `pnpm test`: 407/407 (baseline 403; new coverage for agent + v4 scaffolds) - No `defineBenchV4Task` / `BenchV4TaskContext` references remain - #2570's harness commits verified compatible by cherry-pick on top (local branch `evals-v4-stacked`) Net: −1,653 lines. Merge into `evals-v4-spike`. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Make bench `act`/`extract`/`observe` tasks v4-only while keeping agent tasks on `stagehand-v3`. This drops v3 backcompat for a/e/o, simplifies the API, and prevents running tasks against the wrong SDK. - **Refactors** - `defineBenchTask`/`BenchTaskContext` are now v4-native; removed `defineBenchV4Task` and the runtime guard. - Added `defineAgentBenchTask`/`AgentBenchTaskContext`; `buildBenchContext` renamed to `buildAgentBenchContext`. - Migrated tasks: a/e/o (77) to `defineBenchTask`; agent (47) to `defineAgentBenchTask`. - Removed `tasks/bench/combination/*` and `tasks/bench/experimental/*`; updated category lists, defaults, and docs; TUI scaffolding now pins the v4 `defineBenchTask` template for non-agent categories; tests updated. - Cleaned exports/types: `BenchV4TaskContext` deleted; framework barrel now re-exports only task-author-facing types (dropped builder option/result types); applied formatting fixes. - **Migration** - a/e/o tasks: use `defineBenchTask` with v4 context `{ stagehand, page, logger, input, modelName, debugUrl, sessionUrl }`. - Agent tasks: use `defineAgentBenchTask`; if constructing contexts, switch to `buildAgentBenchContext`. - Harness: drop `--sdk v4`; dispatch by category (v4 for act/extract/observe, v3 for `bench/agent/*`). <sup>Written for commit 724d412. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/browserbase/stagehand/pull/2587?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
d0696d1 to
af52b55
Compare
4c51890 to
aa4bc70
Compare
The deterministic suite is ported to the v4 SDK in place (#2494), but the bench harness only ever built a v3 instance, so every ported task failed its defineBenchV4Task guard before its own code ran. - initStagehand: v4 client factory beside initV3, mirroring its environment/key resolution. selfHeal is on (the server defaults it off, which would let the heal_* benchmarks pass while measuring nothing) and debug SDK log lines are dropped rather than bridged (bridging produces ~18MB Braintrust payloads the API rejects). If init fails after launch, the browser is closed rather than leaked. - benchHarness: act/extract/observe dispatch to the v4 client by task category — the directory a task lives in — so tasks carry no marker and no flag exists. Agent tasks and --api fail loudly on the v4 path. Cleanup closes the browser as well as the client: stagehand.close() tears down the RPC client but leaves the browser running, which leaked one Chrome process (LOCAL) or live Browserbase session per task and kept the CLI from exiting. - benchRunner: forwards { stagehand, page } from the harness context into the task context. - benchRunner.test: the legacy-task fixture moves from category act to combination — act now correctly routes to the v4 client, and legacy v3 tasks only exist outside the deterministic categories. Merge after #2494: category dispatch assumes the a/e/o tasks are the v4 ports; with the v3 tasks still in place it hands them a v4 context. Verified on Browserbase (3-model matrix, -c 10): act 35/40, extract 24/25, observe 11/12 union — at or above the v3 baseline per category.
…acquisition failure Review fixes for the v4 init path: - browserbase.launch now receives BROWSERBASE_PROJECT_ID/BB_PROJECT_ID, matching initV3 and core/targets/browserbase. The Browserbase SDK does not read the project id from the environment, so omitting it lands sessions in the key's default project — the wrong one for keys that own several. - A failure while obtaining the active page now closes both the client and the browser. It ran after the create-failure cleanup and before the harness owns cleanup, so a throw leaked the browser outright and the null-page path closed only the RPC client, which leaves the browser running on this SDK generation.
af52b55 to
12fce44
Compare
aa4bc70 to
5023ce4
Compare
…ext (#2617) ## Summary Review follow-up for #2570, implemented. Three fixes and one test cleanup; only functional change is the session-URL restoration. - **Session/debug URLs on the v4 path** (the one functional gap): `initStagehand` now creates the Browserbase session first via the shared `launchRunnerProvidedBrowserbaseChrome()` creator and attaches with `browserbase.connect({ apiKey, sessionId })`, instead of `browserbase.launch` (which hides the session id behind the opaque `StagehandBrowser` handle, #2517). `sessionUrl`/`debugUrl` flow into every TaskResult and the Braintrust replay click-through again, and the session is explicitly released (`REQUEST_RELEASE`) on cleanup and on every init failure path — `browser.close()` on a connected handle only disconnects. - **`BenchHarnessContext` → discriminated union** (`sdk: "v4" | "v3"`): the runner narrows on the discriminant instead of probing five optional fields; the `v4Page ?? page` fallback is gone, and a legacy task reaching a v4 context is an explicit `EvalsError` instead of `v3: undefined`. - **Dead `systemPrompt` parameter dropped** from `initStagehand` — nothing passes it, and the v3 equivalent is agent-only. - **Test fixtures off the deleted `combination` category** → `agent`, the only non-deterministic category left after #2587. Not touched, for reviewer attention on #2570 itself: the `EVAL_VERIFIER_MODEL` / keyless-provider and Claude Code result-parsing commits are orthogonal to running a/e/o on v4 (they only affect the agent/external-harness grading paths and change nothing unless the env var is set) — candidates for splitting into their own PR. ## Verification - typecheck, fmt, and 410/410 unit tests green - LOCAL end-to-end through the built CLI: `run dropdown -e local -t 1 -m google/gemini-2.5-flash` → **1/1 passed** (first full v4 pass through the harness; requires `packages/extension` built for the local launch's extension preload) - BROWSERBASE connect path not exercised here (no key in this environment) — needs one `run dropdown -e browserbase -t 1` to confirm sessionUrl lands in results <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Restores session and debug URLs for v4 eval runs by creating Browserbase sessions first and connecting, and simplifies the harness context with an explicit v3/v4 split for clearer task handling. - **Bug Fixes** - Restored `sessionUrl` and `debugUrl` for v4 tasks. `initStagehand` now creates the session via `launchRunnerProvidedBrowserbaseChrome` and attaches with `browserbase.connect({ apiKey, sessionId })`. - Releases the session on cleanup and on init failure paths. - **Refactors** - `BenchHarnessContext` is now a discriminated union (`sdk: "v4" | "v3"`). Removes `v4Page ?? page` fallback and throws on legacy tasks reaching the v4 context. - Dropped the unused `systemPrompt` from `initStagehand` in `@browserbasehq/stagehand` v4 flow. - Moved legacy test fixtures from `combination` to `agent`. <sup>Written for commit e31cb72. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/browserbase/stagehand/pull/2617?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
Reverts the verifier-model selection that rode in with #2570. It is orthogonal to running a/e/o on v4 (the verifier only runs on the agent/external grading paths), and the capability is redundant where it sits: V3Evaluator already accepts modelName/modelClientOptions in its constructor, and the verifier's env knobs live in stagehand-v3 (STAGEHAND_EVALUATOR_BACKEND, VERIFIER_RUBRIC_MODEL). If a verifier model override is wanted, it belongs there — alongside a fix for the constructor defaulting a Gemini API key for non-Gemini models, which is the quirk the dropped code was compensating for downstream.
The agent-facing half of the stagehand_code code-mode surface: prepareLLMExposure initializes a v4 Stagehand client via initStagehand and declares a code_handles agent mount — stagehand/page/z in the snippet scope, surface-owned prompt instructions and run-tool copy, final-state evidence capture (screenshot + URL + aria tree), and cleanup that closes the browser as well as the client (stagehand.close() alone leaves the browser running). Nothing consumes the mount here; the claude_code adapter does in #2596 and codex in #2609. Bottom of the nondeterministic-evals stack (#2591 → #2611, with the merge-readiness fixes in #2649 and the MCP tool surfaces in #2650 stacked on top).⚠️ **Base housekeeping before merge**: this PR still targets `evals-v4-root`, which has since merged (via #2570 → #2494) into `v4-spike`. Rebase onto current `v4-spike` and retarget. One known reconciliation: v4-spike removed the `EVAL_VERIFIER_MODEL` override from `verifierAdapter` (2af557b) as redundant with V3Evaluator's own constructor options; this stack's later PRs still carry it — drop it during the rebase rather than reintroducing it. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Adds the `stagehand_code` code-mode tool and registers it in the core tool registry, wiring a v4 `@browserbasehq/stagehand` client with scoped handles, run-tool execution, artifact capture, and full teardown. Enables the v4 code-mode surface for STG-2671 with a fixed SDK model and local/Browserbase startup profiles. - New Features - New `StagehandCodeTool` (`id: stagehand_code`) with session, navigation, evaluation, screenshot, viewport, wait, click, hover, scroll, type, press, tabs, and representation. - Targets: `selector`, `coords`, `focused`. - Agent mount via handles: `stagehand`, `page`, and `z`; scoped prompt instructions; run-tool description, code param description, and deny message. Snippets also get `startUrl`, `task`, and `console` in scope. - Captures screenshot, URL, and ARIA tree; cleanup closes both the Stagehand client and the browser. - Profiles: `tool_launch_local` and `tool_create_browserbase`; connection mode derived; model fixed to `openai/gpt-4.1-mini`. - Registered in `listCoreTools`/`getCoreTool`; tests assert `stagehand_code` is retrievable and that prompt guidance includes awaited locator actions. <sup>Written for commit e579140. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/browserbase/stagehand/pull/2591?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
The deterministic suite is ported to the v4 SDK in place (#2494), but the
bench harness only ever built a v3 instance, so every ported task failed
its defineBenchV4Task guard before its own code ran.
environment/key resolution. selfHeal is on (the server defaults it off,
which would let the heal_* benchmarks pass while measuring nothing) and
debug SDK log lines are dropped rather than bridged (bridging produces
~18MB Braintrust payloads the API rejects). If init fails after launch,
the browser is closed rather than leaked.
category — the directory a task lives in — so tasks carry no marker and
no flag exists. Agent tasks and --api fail loudly on the v4 path.
Cleanup closes the browser as well as the client: stagehand.close()
tears down the RPC client but leaves the browser running, which leaked
one Chrome process (LOCAL) or live Browserbase session per task and
kept the CLI from exiting.
the task context.
combination — act now correctly routes to the v4 client, and legacy v3
tasks only exist outside the deterministic categories.
Merge after #2494: category dispatch assumes the a/e/o tasks are the v4
ports; with the v3 tasks still in place it hands them a v4 context.
Verified on Browserbase (3-model matrix, -c 10): act 35/40, extract
24/25, observe 11/12 union — at or above the v3 baseline per category.
Summary by cubic
Runs act/extract/observe benchmark tasks on the v4
@browserbasehq/stagehandclient. Restores Browserbase session/debug URLs, makes the verifier model selectable, and hardens init/cleanup to prevent leaked browsers and lost errors.New Features
initStagehandto launch local or Browserbase with self‑heal on, resolve provider API keys, passBROWSERBASE_PROJECT_ID/BB_PROJECT_ID, forward SDK logs (drop debug), and create the Browserbase session first, attach viabrowserbase.connect, and providesessionUrl/debugUrlwith explicit session release on cleanup and failures.{ stagehand, page }. Blocks agent tasks/modes and--apion the v4 path. Context is now discriminated (sdk: "v4" | "v3"); legacy tasks on a v4 context throw.EVAL_VERIFIER_MODEL; resolves provider key and applies it to the verifier backend, supporting keyless providers likebedrockandollama.@browserbasehq/stagehand-integrationswith a persistent code‑mode executor, MCP stdio server, env config, and bundledSKILL.md/REFERENCE.md.Bug Fixes
activePage.error.message); parse decorated Claude Code results by extracting the first JSON object afterEVAL_RESULT.Written for commit 555b783. Summary will update on new commits.