refactor: codebase dedup pass — 271 lines deleted#20
Merged
Conversation
Four BiDiClient functions were superseded by Wallabidi.LiveViewAware
and had no live callers outside BiDiClient itself:
* prepare_patch/1, await_patch/2, drain_patches/2 — all replaced by
LiveViewAware.{prepare_patch, await_patch, drain_patches} which
use Protocol.eval/eval_async (protocol-agnostic).
* await_liveview_connected/2 — replaced by
LiveViewAware.await_liveview_connected/2.
The only internal caller, BiDiClient.visit/2, now calls
LiveViewAware.await_liveview_connected directly — same shape, zero
behaviour change.
Also drops four large module attributes that held the corresponding
JS bodies (@prepare_patch_js, @await_patch_js, @drain_patches_js,
@await_lv_connected_js).
Net: 200 lines deleted from bidi_client.ex.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
\`root_session/1\` and \`bidi_pid/1\` were duplicated across CDPClient, BiDiClient, and ChromeCDP — each module had its own private 2-clause recursive helper that walked \`Element.parent → ... → Session\` and pulled the field. Promote both to public functions on \`Wallabidi.Element\` so there's one canonical place for parent-chain traversal. The three private definitions become one-line delegations. (WebSocketClient rename — also flagged in the dedup audit — deferred to its own PR. Pure cosmetic; not bundled here to keep this diff mechanical.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The lazy-ops queue (\`SessionProcess.append_ops/5\`, \`drain_ops/1\`, the \`pending_ops\` field, and the \`maybe_flush_pending\`/\`flush_pending\` plumbing in CDPClient) had zero callers — no path in lib/ ever invoked \`append_ops\`, so the queue was always empty and \`maybe_flush_pending\` was a no-op called on every \`send_cdp_session\`. The \`CDPClient.do_post_click/3-4\` was only called from \`flush_pending\`, so it was equally dead. It was also a near-clone of \`Browser.do_post_click/5\` but missing the \`await_ack\` slow-handle_event fix. **Subtle:** \`maybe_flush_pending\` was load-bearing despite the queue being empty. The synchronous \`GenServer.call(pid, :drain_ops)\` it issued was acting as a *mailbox barrier* on the SessionProcess — draining any pending binding events / page_ready notifications BEFORE the CDP send proceeded. Naive deletion (a previous attempt) introduced a flake regression: 2-failure runs on a previously clean baseline. Replace with an explicit, named barrier: \`Wallabidi.SessionProcess.sync_barrier/1\`. Same mechanism (a no-op \`GenServer.call\` that returns \`:ok\`), but the intent is now visible at the callsite. \`send_cdp_session\` calls it before each CDP RPC. Bisect verified: this is sufficient to restore the clean baseline (9 consecutive clean CDP mc4 runs after the change vs. 1-2 failure runs without the barrier). Net deletion: 121 lines minus the new barrier helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
\`cdp_client.ex\` had five near-clones around the same \`flat_session_id\` branch: - \`send_cdp_session/3\` - \`send_cdp_raw/2\` - \`cast_cdp_command/3\` - \`cast_release/2\` - \`cast_cdp_with_session/4\` (only-pid variant) Each repeated \`pid = bidi_pid(session); session_id = effective_session_id(session); if flat?, do: cast_command_flat(...) else: Map.put(:sessionId, ...)\` with minor differences. Three of them had drift: e.g. \`cast_release\` manually merged \`objectId\` and \`sessionId\` together instead of relying on the helper that other casts use. Replace with one \`dispatch(session, kind, method, params)\` helper where \`kind in [:send, :cast]\`. Each wrapper becomes a one-line call to dispatch. \`cast_cdp_with_session/4\` (no Session struct yet, used during bootstrap) keeps its own minimal form. Behaviour-equivalent — passes unit, lightpanda, and CDP integration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four sequential dedup commits, all locally test-verified (180 unit + 124 LV + 153 lightpanda + 12-of-13 clean CDP mc4 runs).
Commits in order
dedup chore(main): release 0.31.0 #1: delete dead `BiDiClient.{prepare_patch, await_patch, drain_patches, await_liveview_connected}` and the four large JS module attributes that backed them. Superseded by `Wallabidi.LiveViewAware` (protocol-agnostic). `BiDiClient.visit/2` was the only internal caller; switched to `LiveViewAware.await_liveview_connected`. −200 lines.
dedup Session pool doesn't propagate sandbox metadata on checkout #3: hoist `root_session/1` and `bidi_pid/1` to public `Wallabidi.Element` functions. CDPClient/BiDiClient/ChromeCDP each had their own private 2-clause recursive duplicate. −12 lines net.
dedup sandbox_case not detected at compile time — @sandbox_case_available is false #2: delete the unreachable lazy-ops queue (`SessionProcess.append_ops`/`drain_ops`/`pending_ops` field, `CDPClient.maybe_flush_pending`/`flush_pending`/4-arg `do_post_click`). The queue had zero callers. −118 lines net.
Subtle: `maybe_flush_pending` was load-bearing despite the queue being empty — the synchronous `GenServer.call(:drain_ops)` it issued was acting as a mailbox barrier on the SessionProcess, ordering CDP sends against pending binding events. A first attempt at deletion introduced a flake regression (2 failures on a previously clean baseline; bisected to this commit). Fix: keep the barrier explicit via a new `Wallabidi.SessionProcess.sync_barrier/1` (a no-op `GenServer.call` whose value is its mailbox-serialization side-effect). `send_cdp_session` calls it before each CDP RPC.
dedup XPath Query.button cannot classify phx-click — causes incorrect await_patch #5: collapse five near-clones in cdp_client.ex (`send_cdp_session`, `send_cdp_raw`, `cast_cdp_command`, `cast_release`, the `if flat_session_id` ladder in each) into one `dispatch(session, kind, method, params)` helper where `kind in [:send, :cast]`. Each wrapper becomes a one-line call. −10 lines net.
Total: 271 lines deleted
```
lib/wallabidi/bidi_client.ex | 207 +-------
lib/wallabidi/cdp_client.ex | 201 ++-------
lib/wallabidi/chrome_cdp.ex | 5 +-
lib/wallabidi/element.ex | 20 ++-
lib/wallabidi/session_process.ex | 62 ++--
5 files changed, 112 insertions(+), 383 deletions(-)
```
What's NOT in this PR
Test plan
🤖 Generated with Claude Code