Repository navigation
feat(extension): decision-model client (TypeSafe Jev) and candidate-picking library - #2952
miguelg719 wants to merge 3 commits into
Conversation
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 7 files
Confidence score: 2/5
packages/extension/services/jevAct/tree.tscan omit contenteditable targets when structural pruning removes theirOutlineNode, and can also misparse names ending in[selected]or[checked]; preserve the editable marker and use unambiguous state encoding.packages/extension/services/jevAct/args.tsleaves one- and two-character resolved values unredacted in process snapshots and traces, creating a concrete sensitive-data exposure; redact every non-empty resolved value.packages/extension/services/jevAct/typesafeClient.tsmay leave public methods on the stagehand, agent, or understudy interfaces withoutflowLoggerinstrumentation, weakening required operation tracing; instrument every added public method.packages/extension/services/jevAct/pick.tslacks focused coverage for safety-critical strict-none fallbacks, shard finalist aggregation, and protocol-key-preserving redaction, making regressions in those branches harder to detect; add targeted tests.
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/extension/services/jevAct/tree.ts">
<violation number="1" location="packages/extension/services/jevAct/tree.ts:82">
P1: When a contenteditable AX node is pruned as structural, `markEditable` drops its side-channel marker because no `OutlineNode` has that ID, so the input view omits the editable target. Preserve the editable node or propagate the marker to the retained representation before building the input view.</violation>
<violation number="2" location="packages/extension/services/jevAct/tree.ts:110">
P2: When an accessible name literally ends in `[selected]` or `[checked]`, `parseOutline` interprets part of the name as state and loses the suffix. Use an escaped or otherwise unambiguous state encoding in the formatter and parser so literal name text survives.</violation>
</file>
<file name="packages/extension/services/jevAct/typesafeClient.ts">
<violation number="1" location="packages/extension/services/jevAct/typesafeClient.ts:96">
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**
`systemOne()` sends a direct TypeSafe LLM request through `fetch()` without flowLogger or manual LLM instrumentation, so this exported request path is untracked. Route it through a flow-logger-tracked method or add the required request/response logging before exposing the client.</violation>
</file>
<file name="packages/extension/services/jevAct/args.ts">
<violation number="1" location="packages/extension/services/jevAct/args.ts:141">
P1: When a variable value is one or two characters, `redactor` omits it and that value can leave the process in snapshots and traces. Redact every non-empty resolved value instead of applying a three-character cutoff.</violation>
</file>
<file name="packages/extension/services/jevAct/pick.ts">
<violation number="1" location="packages/extension/services/jevAct/pick.ts:129">
P2: The new picker has no focused tests for its safety-critical tiering, sharding, and acceptance branches. Add tests for held strict-none fallbacks, shard finalist aggregation, and redaction preserving protocol keys before these paths are wired into actions.
(Based on your team's feedback about adding unit tests for new behavior.)</violation>
</file>
Architecture diagram
sequenceDiagram
participant Ext as Extension Service Worker
participant Outline as Snapshot Outline
participant Tree as tree.ts
participant Args as args.ts
participant Pick as pick.ts
participant Client as typesafeClient.ts
participant API as TypeSafe API
Note over Ext,API: Jev Act Pipeline - Candidate Selection Flow
Ext->>Tree: Parse snapshot outline
Tree->>Tree: parseOutline() - build node tree with depth/parents/flags
alt Editable regions available
Ext->>Tree: markEditable(nodes, editableIds)
Tree->>Tree: Flag contenteditable paragraphs
end
Ext->>Args: Parse instruction for fill values
Args->>Args: Extract quoted strings, %variables%, percentages, keys
Ext->>Pick: pickTarget(ctx, node, snapshot, kinds, instructions)
Pick->>Tree: buildView(nodes, kind) for each view tier
Note over Pick: Tiered views: pointer → input → select → scroll → option → broad
alt Single quoted target with exact name match
Pick->>Pick: exactNameMatches(candidates, quotedTarget)
Pick->>Pick: pickCandidate() confirm single candidate
end
opt Candidate list > 40
Pick->>Tree: scoreCandidates() lexical pre-ranking
Tree-->>Pick: Pruned top 30 sharing instruction words
end
Pick->>Pick: shardByBudget() - shard large lists by char budget
alt Multiple shards needed
loop Each shard (max 8)
Pick->>Client: systemOne() parallel shard nomination
Client->>API: POST /v1/systemone
API-->>Client: Shard finalists
Client-->>Pick: Nominated candidates
end
end
Pick->>Client: systemOne() with best + strict questions
Client->>Client: Check circuit breaker status
alt Circuit breaker open
Client-->>Pick: Throw JevRequestError
else Circuit breaker closed
Client->>API: POST /v1/systemone (Bearer token)
alt Success
API-->>Client: Choice + noul answers
Client->>Client: Reset failure counter
Client-->>Pick: JevResponse
else Retryable (429/529)
API-->>Client: Retry with backoff (max 3)
else Auth failure (401/403)
API-->>Client: Open breaker 60s
else 5xx error
API-->>Client: Track failure, open at 3
end
end
Pick->>Pick: Evaluate best vs strict answers
alt Strict none > 0.9
Pick->>Pick: Veto pick - nothing matches
else Best accepted and strict none ≤ 0.5
Pick-->>Ext: Return picked target
else Best accepted but none ≤ 0.7 (uneasy)
Pick->>Pick: Hold pick, try next tier
end
opt Target not found
Pick->>Tree: pageDigest() for page state
Pick->>Client: systemOne() page_state noul questions
Client->>API: POST /v1/systemone
API-->>Client: Page state probabilities
Client-->>Pick: Page signals
Pick->>Pick: blockingSignal() check
alt Blocking signal ≥ 0.9
Pick-->>Ext: Return access_denied/captcha blocker
end
end
Note over Args: Grounding and redaction
Args->>Args: groundedSpan() - locate model text in instruction
Args->>Args: redactor() - replace %variable% values with placeholders
Pick->>Args: redact() applied to all outgoing requests and traces
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const editable = new Set(editableIds ?? []); | ||
| if (editable.size === 0) return; | ||
| for (const node of nodes) { | ||
| if (!editable.has(node.id)) continue; |
There was a problem hiding this comment.
P1: When a contenteditable AX node is pruned as structural, markEditable drops its side-channel marker because no OutlineNode has that ID, so the input view omits the editable target. Preserve the editable node or propagate the marker to the retained representation before building the input view.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/extension/services/jevAct/tree.ts, line 82:
<comment>When a contenteditable AX node is pruned as structural, `markEditable` drops its side-channel marker because no `OutlineNode` has that ID, so the input view omits the editable target. Preserve the editable node or propagate the marker to the retained representation before building the input view.</comment>
<file context>
@@ -0,0 +1,741 @@
+ const editable = new Set(editableIds ?? []);
+ if (editable.size === 0) return;
+ for (const node of nodes) {
+ if (!editable.has(node.id)) continue;
+ const parent = node.parent === undefined ? undefined : nodes[node.parent];
+ if (!parent || !editable.has(parent.id)) node.editable = true;
</file context>
There was a problem hiding this comment.
Not changing this — a node the outline pruned has no entry, no selector and no xpath, so there is nothing the input view could offer or act on; propagating a marker to a different retained node would point the act at the wrong element. The editable case this feature exists for (contenteditable paragraph inside an iframe) is covered by a live eval and a pipeline test in #2953.
| export function redactor(variables: Variables | undefined): ((text: string) => string) | undefined { | ||
| const secrets = Object.entries(variables ?? {}) | ||
| .map(([name, value]) => [name, resolveVariableValue(value)] as const) | ||
| .filter(([, value]) => value.length >= 3) |
There was a problem hiding this comment.
P1: When a variable value is one or two characters, redactor omits it and that value can leave the process in snapshots and traces. Redact every non-empty resolved value instead of applying a three-character cutoff.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/extension/services/jevAct/args.ts, line 141:
<comment>When a variable value is one or two characters, `redactor` omits it and that value can leave the process in snapshots and traces. Redact every non-empty resolved value instead of applying a three-character cutoff.</comment>
<file context>
@@ -0,0 +1,153 @@
+export function redactor(variables: Variables | undefined): ((text: string) => string) | undefined {
+ const secrets = Object.entries(variables ?? {})
+ .map(([name, value]) => [name, resolveVariableValue(value)] as const)
+ .filter(([, value]) => value.length >= 3)
+ .sort((a, b) => b[1].length - a[1].length);
+ if (secrets.length === 0) return undefined;
</file context>
| .filter(([, value]) => value.length >= 3) | |
| .filter(([, value]) => value.length > 0) |
There was a problem hiding this comment.
Not changing this — the three-character floor is deliberate: replacing every a, no or 12 in a page's text would shred the candidate descriptions Jev reads, and such values are not secrets. Made explicit in a code comment here and in the README wording in #2953 ("values of three or more characters").
| const depth = Math.floor(indent.length / 2); | ||
|
|
||
| let body = rest; | ||
| const flagMatch = FLAGS.exec(body); |
There was a problem hiding this comment.
P2: When an accessible name literally ends in [selected] or [checked], parseOutline interprets part of the name as state and loses the suffix. Use an escaped or otherwise unambiguous state encoding in the formatter and parser so literal name text survives.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/extension/services/jevAct/tree.ts, line 110:
<comment>When an accessible name literally ends in `[selected]` or `[checked]`, `parseOutline` interprets part of the name as state and loses the suffix. Use an escaped or otherwise unambiguous state encoding in the formatter and parser so literal name text survives.</comment>
<file context>
@@ -0,0 +1,741 @@
+ const depth = Math.floor(indent.length / 2);
+
+ let body = rest;
+ const flagMatch = FLAGS.exec(body);
+ const flags = flagMatch
+ ? [...flagMatch[1]!.matchAll(/\[(\w+)\]/g)].map((flag) => flag[1]!)
</file context>
There was a problem hiding this comment.
Not changing this — an accessible name that literally ends in [selected]/ [checked] is an edge the formatter/parser pair does not distinguish today; changing the outline encoding is a cross-cutting change to the shared snapshot format and out of scope for this library PR. Noted as a known limit.
| @@ -0,0 +1,414 @@ | |||
| import type { StagehandLogger } from "../../logger.js"; | |||
There was a problem hiding this comment.
P2: The new picker has no focused tests for its safety-critical tiering, sharding, and acceptance branches. Add tests for held strict-none fallbacks, shard finalist aggregation, and redaction preserving protocol keys before these paths are wired into actions.
(Based on your team's feedback about adding unit tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/extension/services/jevAct/pick.ts, line 129:
<comment>The new picker has no focused tests for its safety-critical tiering, sharding, and acceptance branches. Add tests for held strict-none fallbacks, shard finalist aggregation, and redaction preserving protocol keys before these paths are wired into actions.
(Based on your team's feedback about adding unit tests for new behavior.) </comment>
<file context>
@@ -0,0 +1,414 @@
+ * the candidates that share words with the instruction, so the common case is
+ * one Jev request; the full list is only asked when that pruned pick is rejected.
+ */
+export async function pickTarget(
+ ctx: AskContext,
+ node: string,
</file context>
There was a problem hiding this comment.
Not changing this — the picker's tiering, held picks, shard aggregation and acceptance branches are exercised through runJevActPipeline in #2953's jevActPipeline.test.ts (held pick used / rejected, twins, shards, none veto, clear-leader), which is the contract that matters; duplicating them at the pickTarget level would test the same code twice. Redaction now has focused tests in this PR (see the other thread).
| for (let attempt = 1; ; attempt++) { | ||
| let response: Response; | ||
| try { | ||
| response = await fetch(url, { |
There was a problem hiding this comment.
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
systemOne() sends a direct TypeSafe LLM request through fetch() without flowLogger or manual LLM instrumentation, so this exported request path is untracked. Route it through a flow-logger-tracked method or add the required request/response logging before exposing the client.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/extension/services/jevAct/typesafeClient.ts, line 96:
<comment>`systemOne()` sends a direct TypeSafe LLM request through `fetch()` without flowLogger or manual LLM instrumentation, so this exported request path is untracked. Route it through a flow-logger-tracked method or add the required request/response logging before exposing the client.</comment>
<file context>
@@ -0,0 +1,162 @@
+ for (let attempt = 1; ; attempt++) {
+ let response: Response;
+ try {
+ response = await fetch(url, {
+ signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
+ method: "POST",
</file context>
There was a problem hiding this comment.
Not changing this — Jev is not an LLM call in the flowLogger sense (a classifier answering typed questions, no generated text) and this PR adds no public Stagehand/understudy method; every request is recorded in the per-act trace (node, ms, tokens) that the act logs. Surfacing Jev usage in metadata.usage/flow logging is listed as a known limit and follow-up in the README.
7a8feaa to
361fb16
Compare
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
361fb16 to
547c6bc
Compare
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
547c6bc to
63a207a
Compare
63a207a to
f5f82d0
Compare
miguelg719
changed the title
This branch has not been deployed
1 participant
Stack
Stack: #2951 snapshot editable ids → #2952 library → #2953 act pipeline → #2954 observe + cache check → #2955 extract. This is 2/5 (#2952). Each PR targets its predecessor.
Why
TypeSafe Jev is a "System One" model: it answers typed questions (Choice / Noul) with probabilities, cannot generate text, and answers in ~100–300 ms for a few thousand input tokens. To use it for element selection we need (a) a client, (b) a way to turn the a11y outline into small, well-described candidate lists, and (c) acceptance rules that know when not to trust an answer. This PR is that library with no wiring — nothing imports it yet, so the built extension is unchanged (it is tree-shaken out).
Split from the act pipeline so the selection logic can be reviewed on its own.
What (
packages/extension/services/decisions/)typesafeClient.ts— fetch client for/v1/systemone; 8 s timeout; per endpoint+key circuit breaker (auth failure 60 s, 3 consecutive failures 30 s); error messages carry the error code only, never the body or key.tree.ts— parses the outline into nodes; builds per-intent candidate views (pointer / input / select / scroll / option / broad / text / link); describes each candidate with the context a model needs to tell twins apart (ancestors, heading, card/row text, table row+column,n of m, iframe flag); lexical pre-ranking for long lists; exact-name matching; page digest.pick.ts— the picking procedure: role view first, then every named element; lists over 40 are cut to the 30 sharing words with the instruction; two questions per request —best(no "none" option) andstrict(with "none", which vetoes above 0.9); a pickstrictis uneasy about is held while the next tier tries; ambiguity stops rather than guesses; indistinguishable twins share their vote; huge lists are sharded by character budget.args.ts— deterministic argument parsing (quoted strings,%variables%, percentages, keys), grounding of model-returned text to the instruction's own characters, andredactor()which replaces resolved%variable%values with their placeholder in anything that leaves the process.pageState.ts— one request classifying why a target is missing (access denied / captcha / login wall / …).Testing
decisionsTree.test.ts,decisionsModules.test.ts(views, descriptions, pruning, args/grounding, redaction, breaker, page state). Full extension suite, typecheck, lint, fmt pass locally.