[java] Parse each BiDi command response only once

[pujagani]

🔗 Related Issues

💥 What does this PR do?

The message received from browsers during a BiDi session was decoded 3 times or more for each incoming message. Fixing that to decode almost only once and use the decoded result to map to typed object.

🔧 Implementation Notes

Simplest way to do without interfering with other JSON flows.

🤖 AI assistance

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

💡 Additional Considerations

🔄 Types of changes

• Cleanup

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[pujagani]

/agentic_review

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📜 Skill insights (0)

1. Existing command mappers stop compiling 📘 Rule violation ≡ Correctness

Description

The public Command constructors now accept Function<@Nullable Object, X> instead of
Function<JsonInput, X>, and ConverterFunctions.map exposes the same incompatible generic change.
Existing clients that explicitly pass, return, or store Function<JsonInput, X> cannot recompile
because Java function types are invariant, even though the erased binary signature remains
unchanged.

Code

java/src/org/openqa/selenium/bidi/Command.java[R55-56]

  public Command(
-      String method, Map<String, @Nullable Object> params, Function<JsonInput, X> mapper) {
+      String method, Map<String, @Nullable Object> params, Function<@Nullable Object, X> mapper) {

Evidence

Rule 1 requires public API compatibility. The changed constructor parameter and helper return type
replace Function<JsonInput, X> with invariant Function<Object, X>, which is a
source-incompatible public API change.

AGENTS.md: Preserve Public API and ABI Compatibility
java/src/org/openqa/selenium/bidi/Command.java[55-64]
java/src/org/openqa/selenium/bidi/ConverterFunctions.java[51-58]

Agent prompt

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

## Issue description
Public command mapper signatures changed from `Function<JsonInput, X>` to `Function<Object, X>`, breaking recompilation of existing client code that explicitly uses the former type.

## Fix Focus Areas
- java/src/org/openqa/selenium/bidi/Command.java[55-64]
- java/src/org/openqa/selenium/bidi/ConverterFunctions.java[51-58]

## Recommended Fix
Preserve the existing `JsonInput`-based public signatures and introduce a distinctly named parsed-result factory or functional interface with a different erased type for the new internal path. Migrate Selenium's internal mappers to that new API while adapting legacy mappers so existing client source continues to compile.

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

---

2. Response parsing lacks a focused test ✓ Resolved 📘 Rule violation ☼ Reliability

Description

Connection.handleResponse now dispatches an already-parsed nullable result or reconstructed error
without a focused unit test of either callback path. The added JsonTest cases exercise only
Json.convert, while successful response dispatch, error propagation, callback removal, and mapper
invocation remain dependent on broader integration coverage.

Code

java/src/org/openqa/selenium/bidi/Connection.java[R315-316]

+  private void handleResponse(Map<String, Object> rawDataMap) {
+    Consumer<Either<Throwable, @Nullable Object>> consumer =

Evidence

Rule 4 requires focused unit coverage for relevant changes where practical. The PR changes the
central BiDi response-dispatch representation and callback behavior, but its added tests cover only
the standalone JSON conversion method.

AGENTS.md: Prefer Small Unit Tests and Avoid Contract-Misrepresenting Mocks
java/src/org/openqa/selenium/bidi/Connection.java[315-326]
java/test/org/openqa/selenium/json/JsonTest.java[170-222]

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 central BiDi response-dispatch contract changed, but the PR only adds tests for the lower-level conversion helper and does not directly verify the new callback behavior.

## Fix Focus Areas
- java/src/org/openqa/selenium/bidi/Connection.java[315-326]
- java/test/org/openqa/selenium/json/JsonTest.java[170-222]

## Recommended Fix
Add focused connection-level unit tests that feed representative successful and error responses into the handler and assert callback removal, parsed result delivery, mapper invocation, and exception propagation without requiring a browser.

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

---

3. Browser responses are decoded twice 🐞 Bug ➹ Performance

Description

Json.convert serializes its already-parsed source with toJson and immediately passes the
resulting text to toType, invoking the JSON parser again instead of coercing the object tree
directly. After Connection.handle has parsed each WebSocket message into a map, every type-based
Command and typed ConverterFunctions.map result reaches this helper, retaining a full
serialization and parse on the common BiDi response path.

Code

java/src/org/openqa/selenium/json/Json.java[239]

+    return toType(toJson(source), typeOfT);

Evidence

Connection.handle first parses each incoming message into raw and passes the parsed result
object to command mappers, while the generic Command(Type) constructor and typed field converter
both invoke Json.convert. The implementation of convert then calls toJson followed by
toType, proving that these already-parsed values are serialized and parsed a second time.

java/src/org/openqa/selenium/bidi/Connection.java[297-325]
java/src/org/openqa/selenium/bidi/Command.java[45-52]
java/src/org/openqa/selenium/bidi/ConverterFunctions.java[51-58]
java/src/org/openqa/selenium/json/Json.java[235-240]
java/src/org/openqa/selenium/bidi/Connection.java[297-307]
java/src/org/openqa/selenium/bidi/Connection.java[315-325]
java/src/org/openqa/selenium/json/Json.java[217-239]
java/src/org/openqa/selenium/bidi/ConverterFunctions.java[51-57]

Agent prompt

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

## Issue description
`Json.convert` currently turns an already-parsed value back into JSON text and parses it again, so typed BiDi command and result-field mapping still performs a second full JSON traversal.

## Fix Focus Areas
- java/src/org/openqa/selenium/json/Json.java[217-240]
- java/src/org/openqa/selenium/bidi/Command.java[47-52]
- java/src/org/openqa/selenium/bidi/ConverterFunctions.java[55-57]

## Recommended Fix
Implement a direct object-to-type coercion path in the JSON coercion layer that consumes parsed `Map`, `List`, scalar, and null values without producing intermediate JSON text. Make `Json.convert`, type-based commands, and typed converter functions use that path instead of `toType(toJson(source), typeOfT)`, preserve the existing null return contract, and add tests covering scalar, bean, and typed-list conversion while proving that conversion neither serializes nor parses the source.

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

---

4. Deep browser responses fail to map 🐞 Bug ≡ Correctness

Description

Json.convert round-trips an already-parsed map or list through the default toJson overload,
imposing JsonOutput.MAX_DEPTH of 100 instead of directly coercing the object tree as the prior
JsonInput mapping did. When script serialization requests more than 100 nested object levels,
evaluate, call-function, and other deeply nested typed command results throw during mapping and
complete exceptionally despite having been parsed successfully.

Code

java/src/org/openqa/selenium/json/Json.java[239]

+    return toType(toJson(source), typeOfT);

Evidence

The new conversion path explicitly accepts parsed map and list structures but calls toJson(source)
before toType; toJson(Object) uses JsonOutput.MAX_DEPTH, defined as 100 and enforced while
traversing nested collections and maps. BiDi callers can request object serialization deeper than
that threshold, so the output-depth exception occurs during typed command mapping and completes the
command future exceptionally.

java/src/org/openqa/selenium/json/Json.java[127-145]
java/src/org/openqa/selenium/json/Json.java[217-240]
java/src/org/openqa/selenium/json/JsonOutput.java[189-215]
java/src/org/openqa/selenium/json/JsonOutput.java[375-397]
java/src/org/openqa/selenium/json/Json.java[235-239]
java/src/org/openqa/selenium/json/JsonOutput.java[53-55]
java/src/org/openqa/selenium/json/JsonOutput.java[189-220]
java/src/org/openqa/selenium/bidi/script/SerializationOptions.java[34-54]
java/src/org/openqa/selenium/bidi/Connection.java[121-130]

Agent prompt

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

## Issue description
`Json.convert` serializes already-parsed maps and lists through `JsonOutput` before coercion, imposing its 100-level `JsonOutput.MAX_DEPTH` on valid incoming BiDi results that were previously mapped directly from `JsonInput`.

## Fix Focus Areas
- java/src/org/openqa/selenium/json/Json.java[235-240]
- java/test/org/openqa/selenium/json/JsonTest.java[170-222]

## Recommended Fix
Replace the serialize-then-parse implementation with object-backed coercion that traverses parsed maps, lists, and scalar values directly, so received values never pass through `JsonOutput` or inherit its depth limit. Add a regression test that converts a parsed map or list nested beyond 100 levels (`JsonOutput.MAX_DEPTH`) and verifies that it reaches the requested target type without an output-depth failure.

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

---

Context sources

Review mode: 🚀 Fast: The push mainly adds localized BiDi unit tests and test-target configuration, with only documentation changes in runtime code and no new production logic.

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-code-review]

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

[qodo-code-review]

No code changes since the last review — review skipped