Repository navigation
feat(extension): experimental decision tree for act() - #2953
miguelg719 wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: d06be16 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
6 issues found across 16 files
Confidence score: 2/5
README.mddoes not redact resolved values shorter than three characters, so the guarantee that values stay out of Jev requests and traces is not met. Redact every resolved value before documenting that guarantee.pipeline.tssends some error messages downstream without the pipeline’s variable redaction, which can expose resolved values through error output. Sanitizeerror.messageandresult.messagebefore logging or emitting them.stagehandController.tsrejects a missing model or gateway before Jev’s documented LLM-free mode can run. Allow that configuration when bothargumentLlmandllmFallbackare false.stagehand.v4.jsonaccepts non-HTTPS Jev endpoints that the extension’s Zod schema rejects during initialization. Keep the HTTPS constraint in the protocol schema.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/extension/services/jevAct/README.md">
<violation number="1" location="packages/extension/services/jevAct/README.md:54">
P1: Values shorter than three characters are not replaced, so this does not guarantee that every resolved variable stays out of Jev requests and traces. Redact all resolved variable values before documenting this guarantee.</violation>
</file>
<file name="packages/extension/controllers/stagehandController.ts">
<violation number="1" location="packages/extension/controllers/stagehandController.ts:89">
P2: When Jev is configured for its documented LLM-free mode (`argumentLlm: false` and `llmFallback: false`), this wiring still cannot run because the controller rejects missing `model` and `gateway` first. Allow that explicit Jev-only configuration through the LLM precondition while retaining the guard for configurations that may fall back to LLM inference.</violation>
</file>
<file name="packages/extension/services/jevAct/pipeline.ts">
<violation number="1" location="packages/extension/services/jevAct/pipeline.ts:224">
P1: Custom agent: **Exception and error message sanitization**
These added error paths emit downstream messages without the pipeline's variable redaction. Sanitize `error.message` and `result.message` before logging or embedding them in fallback reasons, and preserve typed error handling so selectors, URLs, and action arguments cannot leak secrets.</violation>
</file>
<file name="packages/extension/services/actService.ts">
<violation number="1" location="packages/extension/services/actService.ts:193">
P2: Custom agent: **Ensure all public methods added to the stagehand class, agent, or understudy (page, locator, etc.) interfaces are properly instrumented with the flowLogger**
The new argument-only LLM request is not flowLogger-tracked. `actTextArgument` passes a raw generator to `llmService.generate`, which reaches the direct AI SDK call without request or response logging. Route this request through the tracked LLM path or add sanitized manual LLM request and response logging.</violation>
</file>
<file name="packages/protocol/stagehand.v4.json">
<violation number="1" location="packages/protocol/stagehand.v4.json:1219">
P2: Clients validating against this protocol schema can send a non-HTTPS Jev endpoint, then fail initialization when the extension applies the stricter Zod schema. Preserve the `https://` constraint in the generated schema (and its source/generator) so clients reject invalid endpoints before sending them.</violation>
</file>
<file name="packages/extension/tests/jevActPipeline.test.ts">
<violation number="1" location="packages/extension/tests/jevActPipeline.test.ts:403">
P3: The `stuck` sub-case runs `chooseFromAppeared` to its polling loop's exhaustion, and each iteration awaits a real `setTimeout(450)` (APPEAR_POLL_MS in services/jevAct/pipeline.ts). Because `captureSnapshot` is the default static mock that never shows the options, the loop performs 3 real 450ms sleeps per suite run (~1.35s of wall time) to reach the `option_none_appeared` fallback. Use fake timers around this sub-case (the rest of the flow is promise-driven and microtasks advance under `vi.useFakeTimers()`), or allow the poll interval to be injected so tests can shrink it.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| category: "jev", | ||
| instruction: deps.instruction, | ||
| outcome: "error", | ||
| error: error instanceof Error ? error.message : String(error), |
There was a problem hiding this comment.
P1: Custom agent: Exception and error message sanitization
These added error paths emit downstream messages without the pipeline's variable redaction. Sanitize error.message and result.message before logging or embedding them in fallback reasons, and preserve typed error handling so selectors, URLs, and action arguments cannot leak secrets.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/extension/services/jevAct/pipeline.ts, line 224:
<comment>These added error paths emit downstream messages without the pipeline's variable redaction. Sanitize `error.message` and `result.message` before logging or embedding them in fallback reasons, and preserve typed error handling so selectors, URLs, and action arguments cannot leak secrets.</comment>
<file context>
@@ -0,0 +1,1333 @@
+ category: "jev",
+ instruction: deps.instruction,
+ outcome: "error",
+ error: error instanceof Error ? error.message : String(error),
+ trace: JSON.stringify(trace),
+ });
</file context>
There was a problem hiding this comment.
Fixed on the branch: error text embedded in fallback reasons (jev_error:…, action_failed:…) is passed through the same %variable% redactor as requests and capped at 200 chars.
| Sent to TypeSafe: the instruction, candidate descriptions built from the accessibility outline | ||
| (names, nearby text, card/row text, DOM attributes of nameless controls), the page URL without | ||
| query string or fragment, and a digest of the first visible content (page state). | ||
| Resolved `%variable%` values are replaced by their placeholder in every request and in the trace |
There was a problem hiding this comment.
P1: Values shorter than three characters are not replaced, so this does not guarantee that every resolved variable stays out of Jev requests and traces. Redact all resolved variable values before documenting this guarantee.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/extension/services/jevAct/README.md, line 54:
<comment>Values shorter than three characters are not replaced, so this does not guarantee that every resolved variable stays out of Jev requests and traces. Redact all resolved variable values before documenting this guarantee.</comment>
<file context>
@@ -0,0 +1,64 @@
+Sent to TypeSafe: the instruction, candidate descriptions built from the accessibility outline
+(names, nearby text, card/row text, DOM attributes of nameless controls), the page URL without
+query string or fragment, and a digest of the first visible content (page state).
+Resolved `%variable%` values are replaced by their placeholder in every request and in the trace
+log, including when an earlier act already typed them into the page. With the flag on, each act
+logs its instruction and a trace of candidate descriptions at info level.
</file context>
There was a problem hiding this comment.
Fixed on the branch: README now says values of three or more characters; the floor is deliberate (see the thread in #2952).
|
|
||
| // Nothing opens after clicking something that should have opened a list: | ||
| // the option was not chosen, so the LLM path continues from the click. | ||
| stubJev({ family: choice("select"), strict: choice("0-3"), best: noul(0.95) }); |
There was a problem hiding this comment.
P3: The stuck sub-case runs chooseFromAppeared to its polling loop's exhaustion, and each iteration awaits a real setTimeout(450) (APPEAR_POLL_MS in services/jevAct/pipeline.ts). Because captureSnapshot is the default static mock that never shows the options, the loop performs 3 real 450ms sleeps per suite run (~1.35s of wall time) to reach the option_none_appeared fallback. Use fake timers around this sub-case (the rest of the flow is promise-driven and microtasks advance under vi.useFakeTimers()), or allow the poll interval to be injected so tests can shrink it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/extension/tests/jevActPipeline.test.ts, line 403:
<comment>The `stuck` sub-case runs `chooseFromAppeared` to its polling loop's exhaustion, and each iteration awaits a real `setTimeout(450)` (APPEAR_POLL_MS in services/jevAct/pipeline.ts). Because `captureSnapshot` is the default static mock that never shows the options, the loop performs 3 real 450ms sleeps per suite run (~1.35s of wall time) to reach the `option_none_appeared` fallback. Use fake timers around this sub-case (the rest of the flow is promise-driven and microtasks advance under `vi.useFakeTimers()`), or allow the poll interval to be injected so tests can shrink it.</comment>
<file context>
@@ -0,0 +1,904 @@
+
+ // Nothing opens after clicking something that should have opened a list:
+ // the option was not chosen, so the LLM path continues from the click.
+ stubJev({ family: choice("select"), strict: choice("0-3"), best: noul(0.95) });
+ const stuck = deps(
+ custom,
</file context>
There was a problem hiding this comment.
Not changing this — the three real 450 ms sleeps (~1.35 s) are the cost of testing the polling loop's exhaustion with real timers; fake timers interact badly with the performance.now()-based waits in the same code path. Accepting the wall time for one test.
d4d62e2 to
984bdc7
Compare
984bdc7 to
66e9697
Compare
66e9697 to
b87dbc9
Compare
b87dbc9 to
a10952b
Compare
Stack
Stack: #2951 snapshot editable ids → #2952 library → #2953 act pipeline → #2954 observe + cache check → #2955 extract. This is 3/5 (#2953). Each PR targets its predecessor.
Why
act()today is one LLM call over the whole a11y tree (~2 s p50 on gemini-3.8-flash). Most acts are "click X" / "type 'y' into Z": a classification over a closed set, which is what the decision model does in ~150 ms. This PR resolvesact()through a decision tree of decision-model questions and hands the act to the unchanged LLM pipeline whenever the decision model is not confident.Experimental and off by default. Nothing changes unless
experimentalDecisionsis present in the init params.What
decisions/pipeline.ts— modifier-chord check → intent (one request, no snapshot: action family, key, scroll scope, mouse button, toggle end state, which quoted string is the value) → arguments (parsed in code; unquoted text via a tiny argument-only LLM call whose answer must be a span of the instruction) → candidates + pick (library from feat(extension): decision-model client (TypeSafe Jev) and candidate-picking library #2952) → act via the existingperformUnderstudyMethod→ deterministic checks (fill read-back, native<select>selected flag) → page-state on "not on this page" → LLM fallback, seeded with the decision model's shortlist. Flow table and config reference indecisions/README.md.actService.ts— runs the pipeline before LLM inference when configured; the result is the sameActionshape, so caching, replay and self-heal are untouched. Logs oneAct pipeline finishedline (pathllm | decisions | decisions+arg-llm | decisions+llm, duration, LLM tokens) only when the flag is set.experimentalDecisionsstrict object onStagehandInitParams(act fields only in this PR); regeneratedstagehand.v4.json, Python and Go models.STAGEHAND_EXPERIMENTAL_DECISIONS(JSON). Evals build that fromEVAL_DECISIONS=1+EVAL_DECISIONS_*switches ininitStagehand.ts.What leaves the process
Instruction, candidate descriptions from the a11y outline, the page URL without query/fragment, and a digest of visible content (page state only). Resolved
%variable%values are replaced by their placeholder in every request and in trace logs.apiUrlmust be https.Results (gemini-3.8-flash fallback, Browserbase, local runs)
Breadth at 3 trials on two models: 204/240 LLM-only → 229/240 with the decision model. With the LLM disabled entirely: act 27/40, breadth 26/40.
Caveats: thresholds were tuned on these suites (in-sample); decision-model token usage is logged but not yet in
result.metadata.usage; page-state fail-fast andretryNoEffecthave unit tests only. The breadth tasks and the run/report scripts are intentionally not part of the stack.Testing
decisionsPipeline.test.ts(stubbed the decision model: every family, fallbacks, held picks, twins, redaction, arg grounding, no-LLM mode), full extension/protocol/sdk-ts suites, root parity tests, typecheck, lint, fmt,extensionpack --check. One liveact/dropdownrun through the env-driven flag passed on Browserbase (0 LLM tokens, 666 ms).