Skip to content

[Perf-SS] Skip unused accessibility summary walks - #13559

Open
nwparker wants to merge 1 commit into
stablyai:mainfrom
nwparker:nwparker/perf-28-lazy-accessibility-summaries
Open

[Perf-SS] Skip unused accessibility summary walks#13559
nwparker wants to merge 1 commit into
stablyai:mainfrom
nwparker:nwparker/perf-28-lazy-accessibility-summaries

Conversation

@nwparker

Copy link
Copy Markdown
Contributor

Summary

Linux and Windows accessibility snapshots used to perform a depth-four descendant text scan for every node, even though the result can only be published for an unnamed, value-empty generic container. This moves that scan behind the existing eligibility predicate while keeping the row/list-specific summary path unchanged.

ELI5: every item used to ask its descendants for a detailed summary, then most items threw the answer away. Now only the few container items that can show that summary ask for it.

The PR also adds direct Linux and Windows renderer harnesses to the native verification gate. They cover the optimized paths plus anonymous summaries, row summaries, wrapper elision, fail-soft child reads, and node/depth limits.

Performance proof

A controlled harness loaded the exact parent and candidate runtime.py sources, rendered the same counter-backed accessibility trees, and canonicalized the outputs. Each timing used 5 warmups and 101 alternating parent/candidate pairs, with GC outside the timed region.

Linux fixture Parent calls Candidate calls Parent median Candidate median Parent p95 Candidate p95
1,195-node grid 47,662 28,272 (-40.7%) 16.885 ms 10.562 ms (-37.4%) 18.423 ms 11.506 ms (-37.5%)
1,093-node balanced rows 61,509 34,087 (-44.6%) 22.321 ms 13.023 ms (-41.7%) 32.796 ms 18.243 ms (-44.4%)
200-record summary-heavy cards 11,658 11,556 (-0.9%) 4.274 ms 4.259 ms 7.066 ms 6.008 ms

The grid's child-enumeration calls fell from 7,782 to 4,578 (-41.2%); balanced rows fell from 14,778 to 7,748 (-47.6%).

A source-faithful Windows model showed the same deleted work without making an unverified wall-time claim:

Windows fixture Parent FindAll Candidate FindAll
1,196-node grid 4,600 2,390 (-48.0%)
1,300-child node-cap case 4,899 2,399 (-51.0%)
12-tab browser compaction 53 14 (-73.6%)
depth-limit case 650 325 (-50.0%)

Caveat: the timing harness uses in-process fake accessibility objects, so these are controlled renderer costs, not live AT-SPI/UIA device timings. The deterministic reduction in accessibility getter/child calls is the primary proof; real providers make those calls cross process.

No-regression proof

  • Four Linux benchmark shapes and seven Windows-shaped fixtures retained byte-identical serialized records, lines, indices/runtime IDs, and truncation metadata.
  • An independent differential review matched 2,000 randomized Linux trees in both browser-compaction modes.
  • Anonymous generic containers still run the same depth-four summary in the same order.
  • Rows, data items, and list items still run their existing depth-three summary.
  • Elision, compact-control suppression, duplicate runtime IDs, browser-tab compaction, defunct-node fallbacks, node/depth limits, and secure-value handling were reviewed and exercised.
  • There is no cache, retained accessibility object, new retry, persistent state, payload/schema/opcode, or dependency change.
  • Mobile, SSH, relay, folder-workspace, Git-provider, and mixed-version wire surfaces are untouched.

Screenshots

No visual change.

Testing

  • pnpm lint
  • pnpm typecheck
  • pnpm test — direct full Vitest passed 4,582 files / 49,200 tests. The local Node 24 node-pty mismatch failed two unrelated fd tests; one renderer test timed out under full-suite contention and passed focused; the cross-version checkout harness independently reproduces a missing staging-directory failure.
  • pnpm build
  • Added high-quality Linux and Windows renderer regression tests
  • pnpm verify:computer-native — 72 macOS tests, 8 Linux renderer tests, syntax/guardrails, and signed helper verification passed
  • 26 focused computer-use files / 219 tests
  • Changed-code quality, max-lines, reliability, formatting, and diff checks

AI Review Report

Three independent final reviews found no P0/P1/P2 issues across Linux semantic equivalence, PowerShell 5.1/UIA behavior, failure/mutation handling, resource bounds, platform/SSH/folder behavior, remote-wire compatibility, packaging, and performance-claim accuracy.

Review caught an initial test-harness fidelity bug before commit: a PowerShell PSCustomObject collection would not enumerate like AutomationElementCollection. The harness was replaced with a CLR IEnumerable/Count/Item collection, expanded with failure and boundary cases, and re-reviewed clean.

The readiness audit also checked crash/retry/growth, data loss, mobile backward compatibility, macOS/Linux/Windows paths and shells, and Electron packaging. No release-blocking or acceptance-required finding remains.

Security Audit

No production input handling, command execution, auth, secret, network, IPC, dependency, lockfile, or binary surface changes. The optimization removes read-only accessibility calls and introduces no persistent data. Test-only PowerShell compiles fixed local fake types; its GUID-scoped handshake file is created under the system temp directory and removed in finally. Native tests are not copied into the packaged runtime.

Notes

  • The new PowerShell renderer harness runs on the Windows CI verifier; this macOS host could statically validate it but could not execute Windows UI Automation locally.
  • No mobile-facing surface touched.
  • No visual change.
  • X: @nwparker_

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The Linux and Windows accessibility renderers now restrict generic text summaries to unnamed, valueless containers with sufficient plain-text snippets. New platform-specific tests use fake accessibility and UI Automation objects to validate rendering, traversal failures, wrapper elision, and node or depth limits. Native verification now runs the Linux test and enables the Windows PowerShell test on Windows.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main performance optimization in the pull request.
Description check ✅ Passed The description includes all required sections and clearly documents the change, testing, review, security audit, and known full-suite limitations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a5033f5-9b1d-4b61-a63c-7ce2f62b7138

📥 Commits

Reviewing files that changed from the base of the PR and between bbcf89e and 3464be8.

📒 Files selected for processing (5)
  • config/scripts/verify-computer-native.mjs
  • native/computer-use-linux/runtime.py
  • native/computer-use-linux/runtime_render_test.py
  • native/computer-use-windows/runtime-render.test.ps1
  • native/computer-use-windows/runtime.ps1

Comment thread config/scripts/verify-computer-native.mjs
Comment thread native/computer-use-linux/runtime.py
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.

1 participant