fix(vom): bug fix

2026-09-10 13:01:23 +08:00 · 2026-09-08 10:54:33 +08:00
parent 905ba6a8a6
commit aeece8d7e4
5 changed files with 384 additions and 56 deletions
@@ -47,8 +47,9 @@ describe("renderVom single-layer page", () => {
    ];
    const out = renderVom(input);

+    expect(out.text).toContain("[visual surfaces]");
    expect(out.text).toContain(
-      '@e1 surface "Data grid" [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; requires=image-understanding; use: bsk screenshot --ref @e1]',
    );
    expect(out.refs).toContainEqual(
      expect.objectContaining({
@@ -77,7 +78,7 @@ describe("renderVom single-layer page", () => {
    const out = renderVom(input);

    expect(out.text).toContain(
-      '@e1 surface "canvas visual surface" [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; requires=image-understanding; use: bsk screenshot --ref @e1]',
    );
    expect(out.refs).toContainEqual(
      expect.objectContaining({
@@ -89,6 +90,78 @@ describe("renderVom single-layer page", () => {
    );
  });

+  it("appends visual refs after DOM refs so canvas density cannot renumber controls", () => {
+    const input = scene([
+      node({ id: 1, role: "RootWebArea", frameId: "main" }),
+      node({ id: 2, parentId: 1, role: "button", name: "Submit", tag: "button" }),
+    ]);
+    input.visualSurfaces = [
+      {
+        parentId: 1,
+        backendNodeId: 99,
+        frameId: "main",
+        renderingKind: "canvas",
+        visibleRect: { x: 10, y: 20, w: 80, h: 50 },
+        memberCount: 1,
+      },
+    ];
+
+    const withoutVisuals = renderVom({ ...input, visualSurfaces: [] });
+    const withVisuals = renderVom(input);
+
+    expect(withoutVisuals.refs[0]).toMatchObject({ ref: "e1", backendNodeId: 2, kind: "dom" });
+    expect(withVisuals.refs[0]).toMatchObject({ ref: "e1", backendNodeId: 2, kind: "dom" });
+    expect(withVisuals.refs[1]).toMatchObject({ ref: "e2", backendNodeId: 99, kind: "surface" });
+    expect(withVisuals.text.indexOf('@e1 button "Submit"')).toBeLessThan(
+      withVisuals.text.indexOf("[visual surfaces]"),
+    );
+  });
+
+  it("bounds visual output and prefers explicitly labeled surfaces", () => {
+    const input = scene([node({ id: 1, role: "RootWebArea", frameId: "main" })]);
+    input.visualSurfaces = Array.from({ length: 12 }, (_, index) => ({
+      parentId: 1,
+      backendNodeId: 100 + index,
+      frameId: "main",
+      renderingKind: "canvas" as const,
+      visibleRect: { x: index * 10, y: 20, w: 8, h: 8 },
+      ...(index === 11 ? { label: "Important status" } : {}),
+      memberCount: 1,
+    }));
+
+    const out = renderVom(input);
+    const surfaceRefs = out.refs.filter((ref) => ref.kind === "surface");
+
+    expect(surfaceRefs).toHaveLength(8);
+    expect(surfaceRefs[0]).toMatchObject({ backendNodeId: 111, name: "Important status" });
+    expect(out.text).toContain("… 4 additional visual surfaces omitted");
+    expect(out.truncated).toBe(true);
+  });
+
+  it("does not let a visual surface exhaust the budget before later DOM content", () => {
+    const input = scene([
+      node({ id: 1, role: "RootWebArea", name: "Doc", frameId: "main" }),
+      node({ id: 2, parentId: 1, role: "button", name: "First", tag: "button" }),
+      node({ id: 3, parentId: 1, role: "button", name: "Later", tag: "button" }),
+    ]);
+    input.visualSurfaces = [
+      {
+        parentId: 1,
+        backendNodeId: 99,
+        frameId: "main",
+        renderingKind: "canvas",
+        visibleRect: { x: 10, y: 20, w: 800, h: 500 },
+        memberCount: 1,
+      },
+    ];
+
+    const out = renderVom(input, { maxTokens: 30 });
+
+    expect(out.text).toContain('@e2 button "Later"');
+    expect(out.refs.filter((ref) => ref.kind === "surface")).toHaveLength(0);
+    expect(out.truncated).toBe(true);
+  });
+
  it("does not derive handle context across frame scopes", () => {
    const out = renderVom(
      scene([
@@ -88,6 +88,7 @@ const MAX_CUSTOM_RECOVERY_AREA = 180_000;
 const MAX_RECOVERABLE_SINGLE_INTENT_AREA = 360_000;
 const MAX_HANDLE_CONTEXT_ITEMS = 3;
 const MAX_SURFACE_ITEMS = 12;
+const MAX_VISUAL_SURFACES = 8;
 const MAX_SCOPE_LINES = 40;
 const RECOVERY_HANDLER_ATTRS = ["onclick", "onmousedown", "onkeydown", "onkeyup", "onkeypress"];
 const NATIVE_TAGS = new Set(["button", "input", "select", "textarea"]);
@@ -794,40 +795,81 @@ interface RenderState {
  duplicateRefNames: Set<string>;
  surfaceMap: Map<number, CondSurface>;
  scopeMap: Map<number, ActiveScopeBlock>;
-  visualSurfaces: Map<number | null, VomVisualSurface[]>;
+  visualSurfaces: VomVisualSurface[];
 }

-function groupedByParent<T extends { parentId: number | null }>(
-  items: T[],
-): Map<number | null, T[]> {
-  const grouped = new Map<number | null, T[]>();
-  for (const item of items) {
-    const siblings = grouped.get(item.parentId) ?? [];
-    siblings.push(item);
-    grouped.set(item.parentId, siblings);
-  }
-  return grouped;
+function visualSurfaceArea(surface: VomVisualSurface): number {
+  return Math.max(0, surface.visibleRect.w) * Math.max(0, surface.visibleRect.h);
 }

-function emitVisualEntries(parentId: number | null, depth: number, state: RenderState): void {
-  if (state.stopped) return;
-  for (const surface of state.visualSurfaces.get(parentId) ?? []) {
-    if (depth > state.maxDepth) {
-      state.truncated = true;
+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}]`;
+}
+
+function selectVisualSurfaces(
+  surfaces: readonly VomVisualSurface[],
+  nodesById: ReadonlyMap<number, VomNode>,
+): { selected: VomVisualSurface[]; eligibleCount: number } {
+  const selected: VomVisualSurface[] = [];
+  let eligibleCount = 0;
+
+  for (const surface of surfaces) {
+    if (surface.parentId !== null && !nodesById.has(surface.parentId)) continue;
+    eligibleCount += 1;
+
+    const insertionIndex = selected.findIndex(
+      (candidate) => compareVisualSurfaces(surface, candidate) < 0,
+    );
+    if (insertionIndex < 0) {
+      if (selected.length < MAX_VISUAL_SURFACES) selected.push(surface);
      continue;
    }
+    selected.splice(insertionIndex, 0, surface);
+    if (selected.length > MAX_VISUAL_SURFACES) selected.pop();
+  }
+
+  return { selected, eligibleCount };
+}
+
+function emitVisualAppendix(state: RenderState): void {
+  if (state.stopped) return;
+  const { selected, eligibleCount } = selectVisualSurfaces(state.visualSurfaces, state.nodesById);
+  if (eligibleCount === 0) return;
+
+  const header = "[visual surfaces]";
+  let headerEmitted = false;
+  let emitted = 0;
+  for (const surface of selected) {
    const ref = `e${state.nextRef}`;
-    const label = cleaned(surface.label) ?? `${surface.renderingKind} visual surface`;
-    const layers = surface.memberCount > 1 ? `; layers=${surface.memberCount}` : "";
-    const line = `${"  ".repeat(depth)}@${ref} surface ${JSON.stringify(label)} [rendering=${surface.renderingKind}${layers}; visual-only; requires=image-understanding; use: bsk screenshot --ref @${ref}]`;
-    const nextTokens = state.tokens + estimateTokens(line);
-    if (nextTokens > state.maxTokens) {
-      state.truncated = true;
-      state.stopped = true;
-      return;
+    const line = visualSurfaceLine(surface, ref);
+    const nextTokens =
+      state.tokens + estimateTokens(line) + (headerEmitted ? 0 : estimateTokens(header));
+    if (nextTokens > state.maxTokens) break;
+    if (!headerEmitted) {
+      state.lines.push(header);
+      state.tokens += estimateTokens(header);
+      headerEmitted = true;
    }
    state.lines.push(line);
-    state.tokens = nextTokens;
+    state.tokens += estimateTokens(line);
+    const label = cleaned(surface.label) ?? `${surface.renderingKind} visual surface`;
    state.refs.push({
      ref,
      backendNodeId: surface.backendNodeId,
@@ -840,6 +882,20 @@ function emitVisualEntries(parentId: number | null, depth: number, state: Render
      line: state.lines.length - 1,
    });
    state.nextRef += 1;
+    emitted += 1;
+  }
+
+  const omitted = eligibleCount - emitted;
+  if (omitted > 0) {
+    state.truncated = true;
+    if (headerEmitted) {
+      const summary = `  … ${omitted} additional visual surfaces omitted`;
+      const nextTokens = state.tokens + estimateTokens(summary);
+      if (nextTokens <= state.maxTokens) {
+        state.lines.push(summary);
+        state.tokens = nextTokens;
+      }
+    }
  }
 }

@@ -872,34 +928,24 @@ function emitActiveScopeBlock(scope: ActiveScopeBlock, depth: number, state: Ren
  }
 }

-type RenderTask =
-  | { kind: "node"; node: VomNode; depth: number }
-  | { kind: "visual"; parentId: number | null; depth: number };
-
 function pushRenderChildren(
-  stack: RenderTask[],
+  stack: Array<{ node: VomNode; depth: number }>,
  children: readonly VomNode[],
  depth: number,
 ): void {
  for (let index = children.length - 1; index >= 0; index -= 1) {
-    stack.push({ kind: "node", node: children[index], depth });
+    stack.push({ node: children[index], depth });
  }
 }

 function renderTree(children: Map<number | null, VomNode[]>, state: RenderState): void {
-  const stack: RenderTask[] = [{ kind: "visual", parentId: null, depth: 1 }];
+  const stack: Array<{ node: VomNode; depth: number }> = [];
  pushRenderChildren(stack, children.get(null) ?? [], 1);

  while (stack.length > 0 && !state.stopped) {
-    const task = stack.pop() as RenderTask;
-    if (task.kind === "visual") {
-      emitVisualEntries(task.parentId, task.depth, state);
-      continue;
-    }
-    const { node, depth } = task;
+    const { node, depth } = stack.pop() as { node: VomNode; depth: number };

    if (!shouldRender(node)) {
-      stack.push({ kind: "visual", parentId: node.id, depth });
      pushRenderChildren(stack, children.get(node.id) ?? [], depth);
      continue;
    }
@@ -942,10 +988,8 @@ function renderTree(children: Map<number | null, VomNode[]>, state: RenderState)
    if (scope) emitActiveScopeBlock(scope, depth + 1, state);

    if ((ref && shouldSkipRedundantRefChildren(node)) || shouldSkipRedundantChildren(node, state)) {
-      emitVisualEntries(node.id, depth + 1, state);
      continue;
    }
-    stack.push({ kind: "visual", parentId: node.id, depth: depth + 1 });
    pushRenderChildren(stack, children.get(node.id) ?? [], depth + 1);
  }
 }
@@ -976,7 +1020,7 @@ function renderNodes(
    duplicateRefNames: duplicateReferenceNames(nodes),
    surfaceMap: new Map(surfaces.map((surface) => [surface.triggerId, surface])),
    scopeMap: new Map(activeScopeBlocks.map((scope) => [scope.triggerId, scope])),
-    visualSurfaces: groupedByParent(visualSurfaces),
+    visualSurfaces,
  };

  renderTree(children, state);
@@ -1198,7 +1242,10 @@ function renderDoubleLayer(scene: VomScene, layer: BlockingLayer, options: VomOp
    scene.activeScopeBlocks,
    scene.visualSurfaces?.filter((surface) => visibleSurface(surface.parentId)),
  );
-  state.lines.push(renderPageOcclusionLine(hiddenCount));
+  const occlusionLine = renderPageOcclusionLine(hiddenCount);
+  state.lines.push(occlusionLine);
+  state.tokens += estimateTokens(occlusionLine);
+  emitVisualAppendix(state);

  return {
    text: state.lines.join("\n"),
@@ -1227,6 +1274,7 @@ export function renderVom(scene: VomScene, options: VomOptions = {}): VomResult
    scene.activeScopeBlocks,
    scene.visualSurfaces,
  );
+  emitVisualAppendix(state);

  return {
    text: state.lines.join("\n"),