Skip to content

fix(macos): give snapshot nodes with a non-finite AX rect no rect - #3324

Merged
thymikee merged 2 commits into
callstack:mainfrom
janicduplessis:fix/macos-snapshot-nonfinite-rect
Oct 9, 2026
Merged

thymikee merged 2 commits into
callstack:mainfrom
janicduplessis:fix/macos-snapshot-nonfinite-rect

Conversation

@janicduplessis

@janicduplessis janicduplessis commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

On macOS, rectAttribute built RectResponse straight from the AX position and size. SwiftUI reports lazy containers (LazyVGrid, LazyVStack) scrolled or sized out of view as an AXOpaqueProviderGroup at (inf, inf, 0, 0). JSONEncoder throws on that value, so one node failed the whole snapshot: EncodingError.invalidValue: inf (Double). Path: data.nodes[N].rect.x.

rectAttribute now returns nil when any component is non-finite. Nodes already allow a nil rect (they are not hittable), and the other rectAttribute callers already handle nil. Touches 2 files in apple/macos-helper.

The iOS runner and Android paths were not changed: the runner already skips null/empty frames and I found no same-pattern encoding of raw AX doubles there. I did not exercise them with a non-finite frame.

Related to #3323 (independent, no dependency). No existing issue found.

Validation

Tested at 6ddff25.

  • pnpm check:affected --run: all runnable checks passed (macos-helper lanes are GitHub-authoritative).
  • pnpm test:macos-helper: passes, including RectResponseTests on the pure finiteRectResponse(position:size:) that rectAttribute calls: a finite rect is kept, and +inf, -inf and NaN in each component give nil.
  • Live run (evidence in this comment): on a SwiftUI app with an AXOpaqueProviderGroup at (inf, inf, 0, 0), a helper built from main fails snapshot with EncodingError.invalidValue: inf; a helper built from this branch succeeds, with that node present, no rect, hittable: false, and the nodes after it listed. The live helper was built from 099be379a, which has the same content as 6ddff25. Only +inf occurred live; NaN and -inf are covered by the unit tests, and only the live run covers the rectAttribute wiring.

View guided diff

@thymikee

thymikee commented Oct 8, 2026

Copy link
Copy Markdown
Member

