Skip to content

feat(desktop): element-resolved pointing — the crop names an element (D4) - #488

Merged
physercoe merged 3 commits into
j8-d3-ui-screenshotfrom
j8-d4-element-pointing
Jul 31, 2026
Merged

feat(desktop): element-resolved pointing — the crop names an element (D4)#488
physercoe merged 3 commits into
j8-d3-ui-screenshotfrom
j8-d4-element-pointing

Conversation

@physercoe

Copy link
Copy Markdown
Owner

D4 of docs/plans/desktop-ui-context-and-pointing.md (§3.4 step 5). Stacked on #487 (D3) — review the diff against j8-d3-ui-screenshot.

D2 gave the agent pixels of the region the user dragged over. When that region is a <webview> guest, the bridge can do better: the same accessibility machinery browser_snapshot uses already names the elements under those pixels, so the message carries a structural pointer beside the image — { tab_id, ref: "@e42", role: "button", name: "Deploy" }. "Fix this button" stops being ambiguous.

The chain

  1. hit-test the rect centre — elementFromPoint, then closest() to the nearest interactive ancestor (the user drags over a button's label and means the button)
  2. DOM.requestNodeDOM.describeNode for the backendNodeId
  3. the tab's AX tree, compacted by the same compactAxTree browser_snapshot uses — which is what mints the @eN
  4. Accessibility.getPartialAXTree for role + name

Four decisions

  • The ref is live. The compaction registers its refs against the tab (setSnapshotRefs), so the @e42 in the message is one browser_click resolves. Otherwise the plan's "actionable" claim would be decorative.
  • actionable is a field, not an assumption. Only bridge: 'full' partitions are action-drivable; on kimiweb/rerunweb the note says the ref is for reference, because promising a click there is a lie the agent discovers only on refusal.
  • The pointer rides in the note. postAgentInput carries a body and images — a structured field beside them would be dropped on the floor. So the line lands in the user's draft, above the send button, where they can see and edit what the agent will be told; a chip in the target row shows it before that.
  • Every failure degrades to no pointer. The crop is the deliverable. No debugger, no AX tree, a canvas app with nothing to name: the capture returns unchanged, and role falls back to the DOM tag name rather than inventing one.

Accessible names are flattened and clipped to 80 chars — the pointer is a reference, not a content channel (ADR-062 D-2).

Files

src/state/uiPointer.ts (new) the shared type + its two renderings (user chip, agent line) — import-free, both sides
electron/src/uipointer.ts (new) the CDP resolution sequence
annotation_host.ts resolves after the pixels, registers the refs, folds the pointer into the result + audit
AnnotationOverlay.tsx / state/annotation.ts the chip, and the note fold

Verification

  • npm test (electron): 293 pass — D4 adds 10 driving the whole CDP sequence against a fake sender
  • node --test src/state/*.test.ts src/ssh/*.test.ts: 380 pass — D4 adds 4
  • tsc --noEmit clean both sides · lint-desktop-tokens.sh clean (baseline 65, new .annot-pointer rule added, no phantom class) · lint-docs.sh clean
  • Not run: the live hit-test against a real page — needs a display

Stack: D3 (#487) → D4 → D5 → D6.

🤖 Generated with Claude Code

…(D4)

D2 gave the agent pixels of the region the user dragged over. When that
region is a <webview> guest the bridge can do better: the same
accessibility machinery browser_snapshot uses already names the elements
under those pixels, so the message carries a STRUCTURAL pointer beside
the image — { tab_id, ref: "@e42", role: "button", name: "Deploy" }.
"Fix this button" stops being ambiguous.

The chain: hit-test the rect centre (elementFromPoint, then closest() to
the nearest interactive ancestor — the user drags over a button's LABEL
and means the button) → requestNode/describeNode for the backendNodeId →
the tab's AX tree compacted by the SAME compactAxTree browser_snapshot
uses, which mints the @en → getPartialAXTree for role and name.

Four decisions:

- The ref is LIVE. The compaction registers its refs against the tab
  (setSnapshotRefs), so the @e42 in the message is one browser_click
  resolves — otherwise "actionable" would be decorative.
- `actionable` is a field, not an assumption. Only bridge:'full'
  partitions are action-drivable; on kimiweb/rerunweb the note says the
  ref is for reference, because promising a click there is a lie the
  agent discovers only on refusal.
- The pointer rides IN the note. postAgentInput carries a body and
  images; a structured field beside them would be dropped on the floor.
  So the line lands in the user's draft, above the send button, where
  they can see and edit what the agent will be told — and a chip in the
  target row shows it before that.
- Every failure degrades to no pointer. The crop is the deliverable. No
  debugger, no AX tree, a canvas app with nothing to name: the capture
  returns unchanged, and role falls back to the DOM tag name rather than
  inventing one.

Accessible names are flattened and clipped to 80 chars — the pointer is
a reference, not a content channel (ADR-062 D-2).

14 new tests (10 electron driving the whole CDP sequence against a fake
sender, 4 frontend over the two renderings): 293 electron pass, 380
frontend state/ssh pass. The live hit-test needs the desktop pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@physercoe
physercoe force-pushed the j8-d4-element-pointing branch from 7fd13a0 to d89b004 Compare July 31, 2026 03:16
@agentfleets

Copy link
Copy Markdown
Collaborator

Review of D4 (verified against head d89b004d). The resolution mechanism is well-built: coordinate math is correct (reuses D3's guest-space rect; elementFromPoint/capturePage/element bounds all live in the same DIP space — verified no guest ever sets a zoomFactor), degradation to pixels is honest, and the ref-machinery reuse (same compactAxTree + snapshotRefs as browser_snapshot) is exact. Two majors before merge:

Major M1 — the pointer never reaches the agent on the kimi-web path (the plan's first target)

desktop/src/ui/AnnotationOverlay.tsx:294 sends only { file, guest_id } to annotation_attach_kimi; appendPointerNote runs only in handOffToCompanion (state/annotation.ts:145). So on the LOCAL kimi-web loop — which the plan calls the first priority — the agent receives pixels only, and §3.4 step 5's "the message carries both the image AND a structured pointer" is unmet there.

Worse, the pre-send target-row chip and its tooltip ("the element the agent will be pointed at") render on both paths, so the UI promises what the kimi path doesn't deliver. The D4-as-shipped note discusses only postAgentInput and never acknowledges the gap.

Minimal fixes, increasing effort: (a) document the limitation in the plan + suppress/relabel the chip when only the kimi target exists; (b) inject the pointer line into the kimi composer alongside the file (main-side, same user-gesture posture as the file injection).

Major M2 — setSnapshotRefs clobbers the tab's ref map on every annotation

desktop/electron/src/annotation_host.ts:179 registers unconditionally:

if (resolved === null) return null;
setSnapshotRefs(wc.id, resolved.refs);

snapshotRefs is per-tab latest-wins. If the page mutated since the agent's last browser_snapshot, the D4 re-compaction renumbers the refs — an agent-held @e5 now resolves to a different existing element, and browser_click clicks the wrong element with user authority (no REF_STALE, since the ref still exists). Unlike a snapshot, this replacement is invisible to the agent. It fires even for ref-less resolutions (static text hit still replaces the map) and on every capture over a webtab.

Minimal fix: only register when resolved.pointer.ref !== undefined; better, note the semantics in the plan and consider namespacing annotation-minted refs.

Minor

  • m3 — hit-test races the capture: pointer resolves after capturePage; a navigation/scroll/SPA re-render in between means the pointer names whatever is under the point now, not what the crop shows. The pre-send chip is the mitigation; worth a comment in pointerForGuest.
  • m4 — straddling rects hit-test the clipped rect's centre, not the user's drag centre. Defensible, but a silent choice.
  • m5 — untrusted accessible name rides the user-trusted note channel without a caveat; every content-returning bridge tool carries the UNTRUSTED marker. 80-char clip + flattening keeps blast radius small; a "page-derived label" framing would close it.
  • Nits: "a element"/"a statictext" grammar in describeElement; chip fallback tab ${id} is English-only user-facing UI; formatPointerLabel doesn't escape quotes in names.

Test gaps

Rect→hit-point centre math lives in the host and is untested; the store-level handOffToCompanion folding is untested (a store test would have pinned M1); nothing tests that minted refs actually land in snapshotRefs (the "ref is live" claim) or the M2 clobber behavior; the pointer: {ref, role} audit addition is untested.

Ubuntu and others added 2 commits July 31, 2026 04:02
…stead of clobbering

Review fixes for #488 (D4):

- M1: annotation_attach_kimi now delivers the composed note (user words +
  pointer line) into the kimi composer's text box beside the crop —
  DOM.focus + Input.insertText, same user-gesture posture as the file
  injection; failure is reported (note_injected:false → distinct toast +
  audit flag) instead of silently dropping what the chip promised. The
  overlay composes with the SAME appendPointerNote the companion path uses.
- M2: registering the pointer's ref no longer replaces the tab's ref map
  (which renumbered agent-held refs so a stale @E5 could click a different
  element with no REF_STALE). registerAnnotationRef merges: reuse the ref
  the agent's last snapshot minted for that node, else mint a namespaced
  @an beside the existing entries; ref-less resolutions never touch the
  map. resolvePointer now returns the node to register instead of a map.
- m3/m4: comments — the hit-test races the capture (pre-send chip is the
  mitigation); straddling rects hit-test the clipped rect's centre.
- m5: the agent line frames the accessible name as page-derived ("the
  page labels …") — provenance rides with the label.
- Nits: a/an article agreement, page-authored quotes escaped in the
  quoted span, the chip's tab fallback is localizable (en+zh key).
- Tests: 3 kimi-note injection tests, 2 registerAnnotationRef
  merge/no-clobber regressions, quote-escape + article cases; existing
  pointer tests updated to the new resolution shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@physercoe

Copy link
Copy Markdown
Owner Author

Second review pass (agent): both majors confirmed against code and fixed on the branch — commit 9d32798a (the branch also merges D3's fix f7da605f so the stack stays consistent).

M1 (fixed, option b): annotation_attach_kimi now takes the composed note (user words + pointer line, built with the SAME appendPointerNote the companion path uses) and injects it into the kimi composer's text box beside the crop — DOM.focus + Input.insertText, so it rides the browser's input pipeline (React value-tracker safe) with the same user-gesture posture as the file injection. A failed note injection is reported (note_injected:false → distinct toast + audit flag), never silently dropped. This also delivers the user's typed note on the kimi path, which was previously dropped along with the pointer.

M2 (fixed, the stronger variant): ref registration never replaces the tab map. setSnapshotRefs is gone; registerAnnotationRef(tabId, backendNodeId) merges — if the agent's last snapshot already named the node, that existing ref is reused (the agent recognizes it); otherwise a namespaced @aN is minted beside the existing entries. Ref-less resolutions never touch the map. A later browser_snapshot still replaces the whole map, so @aN expires on the ordinary REF_STALE contract. resolvePointer now returns the node-to-register instead of a whole map; merge/no-clobber pinned by tests.

Minors: m3/m4 comments added (hit-test races the capture; clipped-rect centre); m5 — the agent line frames the name as page-derived (the button the page labels \"Deploy\"); nits — a/an article agreement, page-authored quotes escaped, chip tab-fallback localized (en+zh).

Tests: 3 note-injection, 2 registerAnnotationRef regressions, quote/article cases; electron 307/307, frontend state/ssh green, tsc clean. The store-level handOffToCompanion fold stays untestable under node (the store imports the shell bridge — pre-existing, per annotation.test.ts's own header); the fold's pure half (appendPointerNote) is pinned.

🤖 Generated with Claude Code

@physercoe
physercoe merged commit 684631a into j8-d3-ui-screenshot Jul 31, 2026
8 checks passed
physercoe pushed a commit that referenced this pull request Jul 31, 2026
…490)

The stacked PRs squash-merged into their base branches (the GitHub
stack cascade), so main received only D3 (#487); this merge brings the
cascaded j8-d5-remote-desktop-ui head — element-resolved pointing (D4),
hub-relayed desktop UI context (D5), and agent pointing (D6), each
including its review-fix commit — onto main. Conflicts (main's D3-squash
regions vs the branch's D5/D6-extended versions of the same code)
resolved by taking the branch side, which is a strict superset; the
plan's status block updated to all-shipped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants