[rb] route BiDiBridge navigation through the generated Protocol::BrowsingContext

[titusfortner]

🔗 Related Issues

Part of the Ruby BiDi protocol generator rollout (foundation: #17731).

💥 What does this PR do?

Routes the Ruby BiDiBridge's own navigation — get, back, forward, refresh — through the generated BiDi::Protocol::BrowsingContext instead of the hand-written BiDi::BrowsingContext. This makes driver navigation the first production consumer of the generated BiDi protocol layer, rather than it being reachable only through integration specs. No user-facing behavior change.

🔧 Implementation Notes

The generated layer never reads capabilities, so the bridge supplies the two things it omits: the context (current window handle) and the wait readiness mapped from page_load_strategy.

One wire nuance: when page_load_strategy is absent the bridge now defaults to normal (→ complete) rather than the previous wait: null — the generated readiness enum is non-nullable. The common normal/eager/none cases are unchanged.

refresh omits ignoreCache — a plain reload doesn't need it, and Firefox rejects the (correctly-named) param as unimplemented. The old path sent it mis-cased, so Firefox ignored it anyway; behavior is unchanged.

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s): Claude Code
  • What was generated: the BiDiBridge navigation implementation
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

• Deprecating the hand-written BrowsingContext methods is for a future PR.

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Route BiDiBridge navigation through generated BiDi::Protocol::BrowsingContext

✨ Enhancement 🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Switch BiDiBridge navigation commands to the generated BiDi protocol BrowsingContext domain.
• Explicitly pass current window handle and readiness wait derived from page_load_strategy.
• Default missing page_load_strategy to normal/complete readiness to satisfy non-nullable enum.

Diagram

graph TD
  A["Driver navigation API"] --> B["Remote::BiDiBridge"] --> C["BiDi::Protocol::BrowsingContext"] --> D["BiDi::Transport"] --> E["WebSocket connection"] --> F["Browser BiDi endpoint"]
  B --> G["Readiness mapping"] --> C

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Omit `wait` when page_load_strategy is unset

• ➕ Preserves previous wire behavior of sending wait: null/omitting readiness semantics when not configured
• ➕ Avoids imposing a default that could theoretically change timing edge-cases
• ➖ May diverge from typical WebDriver expectations where unset implies normal
• ➖ Would require plumbing Serialization::UNSET (or similar) from the bridge, coupling bridge logic to protocol serialization details

2. Teach the generated layer to infer `context`/`wait`

• ➕ Reduces duplication of context/readiness handling at call sites
• ➕ Centralizes navigation defaults
• ➖ Couples generated protocol code to driver capabilities/window-handle concepts
• ➖ Adds complexity to the generator and reduces separation between transport/protocol and bridge layers

Recommendation: Current approach (bridge supplies context and a concrete readiness derived from page_load_strategy) is the cleanest separation of concerns: the generated protocol stays capability-agnostic while the bridge owns WebDriver semantics. The only nuance to double-check is the intentional default to normal when page_load_strategy is absent; if that aligns with the Ruby driver's existing implied default, the change is appropriate.

Files changed (1) +17 / -5

Enhancement (1) +17 / -5

bidi_bridge.rbRoute navigation via generated Protocol::BrowsingContext with readiness mapping +17/-5

Route navigation via generated Protocol::BrowsingContext with readiness mapping

• Adds the generated BiDi protocol require and replaces the hand-written BiDi::BrowsingContext usage with BiDi::Protocol::BrowsingContext over the shared Transport. Updates get/back/forward/refresh to pass explicit 'context' (window_handle) and 'wait' readiness derived from page_load_strategy, defaulting missing strategy to 'normal'/':complete'.

rb/lib/selenium/webdriver/remote/bidi_bridge.rb

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. BiDiBridge navigation lacks tests 📘 Rule violation ▣ Testability

Description

The PR changes the core navigation implementation (get, go_back, go_forward, refresh) to use
generated BiDi protocol calls, but the change set includes no corresponding test additions/updates
to cover the new argument wiring and readiness mapping. This increases regression risk for a
critical user path (driver navigation).

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R45-59]

        def get(url)
