[Winify]

What & why

In trace mode every adapter took a per-action capture (page source and screenshot, plus DOM/a11y on web) for read and wait commands as well as for actions. On iOS a capture is expensive, because WDA computes visible for every node. One reported suite made 187 getPageSource calls for 12 interactions and slowed from 3:10 to 7:10.

The capture gate has always been mapCommandToAction, the table that decides which commands render as timeline rows. 5c54ae4 (#247) added the reads (getText, is*, waitFor*, getTitle, …) to ACTION_MAP so they would render as rows. From then on each of them was also captured, including every poll of a waiting matcher. One table was answering two questions.

• shared: METHOD_EFFECT classifies every trace method as changes or reads. ACTION_MAP is typed against it, so an unclassified entry fails to compile. changesPage(command) is the new capture gate.
• service / selenium / nightwatch: capture only when changesPage. The first traced command of each test is also captured, so a test that opens with reads still has a frame. Read and wait rows keep rendering and inherit the latest earlier capture through claimAfter's existing latestAtOrBefore fallback.
• Initial-frame retry: the first capture is retried once, on the next traced command, when it comes back without a screenshot. captureActionSnapshot reports a failed screenshot as a snapshot with no screenshot rather than as null. The retry is capped at one per test, so a driver that cannot take screenshots doesn't capture every read again. The rule is the same in all three adapters.
• service: the end-of-test capture is stamped at the test's last page change rather than at its last row. Otherwise that action's row, and the reads after it, would resolve to the capture taken before it.

Type of change

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Polish (an improvement to an existing feature)
• Breaking change (existing behavior changes for users)
• Documentation
• Internal (build, CI, dependencies, tooling)

Packages touched

• shared (types and contracts)
• core (framework-agnostic capture/reporting)
• elements (published element/snapshot API — @wdio/elements)
• service (WebdriverIO adapter)
• nightwatch-devtools (Nightwatch adapter)
• selenium-devtools (Selenium adapter)
• selenium-devtools-py (Selenium Python adapter)
• backend (server)
• app (UI)
• script (page-injected runtime)
• trace (Trace mode) (tests only)

Notes for reviewers

Why isn't this in core? The gate is one call per adapter, but each adapter captures at a different point: the service before the command, Selenium and Nightwatch after it. The shared part is the classification, and that lives in shared.

Behavior change in the service. The service captures before a command, so an action's row now shows the page just before the next page-changing command, after any waits and reads in between. Before, it showed the page just before the first read that followed. Take click(A) → waitForDisplayed → getText → click(C): A's row used to show the moment after the click, possibly mid-transition, and now shows the settled state. The number of captures per page change is unchanged; only their timing moves. Selenium and Nightwatch capture right after each command, so they are unaffected.

Real runtime, upstream/main vs this branch, 3 runs each, DEVTOOLS_MODE=trace. "Rows differing" compares each row's resolved snapshot page text against the base run. These runs were measured before the retry fix, which only changes the screenshot-failure path.

	captures	wall time (mean)	rows differing from base
WDIO mocha, Chrome	51–52 → 15	34.5 s → 34.9 s	0 / 16
Android native (Appium, emulator)	19 → 8	55.2 s → 45.9 s	0 / 17
Nightwatch (1 branch run)	17 → 10	17.8 s → 17.6 s	0 / 21

The Nightwatch wait after the submit click still resolves to /secure: it inherits the click's own capture, which is taken after the click. Wall time on Chrome doesn't move because captures are cheap there.

Checks: after the rebase onto upstream/main, pnpm build passes, pnpm test reports 2859 passed and 3 skipped, and pnpm lint reports 0 errors. New tests cover:

• the gate in each adapter;
• the initial-frame retry when the first screenshot fails, or when the capture is null;
• the one-retry cap when screenshots always fail;
• the stamp on the end-of-test capture;
• read rows resolving to the latest earlier capture.

Not measured: iOS, the case that motivated this. A rerun of the reporting suite on this build would confirm the gain.

Known limits:

• Nightwatch describe/it: the plugin's per-test hook fires once per module, so the first-command capture happens once per module.
• browser.url() with no argument: this getter still counts as a page change.
• Concurrent commands: in Selenium and Nightwatch the first capture isn't awaited. A traced command issued before it settles doesn't get the retry.
• execute counts as a page change. That includes Appium mobile: reads, which only read.
• Python adapter: it still captures after every command. Mirroring the table there, with a parity test, is a follow-up.

Screenshots / recordings

n/a (no UI change)

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issues remain.

Summary

This PR separates timeline rows from page captures. changesPage now gates captures across the three JavaScript adapters.

• Trace mode captures page state when a command changes it.
• The service ties a test’s final capture to its last page change.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Mapped command] --> B{Changes the page?}
  B -->|Yes| C[Capture page state]
  B -->|No| D{Initial capture needed?}
  D -->|Yes| C
  D -->|No| E[Reuse earlier capture]
  C --> F[Render timeline row]
  E --> F

Loading

_{Reviews (2) · Last reviewed commit: "fix(trace): retry the initial frame only..." · Reviewed by Greptile}

[greptile-apps]

Failed screenshot stops later attempts

If the first screenshot fails, captureActionSnapshot can still return a truthy object with screenshot: undefined. This branch leaves awaitingInitialFrame false, so later reads never retry. FrameSnapshotIndex skips that capture, and Selenium has no final capture to recover it. A read-only test can therefore finish without frames for its rows.

Clear the initial-frame flag only after a usable screenshot arrives. Apply the same check in Nightwatch and the service.

[Winify]

Review

The direction is right and the diff is small. Splitting "does this get a timeline row" from "does this get a capture" via METHOD_EFFECT / changesPage fixes the root cause, and typing ACTION_MAP against METHOD_EFFECT means an unclassified command fails to compile. I also checked that nested execute calls inside web reads (e.g. isDisplayed) don't bring captures back: the service only captures topLevelUserCommand.

I'd ask for one fix before merging and have one design question.

Must fix

1. The initial-frame retry never fires. Greptile's finding is correct, and it's slightly worse than it describes. captureActionSnapshot (packages/core/src/action-snapshot.ts:126-175) catches every error inside and returns an object even when the screenshot probe failed (screenshot: undefined). It only returns null from its outer catch, and nothing in that body realistically throws. So:

• else if (initialFrame && !changesPage(...)) awaitingInitialFrame = true in Selenium (commandPostActions.ts) and Nightwatch (session.ts) never runs in practice.
• In the service, if (snap) this.#awaitingInitialCapture = false clears the flag even when there's no screenshot.
• The new test at selenium-devtools/tests/action-snapshot.test.ts:161 mocks coreCapture to return null, which the real code doesn't do. The test passes on a path that can't happen.
• The result: a test that opens with reads and whose first screenshot fails gets no frame. trace-frame-snapshots.ts:66 skips captures without a screenshot.

Suggested fix: gate on snap?.screenshot in all three adapters, and have the tests mock a capture that returns no screenshot.

Should address

2. The service's action rows now show a later moment (not mentioned in the PR). The service captures before a command and stamps the capture with the timestamp of the last page change. Take click(A) → waitForDisplayed → getText → click(C):

• Before: A's row was captured just before waitForDisplayed, so it showed the state right after the click (spinner, transition).
• Now: A's row is captured just before click(C), so it shows the state after all the waits.

The capture count is the same (one per change); only when it's taken moves. Showing the settled state may well be what we want, but it is a user-visible change. The "0 rows differing" table compares page text on suites that probably have no async transitions, so it wouldn't catch this. Please either state it in the changeset or check it on a suite with a loading state. Selenium and Nightwatch capture right after the command, so the adapters now differ here as well.

3. A late result from one test can flip the next test's flag (Selenium/Nightwatch). The capture promise isn't awaited. If test N's first capture resolves after test N+1 has already taken its initial frame, the retry branch sets awaitingInitialFrame = true again, and the next read gets a capture it didn't need. The cost is one extra capture, but a per-test generation counter would close it cheaply. This only matters once #1 makes that branch reachable.

Nits / questions

• Inconsistent retry rule. Selenium and Nightwatch retry only when the first command was a read (!changesPage). The service retries whenever the capture failed. Pick one rule.
• execute counts as changes. That's safe in general, but on Appium most execute('mobile: …') calls are reads (mobile: activeAppInfo, getContexts, …), and iOS is the platform this PR is meant to help. Does the 187-capture suite use many of these? If so, a follow-up could classify by the first argument.
• iOS isn't measured, as the PR says. The PR's main justification is the iOS slowdown. A single rerun of the suite that reported it, before merging, would confirm the gain and show how much of the remaining capture count comes from execute and url() with no argument.
• The Python adapter now diverges from the JS adapters on capture behavior. It's acknowledged as a follow-up. Please open a tracking issue so it doesn't get lost.

---

Generated by Claude Code

[Winify]

@greptileai review

[vishnuv688]

Thanks for this. Skipping captures for reads, waits and wait polls is the right fix for the iOS slowdown, and generating Python's read list from shared (through wire-schema.json into _contract.py) avoids a hand-copied table. The first-frame retry fix, the per-test generation counter and the url() getter fix all look good, and CI including examples-smoke is green.

I reviewed the current head (1eed261). It has moved past the description, so a few things need to line up before merge.

Before merge

1. Docs and description contradict the code.

   • The description says changesPage is the capture gate in every adapter and that Python is a follow-up. Selenium, Nightwatch and Python now use the skipsCapture deny-list, and Python is changed in this PR.
   • In CLAUDE.md, the new Known debt entry says "Only a command that changes the page is captured (changesPage ...)", while the paragraph added under "One DOM snapshot per action" says Selenium, Nightwatch and Python capture every command except known reads. Please make both describe the current behaviour.

