Skip to content

chore(trace): remove snapshot pointers from action events - #42431

Merged
Dmitry Gozman (dgozman) merged 3 commits into
microsoft:mainfrom
dgozman:chore-remove-action-page-id
Aug 28, 2026
Merged

chore(trace): remove snapshot pointers from action events#42431
Dmitry Gozman (dgozman) merged 3 commits into
microsoft:mainfrom
dgozman:chore-remove-action-page-id

Conversation

@dgozman

Copy link
Copy Markdown
Collaborator

Summary

  • Actions no longer point at their DOM snapshots. Snapshots are addressed by callId + phase, the same way screenshots and aria snapshots already are.
  • Removes BeforeActionTraceEvent.pageId, beforeSnapshot, inputSnapshot and afterSnapshot, plus the now unused CallMetadata.pageId / CallMetadata.frameId.
  • Page entries in the model come from page open/close events; internal pages are no longer reported to the instrumentation.
  • trace snapshot takes --phase <before|action|after> instead of --name <before|input|after>.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Snapshots are now addressed by their name, which includes the call id and
is therefore unique across the trace, with an optional frameId query
parameter for child frames. This makes `BeforeActionTraceEvent.pageId`
unnecessary, along with `CallMetadata.pageId` and the never used
`CallMetadata.frameId`.
The list of pages in the model is now derived from page open/close events.
To keep it complete, tracing reports the already open pages when a chunk
starts, and no longer reports internal pages, e.g. the one used to collect
the storage state, to the instrumentation.
Frame snapshots already carry the call id, so recording their name in the
before/input/after action events was redundant. Snapshots now carry the
`phase` they were captured at, matching how screenshots and aria snapshots
are already keyed, and actions no longer point at them at all.
Which phases have a snapshot is derived from the recorded snapshots, so an
action whose capture failed now correctly falls back to a neighbouring
snapshot instead of pointing at one that was never written.
The `trace snapshot` command takes `--phase <before|action|after>` instead
of `--name <before|input|after>`.
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

1 failed
❌ [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport @chromium-ubuntu-22.04-node22

3 flaky⚠️ [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup `@chromium-ubuntu-22.04-node24`
⚠️ [webkit-library] › library/browsercontext-storage-state.spec.ts:543 › should not leave IndexedDB connections open `@webkit-ubuntu-22.04-node20`

51249 passed, 1240 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

3 failed
❌ [chromium] › mcp/clipboard.spec.ts:19 › clipboard write without permission dialog @mcp-macos-latest-chromium
❌ [chromium] › mcp/clipboard.spec.ts:19 › clipboard write without permission dialog @mcp-windows-latest-chromium
❌ [chromium] › mcp/clipboard.spec.ts:19 › clipboard write without permission dialog @mcp-ubuntu-latest-chromium

8256 passed, 1361 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Hi, I'm the Playwright bot and I took a first look at the CI failures here.

🟢 The one failure is a pre-existing screencast flake — this PR looks clear

library/video.spec.ts:664 › screencast › should capture full viewport failed on chromium-ubuntu-22.04-node22, but it's a well-known pixel-timing flake and the identical failure rescued itself on retry in this very same run on another bot. This PR removes snapshot pointers from action events (trace addressing by callId + phase) — it doesn't touch video/screencast capture. The 3 flaky results need no triage.

Details

This PR changes the trace model, snapshotter, trace-viewer and the trace snapshot CLI. The failing test asserts on captured screencast frame pixels, which none of that code path exercises.

Pre-existing flake / infra

  • [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport@chromium-ubuntu-22.04-node22 — the test fails with Expected all pixels to satisfy isAlmostRed, found bad pixel (255, 255, 255, 255): a frame that should be solid red is momentarily white, the classic screencast render-timing flake. Across the test-results DB this test is chronically rescued on retry — 334 retry-rescues and only 1 final failure ever recorded (this run). It flakes on push (main-branch, no PR) and unrelated PRs alike, so the PR can't be responsible.

    The proof it's the same flake here: in this same run (33158602402) it failed on chromium-arm-node20 too with the identical isAlmostRed signature, but a retry rescued it there (so it's reported only as flaky) — on node22 it happened to exhaust all four attempts and went red. Same flake, worse luck.

    Final-verdict history on chromium-ubuntu-22.04-node22 (this PR is the trailing 🟥; the next main run right after is green):

    🟩🟩🟥🟩

    History on chromium-ubuntu-22.04-arm-node20 (🟧 = a retry rescued a failed attempt — 65 of 653 runs; this PR is the trailing 🟧):

    🟧🟩

Not triaged (flaky, passed on retry)

  • [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport@chromium-ubuntu-22.04-arm-node20, [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup@chromium-ubuntu-22.04-node24, [webkit-library] › library/browsercontext-storage-state.spec.ts:543 › should not leave IndexedDB connections open@webkit-ubuntu-22.04-node20 — reported flaky, all rescued on retry, none in the trace/snapshot code paths this PR changes.

Triaged by the Playwright bot - agent run

@dgozman
Dmitry Gozman (dgozman) merged commit 9a450a2 into microsoft:mainAug 28, 2026
42 of 46 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Hi, I'm the Playwright bot and I took a look at the CI failures here.

🟢 Both failures are pre-existing flakes/infra — this PR looks clear

The latest runs report a video.spec.ts screencast failure and three mcp/clipboard.spec.ts failures. Both fail the same way on main and on unrelated PRs, so neither is caused by this PR. This PR only reworks trace snapshot addressing (action events, snapshotter, trace-viewer, trace snapshot CLI) — it doesn't touch video recording or the MCP clipboard flow.

Details

Pre-existing flake / infra

  • [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport@chromium-ubuntu-22.04-node22 — a classic screencast pixel-timing flake: the frame comes back white (bad pixel (255,255,255,255), expected almost-red). It flaked (rescued on retry) on @chromium-ubuntu-22.04-arm-node20 in this very run, and across the results DB it's rescued on retry 299 times on shas unrelated to this PR, failing across many bots (arm, macos, ubuntu node20/22/24). 2639 passing final verdicts vs this single first-attempt failure. Nothing this PR changes affects screencast pixels.

  • [chromium] › mcp/clipboard.spec.ts:19 › clipboard write without permission dialog@mcp-{ubuntu,windows,macos}-latest-chromium — same error on all three OSes: the clipboard reads back "" instead of "Hello from Playwright!". It's currently failing everywhere, not just here — same failure on the main push 6748802c (this PR's own base, no PR involved) in run 33157511907, and on unrelated PRs #42440, #42441, #42442 and #42434 within the same window. It's an environment/permission issue (first seen 2026-08-21, spiking now), not something this PR could cause.

Failing on main and four other PRs at the same time is the tell — this is shared infra noise, not a regression from this PR.

Triaged by the Playwright bot - agent run

Sign up for freeto 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

@dgozman@pavelfeldman