The macOS change is small and reads correctly, but nothing yet shows it fixes the reported failure at 38fb7bf. rectAttribute now returns nil for any non-finite component, so a node with an infinite rect should no longer break encoding. That is the device-facing AX snapshot route, and the PR says it was not run live. The unit tests cannot reach rectAttribute, because it reads a live AXUIElement (SnapshotTraversal.swift#L813). So nothing shows that a snapshot of a macOS app with an offscreen LazyVGrid or LazyVStack now succeeds, that the AXOpaqueProviderGroup node's children and siblings still come through, or that other non-finite values such as a NaN size behave. Please run the repo CLI on a native macOS app with a scrolled-out SwiftUI lazy container, using a helper built from this head. The app that produced the original EncodingError.invalidValue: inf is the right one. Show snapshot succeeding where it failed before, the inf-rect node present with no rect and hittable=false, and the nodes after it still listed. Please keep that output as the evidence.

Not blocking: testUnguardedInfiniteRectFailsEncoding checks Foundation's JSONEncoder, not code in this repo, and no test goes through rectAttribute or snapshot node building, so restoring the unguarded constructor would pass every new test. You could extract a pure position/size to RectResponse? function and test that, or state in the PR that this is untestable without an AX seam. Either is fine, and you can take or leave this.

The one failing check is the lifecycle.test.ts crashed-helper test. It is a mock-based iOS Simulator bridge test that timed out by a few milliseconds on a Linux runner. This PR touches only macOS helper Swift files, so I think it is unrelated, but I did not rerun it, and I could not confirm the macOS helper Swift lanes passed. There are no conflicts. Before merge, the live macOS snapshot run above needs to be attached.

@janicduplessis

Copy link
Copy Markdown
Contributor Author

Ran it live. Target was Stim Desktop built before our fix that gave lazy containers a finite frame, with the repo CLI, native macOS backend and --surface app. The app has an AXOpaqueProviderGroup at (inf, inf, 0, 0) inside the Overview scroll area.

  • Helper built from main (24b2ce6): snapshot fails with EncodingError.invalidValue: inf (Double). Path: data.nodes[157].rect.x.
  • Helper built from this PR: snapshot succeeds. The raw output has that node (e158, AXOpaqueProviderGrid) with no rect and hittable: false. The scrollbar, value indicator, buttons and toolbar after it are all listed. The sibling grid before it keeps its rect.

Full output is in the collapsed section below. Only +inf appeared live. NaN and -inf are covered by unit tests.

I took the optional suggestion. The guard is now a pure finiteRectResponse(position:size:) that rectAttribute calls. The tests cover +inf, -inf and NaN in each component, and the Foundation encoder test is gone. Swift helper tests, lint, format, typecheck and build pass locally. The lifecycle.test.ts flake did not reproduce locally and I left it alone.

Live snapshot evidence
# Evidence for callstack/agent-device#3324
Date: Thu Oct  8 21:06:12 EDT 2026
Target: Stim Desktop built at stim commit 6ee26cfb8 (parent of #3025), bundle dev.stim.desktop.development.stim.f056b2fa5c03, pid 82217 (running via stim macos)
CLI: repo checkout of PR head branch (099be379a); env AGENT_DEVICE_MACOS_APP_BACKEND=native, private STATE_DIR/CLAIMS_DIR
Base helper: upstream/main 24b2ce638; Head helper: 099be379a

## Non-finite AX elements in the app (scripts/ax-finite.swift from #3025)
NON-FINITE pos=Optional((inf, inf)) size=Optional((0.0, 0.0)) frame=Optional((inf, inf, 0.0, 0.0))
  AXApplication[id= title=StimDesktop · ad3324-pre3025-desktop] > AXWindow[id=main title=Stim] > AXGroup[id= title=] > AXSplitGroup[id=main, SidebarNavigationSplitView title=] > AXGroup[id= title=] > AXScrollArea[id= title=] > AXOpaqueProviderGroup[id= title=]
389 elements, 1 non-finite

## 1. BEFORE: helper built from PR base
$ snapshot --platform macos
Opened: dev.stim.desktop.development.stim.f056b2fa5c03
Error (COMMAND_FAILED): EncodingError.invalidValue: inf (Double). Path: data.nodes[157].rect.x. Debug description: Unable to encode Double.inf directly in JSON.
Hint: Retry with --debug and inspect diagnostics log for details.
Diagnostic ID: mv09lioq-0d9ad4c8
exit=1

## 2. AFTER: helper built from PR head
Opened: dev.stim.desktop.development.stim.f056b2fa5c03
$ snapshot --platform macos
exit=0
Page: dev.stim.desktop.development.stim.f056b2fa5c03
App: dev.stim.desktop.development.stim.f056b2fa5c03
Snapshot: 171 nodes
@e70 [AXUnknown] merges ~11 labels into a single accessibility element. The app likely marks a container as accessible, which hides every descendant from assistive tech and automation — the children
--- tail of tree (nodes around the inf-rect node)
87:        @e131 [axopaqueprovidergrid]
88-          @e132 [button] "ad3324-pre3025"
89-          @e133 [button] "This is the viewer app. View its window from another Stim Desktop instance or your phone."
90-          @e134 [button] "fix/3051-auto-update-build-machines"
91:        @e154 [axopaqueprovidergrid]
92-        @e155 [axscrollbar] [scrollable]
93-          @e156 [axvalueindicator]
94-          @e157 [button]
95-          @e158 [button]
96-          @e159 [button]
97-          @e160 [button]
98-        [content below scroll-area hidden]
99-    @e161 [axtoolbar]

$ snapshot --platform macos --raw   (rect / hittable fields)
exit=0
{'index': 134, 'ref': 'e135', 'role': 'AXOpaqueProviderGroup', 'subrole': 'AXOpaqueProviderGrid', 'hittable': True, 'rect': {'height': 1431, 'width': 615, 'x': 754, 'y': 257}, 'parentIndex': 132}
{'index': 157, 'ref': 'e158', 'role': 'AXOpaqueProviderGroup', 'subrole': 'AXOpaqueProviderGrid', 'hittable': False, 'rect': None, 'parentIndex': 132}
--- nodes after the inf-rect node (index > that node):
nodes without rect: [(157, 'e158', 'AXOpaqueProviderGroup', 'AXOpaqueProviderGrid', False)]
listed after: [(158, 'e159', 'AXScrollBar', None), (159, 'e160', 'AXValueIndicator', None), (160, 'e161', 'AXButton', None), (161, 'e162', 'AXButton', None), (162, 'e163', 'AXButton', None), (163, 'e164', 'AXButton', None), (164, 'e165', 'AXToolbar', None), (165, 'e166', 'AXButton', 'Hide Sidebar'), (166, 'e167', 'AXGroup', None), (167, 'e168', 'AXButton', 'Back'), (168, 'e169', 'AXButton', 'Back'), (169, 'e170', 'AXGroup', None), (170, 'e171', 'AXButton', 'CPU details'), (171, 'e172', 'AXProgressIndicator', 'Memory details'), (172, 'e173', 'AXButton', 'Disk details'), (173, 'e174', 'AXButton', 'Notifications'), (174, 'e175', 'AXButton', None), (175, 'e176', 'AXButton', None), (176, 'e177', 'AXGroup', None), (177, 'e178', 'AXGroup', None), (178, 'e179', 'AXButton', None)]
total nodes 179
Error (SESSION_NOT_FOUND): No active session
Hint: Run open first or pass an explicit device selector.

@thymikee

thymikee commented Oct 9, 2026

Copy link
Copy Markdown
Member

This PR is ready. The earlier review at 38fb7bf was waiting on evidence, and the live macOS run you pasted now covers it. At the reviewed commit 6ddff25 the non-finite rect fix and its tests look correct, and all 13 checks pass. The earlier lifecycle.test.ts timeout was in an iOS bridge mock test that this diff does not touch, and it does not recur at this head. There are no conflicts, and no review threads are open. You can take the PR out of draft.

I did not re-run the live snapshot or the Swift tests. I relied on your pasted output. The live helper was built from 099be379a, which I could not check out here. I took it as equivalent to 6ddff25 because the delta is a pure extraction. Only +inf occurred live. NaN and -inf are covered by the unit tests only.

Not blocking, and you can take or leave these: the tests in RectResponseTests.swift call finiteRectResponse directly and never reach rectAttribute, so only the live run covers that wiring. The PR body validation section still says "Tested at 38fb7bf" and "Not run live", and it still says an unguarded inf rect fails encoding. Updating it to cite the live run and the head commit would help the next reader.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 9, 2026
@janicduplessis
janicduplessis marked this pull request as ready for review October 9, 2026 02:36
Copilot AI balanced review requested due to automatic review settings October 9, 2026 02:36

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

View guided diff | Re-trigger cubic

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused fix is correct, comprehensively tested, and supported by live macOS validation.

0 open findings

What changed in this PR

Prevents macOS snapshots from failing JSON encoding when AX elements report non-finite geometry.

Changes:

  • Converts only finite AX geometry into snapshot rectangles.
  • Adds coverage for infinity, negative infinity, NaN, and valid rectangles.
File Description
SnapshotTraversal.swift Omits invalid AX rectangles from snapshot nodes.
RectResponseTests.swift Tests finite and non-finite geometry handling.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@thymikee
thymikee merged commit 5511ada into callstack:main Oct 9, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants