perf(trace): capture page state only for commands that change it

[Winify]

Why

In trace mode every adapter took a per-action capture (page source + screenshot, plus DOM/a11y on web) for read and wait commands as well as actions. On iOS a capture is expensive (WDA computes visible for every node): a 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 timeline-rendering table. 5c54ae4 (#247) added the reads (getText, is*, waitFor*, getTitle, …) to ACTION_MAP so they would render as rows, and from then on each of them, including every poll of a waiting matcher, was also captured. One table was answering two questions.

What

• shared: METHOD_EFFECT classifies every trace method as changes or reads. ACTION_MAP is typed against it, so an unclassified entry is a compile error. changesPage(command) is the capture gate.
• service / selenium / nightwatch: capture only when changesPage, plus the first traced command of each test, 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.
• service: the end-of-test capture is stamped at the test's last page change rather than the last row. Otherwise that action's row, and the reads after it, resolve to the capture taken before it.

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.

Verification

• pnpm build, pnpm lint (0 errors), tsc clean on the touched packages, conformance passes, and the dist leak grep is empty.
• pnpm test: 2755 passed, 1 failed. The failure is backend/tests/run-state-continuity.test.ts with EADDRINUSE 127.0.0.1:3000, and it fails the same way on upstream/main.
• New tests cover the gate in each adapter, retrying the first-command capture when it returns nothing, the stamp of the end-of-test capture, and read rows resolving to the latest earlier capture. The test for that stamp fails without the fix.
• Real runtime, base (upstream/main) vs branch, 3 runs each, DEVTOOLS_MODE=trace. "Rows differing" compares each row's resolved snapshot page text against base.

	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.

iOS is not measured. A rerun of the reporting suite on this build would confirm the gain where it matters most.

Known limits

• Nightwatch's describe/it interface fires the plugin's per-test hook once per module, so the first-command capture happens once per module there.
• browser.url() with no argument (a getter) still counts as a page change.
• The Python adapter still captures after every command. Mirroring the table there, with a parity test, is a follow-up.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The change affects trace captures across three adapter workflows.

The PR appears safe to merge, with a non-blocking retry gap when the first screenshot fails.

Findings

1.  Failed screenshot stops later attempts ▶

Summary

Separates timeline rows from commands that need a page capture.

• Trace captures now follow page changes, not every read.

Known limits acknowledged by Winify: Nightwatch BDD captures once per module; getter-only browser.url() still counts as a page change; the Python adapter remains unchanged as a follow-up.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Command[Traced command] --> Gate{Changes page or needs initial frame?}
  Gate -->|Yes| Capture[Capture page state]
  Gate -->|No| Reuse[Use latest earlier capture]
  Capture --> Row[Render timeline row]
  Reuse --> Row

Loading

_{Reviews (1) · Last reviewed commit: "perf(trace): capture page state only for..." · 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