-          browsing_context.navigate(url)
+          browsing_context.navigate(context: window_handle, url: url, wait: readiness_state)
        end

        def go_back
-          browsing_context.traverse_history(-1)
+          browsing_context.traverse_history(context: window_handle, delta: -1)
        end

        def go_forward
-          browsing_context.traverse_history(1)
+          browsing_context.traverse_history(context: window_handle, delta: 1)
        end

        def refresh
-          browsing_context.reload
+          browsing_context.reload(context: window_handle, ignore_cache: false, wait: readiness_state)
        end

Evidence

PR Compliance ID 389273 requires tests for new functionality/bug fixes. The diff changes BiDiBridge
navigation methods to call generated protocol APIs with new parameters, but no tests were
added/updated in this change set to validate the new behavior.

Rule 389273: Require tests for all new functionality and bug fixes
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[45-59]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR changes navigation to go through `BiDi::Protocol::BrowsingContext`, but there are no accompanying test changes to ensure the new protocol calls and parameters (`context`, `delta`, `ignore_cache`, `wait`) are correctly passed.

## Issue Context
This touches high-traffic public behavior (`driver.navigate.to/back/forward/refresh`) and also introduces a new readiness-state mapping used by the generated protocol layer.

## Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[45-81]
- rb/spec/integration/selenium/webdriver/navigation_spec.rb[23-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. BiDiBridge missing @api private ✗ Dismissed 📘 Rule violation ✧ Quality ⭐ New

Description

The newly added internal helper readiness_state (and related READINESS_STATE constant) is not
marked with a YARD @api private tag, even though Selenium::WebDriver::Remote internals are
typically annotated as private API. This can accidentally expose internal implementation details in
generated docs and mislead downstream users about support/compatibility guarantees.

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R92-94]

+        def readiness_state
+          READINESS_STATE.fetch(capabilities[:page_load_strategy] || 'normal')
        end

Evidence

PR Compliance ID 389237 requires internal Ruby APIs to be marked with # @api private in YARD docs.
The new private method readiness_state was added without any adjacent YARD doc block containing
@api private.

Rule 389237: Mark internal Ruby APIs with @api private in YARD docs
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[88-94]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Selenium::WebDriver::Remote::BiDiBridge` gained new internal implementation (`READINESS_STATE` and `readiness_state`) but the file/method lacks a YARD `# @api private` annotation, which this project uses to mark internal Ruby APIs.

## Issue Context
Other `Selenium::WebDriver::Remote` internals (e.g., `Remote::Driver`) include `# @api private` to avoid documenting/advertising internal APIs.

## Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[24-35]
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[88-94]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. readiness_state defaults to normal 📘 Rule violation ≡ Correctness

Description

The new readiness_state logic forces page_load_strategy to default to normal (:complete)
when absent, changing the wire behavior from previously passing a nil readiness (wait: null). This
is a user-visible behavioral default and should be compared against at least one other language
binding and documented if intentionally divergent.

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R79-80]

+        def readiness_state
+          READINESS_STATE.fetch(capabilities[:page_load_strategy] || 'normal')

Evidence

PR Compliance ID 389265 requires a cross-language comparison when changing user-visible behavior.
The new code defaults absent page_load_strategy to normal, whereas the prior implementation
derived readiness from a hash lookup that could return nil, changing the resulting wait
parameter semantics.

Rule 389265: Compare cross-language bindings when changing user-visible behavior
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[79-80]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[39-51]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A default behavior change was introduced for navigation readiness when `page_load_strategy` is absent (`nil`), and the PR should include evidence of cross-binding comparison or document any intentional divergence.

## Issue Context
Previously the handwritten BiDi browsing context used `@readiness = READINESS_STATE[page_load_strategy]`, which yielded `nil` when the capability was absent; the new code uses `fetch(... || 'normal')`, which always forces a non-nil readiness (`:complete`).

## Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[79-80]
- rb/lib/selenium/webdriver/bidi/browsing_context.rb[39-51]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Readiness mapping raises KeyError 🐞 Bug ☼ Reliability

Description

