From 574e083e75b679a504b1eda8d3afb0109c38741d Mon Sep 17 00:00:00 2001 From: JB Date: Mon, 21 Sep 2026 13:18:18 -0400 Subject: [PATCH] perf(fast-browser): batch value-free observation metadata --- extensions/fast-browser/browser_worker.py | 211 +++++++++++++----- .../fast-browser/test_browser_worker.py | 144 ++++++++++++ 2 files changed, 301 insertions(+), 54 deletions(-) diff --git a/extensions/fast-browser/browser_worker.py b/extensions/fast-browser/browser_worker.py index c8f4de6..c98ca98 100644 --- a/extensions/fast-browser/browser_worker.py +++ b/extensions/fast-browser/browser_worker.py @@ -21,7 +21,7 @@ import time import uuid import base64 import re -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path from typing import Any, Iterable, Protocol from urllib.parse import urlsplit, urlunsplit @@ -33,6 +33,7 @@ from urllib.parse import urlsplit, urlunsplit MAX_LINE_BYTES = 12 * 1024 * 1024 MAX_OUTPUT_BYTES = 12 * 1024 * 1024 MAX_CANDIDATES = 60 +MAX_METADATA_BATCH = 16 MAX_OBSERVATIONS = 8 MAX_STRING = 4_000 MAX_NAME = 240 @@ -348,6 +349,50 @@ class BrowserWorker: observation_id = f"obs-{self._counter}-{secrets.token_hex(4)}" records: list[CandidateRecord] = [] seen: set[int] = set() + batch_metadata = getattr(self.backend, "metadata_many", None) + pending: list[tuple[int, str, str]] = [] + + def consume(items: list[tuple[int, str, str]]) -> None: + if not items: + return + metadata_by_id: dict[int, ElementMetadata] = {} + if callable(batch_metadata): + try: + result = batch_metadata([backend_id for backend_id, _role, _name in items]) + except Exception: + result = None + if isinstance(result, dict): + for backend_id, meta in result.items(): + if type(backend_id) is int and isinstance(meta, ElementMetadata): + metadata_by_id[backend_id] = meta + for backend_id, ax_role, ax_name in items: + meta = metadata_by_id.get(backend_id) + if meta is None or meta.backend_id != backend_id: + try: + meta = self.backend.metadata(backend_id) + except Exception: + continue + if not meta.connected or meta.backend_id != backend_id: + continue + # Full AX discovery is authoritative for observation roles and + # click names. DOM metadata remains the only source for fill + # labels, so a field's current value cannot enter an observation. + observed_meta = replace(meta, ax_role=ax_role, ax_name=ax_name) + candidate_role = (ax_role or observed_meta.role).lower() + actions = _candidate_actions(observed_meta, candidate_role) + if not actions: + continue + name = _safe_name( + observed_meta.name if "fill" in actions + else (ax_name or observed_meta.ax_name or observed_meta.name) + ) + candidate_id = f"c-{len(records)}" + records.append(CandidateRecord( + candidate_id, backend_id, candidate_role, name, actions, observed_meta + )) + if len(records) >= MAX_CANDIDATES: + break + for node in self.backend.accessibility_nodes(): if len(records) >= MAX_CANDIDATES: break @@ -362,25 +407,11 @@ class BrowserWorker: # Do not spend several CDP round trips on static text/layout nodes. if ax_role not in _FILL_ROLES | _CLICK_ROLES: continue - try: - meta = self.backend.metadata(backend_id) - except Exception: - continue - if not meta.connected or meta.backend_id != backend_id: - continue - candidate_role = (ax_role or meta.ax_role or meta.role).lower() - actions = _candidate_actions(meta, candidate_role) - if not actions: - continue - # For fields prefer an author-supplied label/placeholder returned by - # metadata. AX names for a textbox must not accidentally echo its - # current value. The real backend's metadata.name follows this rule. - # A field's AX name can include state/value text on some custom - # controls. Never use it as a fallback for fill candidates. - name = _safe_name(meta.name if "fill" in actions else (meta.ax_name or meta.name or ax_name)) - candidate_id = f"c-{len(records)}" - record = CandidateRecord(candidate_id, backend_id, candidate_role, name, actions, meta) - records.append(record) + pending.append((backend_id, ax_role, ax_name)) + if len(pending) >= MAX_METADATA_BATCH: + consume(pending) + pending.clear() + consume(pending) snapshot = ObservationRecord( observation_id, tab["target_id"], frame_id, loader_id, tab["url"], tab["title"], tuple(records) ) @@ -426,7 +457,7 @@ class BrowserWorker: raise WorkerError("action is not allowed for candidate") current = self.backend.metadata(candidate.backend_id) current_role = (current.ax_role or current.role).lower() - current_name = current.name if action == "fill" else (current.name or current.ax_name) + current_name = current.name if action == "fill" else (current.ax_name or current.name) current_actions = _candidate_actions(current, current_role) if ( not current.connected @@ -547,6 +578,39 @@ class BrowserWorker: self.backend.close() +_METADATA_FUNCTION = """function () { + const e = this; + const style = getComputedStyle(e); + const type = (e.getAttribute('type') || '').toLowerCase(); + const text = node => node ? String(node.innerText || node.textContent || '').trim() : ''; + const labelledBy = (e.getAttribute('aria-labelledby') || '').split(/\\s+/) + .map(id => id ? text(document.getElementById(id)) : '').filter(Boolean).join(' '); + const labels = e.labels ? [...e.labels].map(text).filter(Boolean).join(' ') : ''; + const visibleControlText = ['BUTTON', 'A', 'SUMMARY'].includes(e.tagName) ? text(e) : ''; + // Never read .value: for fill candidates this is user data. + const label = e.getAttribute('aria-label') || labelledBy || labels || + (visibleControlText || e.getAttribute('placeholder') || + e.getAttribute('name') || e.getAttribute('id') || ''); + return { + tag: e.tagName.toLowerCase(), type, role: e.getAttribute('role') || '', + name: String(label).slice(0, 240), connected: !!e.isConnected, + mainFrame: window.top === window, + contenteditable: e.isContentEditable, + visible: !!(e.getClientRects().length && style.display !== 'none' && + style.visibility !== 'hidden' && style.opacity !== '0' && + e.getAttribute('aria-hidden') !== 'true'), + disabled: !!(e.disabled || e.getAttribute('aria-disabled') === 'true' || + e.matches(':disabled')) + }; +}""" +_METADATA_MANY_FUNCTION = ( + "function (...nodes) {\n" + f" const read = {_METADATA_FUNCTION};\n" + " return nodes.map(node => node && typeof node === 'object' ? read.call(node) : null);\n" + "}" +) + + class RealBrowserBackend: """Adapter over browser-harness; no CDP implementation is duplicated here.""" @@ -875,34 +939,20 @@ class RealBrowserBackend: except Exception: pass - def metadata(self, backend_id: int) -> ElementMetadata: - value = self._call(backend_id, """function () { - const e = this; - const style = getComputedStyle(e); - const type = (e.getAttribute('type') || '').toLowerCase(); - const text = node => node ? String(node.innerText || node.textContent || '').trim() : ''; - const labelledBy = (e.getAttribute('aria-labelledby') || '').split(/\\s+/) - .map(id => id ? text(document.getElementById(id)) : '').filter(Boolean).join(' '); - const labels = e.labels ? [...e.labels].map(text).filter(Boolean).join(' ') : ''; - const visibleControlText = ['BUTTON', 'A', 'SUMMARY'].includes(e.tagName) ? text(e) : ''; - // Never read .value: for fill candidates this is user data. - const label = e.getAttribute('aria-label') || labelledBy || labels || - (visibleControlText || e.getAttribute('placeholder') || - e.getAttribute('name') || e.getAttribute('id') || ''); - return { - tag: e.tagName.toLowerCase(), type, role: e.getAttribute('role') || '', - name: String(label).slice(0, 240), connected: !!e.isConnected, - mainFrame: window.top === window, - contenteditable: e.isContentEditable, - visible: !!(e.getClientRects().length && style.display !== 'none' && - style.visibility !== 'hidden' && style.opacity !== '0' && - e.getAttribute('aria-hidden') !== 'true'), - disabled: !!(e.disabled || e.getAttribute('aria-disabled') === 'true' || - e.matches(':disabled')) - }; - }""") + @staticmethod + def _metadata_from_dom_value(backend_id: int, value: Any) -> ElementMetadata: if not isinstance(value, dict): raise WorkerError("element metadata unavailable") + return ElementMetadata( + backend_id, str(value.get("tag", "")), str(value.get("type", "")), + str(value.get("role", "")), " ".join(str(value.get("name", "")).split())[:MAX_NAME], + bool(value.get("visible")), bool(value.get("disabled")), bool(value.get("connected")), + bool(value.get("mainFrame", True)), bool(value.get("contenteditable", False)), + ) + + def metadata(self, backend_id: int) -> ElementMetadata: + value = self._call(backend_id, _METADATA_FUNCTION) + metadata = self._metadata_from_dom_value(backend_id, value) ax_role, ax_name = "", "" try: partial = self._helpers.cdp( @@ -919,13 +969,66 @@ class RealBrowserBackend: # DOM metadata still provides safe labels; accessibility is a # refinement and not a reason to expose a broad CDP surface. pass - return ElementMetadata( - backend_id, str(value.get("tag", "")), str(value.get("type", "")), - str(value.get("role", "")), " ".join(str(value.get("name", "")).split())[:MAX_NAME], - bool(value.get("visible")), bool(value.get("disabled")), bool(value.get("connected")), - bool(value.get("mainFrame", True)), bool(value.get("contenteditable", False)), - ax_role, ax_name, - ) + return replace(metadata, ax_role=ax_role, ax_name=ax_name) + + def metadata_many(self, backend_ids: Iterable[int]) -> dict[int, ElementMetadata]: + """Read value-free DOM metadata for one bounded observation chunk. + + The full AX tree is already the observation's source for role/name. This + seam only batches the DOM extraction; action revalidation continues to + use metadata(), including its fresh partial AX read. + """ + ids = [backend_id for backend_id in backend_ids if type(backend_id) is int and backend_id > 0][:MAX_METADATA_BATCH] + if not ids: + return {} + object_group = f"fast-browser-metadata-{uuid.uuid4().hex}" + object_ids: list[str | None] = [] + try: + for backend_id in ids: + try: + resolved = self._helpers.cdp( + "DOM.resolveNode", backendNodeId=backend_id, objectGroup=object_group + ) + except Exception: + object_ids.append(None) + continue + object_id = (resolved.get("object") or {}).get("objectId") if isinstance(resolved, dict) else None + object_ids.append(object_id if isinstance(object_id, str) else None) + + target_object = next((object_id for object_id in object_ids if object_id is not None), None) + if target_object is None: + return {} + arguments = [ + {"objectId": object_id} if object_id is not None else {"value": None} + for object_id in object_ids + ] + try: + response = self._helpers.cdp( + "Runtime.callFunctionOn", objectId=target_object, + functionDeclaration=_METADATA_MANY_FUNCTION, arguments=arguments, + returnByValue=True, awaitPromise=False, objectGroup=object_group, + ) + except Exception: + return {} + details = response.get("exceptionDetails") if isinstance(response, dict) else None + remote = response.get("result") if isinstance(response, dict) else None + values = remote.get("value") if isinstance(remote, dict) else None + if details or not isinstance(remote, dict) or remote.get("subtype") == "error" or not isinstance(values, list): + return {} + result: dict[int, ElementMetadata] = {} + for backend_id, value in zip(ids, values): + try: + result[backend_id] = self._metadata_from_dom_value(backend_id, value) + except WorkerError: + # Missing or malformed entries use the safe legacy path in + # BrowserWorker.observe rather than being treated as absent. + continue + return result + finally: + try: + self._helpers.cdp("Runtime.releaseObjectGroup", objectGroup=object_group) + except Exception: + pass def click(self, backend_id: int) -> None: box = self._helpers.cdp("DOM.getBoxModel", backendNodeId=backend_id) diff --git a/extensions/fast-browser/test_browser_worker.py b/extensions/fast-browser/test_browser_worker.py index 23c3849..d3752bb 100644 --- a/extensions/fast-browser/test_browser_worker.py +++ b/extensions/fast-browser/test_browser_worker.py @@ -103,6 +103,25 @@ class FakeBackend: pass +class BatchFakeBackend(FakeBackend): + def __init__(self): + super().__init__() + self.metadata_many_calls = [] + self.metadata_calls = [] + self.batch_result = None + + def metadata(self, backend_id): + self.metadata_calls.append(backend_id) + return super().metadata(backend_id) + + def metadata_many(self, backend_ids): + ids = list(backend_ids) + self.metadata_many_calls.append(ids) + if self.batch_result is not None: + return self.batch_result + return {backend_id: self.elements[backend_id] for backend_id in ids if backend_id in self.elements} + + class PopupFakeBackend(FakeBackend): def __init__(self): super().__init__() @@ -550,6 +569,82 @@ class WorkerTests(unittest.TestCase): self.assertNotIn("values", json.dumps(observation)) self.assertNotIn("Disabled", json.dumps(observation)) + def test_observe_batches_dom_metadata_but_keeps_full_ax_roles_and_names(self): + backend = BatchFakeBackend() + backend.elements[1] = replace(backend.elements[1], name="DOM Email", ax_role="textbox", ax_name="AX Email") + backend.elements[3] = replace(backend.elements[3], name="DOM Submit", ax_role="button", ax_name="AX Submit") + backend.nodes[0]["name"] = {"value": "AX Email old-test-secret"} + backend.nodes[1]["name"] = {"value": "AX Submit"} + worker = BrowserWorker(backend, ["http://fixture.test"]) + + observation = worker.observe() + + self.assertEqual(backend.metadata_many_calls, [[1, 3, 4, 2]]) + self.assertEqual(backend.metadata_calls, []) + self.assertEqual([candidate["name"] for candidate in observation["candidates"]], + ["DOM Email", "AX Submit", "Password"]) + self.assertEqual([candidate["role"] for candidate in observation["candidates"]], + ["textbox", "button", "textbox"]) + self.assertNotIn("old-test-secret", json.dumps(observation)) + + result = worker.act({ + "observationId": observation["observationId"], "candidateId": "c-1", "action": "click", + "postcondition": {"kind": "url", "value": "http://fixture.test/done"}, + }) + self.assertTrue(result["verified"]) + self.assertEqual(backend.metadata_calls, [3]) + + def test_observe_falls_back_for_missing_batch_metadata(self): + backend = BatchFakeBackend() + backend.batch_result = {1: backend.elements[1]} + worker = BrowserWorker(backend, ["http://fixture.test"]) + + observation = worker.observe() + + self.assertEqual([candidate["name"] for candidate in observation["candidates"]], + ["Email", "Submit", "Password"]) + self.assertEqual(backend.metadata_calls, [3, 4, 2]) + + def test_observe_batches_in_bounded_chunks_and_preserves_order_and_deduplication(self): + backend = BatchFakeBackend() + backend.elements = { + index: ElementMetadata(index, "button", "", "button", f"Button {index}", True, False, + ax_role="button", ax_name=f"Button {index}") + for index in range(1, 34) + } + backend.nodes = [ + {"backendDOMNodeId": index, "role": {"value": "button"}, "name": {"value": f"AX {index}"}} + for index in range(1, 34) + ] + backend.nodes.insert(10, dict(backend.nodes[9])) + worker = BrowserWorker(backend, ["http://fixture.test"]) + + observation = worker.observe() + + self.assertEqual([len(chunk) for chunk in backend.metadata_many_calls], [16, 16, 1]) + self.assertEqual([candidate["name"] for candidate in observation["candidates"]], + [f"AX {index}" for index in range(1, 34)]) + self.assertEqual(len(observation["candidates"]), 33) + + def test_observe_skips_detached_nodes_and_keeps_candidate_limit(self): + backend = BatchFakeBackend() + backend.elements = { + index: ElementMetadata(index, "button", "", "button", f"Button {index}", True, False, + connected=index != 10, ax_role="button", ax_name=f"Button {index}") + for index in range(1, 63) + } + backend.nodes = [ + {"backendDOMNodeId": index, "role": {"value": "button"}, "name": {"value": f"AX {index}"}} + for index in range(1, 63) + ] + worker = BrowserWorker(backend, ["http://fixture.test"]) + + observation = worker.observe() + + self.assertEqual(len(observation["candidates"]), 60) + self.assertNotIn("AX 10", json.dumps(observation)) + self.assertTrue(all(len(chunk) <= 16 for chunk in backend.metadata_many_calls)) + def test_password_fill_uses_literal_without_exposing_value_in_metadata_or_result(self): backend = FakeBackend() backend.values[2] = "old-test-secret" @@ -907,6 +1002,55 @@ class BridgeRegressionTests(unittest.TestCase): self.assertEqual(keydowns[0].kwargs["modifiers"], 4) backend._helpers.press_key.assert_called_once_with("Backspace") + def test_metadata_many_uses_one_call_and_releases_temporary_object_group(self): + backend = RealBrowserBackend.__new__(RealBrowserBackend) + helper = Mock() + backend._helpers = helper + + def cdp(method, **kwargs): + if method == "DOM.resolveNode": + return {"object": {"objectId": f"object-{kwargs['backendNodeId']}"}} + if method == "Runtime.callFunctionOn": + return {"result": {"value": [ + {"tag": "input", "type": "text", "role": "", "name": "Email", + "connected": True, "mainFrame": True, "contenteditable": False, + "visible": True, "disabled": False}, + {"tag": "button", "type": "", "role": "button", "name": "Submit", + "connected": True, "mainFrame": True, "contenteditable": False, + "visible": True, "disabled": False}, + ]}} + return {} + helper.cdp.side_effect = cdp + + result = backend.metadata_many([1, 2]) + + self.assertEqual([metadata.name for metadata in result.values()], ["Email", "Submit"]) + calls = [call.args[0] for call in helper.cdp.call_args_list] + self.assertEqual(calls.count("DOM.resolveNode"), 2) + self.assertEqual(calls.count("Runtime.callFunctionOn"), 1) + self.assertEqual(calls.count("Runtime.releaseObjectGroup"), 1) + call = next(call for call in helper.cdp.call_args_list if call.args[0] == "Runtime.callFunctionOn") + self.assertEqual([argument.get("objectId") for argument in call.kwargs["arguments"]], + ["object-1", "object-2"]) + + def test_metadata_many_releases_object_group_when_batch_call_fails(self): + backend = RealBrowserBackend.__new__(RealBrowserBackend) + helper = Mock() + backend._helpers = helper + + def cdp(method, **kwargs): + if method == "DOM.resolveNode": + return {"object": {"objectId": f"object-{kwargs['backendNodeId']}"}} + if method == "Runtime.callFunctionOn": + raise RuntimeError("detached") + return {} + helper.cdp.side_effect = cdp + + self.assertEqual(backend.metadata_many([1, 2]), {}) + self.assertEqual( + [call.args[0] for call in helper.cdp.call_args_list].count("Runtime.releaseObjectGroup"), 1 + ) + def test_call_releases_remote_object_on_browser_exception(self): backend = RealBrowserBackend.__new__(RealBrowserBackend) backend._helpers = Mock()