diff --git a/CLAUDE.md b/CLAUDE.md index 0525422a..6d2204f4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -265,6 +265,12 @@ Documented divergences from the conventions above. They exist today as debt to b - **A command row is stamped at COMPLETION, and the DOM anchor carries the document's own birth time.** These two together are what make the replay line up; both adapters got them wrong in the same way and the fix is symmetric. (a) `selenium-devtools/src/driverPatcher.ts` and `nightwatch-devtools/src/helpers/browserProxy.ts` both ran their capture at completion but stamped `timestamp` with the *invocation* clock, keeping the invocation time as `startTime` only after this fix. The page-side mutation stream is on real time, so an invocation-stamped row ended before its own effect landed and replayed the page from before it — the `#username` fill rendered an empty field, the `#password` fill rendered only the username, and a navigation row rendered the page it had just left. Rows also now span their real duration instead of a synthetic 1 ms. (b) `collector.captureCurrentDom` (the only producer of a mutation with a `url`) stamps `performance.timeOrigin`, not the drain clock. A drain is forced from Node whenever a collector might be fresh, which is always after the navigation — a round trip at best, a whole page load at worst — so drain-stamping put the anchor after several later actions (measured: 9/15 Selenium and 8/15 Nightwatch rows on the wrong DOM). With both in place a navigation row ends after its destination document was born, so the anchor needs no repositioning at all. - `core/trace-mutations.ts` `reattributeDomAnchors` remains as a narrow backstop for the one case the stamps can't cover: an anchor born *after* the last logged command, i.e. a click whose navigation commits once the click has already returned. It snaps such an anchor to the newest logged command, but **only when no logged command completed after it** — if one did, that command's row already resolves the anchor and pulling it earlier mis-credits it to a preceding action and steals the new page's DOM from rows still on the old one (measured: a 206 ms pull moved `/login` onto two rows that were on `/add_remove_elements`). Anchors are only pulled earlier, never past the newest timestamp already in the stream, or replay would apply the outgoing document's refs to the incoming tree. - Residual, accepted: Nightwatch's `click` resolves *before* its navigation commits (measured 5 ms), so a submit-click row can still show its pre-navigation page. Selenium is immune — its click waits for page load. Not worth another heuristic; every heuristic tried here regressed a different row. +- **A pushed screencast needs bounding at both ends, and the obvious bound biases toward the end of the run.** Per-command capture is self-limiting — one frame per command, so the test's own length caps it — which is why `selenium-devtools-py` had no frame cap at all. Chrome's `Page.startScreencast` removes that property: `cdp_screencast.py` subscribes over a websocket of its OWN, which is a *different connection* from the session's command channel and therefore safe where the poll thread the module's docstring warns about was not. Every frame must be acked (Chrome sends nothing after an unacknowledged one, so a missed ack ends the recording rather than degrading it), and the rate is thinned at the source with `every_nth_frame` rather than buffered and discarded here. + - The buffer cap then needs care. Halving the buffer and keeping first/last — core's documented `maxBufferFrames` shape — drifts toward the run's end, because each decimation thins what is already held while new frames keep arriving unthinned: measured on a 40-frame run at a cap of 6, it kept frames 0, 1, 35, 37, 38, 39, i.e. the last moments and nothing from the middle. `_buffer` therefore thins the INCOMING frames by the same factor it has halved the buffer (`_stride` doubles per decimation), giving 0, 1, 11, 23, 31 for the same run and, at the real 2000 cap over 12000 frames, 1503 frames with 751 from the middle half. Thinning then costs the END of the run, because the last frame offered is only kept when the run happens to stop on a stride position — 41 frames at a cap of 6 ended on frame 31, eight frames stale, and 12000 at 2000 kept its last only because the two aligned. The newest skipped frame is therefore HELD rather than dropped and folded in by `_keep_tail` when the recorder stops (`finalize` stops before reading the buffer), so the video always ends where the run did — which is the part a failure is inspected for. Asserting only the endpoints does not catch this — tail truncation also leaves frame 0 plus whatever arrived since the last decimation, so the test has to assert something from the *middle* survives. + - **`driver.start_devtools()` cannot be used for this, because selenium caches ONE `_websocket_connection` per driver and hands it to whichever of BiDi or CDP asks first.** The adapter attaches BiDi before arming the screencast, so `start_devtools()` returned the BiDi socket and `Page.startScreencast` reached a BiDi endpoint: measured, `unknown command: Unknown command 'Page.startScreencast'`, followed by `BiDi command has no 'params' of type dictionary: {"method": "Page.stopScreencast"}` — that second line being the proof of which endpoint it was, and coming from a `stop` the failed start should never have sent. BiDi carries console and network, so it keeps the shared connection and the screencast opens its own. + - Resolving that endpoint needs BOTH routes. `se:cdp` is a **Grid** capability and is absent for a locally started chromedriver — the common case, and the one the demo runs — so without the `debuggerAddress` → `/json/version` → `webSocketDebuggerUrl` lookup that selenium's own `_get_cdp_details` performs, push mode would decline on every local run and the feature would be dead code. Done with stdlib urllib rather than that private method, since selenium moving internals is what broke network capture in #293. An `se:cdp` equal to `webSocketUrl` is rejected as the BiDi socket. + - **Performance timings ride on the command ROW, not a scope of their own** (`CommandLog.performance`, plus `cookies`/`documentInfo`/`result`), so the row is sent when the command completes and sent again under `replaceCommand` once the page has answered — which is why `capture_command` returns the row it built and `send_replace_command` keys on its `timestamp` rather than the per-process `id` counter. Python does **not** sleep before reading, where the JS adapters wait 500 ms: their navigation command can resolve before the load event, selenium's `get()` returns after it, and a sleep on this thread would be a real delay in the user's test rather than a detached await. A read that lands early anyway carries no `navigation` entry and is discarded rather than replacing a good row with an empty one. The read goes through `_guarded_execute_script`, or it lands in the same `execute` hook it was called from and shows up as an `executeScript` row beside every navigation — a fake driver whose `execute_script` does not route through `execute` cannot catch that, and did not. + - Per-command screenshots keep being taken for the command ROWS while a stream is live, but stop feeding the video: the pushed frames already cover the timeline and interleaving would duplicate one of them a few milliseconds off. - **A drain must anchor the document it reads, and the flag for that has only ever had one value.** `core/script-loader.ts` `collectorDrainExpression(forceAnchor)` prepends `captureCurrentDom()` so a freshly injected collector's *async* initial anchor is not lost: the collector schedules it after `waitForBody`, so a drain issued right after a navigation beats it, reads an empty buffer, and the destination's buffer then dies with the page — leaving the navigating action with no DOM. Every production caller in both JS adapters passes `true` (selenium's `drainAfterLiveCommand`, its re-inject-after-navigation and teardown paths; nightwatch's five sites), so the `false` default is vestigial. Python's drain read `getTraceData()` with no anchor at all, which is the same missing backstop the preload does not cover; `selenium-devtools-py/src/selenium_devtools/snapshot.py` `_DRAIN_SCRIPT` now forces it **unconditionally and carries no flag** — one setting is not a knob. Forcing is free after the first anchor of a document because `packages/script` guards `captureCurrentDom` with an `#anchored` flag that deliberately survives its `reset()`, which is why selenium anchors on every live command and still emits ~3 anchors across a 16-row run rather than 16. - **Document-start injection is what removes the whole race class; everything else is reconstruction.** `