BiDiBridge#readiness_state uses READINESS_STATE.fetch(...), so an unexpected non-nil
capabilities[:page_load_strategy] will raise KeyError and break get/refresh before any BiDi command
is sent. Previously, the hand-written BiDi::BrowsingContext used a tolerant lookup
(READINESS_STATE[...]) and would pass wait: nil when the strategy was missing/unknown.

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R79-80]

+        def readiness_state
+          READINESS_STATE.fetch(capabilities[:page_load_strategy] || 'normal')

Evidence

The new code uses READINESS_STATE.fetch(...), which raises on unknown keys; get and refresh
call readiness_state as part of argument evaluation, so they will error out before calling into
the protocol layer. The previous BiDi browsing context mapping used READINESS_STATE[...] and
passed the resulting (possibly nil) readiness to the wire command instead of raising.

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[30-35]
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[45-59]
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[79-81]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[28-41]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[48-52]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[69-73]
rb/lib/selenium/webdriver/bidi/protocol/browsing_context.rb[47-51]
rb/lib/selenium/webdriver/bidi/protocol/browsing_context.rb[555-559]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`BiDiBridge#readiness_state` currently uses `READINESS_STATE.fetch(...)`, which raises `KeyError` for any unexpected non-nil `page_load_strategy`. This makes navigation methods (`get`, `refresh`) fail synchronously before issuing any BiDi command.

### Issue Context
The prior hand-written implementation effectively allowed unknown/missing values by producing `nil` readiness and sending `wait: nil`.

### Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[30-35]
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[79-81]

### Suggested fix
Change `readiness_state` to either:
- Provide a safe fallback (e.g., default to `'normal'`/`:complete` when the value is unknown), or
- Raise a clearer Selenium error (e.g., `Error::WebDriverError`) with an explicit message listing allowed values, rather than bubbling up a `KeyError`.

Example approach (fallback):
```ruby
strategy = capabilities[:page_load_strategy] || 'normal'
READINESS_STATE[strategy] || READINESS_STATE['normal']
```

Example approach (clear error):
```ruby
strategy = capabilities[:page_load_strategy] || 'normal'
READINESS_STATE.fetch(strategy) do
 raise Error::WebDriverError, "Unsupported page_load_strategy=#{strategy.inspect} (expected one of: none, eager, normal)"
end
```

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Previous review results

Review updated until commit a0d506c

Results up to commit 42247c3

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. BiDiBridge navigation lacks tests 📘 Rule violation ▣ Testability

Description

The PR changes the core navigation implementation (get, go_back, go_forward, refresh) to use
generated BiDi protocol calls, but the change set includes no corresponding test additions/updates
to cover the new argument wiring and readiness mapping. This increases regression risk for a
critical user path (driver navigation).

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R45-59]

        def get(url)
-          browsing_context.navigate(url)
+          browsing_context.navigate(context: window_handle, url: url, wait: readiness_state)
        end

        def go_back
-          browsing_context.traverse_history(-1)
+          browsing_context.traverse_history(context: window_handle, delta: -1)
        end

        def go_forward
-          browsing_context.traverse_history(1)
+          browsing_context.traverse_history(context: window_handle, delta: 1)
        end

        def refresh
-          browsing_context.reload
+          browsing_context.reload(context: window_handle, ignore_cache: false, wait: readiness_state)
        end

Evidence

PR Compliance ID 389273 requires tests for new functionality/bug fixes. The diff changes BiDiBridge
navigation methods to call generated protocol APIs with new parameters, but no tests were
added/updated in this change set to validate the new behavior.

Rule 389273: Require tests for all new functionality and bug fixes
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[45-59]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR changes navigation to go through `BiDi::Protocol::BrowsingContext`, but there are no accompanying test changes to ensure the new protocol calls and parameters (`context`, `delta`, `ignore_cache`, `wait`) are correctly passed.

## Issue Context
This touches high-traffic public behavior (`driver.navigate.to/back/forward/refresh`) and also introduces a new readiness-state mapping used by the generated protocol layer.

## Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[45-81]
- rb/spec/integration/selenium/webdriver/navigation_spec.rb[23-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. readiness_state defaults to normal 📘 Rule violation ≡ Correctness

Description

The new readiness_state logic forces page_load_strategy to default to normal (:complete)
when absent, changing the wire behavior from previously passing a nil readiness (wait: null). This
is a user-visible behavioral default and should be compared against at least one other language
binding and documented if intentionally divergent.

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R79-80]

