diff --git a/apps/extension/src/tools/__tests__/observation.test.ts b/apps/extension/src/tools/__tests__/observation.test.ts index f5cdcfd..352431f 100644 --- a/apps/extension/src/tools/__tests__/observation.test.ts +++ b/apps/extension/src/tools/__tests__/observation.test.ts @@ -403,6 +403,43 @@ describe("handleScreenshot", () => { expect((calls[1]?.params as { clip?: { scale?: number } }).clip?.scale).toBeLessThan(0.5); }); + it("fails closed when the retried surface PNG still exceeds the pixel budget", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + const ctx = await sm.start("aa11"); + ctx.refStore.set("e5", 999, { + tabId: 7, + kind: "surface", + visibleRect: { x: 0, y: 0, w: 1_000, h: 1_000 }, + }); + const { cdp, sent } = makeFakeCdp({ + "Runtime.evaluate": () => ({ result: { value: 1 } }), + "Page.getLayoutMetrics": () => ({ + cssLayoutViewport: { clientWidth: 1_000, clientHeight: 1_000 }, + }), + "DOM.getContentQuads": () => ({ + quads: [[0, 0, 1_000, 0, 1_000, 1_000, 0, 1_000]], + }), + "Page.captureScreenshot": () => ({ data: pngWithIhdrDimensions(4_000, 4_000) }), + }); + + const res = await handleScreenshot( + sm, + { session_id: "aa11", ref: "@e5", tab_id: 7 }, + makeScreenshotDeps({ + cdp, + get: vi.fn(async () => ({ id: 7, windowId: 100, active: false }) as chrome.tabs.Tab), + query: vi.fn(), + captureVisibleTab: vi.fn(), + }), + ); + + expect(res).toEqual({ + code: "cdp_failed", + message: "Page.captureScreenshot exceeded the visual surface pixel budget", + }); + expect(sent.filter((call) => call.method === "Page.captureScreenshot")).toHaveLength(2); + }); + it("crops a visual surface screenshot to its observation-time visible region", async () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); const ctx = await sm.start("aa11"); diff --git a/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts b/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts index 7450426..df659f5 100644 --- a/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts +++ b/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts @@ -50,6 +50,36 @@ function childSnapshot(frameId: string, backendNodeId: number) { } describe("captureFrameData", () => { + it("does not request frame geometry for a root-only page", async () => { + const root = { ...ownerNode(1, 0), tag: "body" }; + const captured: CapturedViewModel = { + nodes: [root], + viewport: { width: 1000, height: 800 }, + iframeNodes: new Map(), + frameNodes: new Map([["main", [root]]]), + rootFrameId: "main", + excludedBackendNodeIds: new Set(), + }; + const send = vi.fn(async (_tabId, method) => { + if (method === "Accessibility.enable") return {}; + if (method === "Accessibility.getFullAXTree") return { nodes: [] }; + throw new Error(`unexpected ${method}`); + }); + const cdp: CdpRunner = { + send: send as CdpRunner["send"], + getFrameGraph: vi.fn(async () => ({ + rootFrameId: "main", + frames: [{ frameId: "main", target: { tabId: 4 } }], + })), + }; + + await captureFrameData(cdp, 4, captured); + + expect(send.mock.calls.some(([, method]) => method === "Page.createIsolatedWorld")).toBe(false); + expect(send.mock.calls.some(([, method]) => method === "DOM.getBoxModel")).toBe(false); + expect(send.mock.calls.some(([, method]) => method === "Page.getLayoutMetrics")).toBe(false); + }); + it("projects same-origin frame-local rects through the iframe content box", async () => { const owner = ownerNode(10, 50); const canvas: CapturedNode = { @@ -202,6 +232,87 @@ describe("captureFrameData", () => { ); }); + it("bounds concurrent same-target frame geometry resolution", async () => { + const owners = Array.from({ length: 8 }, (_, index) => ownerNode(10 + index, index * 40)); + const childFrames = owners.map((owner, index) => { + const frameId = `child-${index}`; + const node: CapturedNode = { + backendNodeId: 100 + index, + parentBackendNodeId: null, + frameId, + ownerFrameBackendNodeId: owner.backendNodeId, + tag: "canvas", + attrs: {}, + rect: null, + localRect: { x: 1, y: 2, w: 20, h: 10 }, + paintOrder: 1, + position: "static", + pointerEvents: "auto", + }; + return { frameId, owner, node }; + }); + const captured: CapturedViewModel = { + nodes: owners, + viewport: { width: 1000, height: 800 }, + iframeNodes: new Map(childFrames.map(({ owner, node }) => [owner.backendNodeId, [node]])), + frameNodes: new Map([ + ["main", owners], + ...childFrames.map(({ frameId, node }): [string, CapturedNode[]] => [frameId, [node]]), + ]), + frameOwnerBackendNodeIds: new Map( + childFrames.map(({ frameId, owner }) => [frameId, owner.backendNodeId]), + ), + rootFrameId: "main", + excludedBackendNodeIds: new Set(), + }; + let activeWorlds = 0; + let maxActiveWorlds = 0; + const cdp: CdpRunner = { + send: vi.fn(async (_tabId, method, params) => { + if (method === "Page.getLayoutMetrics") { + return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 } }; + } + if (method === "DOM.getBoxModel") { + const backendNodeId = (params as { backendNodeId?: number }).backendNodeId ?? 10; + const x = (backendNodeId - 10) * 40; + return { model: { content: [x, 100, x + 30, 100, x + 30, 120, x, 120] } }; + } + if (method === "Page.createIsolatedWorld") { + activeWorlds += 1; + maxActiveWorlds = Math.max(maxActiveWorlds, activeWorlds); + await new Promise((resolve) => setTimeout(resolve, 5)); + activeWorlds -= 1; + return { executionContextId: 90 }; + } + if (method === "Runtime.evaluate") { + return { result: { value: { width: 30, height: 20 } } }; + } + if (method === "Accessibility.enable") return {}; + if (method === "Accessibility.getFullAXTree") return { nodes: [] }; + throw new Error(`unexpected ${method}`); + }) as CdpRunner["send"], + getFrameGraph: vi.fn(async () => ({ + rootFrameId: "main", + frames: [ + { frameId: "main", target: { tabId: 4 } }, + ...childFrames.map(({ frameId, owner }) => ({ + frameId, + parentFrameId: "main", + ownerBackendNodeId: owner.backendNodeId, + target: { tabId: 4 }, + })), + ], + })), + }; + + await captureFrameData(cdp, 4, captured); + + expect(maxActiveWorlds).toBe(4); + expect(childFrames.every(({ frameId }) => captured.frameNodes?.get(frameId)?.[0]?.rect)).toBe( + true, + ); + }); + it("keeps OOPIF semantics when viewport projection is unavailable", async () => { const owner = { ...ownerNode(10, 50), rect: null, localRect: null }; const captured: CapturedViewModel = { diff --git a/apps/extension/src/tools/vom/__tests__/rendered-surfaces.test.ts b/apps/extension/src/tools/vom/__tests__/rendered-surfaces.test.ts index 03f1cb9..da97e1e 100644 --- a/apps/extension/src/tools/vom/__tests__/rendered-surfaces.test.ts +++ b/apps/extension/src/tools/vom/__tests__/rendered-surfaces.test.ts @@ -313,4 +313,74 @@ describe("rendered surface clustering and projection", () => { ]); expect(clustered.truncated).toBe(false); }); + + it("keeps large non-overlapping surface sets linear and independent", () => { + const surfaces = Array.from({ length: 10_000 }, (_, index) => ({ + renderingKind: "canvas" as const, + frameId: "main", + backendNodeId: index + 1, + parentBackendNodeId: 1, + visibleRect: { x: index * 20, y: 0, w: 10, h: 10 }, + paintOrder: index, + })); + + const clustered = clusterRenderedSurfaces(surfaces); + + expect(clustered.groups).toHaveLength(10_000); + expect(clustered.groups.every((group) => group.memberCount === 1)).toBe(true); + expect(clustered.truncated).toBe(false); + }); + + it("keeps the highest-priority surfaces when the computation guard truncates input", () => { + const surfaces = [ + { + renderingKind: "canvas" as const, + frameId: "main", + backendNodeId: 1, + parentBackendNodeId: 10, + visibleRect: { x: 0, y: 0, w: 10, h: 10 }, + paintOrder: 1, + }, + { + renderingKind: "canvas" as const, + frameId: "main", + backendNodeId: 2, + parentBackendNodeId: 10, + visibleRect: { x: 20, y: 0, w: 100, h: 100 }, + paintOrder: 1, + }, + { + renderingKind: "canvas" as const, + frameId: "main", + backendNodeId: 3, + parentBackendNodeId: 10, + visibleRect: { x: 140, y: 0, w: 8, h: 8 }, + paintOrder: 1, + label: "Important status", + }, + ]; + + const clustered = clusterRenderedSurfaces(surfaces, { maxSurfaces: 2 }); + + expect(clustered.groups.map((group) => group.representative.backendNodeId).sort()).toEqual([ + 2, 3, + ]); + expect(clustered).toMatchObject({ truncated: true, omittedCount: 1 }); + }); + + it("stops at the comparison budget without dropping already completed groups", () => { + const surfaces = Array.from({ length: 3 }, (_, index) => ({ + renderingKind: "canvas" as const, + frameId: "main", + backendNodeId: index + 1, + parentBackendNodeId: 10, + visibleRect: { x: 0, y: 0, w: 100, h: 100 }, + paintOrder: index + 1, + })); + + const clustered = clusterRenderedSurfaces(surfaces, { maxComparisons: 1 }); + + expect(clustered.groups).toEqual([expect.objectContaining({ memberCount: 2 })]); + expect(clustered).toMatchObject({ truncated: true, omittedCount: 1 }); + }); }); diff --git a/apps/extension/src/tools/vom/frame-capture.ts b/apps/extension/src/tools/vom/frame-capture.ts index 6211243..2331210 100644 --- a/apps/extension/src/tools/vom/frame-capture.ts +++ b/apps/extension/src/tools/vom/frame-capture.ts @@ -12,6 +12,8 @@ import { export type FrameAxNode = FrameOwnedAxNode; export type CapturedFrameDocument = FrameDocument; +const FRAME_GEOMETRY_CONCURRENCY = 4; + function throwIfAborted(signal?: AbortSignal): void { if (signal?.aborted) throw new DOMException("observation aborted", "AbortError"); } @@ -20,6 +22,22 @@ function isAbortError(err: unknown): boolean { return err instanceof Error && err.name === "AbortError"; } +async function forEachWithConcurrency( + items: readonly T[], + concurrency: number, + worker: (item: T) => Promise, +): Promise { + let next = 0; + const run = async () => { + while (next < items.length) { + const index = next; + next += 1; + await worker(items[index]); + } + }; + await Promise.all(Array.from({ length: Math.min(concurrency, items.length) }, run)); +} + async function discoverFrameGraph(cdp: CdpRunner, tabId: number): Promise { if (!cdp.getFrameGraph) return null; try { @@ -92,20 +110,24 @@ async function normalizeFrameLocalRects( if (!captured.frameNodes) return; const geometry = createFrameGeometryContext(cdp, graph); const frameById = new Map(graph.frames.map((frame) => [frame.frameId, frame])); - await Promise.all( - [...captured.frameNodes].map(async ([frameId, nodes]) => { + await forEachWithConcurrency( + [...captured.frameNodes], + FRAME_GEOMETRY_CONCURRENCY, + async ([frameId, nodes]) => { if (frameId === graph.rootFrameId || !frameById.has(frameId)) return; try { - const normalized = await Promise.all( - nodes.map(async (node) => { - if (!node.localRect) return { ...node, rect: null }; + const normalized: CapturedNode[] = []; + for (const node of nodes) { + if (!node.localRect) { + normalized.push({ ...node, rect: null }); + } else { const bounds = await geometry.projectFrameLocalRect(frameId, node.localRect); - return { + normalized.push({ ...node, rect: bounds ? { x: bounds.x, y: bounds.y, w: bounds.width, h: bounds.height } : null, - }; - }), - ); + }); + } + } throwIfAborted(signal); captured.frameNodes?.set(frameId, normalized); const ownerBackendNodeId = @@ -129,7 +151,7 @@ async function normalizeFrameLocalRects( captured.iframeNodes.set(ownerBackendNodeId, unavailable); } } - }), + }, ); } diff --git a/apps/extension/src/tools/vom/rendered-surfaces/cluster.ts b/apps/extension/src/tools/vom/rendered-surfaces/cluster.ts index 8cf34b5..8b2ecb8 100644 --- a/apps/extension/src/tools/vom/rendered-surfaces/cluster.ts +++ b/apps/extension/src/tools/vom/rendered-surfaces/cluster.ts @@ -1,4 +1,4 @@ -import type { Rect } from "@browser-skill/vom"; +import { compareVisualSurfacePriority, type Rect } from "@browser-skill/vom"; import type { ClusteredRenderedSurfaces, RenderedSurface, RenderedSurfaceGroup } from "./types"; const STACK_IOU_RATIO = 0.9; @@ -8,6 +8,11 @@ const POSITION_CELL_RATIO = 0.1; const MAX_CLUSTER_COMPARISONS = 1_000_000; const MAX_CLUSTER_SURFACES = 50_000; +export interface ClusterRenderedSurfaceOptions { + maxComparisons?: number; + maxSurfaces?: number; +} + interface IndexedGroup extends RenderedSurfaceGroup { creationIndex: number; indexKey: string; @@ -92,15 +97,72 @@ function partitionKey(surface: RenderedSurface): string { return `${surface.frameId}\u0000${surface.parentBackendNodeId ?? "root"}`; } -export function clusterRenderedSurfaces(surfaces: RenderedSurface[]): ClusteredRenderedSurfaces { +function siftWorstUp(heap: RenderedSurface[], start: number): void { + let index = start; + while (index > 0) { + const parent = Math.floor((index - 1) / 2); + if (compareVisualSurfacePriority(heap[index], heap[parent]) <= 0) break; + [heap[index], heap[parent]] = [heap[parent], heap[index]]; + index = parent; + } +} + +function siftWorstDown(heap: RenderedSurface[], start: number): void { + let index = start; + while (true) { + const left = index * 2 + 1; + if (left >= heap.length) return; + const right = left + 1; + let worst = left; + if (right < heap.length && compareVisualSurfacePriority(heap[right], heap[left]) > 0) { + worst = right; + } + if (compareVisualSurfacePriority(heap[worst], heap[index]) <= 0) return; + [heap[index], heap[worst]] = [heap[worst], heap[index]]; + index = worst; + } +} + +function highestPrioritySurfaces( + surfaces: readonly RenderedSurface[], + limit: number, +): RenderedSurface[] { + if (limit <= 0) return []; + if (surfaces.length <= limit) return [...surfaces].sort(compareVisualSurfacePriority); + const heap: RenderedSurface[] = []; + for (const surface of surfaces) { + if (heap.length < limit) { + heap.push(surface); + siftWorstUp(heap, heap.length - 1); + } else if (compareVisualSurfacePriority(surface, heap[0]) < 0) { + heap[0] = surface; + siftWorstDown(heap, 0); + } + } + return heap.sort(compareVisualSurfacePriority); +} + +function normalizedLimit(value: number | undefined, fallback: number): number { + const resolved = value ?? fallback; + return Number.isFinite(resolved) ? Math.max(0, Math.floor(resolved)) : fallback; +} + +export function clusterRenderedSurfaces( + surfaces: RenderedSurface[], + options: ClusterRenderedSurfaceOptions = {}, +): ClusteredRenderedSurfaces { + const maxComparisons = normalizedLimit(options.maxComparisons, MAX_CLUSTER_COMPARISONS); + const maxSurfaces = normalizedLimit(options.maxSurfaces, MAX_CLUSTER_SURFACES); + const prioritized = highestPrioritySurfaces(surfaces, maxSurfaces); const partitions = new Map>>(); const groups: IndexedGroup[] = []; let comparisons = 0; let processed = 0; - let truncated = false; + let truncated = prioritized.length < surfaces.length; + let comparisonLimitReached = false; - for (const surface of surfaces) { - if (processed >= MAX_CLUSTER_SURFACES || comparisons >= MAX_CLUSTER_COMPARISONS) { + for (const surface of prioritized) { + if (comparisons >= maxComparisons) { truncated = true; break; } @@ -117,11 +179,12 @@ export function clusterRenderedSurfaces(surfaces: RenderedSurface[]): ClusteredR let best: { group: IndexedGroup; score: number; areaDelta: number } | null = null; for (const group of candidates) { - comparisons += 1; - if (comparisons > MAX_CLUSTER_COMPARISONS) { + if (comparisons >= maxComparisons) { truncated = true; + comparisonLimitReached = true; break; } + comparisons += 1; const score = stackScore(group.representative, surface); if (score === null) continue; const areaDelta = Math.abs( @@ -138,7 +201,7 @@ export function clusterRenderedSurfaces(surfaces: RenderedSurface[]): ClusteredR best = { group, score, areaDelta }; } } - if (truncated) break; + if (comparisonLimitReached) break; if (best) { const group = best.group; diff --git a/apps/extension/src/tools/vom/rendered-surfaces/index.ts b/apps/extension/src/tools/vom/rendered-surfaces/index.ts index d1c1873..9f220a9 100644 --- a/apps/extension/src/tools/vom/rendered-surfaces/index.ts +++ b/apps/extension/src/tools/vom/rendered-surfaces/index.ts @@ -1,4 +1,4 @@ -export { clusterRenderedSurfaces } from "./cluster"; +export { type ClusterRenderedSurfaceOptions, clusterRenderedSurfaces } from "./cluster"; export { discoverRenderedSurfaces } from "./discovery"; export { projectRenderedSurfaces } from "./project"; export type { diff --git a/packages/vom/src/__tests__/render.test.ts b/packages/vom/src/__tests__/render.test.ts index 42b085f..0050086 100644 --- a/packages/vom/src/__tests__/render.test.ts +++ b/packages/vom/src/__tests__/render.test.ts @@ -49,7 +49,7 @@ describe("renderVom single-layer page", () => { expect(out.text).toContain("[visual surfaces]"); expect(out.text).toContain( - '@e1 surface "Data grid" [bounds=10,20,800,500; rendering=canvas; layers=2; visual-only; requires=image-understanding; use: bsk screenshot --ref @e1]', + '@e1 surface "Data grid" [bounds=10,20,800,500; rendering=canvas; layers=2; visual-only; screenshot-only; requires=image-understanding]', ); expect(out.refs).toContainEqual( expect.objectContaining({ @@ -78,7 +78,7 @@ describe("renderVom single-layer page", () => { const out = renderVom(input); expect(out.text).toContain( - '@e1 surface "canvas visual surface" [bounds=10,20,800,500; rendering=canvas; visual-only; requires=image-understanding; use: bsk screenshot --ref @e1]', + '@e1 surface "canvas visual surface" [bounds=10,20,800,500; rendering=canvas; visual-only; screenshot-only; requires=image-understanding]', ); expect(out.refs).toContainEqual( expect.objectContaining({ diff --git a/packages/vom/src/index.ts b/packages/vom/src/index.ts index c0f1981..1a10f70 100644 --- a/packages/vom/src/index.ts +++ b/packages/vom/src/index.ts @@ -20,3 +20,7 @@ export type { VomScene, VomVisualSurface, } from "./types"; +export { + compareVisualSurfacePriority, + type VisualSurfacePriorityInput, +} from "./visual-surface-priority"; diff --git a/packages/vom/src/render.ts b/packages/vom/src/render.ts index df94a32..8260159 100644 --- a/packages/vom/src/render.ts +++ b/packages/vom/src/render.ts @@ -11,6 +11,7 @@ import type { VomScene, VomVisualSurface, } from "./types"; +import { compareVisualSurfacePriority } from "./visual-surface-priority"; const SKIP_ROLES = new Set(["generic", "none", "presentation", "inlinetextbox"]); @@ -809,29 +810,12 @@ function tryAppendLine(state: RenderState, line: string, tokenLimit = state.maxT return true; } -function visualSurfaceArea(surface: VomVisualSurface): number { - return Math.max(0, surface.visibleRect.w) * Math.max(0, surface.visibleRect.h); -} - -function compareVisualSurfaces(a: VomVisualSurface, b: VomVisualSurface): number { - const aLabeled = cleaned(a.label) !== undefined; - const bLabeled = cleaned(b.label) !== undefined; - return ( - Number(bLabeled) - Number(aLabeled) || - visualSurfaceArea(b) - visualSurfaceArea(a) || - a.frameId.localeCompare(b.frameId) || - a.visibleRect.y - b.visibleRect.y || - a.visibleRect.x - b.visibleRect.x || - a.backendNodeId - b.backendNodeId - ); -} - function visualSurfaceLine(surface: VomVisualSurface, ref: string): string { const label = cleaned(surface.label) ?? `${surface.renderingKind} visual surface`; const layers = surface.memberCount > 1 ? `; layers=${surface.memberCount}` : ""; const { x, y, w, h } = surface.visibleRect; const bounds = [x, y, w, h].map(Math.round).join(","); - return ` @${ref} surface ${JSON.stringify(label)} [bounds=${bounds}; rendering=${surface.renderingKind}${layers}; visual-only; requires=image-understanding; use: bsk screenshot --ref @${ref}]`; + return ` @${ref} surface ${JSON.stringify(label)} [bounds=${bounds}; rendering=${surface.renderingKind}${layers}; visual-only; screenshot-only; requires=image-understanding]`; } function selectVisualSurfaces( @@ -847,7 +831,7 @@ function selectVisualSurfaces( selected.push(surface); } - selected.sort(compareVisualSurfaces); + selected.sort(compareVisualSurfacePriority); return { selected, eligibleCount }; } diff --git a/packages/vom/src/visual-surface-priority.ts b/packages/vom/src/visual-surface-priority.ts new file mode 100644 index 0000000..97c2229 --- /dev/null +++ b/packages/vom/src/visual-surface-priority.ts @@ -0,0 +1,29 @@ +export interface VisualSurfacePriorityInput { + label?: string; + visibleRect: { x: number; y: number; w: number; h: number }; + frameId: string; + backendNodeId: number; +} + +function hasLabel(label: string | undefined): boolean { + return (label?.trim().length ?? 0) > 0; +} + +function area(surface: VisualSurfacePriorityInput): number { + return Math.max(0, surface.visibleRect.w) * Math.max(0, surface.visibleRect.h); +} + +/** Negative means `a` should be emitted before `b`. */ +export function compareVisualSurfacePriority( + a: VisualSurfacePriorityInput, + b: VisualSurfacePriorityInput, +): number { + return ( + Number(hasLabel(b.label)) - Number(hasLabel(a.label)) || + area(b) - area(a) || + a.frameId.localeCompare(b.frameId) || + a.visibleRect.y - b.visibleRect.y || + a.visibleRect.x - b.visibleRect.x || + a.backendNodeId - b.backendNodeId + ); +}