2. The same logic lands in two adapters. Selenium's and Nightwatch's SessionCapturer both gain awaitingInitialFrame, initialFrameRetried, testCaptureGeneration, lastCapturedTimestamp, testStartWallTime and resetTestCapture(), plus the same first-frame retry block in queueActionSnapshot and a near-identical captureFinalFrame. Per CLAUDE.md that belongs in core; SessionCapturerBase looks like the natural home, with each adapter passing only its own captureActionSnapshot.

3. One question, two predicates. The service gates on changesPage and the other adapters on skipsCapture. The service only considers ACTION_MAP commands (#isActionCommand), and for those changesPage(c) === !skipsCapture(c), so the service can use skipsCapture and changesPage can go. If the difference is meant to stay, please say why in the code.

4. Python.

   • _final_frame_stamp says it mirrors core's finalFrameStamp, but there is no section for it in packages/shared/mirror-vectors.json. It is pure and deterministic, so it should get one, with cases for no capture yet, a capture from the previous test, no rows after the capture, and rows after it.
   • instrumentation.py adds seven _state keys. The Known debt entry for that file asks the next change touching _state to split it per concern first; a small TestCaptureState (or a module of its own) would do it for these keys.
   • Nit: the # Frames are screenshot-backed ... comment is duplicated in _capture_final_frame.

Suggestions

5. Selenium JS reads missing from the deny-list. The JS adapter wraps every WebDriver method except INTERNAL_DRIVER_METHODS. Checked against selenium-webdriver 4.50, these reads would now each take a capture: getAllWindowHandles, printPage, getCredentials, getDownloadableFiles. getAllWindowHandles is common in multi-window tests. Worth adding to READ_ONLY_COMMANDS.

6. afterActions fires on failure too. Its comment in types.ts says it fires once perform() succeeds, but patchedPerform calls it from finally, so a rejected perform() also queues a capture, unlike every other command, where an error skips it. Either move the call to the success path or update the comment and say why a failed sequence should capture.

Questions and nits

• iOS is still unmeasured, and it is the case this PR exists for. A run of the reporting suite on this build before merge would confirm the gain. On Python a screenshot is still taken on every command, so there only the element-tree reads are saved; worth saying in the fragment.
• wire-schema.json now carries actionCommands, which is not part of the wire format. ADAPTERS.md documents it, so this is fine if intended; just checking it was a deliberate choice over a separate generated file.
• Nightwatch session.ts: the doc comment on captureTraceAtTestEnd was replaced by the one for the new captureFinalFrame; please keep both.

[Winify]

@vishnuv688 I have measured the changes on BrowserStack against iOS

iOS measurement (BrowserStack App Automate, iPhone 17 / iOS 26)

Base is main at 269a4365. Head is this branch at 1eed2617 plus the fixes for the review above (a83e6309). Both were measured on the WDIO service with the BrowserStack sample app (native), in trace mode with traceGranularity: 'session'. Each side ran 3 times, alternating. The spec has two tests, and each runs a click, waitForDisplayed, getText, isDisplayed, setValue, two mobile: execute reads, the polling matchers toHaveText and toBeDisplayed, isEnabled, and a back navigation. Request counts come from the BrowserStack session logs.

Median	main	head	Change
Mocha duration	27.9 s	17.0 s	-10.9 s
BrowserStack session duration	49 s	35 s	-14 s
GET /source	26	8	-18
GET /screenshot	26	8	-18
All driver requests	97	61	-36

• Every head run was faster than every main run. The spread within each side was 2.5 s on main and 0.7 s on head.
• Every action row still resolves to a frame in both trees. The trace zip shrinks from about 13.5 MB to 10.5 MB.
• Head shows 26 rows against 22 on main. The extra rows are isDisplayed polls nested under each waitForDisplayed, which comes from the wait-nesting change, not from the capture gate.

Smoke runs on head, one each, all passed with a trace written and no unresolved rows: Selenium JS on iOS web and on native, Python on iOS web, and Nightwatch on iOS web. Nightwatch native ran twice on head and wrote a full trace both times. The earlier trace write failed: base64.startsWith is not a function did not recur in those 2 runs.

The reported suite itself (187 page-source reads for 12 interactions) was not rerun. These numbers come from the sample app.

[vishnuv688]

Thanks, this addresses the review, and the iOS numbers settle the open question (GET /source 26 to 8, Mocha 27.9 s to 17.0 s, every head run faster than every main run).

Three small text fixes left, then this is good to merge from my side:

1. The PR description is still the first version. It names changesPage as the gate in every adapter, says iOS is not measured, and lists the Python adapter as a follow-up. Please update it to the deny-list gate, link the iOS measurement above, and drop the Python follow-up.
2. Changeset: the last sentence, "The WDIO service keeps its page-changing gate.", is out of date now that the service uses skipsCapture too. Something like "The WDIO service applies the same gate to its action rows and keeps capturing before the next command." would match CLAUDE.md.
3. CLAUDE.md, "Logic implemented in both TypeScript and Python": the "Covered today" list should name the new section (end-of-test frame stamp).

[vishnuv688]

LGTM