+        def readiness_state
+          READINESS_STATE.fetch(capabilities[:page_load_strategy] || 'normal')

Evidence

PR Compliance ID 389265 requires a cross-language comparison when changing user-visible behavior.
The new code defaults absent page_load_strategy to normal, whereas the prior implementation
derived readiness from a hash lookup that could return nil, changing the resulting wait
parameter semantics.

Rule 389265: Compare cross-language bindings when changing user-visible behavior
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[79-80]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[39-51]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A default behavior change was introduced for navigation readiness when `page_load_strategy` is absent (`nil`), and the PR should include evidence of cross-binding comparison or document any intentional divergence.

## Issue Context
Previously the handwritten BiDi browsing context used `@readiness = READINESS_STATE[page_load_strategy]`, which yielded `nil` when the capability was absent; the new code uses `fetch(... || 'normal')`, which always forces a non-nil readiness (`:complete`).

## Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[79-80]
- rb/lib/selenium/webdriver/bidi/browsing_context.rb[39-51]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Readiness mapping raises KeyError 🐞 Bug ☼ Reliability

Description

BiDiBridge#readiness_state uses READINESS_STATE.fetch(...), so an unexpected non-nil
capabilities[:page_load_strategy] will raise KeyError and break get/refresh before any BiDi command
is sent. Previously, the hand-written BiDi::BrowsingContext used a tolerant lookup
(READINESS_STATE[...]) and would pass wait: nil when the strategy was missing/unknown.

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R79-80]

+        def readiness_state
+          READINESS_STATE.fetch(capabilities[:page_load_strategy] || 'normal')

Evidence

The new code uses READINESS_STATE.fetch(...), which raises on unknown keys; get and refresh
call readiness_state as part of argument evaluation, so they will error out before calling into
the protocol layer. The previous BiDi browsing context mapping used READINESS_STATE[...] and
passed the resulting (possibly nil) readiness to the wire command instead of raising.

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[30-35]
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[45-59]
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[79-81]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[28-41]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[48-52]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[69-73]
rb/lib/selenium/webdriver/bidi/protocol/browsing_context.rb[47-51]
rb/lib/selenium/webdriver/bidi/protocol/browsing_context.rb[555-559]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`BiDiBridge#readiness_state` currently uses `READINESS_STATE.fetch(...)`, which raises `KeyError` for any unexpected non-nil `page_load_strategy`. This makes navigation methods (`get`, `refresh`) fail synchronously before issuing any BiDi command.

### Issue Context
The prior hand-written implementation effectively allowed unknown/missing values by producing `nil` readiness and sending `wait: nil`.

### Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[30-35]
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[79-81]

### Suggested fix
Change `readiness_state` to either:
- Provide a safe fallback (e.g., default to `'normal'`/`:complete` when the value is unknown), or
- Raise a clearer Selenium error (e.g., `Error::WebDriverError`) with an explicit message listing allowed values, rather than bubbling up a `KeyError`.

Example approach (fallback):
```ruby
strategy = capabilities[:page_load_strategy] || 'normal'
READINESS_STATE[strategy] || READINESS_STATE['normal']
```

Example approach (clear error):
```ruby
strategy = capabilities[:page_load_strategy] || 'normal'
READINESS_STATE.fetch(strategy) do
 raise Error::WebDriverError, "Unsupported page_load_strategy=#{strategy.inspect} (expected one of: none, eager, normal)"
end
```

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 5bd9a18

[qodo-code-review]

Code review by qodo was updated up to the latest commit 66a391d

[qodo-code-review]

Code review by qodo was updated up to the latest commit c63ced9

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Eager protocol layer load ✗ Dismissed 🐞 Bug ➹ Performance ⭐ New

Description

BiDiBridge now requires selenium/webdriver/bidi/protocol, which loads all generated BiDi domains
even though this bridge only instantiates BiDi::Protocol::BrowsingContext for navigation. This
increases code loading (and potentially startup time/memory) for BiDi-enabled sessions
unnecessarily.

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R20-21]

require 'selenium/webdriver/bidi'
+require 'selenium/webdriver/bidi/protocol'

