Skip to content

Memoize Playwright page target IDs - #356

Merged
rgarcia merged 2 commits into
mainfrom
hypeship/memoize-page-target-ids
Aug 25, 2026
Merged

Memoize Playwright page target IDs#356
rgarcia merged 2 commits into
mainfrom
hypeship/memoize-page-target-ids

Conversation

@rgarcia

@rgarcia rgarcia commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • cache successful Playwright page-to-CDP target ID lookups in a WeakMap
  • coalesce concurrent discovery and prevent pre-reset work from repopulating the cache
  • refresh cached IDs on resolver retries and reset them on browser disconnect
  • build a per-attempt pageByTargetId index for the tab-to-page join
  • serve each resolution attempt with a single browser-level CDP session (the active-tab listing and every tab-to-page join share one session instead of opening one per active tab)
  • add focused Node tests for reuse, refresh, failures, concurrency, and reset behavior

Stack

Based on #347. Replaces #352/#355 after the resolve_active_page request option was dropped from the stack.

Validation

  • make test-runtime (9/9)
  • go vet ./...
  • Playwright daemon esbuild bundle build

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

reviewed the stacked layer at 16d6b3a. one root CDP session per attempt removes the nested session churn, while the weak cache is refreshed on retry and reset on browser disconnect/timeout. the cache tests cover reuse, failed discovery, refresh, in-flight work, and reset.

validation beyond BugBot: runtime tests and the full non-e2e race suite pass; both Dockerfiles pass build checks; the bundled daemon passed the deterministic cross-context case, 30 rapid cross-origin transitions with no failures/wrong pages, page recreation, and an unmatched raw-CDP tab. with 41 pages, warm active-first/active-last resolution was ~10ms in both positions.

this PR targets the lower stack branch, so the main-targeted image/server workflows have not run on this layer yet. please let those run after #347 merges and this retargets to main, before merging/releasing. approved subject to that normal CI gate.

Base automatically changed from hypeship/fix-cross-context-page to main August 25, 2026 15:16
@rgarcia
rgarcia force-pushed the hypeship/memoize-page-target-ids branch from 16d6b3a to 38956b2 Compare August 25, 2026 15:16
@rgarcia
rgarcia merged commit 8c5de5c into main Aug 25, 2026
10 checks passed
@rgarcia
rgarcia deleted the hypeship/memoize-page-target-ids branch August 25, 2026 15:19
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.

3 participants