Skip to content

perf(agent): read a shared scope's text once per snapshot - #984

Merged
Kikobeats merged 3 commits into
masterfrom
codex/perf-agent-scope-text
Oct 9, 2026
Merged

Kikobeats merged 3 commits into
masterfrom
codex/perf-agent-scope-text

Conversation

@Kikobeats

@Kikobeats Kikobeats commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Controls that share a parent were each reading that parent's text twice (guard, then scopes). On a 400-cell grid that took the in-page snapshot from 47.6 ms to 11.4 ms, under the 29.9 ms baseline from v14.0.2.
  • The memo lives only for the snapshot call. cache.guard() with no reader still reads live, so a text change after observe stays stale.
  • shared scope text is read once per snapshot fails the agent test job if a shared scope is read once per action again. A wall-clock budget on the runner would flake; the read count is the cost that showed up.

Test plan

  • pnpm exec ava test/snapshot.js (28 passed)
  • Grid fixture, same Chrome, 1280×800: in-page p50 47.6 ms → 11.4 ms, 252 actions and 616 scopes unchanged
  • CI: Test @browserless/agent

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Actions now use consistent text from their surrounding area when describing context, including when multiple actions share that area.
    • Changes to surrounding text are detected, helping prevent actions from proceeding with outdated context.
    • Scope text is limited in length to keep action context concise and readable, while preserving enough surrounding information to describe actions accurately.

Controls in the same parent were each reading that parent's text twice, which is what slowed the grid snapshot. The input-time guard still reads live.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9b9fb613-e599-4ea5-ae63-c49b87d36c1a

📥 Commits

Reviewing files that changed from the base of the PR and between 6dc080b and c8adf33.


📒 Files selected for processing (2)
  • .github/workflows/pull_request.yml
  • packages/agent/test/chrome.js

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.



📝 Walkthrough

Walkthrough

Snapshot scope text is cached per snapshot call and shared by guard data and action scope summaries. Tests verify that shared scope text is read once and that text changes invalidate action freshness. A browser test compares snapshot timing with a base version.

Changes

Snapshot scope text

Layer / File(s) Summary
Read and reuse scope text
packages/agent/src/snapshot.js, packages/agent/test/snapshot.js
A snapshot-local reader caches scope text for guard data and action scope summaries. Guard text remains limited to 6,000 characters, and action scope summaries remain limited to 1,000 characters. Tests check that shared text is read once and that text changes invalidate action freshness.
Compare snapshot performance
packages/agent/test/chrome.js, .github/workflows/pull_request.yml
A serial browser test compares snapshot timing with a base source across three page shapes. The workflow fetches the pull request base commit and sets PERF_BASE for pull request events.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor


Merge Risk: ⚪ Minimal · up to c8adf

The change makes snapshot scope text reads once per snapshot and adds a base-versus-head performance comparison to the tests. No concrete merge-blocking risk remains from this review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main performance change: reading shared scope text once per snapshot.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
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.


✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coveralls

coveralls commented Oct 9, 2026 •

Copy link
Copy Markdown

Coverage Status

coverage: 77.158%. first build — codex/perf-agent-scope-text into master

Kikobeats and others added 2 commits October 9, 2026 10:07
Absolute milliseconds move with the runner. The budget is the snapshot's time against a rect walk on the same page, which is where the shared-scope regression showed up.

Co-authored-by: Cursor <cursoragent@cursor.com>
A fixed ceiling still lets a slower snapshot pass. Each pull request times its snapshot against the base commit on the same page.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Kikobeats

Copy link
Copy Markdown
Member Author

Snapshot performance

In-page snapshot p50, same Chrome, 1280×800, interleaved with the base commit. A later change to snapshot.js has to stay at or under these base numbers. The agent test fails when a fixture is more than 10% slower and more than 1 ms slower. That band is timer noise. Identical source skips the timing.

Fixture This PR Base
Listings 0.9 ms 0.9 ms
Grid 11.8 ms 49.7 ms
Long document 9.2 ms 8.6 ms

@Kikobeats
Kikobeats merged commit 23987d6 into master Oct 9, 2026
29 checks passed
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