Evidence

The new BiDiBridge require pulls in bidi/protocol.rb, which explicitly requires many generated
domain files, so a BiDi-enabled driver load ends up loading the full generated protocol surface area
even if only browsing context navigation is used.

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[20-22]
rb/lib/selenium/webdriver/bidi/protocol.rb[20-39]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rb/lib/selenium/webdriver/remote/bidi_bridge.rb` now does `require 'selenium/webdriver/bidi/protocol'`, which in turn requires *all* generated BiDi protocol domains. Since this bridge only needs the generated `BrowsingContext` domain for navigation, this eager load is broader than necessary.

### Issue Context
The protocol entrypoint (`bidi/protocol.rb`) is an aggregator that pulls in every generated domain file. For `BiDiBridge` navigation, only the protocol runtime (serialization/transport/domain base) + `protocol/browsing_context` appear necessary.

### Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[20-22]
- rb/lib/selenium/webdriver/bidi/protocol.rb[20-39]

### Suggested fix
Replace `require 'selenium/webdriver/bidi/protocol'` with a minimal set of requires needed to construct `BiDi::Protocol::BrowsingContext`, e.g.:

- `require 'selenium/webdriver/bidi/serialization'`
- `require 'selenium/webdriver/bidi/transport'`
- `require 'selenium/webdriver/bidi/protocol/domain'`
- `require 'selenium/webdriver/bidi/protocol/browsing_context'`

(Or introduce a smaller entrypoint like `bidi/protocol/browsing_context_only` and require that here.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Navigation return type drift ✗ Dismissed 🐞 Bug ≡ Correctness

Description

BiDiBridge#get and #refresh now return a generated NavigateResult record object (not a plain
Hash), and this return value flows through the public Driver#navigate.to/#navigate.refresh
APIs. Code that previously treated the result as a hash (e.g., result['navigation']) will break in
BiDi-enabled sessions.

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R49-63]

        def get(url)
-          browsing_context.navigate(url)
+          browsing_context.navigate(context: window_handle, url: url, wait: readiness_state)
        end

        def go_back
-          browsing_context.traverse_history(-1)
+          browsing_context.traverse_history(context: window_handle, delta: -1)
        end

        def go_forward
-          browsing_context.traverse_history(1)
+          browsing_context.traverse_history(context: window_handle, delta: 1)
        end

        def refresh
-          browsing_context.reload
+          browsing_context.reload(context: window_handle, wait: readiness_state)
        end

Evidence

BiDiBridge#get/#refresh return the result of generated protocol calls; those generated calls
explicitly return NavigateResult records. The return value is exposed publicly because
Navigation#to and Navigation#refresh return the bridge method results directly. The previous
handwritten browsing context used @bidi.send_cmd, which returns the raw reply result hash.

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[49-63]
rb/lib/selenium/webdriver/bidi/protocol/browsing_context.rb[661-701]
rb/lib/selenium/webdriver/common/navigation.rb[22-57]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[43-73]
rb/lib/selenium/webdriver/bidi/serialization/record.rb[256-269]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`BiDiBridge#get` and `BiDiBridge#refresh` currently return the generated protocol result object (`BrowsingContext::NavigateResult`). That object is a `Serialization::Record` (a `Data` subclass) and is not hash-like, while the previous implementation returned the raw websocket `result` hash. Because `Navigation#to` and `Navigation#refresh` return the bridge method result directly, this is an observable return-type/shape change.

### Issue Context
- Generated protocol: `BrowsingContext#navigate`/`#reload` return `NavigateResult` (a `Serialization::Record`).
- Old handwritten path returned the websocket reply `result` hash.
- Public API exposure: `Selenium::WebDriver::Navigation#to` calls `@bridge.get` and returns its value.

### Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[49-63]

### Suggested fix
In `BiDiBridge#get` and `BiDiBridge#refresh`, normalize the generated `NavigateResult` back to a plain Ruby hash to preserve the prior shape, e.g.:
- `browsing_context.navigate(...).as_json`
- `browsing_context.reload(...).as_json`

