diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 6d2a7a72..a2429475 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -296,12 +296,17 @@ async def execute_browser_tool( # Self-healing click toggle (renderer escalates a stale-index failure to a fresh by-name click). Default on; OSW_SELFHEAL_CLICK=0 disables it for the A/B off-arm. if action == "click_index": params["selfheal"] = os.environ.get("OSW_SELFHEAL_CLICK", "1") != "0" + # Metric only: measure how often a "successful" click changed nothing (wrong/dead element). Off by default = zero cost. + if os.environ.get("OSW_CLICK_EFFECT_PROBE") == "1": + params["effectProbe"] = True request_id = uuid4().hex result = await ws_manager.send_browser_command( request_id, action, browser_id, params, tab_id=tab_id, ) if isinstance(result, dict) and result.get("selfHealed"): logger.info(f"[browser-selfheal] recovered a stale-index click via {result['selfHealed']} -> {browser_id}") + if isinstance(result, dict) and result.get("clickEffect"): + logger.info(f"[click-effect] {result['clickEffect']} ({p_action or action}) -> {browser_id}") return result diff --git a/frontend/src/shared/browserCommandHandler.ts b/frontend/src/shared/browserCommandHandler.ts index 15387d44..851e2fc2 100644 --- a/frontend/src/shared/browserCommandHandler.ts +++ b/frontend/src/shared/browserCommandHandler.ts @@ -1,5 +1,6 @@ import { getWebview, findWebviewByDomain, hasDomReady, markDomReady, type BrowserWebview } from './browserRegistry'; import { shouldSelfHealClick } from './selfHealClick'; +import { FP_EXPR, clickEffect } from './clickEffect'; import { store } from './state/store'; import { resumeBrowserCard } from './state/dashboardLayoutSlice'; import { dashboardWs } from './ws/WebSocketManager'; @@ -655,7 +656,7 @@ async function enumerateCandidates(wv: BrowserWebview): Promise { // Resolve + click a specific backend node (revalidate, frame-local box model, OS-level dispatch in the element's own frame, cosmetic top-level ripple). Shared by click_index (cache lookup) and click_by_name (fresh resolution). async function clickBackendNode( wv: BrowserWebview, backendNodeId: number, sessionId: string | undefined, label: string, - opts: { role?: string; text?: string } = {}, + opts: { role?: string; text?: string; effectProbe?: boolean } = {}, ): Promise> { let resolvedObjectId: string | undefined; try { @@ -784,16 +785,24 @@ async function clickBackendNode( } } + // Metric only (flag-gated): fingerprint the page just before the click so we can + // tell afterwards whether it actually did anything. Off by default = zero cost. + let fpBefore = ''; + if (opts.effectProbe) { try { fpBefore = String(await wv.executeJavaScript(FP_EXPR)); } catch { /* ignore */ } } try { await sendCdp(wv, 'Input.dispatchMouseEvent', { type: 'mousePressed', x: lx, y: ly, button: 'left', clickCount: 1 }, sessionId); await sendCdp(wv, 'Input.dispatchMouseEvent', { type: 'mouseReleased', x: lx, y: ly, button: 'left', clickCount: 1 }, sessionId); } catch (err: any) { return { error: `Click failed: ${err.message || String(err)}` }; } - return { - text: `Clicked ${label} at (${Math.round(rx)}, ${Math.round(ry)})`, - ...ripple, - }; + const out: Record = { text: `Clicked ${label} at (${Math.round(rx)}, ${Math.round(ry)})`, ...ripple }; + if (opts.effectProbe) { + await new Promise((r) => setTimeout(r, 400)); + let fpAfter = ''; + try { fpAfter = String(await wv.executeJavaScript(FP_EXPR)); } catch { /* ignore */ } + out.clickEffect = clickEffect(fpBefore, fpAfter); + } + return out; } // Drop list rows the user literally cannot click: zero-size nodes and ones whose center hits a DIFFERENT element (modal backdrop, sticky header, cookie banner). Ground truth via elementFromPoint, the same predicate the click path trusts. Offscreen-but-scrollable elements are kept; the page-wide list is deliberately wider than the viewport. Chunked with a hard budget so a heavy page degrades to an unfiltered list, never a stall. @@ -974,7 +983,7 @@ async function handleClickIndex(wv: BrowserWebview, params: Record) const wantsText = typeof params.text === 'string' && params.text.length > 0; const result = await clickBackendNode(wv, backendNodeId, sessionId, `index ${idx}`, - { role, text: wantsText ? params.text : undefined }); + { role, text: wantsText ? params.text : undefined, effectProbe: params.effectProbe === true }); // Self-healing escalation: a cached index goes stale the instant the page mutates, which is HALF of all runs' tool-errors. An explicit clickBackendNode error PROVES the click never landed (so re-trying the same target can't double-act), so before we hand a ~3s re-strategize turn back to the model, resolve the SAME element fresh from the full DOM by its name+role, the exact rung the model would have climbed to itself. Plain clicks only (a text-fill has its own readback path); gated so an A/B can turn it off. if (shouldSelfHealClick(!!result.error, wantsText, name, params.selfheal)) { const healed = await handleClickByName(wv, { name: name as string, role: role || '' }); diff --git a/frontend/src/shared/clickEffect.test.ts b/frontend/src/shared/clickEffect.test.ts new file mode 100644 index 00000000..5f636bbb --- /dev/null +++ b/frontend/src/shared/clickEffect.test.ts @@ -0,0 +1,20 @@ +// Run: node --test frontend/src/shared/clickEffect.test.ts +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { clickEffect } from './clickEffect.ts'; + +test('a click that changed the page fingerprint = changed', () => { + assert.equal(clickEffect('u1|1200|BUTTON|0', 'u1|1214|BUTTON|0'), 'changed'); // menu opened (+14 nodes) + assert.equal(clickEffect('u1|1200|BUTTON|0', 'u2|1200|BUTTON|0'), 'changed'); // navigated + assert.equal(clickEffect('u1|1200|DIVfalse|0', 'u1|1200|DIVtrue|0'), 'changed'); // aria-expanded toggled + assert.equal(clickEffect('u1|1200|BODY|0', 'u1|1200|BODY|380'), 'changed'); // scrolled +}); + +test('a click that changed NOTHING = none (the invisible wrong/dead-element failure)', () => { + assert.equal(clickEffect('u1|1200|BUTTON|0', 'u1|1200|BUTTON|0'), 'none'); +}); + +test('an unreadable fingerprint (empty) is not counted as a real no-effect', () => { + assert.equal(clickEffect('', 'u1|1200|BUTTON|0'), 'none'); + assert.equal(clickEffect('u1|1200|BUTTON|0', ''), 'none'); +}); diff --git a/frontend/src/shared/clickEffect.ts b/frontend/src/shared/clickEffect.ts new file mode 100644 index 00000000..dc08dc9f --- /dev/null +++ b/frontend/src/shared/clickEffect.ts @@ -0,0 +1,13 @@ +// A cheap page fingerprint taken before and after a click, so we can MEASURE the +// invisible failure the tool-error counter misses: a click that "succeeds" (dispatches +// fine) but lands on the wrong element or a dead one, so nothing on the page changes. +// A real click almost always moves at least one of: the URL, the element count (menu +// opened / row added), the focused element, or the scroll position. +export const FP_EXPR = + "location.href + '|' + document.getElementsByTagName('*').length + '|' + " + + "(document.activeElement ? document.activeElement.tagName + (document.activeElement.getAttribute('aria-expanded')||'') + (document.activeElement.getAttribute('aria-checked')||'') : '') + '|' + " + + "Math.round(window.scrollY)"; + +export function clickEffect(before: string, after: string): 'changed' | 'none' { + return before && after && before !== after ? 'changed' : 'none'; +}