(`Serialization::Record#as_json` emits a hash with the wire keys, matching the prior reply hash’s string-key shape.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. READINESS_STATE comment restates mapping ✓ Resolved 📘 Rule violation ⚙ Maintainability ⭐ New

Description

The new comment above READINESS_STATE describes what the mapping is, but not why this mapping
exists or why it is needed for the generated protocol layer. This reduces maintainability by adding
redundant documentation instead of rationale.

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[29]

+        # Classic page load strategy => the BiDi ReadinessState the generated commands wait on.

Evidence

PR Compliance ID 6 requires comments to explain rationale rather than restating code behavior. The
added comment above READINESS_STATE primarily paraphrases the mapping instead of documenting the
motivation/constraint behind introducing it.

AGENTS.md: Comments Should Explain 'Why' Rather Than 'What'
rb/lib/selenium/webdriver/remote/bidi_bridge.rb[29-34]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly added comment restates what the `READINESS_STATE` constant does rather than explaining why this mapping is required.

## Issue Context
Per repo guidelines, comments should focus on rationale/tradeoffs/constraints; in this case the important context is why the bridge must map `page_load_strategy` to a non-nullable BiDi readiness state for generated protocol commands.

## Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[29-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Previous review results

Review updated until commit a0d506c

Results up to commit e154d80 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Navigation return type drift ✗ Dismissed 🐞 Bug ≡ Correctness

Description

BiDiBridge#get and #refresh now return a generated NavigateResult record object (not a plain
Hash), and this return value flows through the public Driver#navigate.to/#navigate.refresh
APIs. Code that previously treated the result as a hash (e.g., result['navigation']) will break in
BiDi-enabled sessions.

Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R49-63]

        def get(url)
-          browsing_context.navigate(url)
+          browsing_context.navigate(context: window_handle, url: url, wait: readiness_state)
        end

        def go_back
-          browsing_context.traverse_history(-1)
+          browsing_context.traverse_history(context: window_handle, delta: -1)
        end

        def go_forward
-          browsing_context.traverse_history(1)
+          browsing_context.traverse_history(context: window_handle, delta: 1)
        end

        def refresh
-          browsing_context.reload
+          browsing_context.reload(context: window_handle, wait: readiness_state)
        end

Evidence

BiDiBridge#get/#refresh return the result of generated protocol calls; those generated calls
explicitly return NavigateResult records. The return value is exposed publicly because
Navigation#to and Navigation#refresh return the bridge method results directly. The previous
handwritten browsing context used @bidi.send_cmd, which returns the raw reply result hash.

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[49-63]
rb/lib/selenium/webdriver/bidi/protocol/browsing_context.rb[661-701]
rb/lib/selenium/webdriver/common/navigation.rb[22-57]
rb/lib/selenium/webdriver/bidi/browsing_context.rb[43-73]
rb/lib/selenium/webdriver/bidi/serialization/record.rb[256-269]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`BiDiBridge#get` and `BiDiBridge#refresh` currently return the generated protocol result object (`BrowsingContext::NavigateResult`). That object is a `Serialization::Record` (a `Data` subclass) and is not hash-like, while the previous implementation returned the raw websocket `result` hash. Because `Navigation#to` and `Navigation#refresh` return the bridge method result directly, this is an observable return-type/shape change.

### Issue Context
- Generated protocol: `BrowsingContext#navigate`/`#reload` return `NavigateResult` (a `Serialization::Record`).
- Old handwritten path returned the websocket reply `result` hash.
- Public API exposure: `Selenium::WebDriver::Navigation#to` calls `@bridge.get` and returns its value.

### Fix Focus Areas
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[49-63]

### Suggested fix
In `BiDiBridge#get` and `BiDiBridge#refresh`, normalize the generated `NavigateResult` back to a plain Ruby hash to preserve the prior shape, e.g.:
- `browsing_context.navigate(...).as_json`
- `browsing_context.reload(...).as_json`

(`Serialization::Record#as_json` emits a hash with the wire keys, matching the prior reply hash’s string-key shape.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit d7eb9c7

[qodo-code-review]

Code review by qodo was updated up to the latest commit a56357a

[qodo-code-review]

Code review by qodo was updated up to the latest commit e2db8ec

[qodo-code-review]

Code review by qodo was updated up to the latest commit a0d506c