Skip to content

[py] generate the internal BiDi protocol layer from the shared binding-neutral schema - #17761

Open
titusfortner wants to merge 4 commits into
SeleniumHQ:trunkfrom
titusfortner:bidi-gen-py
Open

[py] generate the internal BiDi protocol layer from the shared binding-neutral schema#17761
titusfortner wants to merge 4 commits into
SeleniumHQ:trunkfrom
titusfortner:bidi-gen-py

Conversation

@titusfortner

@titusfortner titusfortner commented Jul 10, 2026

Copy link
Copy Markdown
Member

🔗 Related Issues

💥 What does this PR do?

  • Adds a new internal selenium.webdriver.common._bidi package, generated at build time from the shared binding-neutral schema (projected once from the CDDL). The domain modules are not checked in.
  • Adds the hand-written runtime that the generated modules build on: the serialization runtime, the transport seam, the Domain base, and the package __init__. These four are the only tracked files in the package; the domain modules are generated and gitignored.
  • Consumes the three shared-schema signals from [rb] add objectOnly/preserveExtras/scalar-primitive BiDi schema signals #17784 so the wire boundary is validated at runtime, not merely typed.
  • Links each generated element back to its spec definition.
  • This code is additive; nothing has been re-implemented to use it yet.

✅ Compliance with the behavioral contract (#17786)

This branch meets every behavior in the ADR's conformance table for py 17761. The two the
snapshot had it failing are now closed by consuming #17784's derived signals, each backed by an
executed test that feeds malformed input and asserts the raise/round-trip (the ADR is
runtime-not-declaration):

  • Item 5 (variant/vocabulary resolution) — an all-object union now rejects a non-object payload
    (Union._OBJECT_ONLY, from the schema's objectOnly), while a union with a real scalar arm
    (input.Origin) still passes scalars through. Map [key, value] entries keep round-tripping
    because the scalar signal lets a bare-string key through, checked against its primitive.
  • Item 8 (extras scoped to re-sendable) — the unknown-property store is now gated on
    preserveExtras (extensible ∩ re-sendable), so a received-only type (network.Cookie,
    session.NewResultCapabilities, storage.PartitionKey) drops unknowns while a re-sendable type
    (storage.PartialCookie, proxy configs, …) stores and echoes them.

The other ten behaviors were already met and are unchanged.

🔧 Implementation Notes

  • The underscore namespace marks it unsupported and outside the deprecation policy; nothing is publicly exposed on the driver. The existing common/bidi code and its CDDL generator are left untouched — retiring or migrating that is out of scope.
  • Generated at build time, not committed. A Starlark rule projects the schema into the domain modules under common/_bidi/; only serialization.py, transport.py, domain.py, and __init__.py are hand-written and tracked. Regenerate the checked-out tree for local development with bazel run //py:generate-bidi-protocol.
  • The package lives under common/ alongside the existing generated common/bidi and common/devtools packages, matching the established structure.
  • Because the shared schema is already resolved (types, unions, nullability):
    • the generated methods take precisely-typed arguments, argument values are validated before being sent over the socket, and inbound results are self-validating typed objects rather than raw dicts
    • no per-construct special-casing in the generator via manifests is required
  • All three signals are derived once in the shared projector ([rb] add objectOnly/preserveExtras/scalar-primitive BiDi schema signals #17784); the Python generator reads them, so the bindings stay in parity by construction — each is a one-line consume plus runtime support.
  • Records are frozen/immutable.
  • Inbound validation is strict now — types, required-presence, and enum/discriminator values are all checked. The intent is to relax reactively if a real browser payload ever needs it, rather than start lenient.
  • Each domain wraps the session's BiDi connection in its own lightweight Transport; the driver owns only the shared WebSocketConnection (where the id counter and callbacks live). Nothing on the driver imports the generated package, so non-BiDi sessions never load it.
  • The Domain base (the class each generated domain subclasses) lives in its own common/_bidi/domain.py module.
  • Each generated class/command/type carries a docstring link to its spec definition.
  • Code is properly linted & type checked.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: the generator, serialization runtime, transport seam, and tests — authored and reviewed iteratively
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • Is this the right namespace and the right way to namespace this code?
  • Should we relax inbound validation from the beginning? (this code is very strict with intent to relax as necessary during the implementation phase)
  • Any deprecations and re-implementations will be managed in follow-up PRs.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

@selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations labels Jul 10, 2026
@titusfortner
titusfortner marked this pull request as ready for review July 10, 2026 15:10
@qodo-code-review

qodo-code-review Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

[py] Generate internal BiDi protocol layer from the shared binding-neutral schema

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Adds an internal selenium.webdriver.common._bidi package, generated at build time from the
 shared BiDi schema.
• Implements the hand-written runtime (serialization, transport, Domain) that generated
 modules depend on.
• Adds Bazel + local-dev wiring and a comprehensive test suite validating wire-shape and runtime
 contract.
Diagram

graph TD
  Schema[("Shared BiDi schema.json")] --> Generator["generate_bidi_protocol.py"] --> Domains["Generated _bidi domain modules"]
  Domains --> Serialization["serialization.py (Record/Union)"] --> Transport["transport.py (Transport)"] --> WSConn["WebSocketConnection.send_cmd"] --> Browser{{"Browser BiDi"}}
  Domains --> DomainBase["domain.py (Domain)"] --> Transport
  WebDriver["Remote WebDriver"] --> WSConn --> Browser
  WebDriver --> Transport

  subgraph Legend
    direction LR
    _db[(Data/schema)] ~~~ _svc([Python module]) ~~~ _ext{{External}}
  end
Loading
High-Level Assessment

Generating the Python BiDi protocol layer from the shared, binding-neutral schema is the right long-term approach: it prevents per-binding drift, matches the Ruby precedent, and enables schema-driven runtime validation (objectOnly/preserveExtras/scalar) required by the ADR contract. Keeping generated domain modules out of git (with only the runtime checked in) minimizes review noise while preserving debuggability and local regeneration via Bazel.

Files changed (41) +6170 / -7 · 14 not counted

Enhancement (6) +1621 / -0
generate_bidi_protocol.pyAdd schema→Python generator for internal BiDi protocol modules +1062/-0

Add schema→Python generator for internal BiDi protocol modules

• Adds the Python generator that consumes the projected schema JSON and emits domain modules into 'selenium/webdriver/common/_bidi/', including wiring of runtime metadata (wire names, nullability, enum/ref/primitive, map-scalar handling), union selectors, and spec links.

py/generate_bidi_protocol.py

__init__.pyIntroduce internal _bidi package with warnings and regeneration notes +30/-0

Introduce internal _bidi package with warnings and regeneration notes

• Adds internal-API documentation and explains that domain modules are generated at build time and not checked in, with local regeneration instructions.

py/selenium/webdriver/common/_bidi/init.py

domain.pyAdd Domain base class that reuses the session’s shared transport +54/-0

Add Domain base class that reuses the session’s shared transport

• Implements a base class that accepts either a 'Transport' or a 'WebDriver', starting BiDi lazily when needed and routing domain commands through '_execute'.

py/selenium/webdriver/common/_bidi/domain.py

serialization.pyAdd strict serialization runtime (Record/Union/UNSET) for generated BiDi types +410/-0

Add strict serialization runtime (Record/Union/UNSET) for generated BiDi types

• Implements strict inbound validation and outbound serialization for generated dataclass records and discriminated unions, including UNSET vs null semantics, lazy type registry resolution, enum/primitive checks, 'objectOnly' union rejection of non-objects, and 'scalar' map-key validation. Supports gated extras preservation via '_EXTENSIBLE' (driven by schema 'preserveExtras').

py/selenium/webdriver/common/_bidi/serialization.py

transport.pyAdd Transport seam from generated layer to the websocket round-trip +58/-0

Add Transport seam from generated layer to the websocket round-trip

• Introduces 'Transport.execute()' which serializes params (if present), sends via 'send_cmd', raises on BiDi 'error', and parses 'result' into the declared generated result type.

py/selenium/webdriver/common/_bidi/transport.py

webdriver.pyAdd shared '_bidi_transport' lifecycle to Remote WebDriver +7/-0

Add shared '_bidi_transport' lifecycle to Remote WebDriver

• Adds a '_bidi_transport' attribute, resets it on quit, and instantiates it during '_start_bidi()' via a lazy import so non-BiDi users don’t pay import cost for the internal generated package.

py/selenium/webdriver/remote/webdriver.py

Refactor (1) +18 / -3
websocket_connection.pyRefactor BiDi command send/receive and add 'send_cmd' for non-coroutine callers +18/-3

Refactor BiDi command send/receive and add 'send_cmd' for non-coroutine callers

• Extracts shared id/envelope/round-trip logic into '_round_trip()' and adds 'send_cmd(method, params)' that returns raw replies without raising, used by the new '_bidi' transport. Updates existing 'execute()' to delegate to '_round_trip()'.

py/selenium/webdriver/remote/websocket_connection.py

Tests (29) +4412 / -0
browser_tests.pyAdd generated _bidi Browser domain integration tests +274/-0

Add generated _bidi Browser domain integration tests

• Introduces browser integration coverage for the generated internal protocol layer via real browser sessions.

py/test/selenium/webdriver/common/_bidi/browser_tests.py

browsing_context_tests.pyAdd generated _bidi BrowsingContext integration tests +555/-0

Add generated _bidi BrowsingContext integration tests

• Adds end-to-end tests that exercise generated browsing context commands and typed results against real browsers.

py/test/selenium/webdriver/common/_bidi/browsing_context_tests.py

emulation_tests.pyAdd generated _bidi Emulation integration tests +527/-0

Add generated _bidi Emulation integration tests

• Adds integration tests that validate generated emulation commands work against a browser.

py/test/selenium/webdriver/common/_bidi/emulation_tests.py

errors_tests.pyAdd generated _bidi error-handling integration tests +121/-0

Add generated _bidi error-handling integration tests

• Adds integration tests validating error cases and raising behavior when driving the generated layer.

py/test/selenium/webdriver/common/_bidi/errors_tests.py

input_tests.pyAdd generated _bidi Input integration tests +542/-0

Add generated _bidi Input integration tests

• Adds browser integration tests for generated input commands and relevant payload shapes.

py/test/selenium/webdriver/common/_bidi/input_tests.py

network_tests.pyAdd generated _bidi Network integration tests +62/-0

Add generated _bidi Network integration tests

• Adds integration tests for generated network commands and types.

py/test/selenium/webdriver/common/_bidi/network_tests.py

permissions_tests.pyAdd generated _bidi Permissions integration tests +114/-0

Add generated _bidi Permissions integration tests

• Adds integration tests for generated permissions commands against a browser session.

py/test/selenium/webdriver/common/_bidi/permissions_tests.py

script_tests.pyAdd generated _bidi Script integration tests +623/-0

Add generated _bidi Script integration tests

• Adds integration tests for generated script commands and their typed results.

py/test/selenium/webdriver/common/_bidi/script_tests.py

session_tests.pyAdd generated _bidi Session integration tests +43/-0

Add generated _bidi Session integration tests

• Adds integration tests that validate generated session commands against a browser session.

py/test/selenium/webdriver/common/_bidi/session_tests.py

storage_tests.pyAdd generated _bidi Storage integration tests +525/-0

Add generated _bidi Storage integration tests

• Adds integration tests for generated storage commands and payload shapes, including extras behavior driven by 'preserveExtras'.

py/test/selenium/webdriver/common/_bidi/storage_tests.py

webextension_tests.pyAdd generated _bidi WebExtension integration tests +171/-0

Add generated _bidi WebExtension integration tests

• Adds integration tests for generated web extension commands against a browser session.

py/test/selenium/webdriver/common/_bidi/webextension_tests.py

browser_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout (no substantive behavior changes implied by this diff summary).

py/test/selenium/webdriver/common/bidi/browser_tests.py

browsing_context_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/browsing_context_tests.py

emulation_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/emulation_tests.py

errors_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/errors_tests.py

input_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/input_tests.py

integration_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/integration_tests.py

log_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout (and absorbs coverage removed from the deleted 'bidi_tests.py').

py/test/selenium/webdriver/common/bidi/log_tests.py

network_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/network_tests.py

permissions_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/permissions_tests.py

protocol_tests.pyAdd browser round-trip tests for generated internal '_bidi' layer +70/-0

Add browser round-trip tests for generated internal '_bidi' layer

• Adds end-to-end tests that drive generated '_bidi' modules directly against a real browser, validating real wire payload shapes (including nested record/list results).

py/test/selenium/webdriver/common/bidi/protocol_tests.py

quit_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/quit_tests.py

script_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout (and absorbs coverage removed from the deleted 'bidi_tests.py').

py/test/selenium/webdriver/common/bidi/script_tests.py

session_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/session_tests.py

storage_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/storage_tests.py

webextension_tests.pyReorganize existing BiDi tests under 'bidi/' folder not counted

Reorganize existing BiDi tests under 'bidi/' folder

• Moves/renames the test module into the new 'py/test/selenium/webdriver/common/bidi/' layout.

py/test/selenium/webdriver/common/bidi/webextension_tests.py

bidi_protocol_command_tests.pyAdd unit tests for generated command methods and schema-driven validation +185/-0

Add unit tests for generated command methods and schema-driven validation

• Tests generated domains using a stand-in connection to validate command dispatch, param serialization, typed result parsing, and pre-wire validation (enum values, discriminator membership). Includes tests for extras preservation vs dropping per 'preserveExtras'.

py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py

bidi_serialization_tests.pyAdd unit tests for the serialization runtime behaviors (Record/Union/UNSET) +453/-0

Add unit tests for the serialization runtime behaviors (Record/Union/UNSET)

• Adds isolated runtime tests for required/optional/nullable semantics, strict primitive checks, enum validation, union selection rules (discriminator + presence), 'objectOnly' rejection of non-object payloads, and 'scalar' map-key passthrough with primitive validation.

py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py

bidi_transport_tests.pyAdd unit tests for Transport and Domain seam behavior +147/-0

Add unit tests for Transport and Domain seam behavior

• Adds tests that verify 'Transport.execute()' wire framing expectations against a driving stand-in and confirms Domain can accept a Transport, reuse a driver’s shared transport, or start BiDi lazily when needed.

py/test/unit/selenium/webdriver/common/bidi_transport_tests.py

Documentation (1) +1 / -1
TESTING.mdUpdate documented bazel test target path after test reorg +1/-1

Update documented bazel test target path after test reorg

• Adjusts the example command to reflect that BiDi tests now live under 'test/selenium/webdriver/common/bidi/' rather than flattened filenames.

py/TESTING.md

Other (4) +118 / -3
.gitignoreIgnore generated _bidi domain modules while keeping runtime files tracked +6/-0

Ignore generated _bidi domain modules while keeping runtime files tracked

• Adds ignore rules for all files under 'py/selenium/webdriver/common/_bidi/*' but explicitly un-ignores the four hand-written runtime files ('__init__.py', 'serialization.py', 'transport.py', 'domain.py').

.gitignore

BUILD.bazelAdd Bazel generation + library targets for the internal _bidi protocol layer +50/-2

Add Bazel generation + library targets for the internal _bidi protocol layer

• Introduces 'generate_bidi_protocol' plumbing (rule invocation, generator binaries, and 'bidi_protocol' py_library combining generated sources with the checked-in runtime). Updates test globs to split 'bidi/' and '_bidi/' and adds ':bidi_protocol' as a dependency where tests import generated modules directly.

py/BUILD.bazel

generate_bidi_protocol.bzlAdd Bazel rule to run the BiDi protocol generator over the shared schema +61/-0

Add Bazel rule to run the BiDi protocol generator over the shared schema

• Defines a 'generate_bidi_protocol' rule that executes the generator in the exec configuration, producing one '.py' output per BiDi domain module under the target package directory.

py/private/generate_bidi_protocol.bzl

python.rakeInclude 'common/_bidi' output in Python 'local_dev' task +1/-1

Include 'common/_bidi' output in Python 'local_dev' task

• Updates local-dev copy logic to include 'common/_bidi' alongside 'common/bidi' and 'common/devtools' so build-generated protocol modules land in the working tree for development workflows.

rake_tasks/python.rake

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new internal Python WebDriver BiDi protocol layer under selenium.webdriver._bidi, generated from the shared binding-neutral BiDi schema, plus a small handwritten runtime (serialization + transport seam) and Bazel/CI wiring to keep checked-in generated files in sync.

Changes:

  • Introduces the generated internal _bidi protocol package (domains, types, commands/events metadata) and a handwritten runtime (serialization.py, transport.py).
  • Wires RemoteWebDriver to create a per-session _bidi_transport lazily when BiDi is started.
  • Adds unit + end-to-end tests, plus a verify-bidi-protocol Bazel target and CI step to detect drift/hand-edits.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
py/verify_bidi_protocol.py Verifies checked-in _bidi files match generator output.
py/test/unit/selenium/webdriver/common/bidi_transport_tests.py Unit coverage for the handwritten transport/domain seam.
py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py Unit coverage for strict (de)serialization runtime rules.
py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py Exercises generated command surfaces over a stand-in transport.
py/test/selenium/webdriver/common/bidi_protocol_tests.py Browser-level sanity checks that generated types round-trip against a real BiDi session.
py/selenium/webdriver/remote/webdriver.py Adds _bidi_transport lifecycle + lazy import/creation in _start_bidi().
py/selenium/webdriver/_bidi/__init__.py Generated package initializer exporting domain classes.
py/selenium/webdriver/_bidi/serialization.py Handwritten serialization runtime used by generated records/unions (plus registry).
py/selenium/webdriver/_bidi/transport.py Handwritten websocket seam (Transport) and base domain (Domain).
py/selenium/webdriver/_bidi/browsing_context.py Generated browsingContext domain (types + commands + event metadata).
py/selenium/webdriver/_bidi/bluetooth.py Generated bluetooth domain (types + commands + event metadata).
py/selenium/webdriver/_bidi/browser.py Generated browser domain (types + commands).
py/selenium/webdriver/_bidi/emulation.py Generated emulation domain (types + commands).
py/selenium/webdriver/_bidi/input.py Generated input domain (types + commands + event metadata).
py/selenium/webdriver/_bidi/log.py Generated log domain (types + event metadata).
py/selenium/webdriver/_bidi/network.py Generated network domain (types + commands + event metadata).
py/selenium/webdriver/_bidi/permissions.py Generated permissions domain (types + commands).
py/selenium/webdriver/_bidi/script.py Generated script domain (types + commands + event metadata).
py/selenium/webdriver/_bidi/session.py Generated session domain (types + commands).
py/selenium/webdriver/_bidi/speculation.py Generated speculation domain (types + event metadata).
py/selenium/webdriver/_bidi/storage.py Generated storage domain (types + commands).
py/selenium/webdriver/_bidi/user_agent_client_hints.py Generated userAgentClientHints domain (types + commands).
py/selenium/webdriver/_bidi/web_extension.py Generated webExtension domain (types + commands).
py/BUILD.bazel Adds generator target, _bidi library, and verify test; packages _bidi.
.github/workflows/ci-python.yml Runs //py:verify-bidi-protocol in CI alongside unit tests.

Comment thread py/selenium/webdriver/_bidi/serialization.py
Comment thread py/selenium/webdriver/_bidi/serialization.py
Comment thread py/selenium/webdriver/_bidi/serialization.py
@qodo-code-review

qodo-code-review Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Union scalar bypass ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Union.from_json() returns any non-dict payload unchanged, so union-typed fields can silently
accept unexpected scalar values even when the union has no scalar arm. This weakens the PR’s strict
inbound validation because _read_scalar() routes union refs to Union.from_json() without
enforcing object-ness.
Code

py/selenium/webdriver/_bidi/serialization.py[R300-306]

+    @classmethod
+    def from_json(cls, payload: Any) -> Any:
+        # A bare scalar arm (e.g. input.Origin's "viewport") has no object to
+        # dispatch on, so it is returned unchanged.
+        if not isinstance(payload, dict):
+            return payload
+        variant = cls._select(payload)
Evidence
_read_scalar() allows non-dict values for union refs and delegates to Union.from_json(), and
Union.from_json() immediately returns non-dict values unchanged—so scalars bypass union validation
entirely.

py/selenium/webdriver/_bidi/serialization.py[262-277]
py/selenium/webdriver/_bidi/serialization.py[300-312]

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

## Issue description
`Union.from_json()` currently accepts any non-dict payload by returning it unchanged. This allows malformed payloads (scalar instead of object) to pass through for any union-ref field.

## Issue Context
`_read_scalar()` explicitly skips the "must be dict" check for unions and calls `klass.from_json(raw)`. With the current `Union.from_json` early return, that means *all* unions accept scalars, not just those that legitimately have scalar arms.

## Fix Focus Areas
- py/selenium/webdriver/_bidi/serialization.py[262-277]
- py/selenium/webdriver/_bidi/serialization.py[300-312]

## Implementation direction
- Make scalar acceptance opt-in per union (e.g., `_ALLOW_SCALAR: bool` or `_SCALAR_PRIMITIVE: str|None`), defaulting to rejecting non-dict payloads.
- Have the generator set this flag only for unions that truly include a scalar arm (and optionally validate the scalar type/value when enabled).

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


2. Presence keys mismatch ✗ Dismissed 🐞 Bug ≡ Correctness
Description
The generated script.RemoteReference union uses a presence key sharedId (wire name), but
Union.build() checks presence against Python kwargs keys; because the corresponding Python field
is shared_id, RemoteReference.build(shared_id=...) cannot match a variant and will raise
BiDiSerializationError. This makes outbound construction via Union.build() incorrect for this
union (and any similar presence union where wire keys differ from Python names).
Code

py/selenium/webdriver/_bidi/script.py[R817-820]

+@register("script.RemoteReference")
+class RemoteReference(Union):
+    _PRESENCE = (("script.SharedReference", ("sharedId",)), ("script.RemoteObjectReference", ("handle",)))
+
Evidence
RemoteReference presence selection is keyed on sharedId, but the generated record field name is
shared_id; Union._outbound() checks kwargs.get(key) for each presence key, so a call providing
shared_id=... will never satisfy the sharedId presence check and the union cannot be built via
build().

py/selenium/webdriver/_bidi/script.py[817-823]
py/selenium/webdriver/_bidi/script.py[309-324]
py/selenium/webdriver/_bidi/serialization.py[353-364]

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

## Issue description
`Union.build()` uses `_PRESENCE` keys to check `kwargs.get(key)`, but `script.RemoteReference._PRESENCE` contains the wire key `sharedId` while the Python field/kwargs name is `shared_id`. This prevents selecting the SharedReference arm when building outbound values.

## Issue Context
- Inbound selection wants wire keys (e.g., `sharedId` in the payload).
- Outbound selection wants Python keys (e.g., `shared_id` in kwargs).
Today these are forced to share one `_PRESENCE` table, which breaks when they differ.

## Fix Focus Areas
- py/selenium/webdriver/_bidi/serialization.py[353-364]
- py/selenium/webdriver/_bidi/script.py[817-820]

## Implementation direction
- Introduce separate presence tables for inbound vs outbound (e.g., `_PRESENCE_WIRE` and `_PRESENCE_FIELDS`), and have `_select()` use wire keys while `_outbound()` uses python field names.
- Update the generator to emit both sets for unions that use presence selection (or emit python names and have `_select()` also accept wire-name equivalents via a mapping).

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


3. _command() missing return annotation ✓ Resolved 📘 Rule violation ✧ Quality
Description
The newly added generator helper _command() has no explicit return type annotation, violating the
requirement that new function signatures include return annotations. This reduces type-checking
coverage and can cause inconsistent typing across the new BiDi transport seam.
Code

py/selenium/webdriver/_bidi/transport.py[R37-45]

+def _command(method: str, params: dict):
+    """A command_builder coroutine: yield the wire frame, return the raw result.
+
+    ``WebSocketConnection.execute`` speaks this protocol (``next`` then ``send``).
+    Params arrive already wire-shaped (from ``Record.as_json``), so the connection's
+    dataclass encoder passes them through untouched — no double camelCasing.
+    """
+    result = yield {"method": method, "params": params}
+    return result
Evidence
PR Compliance ID 337802 requires explicit type annotations on new function signatures, including
return types. The added _command() definition has parameter annotations but omits the -> ...
return annotation.

Rule 337802: Require type annotations on new function and method signatures
py/selenium/webdriver/_bidi/transport.py[37-45]

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

## Issue description
`py/selenium/webdriver/_bidi/transport.py` introduces `_command()` without an explicit return type annotation.

## Issue Context
Compliance requires all new/modified Python function and method signatures to include parameter annotations and an explicit return type annotation. `_command()` is a generator-style coroutine used with `WebSocketConnection.execute`, so its signature should still declare a return type (e.g., a `Generator[...]` type, or at minimum `-> Any`).

## Fix Focus Areas
- py/selenium/webdriver/_bidi/transport.py[37-45]

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



Remediation recommended

4. Import-order type failures ✗ Dismissed 🐞 Bug ☼ Reliability
Description
Cross-domain refs (e.g. browsingContext.ElementClipRectangle referencing script.SharedReference)
rely on the global registry, but domain modules don’t import their dependencies at runtime;
importing a single domain module can therefore fail at (de)serialization time with `unknown BiDi
type ... (module not imported?)`. This is an import-order dependent runtime failure (package-level
selenium.webdriver._bidi import avoids it by importing all domains).
Code

py/selenium/webdriver/_bidi/browsing_context.py[R192-196]

+@register("browsingContext.ElementClipRectangle")
+@dataclass(frozen=True)
+class ElementClipRectangle(Record):
+    element: SharedReference = field(metadata=meta("element", required=True, ref="script.SharedReference"))
+    type: str = field(default="element", init=False, metadata=meta("type", required=True, fixed="element"))
Evidence
The generated record contains a ref to script.SharedReference, but the module doesn’t import
script; if script hasn’t been imported elsewhere, resolve('script.SharedReference') raises
with an explicit “module not imported?” message.

py/selenium/webdriver/_bidi/browsing_context.py[23-30]
py/selenium/webdriver/_bidi/browsing_context.py[192-197]
py/selenium/webdriver/_bidi/serialization.py[132-137]

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

## Issue description
Some generated types reference schema names in other domain modules via `ref=...`, but the registry is only populated when those other modules have been imported. If users import only one domain module (not the package `selenium.webdriver._bidi`), deserialization of cross-domain refs can fail.

## Issue Context
This is primarily an import-order hazard. Package-level import (`import selenium.webdriver._bidi`) imports all domain modules and registers all types, but direct module imports do not.

## Fix Focus Areas
- py/selenium/webdriver/_bidi/serialization.py[132-137]
- py/selenium/webdriver/_bidi/browsing_context.py[192-197]
- py/selenium/webdriver/_bidi/browsing_context.py[23-30]

## Implementation direction
- Option A (runtime): Enhance `resolve()` to attempt a lazy import based on the schema name prefix before failing (e.g., importing `selenium.webdriver._bidi.script` when resolving `script.*`).
- Option B (generator): Emit minimal runtime imports for referenced domains (with care to avoid cycles), or provide a documented/obvious "load all" initializer that callers must invoke.

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



Informational

5. Verifier misses extra files 🐞 Bug ⚙ Maintainability
Description
verify_bidi_protocol.py only checks that files produced by the generator match the committed
copies; it never fails if _bidi/ contains extra .py files not produced by the generator. This
can allow removed/renamed generated modules to linger unnoticed.
Code

py/verify_bidi_protocol.py[R36-41]

+    rendered = gen.render_all(schema_path)
+    stale = [
+        name
+        for name, contents in rendered.items()
+        if not (bidi_dir / name).exists() or (bidi_dir / name).read_text() != contents
+    ]
Evidence
The script computes stale solely from rendered.items() and never compares the on-disk directory
listing against the rendered set, so extra files are ignored.

py/verify_bidi_protocol.py[32-41]

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 verification step does not detect extra files present on disk but not produced by `render_all()`, so stale generated modules can persist.

## Issue Context
This is a drift-guard completeness gap: it catches mismatches for expected outputs but not unexpected leftovers.

## Fix Focus Areas
- py/verify_bidi_protocol.py[36-41]

## Implementation direction
- After computing `rendered`, list `bidi_dir/*.py` (and possibly exclude hand-written runtime files like `serialization.py`, `transport.py`) and fail if any generated-looking file is not in `rendered.keys()`.
- Print the unexpected file list alongside stale mismatches.

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


Grey Divider

Qodo Logo

Comment thread py/selenium/webdriver/_bidi/transport.py Outdated
Comment thread py/selenium/webdriver/_bidi/script.py Outdated
Comment thread py/selenium/webdriver/common/_bidi/serialization.py
Comment thread py/selenium/webdriver/_bidi/browsing_context.py Outdated
Comment thread py/verify_bidi_protocol.py Outdated
@titusfortner
titusfortner marked this pull request as draft July 10, 2026 15:21
Comment thread py/selenium/webdriver/_bidi/browser.py Outdated
@AutomatedTester

Copy link
Copy Markdown
Member

The direction of this looks ok so far but need to see how this will be used as that code isn't here

@AutomatedTester

Copy link
Copy Markdown
Member

PR has a lot of unnecessary files in it now in case you're not aware

@titusfortner

Copy link
Copy Markdown
Member Author

@AutomatedTester yes, to work with the latest schema it pulled in #17784 which should merge soon

@titusfortner
titusfortner force-pushed the bidi-gen-py branch 3 times, most recently from 6057552 to a227d86 Compare July 20, 2026 16:10
@titusfortner

Copy link
Copy Markdown
Member Author

need to see how this will be used as that code isn't here

@AutomatedTester to clarify, the intention is that our users will write this code if they need it. The usage in selenium will be limited to re-implementing current API and whatever additional facades we agree to, so I can't give examples for any of that yet, bu to give a better view of what usage looks like, I ported all the existing integration tests to use this code.

py/test/selenium/webdriver/common/_bidi/. (instantiates Domain(driver), pass typed records in, read typed results out, against real browsers).

A few concrete comparisons

# convenience method — returns the handle
handle = driver.browsing_context.create(type=WindowTypes.TAB)

# generated classes — read .context off the typed result
handle = BrowsingContext(driver).create(type=CreateType.TAB).context
# convenience method — plain string name
driver.permissions.set_permission("geolocation", PermissionState.GRANTED, origin)

# generated classes — wrap the name in a descriptor
Permissions(driver).set_permission(PermissionDescriptor(name="geolocation"), PermissionState.GRANTED, origin)
# convenience method — dict in, raw dicts out
nodes = driver.browsing_context.locate_nodes(context=cid, locator={"type": "css", "value": "div.content"})
name = nodes[0]["value"]["localName"]

# generated classes — typed locator in, typed nodes out
nodes = BrowsingContext(driver).locate_nodes(context=cid, locator=CssLocator(value="div.content")).nodes
name = nodes[0].value.local_name
# convenience method — flat bool + folder
driver.browser.set_download_behavior(allowed=True, destination_folder=folder)

# generated classes — pass the typed variant
Browser(driver).set_download_behavior(DownloadBehaviorAllowed(destination_folder=folder))

@titusfortner
titusfortner force-pushed the bidi-gen-py branch 2 times, most recently from 5899a33 to 2b06fe9 Compare July 21, 2026 20:38
@titusfortner
titusfortner marked this pull request as ready for review July 21, 2026 20:38
@qodo-code-review

qodo-code-review Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Record.from_json non-dict crash ✓ Resolved 🐞 Bug ☼ Reliability
Description
Record.from_json() assumes payload is a dict and can raise raw TypeError/AttributeError when
the wire payload is not an object, leaking non-WebDriverException errors to callers. This can
surface via Transport.execute() which calls result.from_json(...) directly on websocket replies
without normalizing failures to BiDiSerializationError.
Code

py/selenium/webdriver/common/_bidi/serialization.py[R223-235]

+    @classmethod
+    def from_json(cls, payload: dict) -> Any:
+        kwargs: dict = {}
+        known: set[str] = set()
+        for f in _fields(cls):
+            w = _wire_of(f)
+            known.add(w.wire)
+            if w.fixed is not UNSET or f.name == "extensions":
+                continue
+            kwargs[f.name] = _read_field(cls, f.name, w, payload)
+        if cls._EXTENSIBLE:
+            kwargs["extensions"] = {k: v for k, v in payload.items() if k not in known}
+        return cls(**kwargs)
Evidence
Record.from_json uses dict operations (_read_field(..., payload) and payload.items()) without
a runtime type check; if payload is not a dict this will raise raw Python exceptions.
Transport.execute calls result.from_json(value) on the reply payload, so these raw exceptions
propagate instead of becoming BiDiSerializationError (a WebDriverException).

py/selenium/webdriver/common/_bidi/serialization.py[223-235]
py/selenium/webdriver/common/_bidi/transport.py[52-58]
py/selenium/webdriver/common/_bidi/serialization.py[44-45]

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

### Issue description
`Record.from_json` assumes the payload is a `dict` and will throw raw Python exceptions if it receives a non-dict payload (e.g., browser returns a scalar/array where an object is expected). This breaks the intended error boundary: deserialization failures should raise `BiDiSerializationError` (a `WebDriverException`), not `TypeError`/`AttributeError`.

### Issue Context
`Transport.execute` calls `result.from_json(value)` directly on the raw websocket reply result, so any non-`BiDiSerializationError` exceptions escape to callers.

### Fix Focus Areas
- py/selenium/webdriver/common/_bidi/serialization.py[223-235]
- py/selenium/webdriver/common/_bidi/transport.py[52-58]
- py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py[1-453]

### Suggested fix
1. Change `Record.from_json` signature to accept `payload: Any` (or keep `dict` but still validate).
2. At the top of `Record.from_json`, add:
  - `if not isinstance(payload, dict): raise BiDiSerializationError(f"{cls.__name__} expected an object on the wire, got {type(payload).__name__} {payload!r}")`
3. Add a unit test that calls a small Record type’s `from_json` with a non-dict (e.g. `[]` or `"x"`) and asserts `BiDiSerializationError` is raised.

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


2. Schema drift breaks Bazel ✓ Resolved 🐞 Bug ☼ Reliability
Description
The Bazel rule declares a fixed set of generated domain module outputs, but the generator renders
modules dynamically from the schema’s domains; a schema domain add/remove/rename can cause missing
declared outputs or undeclared outputs, breaking builds.
Code

py/private/generate_bidi_protocol.bzl[R8-36]

+# Generated domain modules, snake_case (one per BiDi domain in the schema). The
+# package ``__init__.py`` is hand-written and checked in, so it is not generated here.
+_MODULES = [
+    "bluetooth",
+    "browser",
+    "browsing_context",
+    "emulation",
+    "input",
+    "log",
+    "network",
+    "permissions",
+    "script",
+    "session",
+    "speculation",
+    "storage",
+    "user_agent_client_hints",
+    "web_extension",
+]
+
+def _generate_bidi_protocol_impl(ctx):
+    outputs = [ctx.actions.declare_file(ctx.attr.package + "/" + name + ".py") for name in _MODULES]
+
+    ctx.actions.run(
+        inputs = [ctx.file.schema],
+        outputs = outputs,
+        executable = ctx.executable.generator,
+        # The generator writes one file per domain into the output dir.
+        arguments = [ctx.file.schema.path, outputs[-1].dirname],
+        use_default_shell_env = True,
Evidence
The Bazel rule hard-codes _MODULES and uses it to declare outputs, but the generator builds
modules from schema.domains() and writes all of them to the output directory, making the output
set schema-dependent rather than _MODULES-dependent.

py/private/generate_bidi_protocol.bzl[8-39]
py/generate_bidi_protocol.py[262-283]
py/generate_bidi_protocol.py[1026-1035]

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

### Issue description
`generate_bidi_protocol.bzl` declares a fixed `_MODULES` output list, but `generate_bidi_protocol.py` generates one module per domain discovered in the schema at runtime. When the schema changes, Bazel can either:
- fail because a declared output wasn’t created, or
- fail/sandbox-violate by writing extra (undeclared) outputs.

### Issue Context
Bazel actions should only write declared outputs; right now the generator is schema-driven while the rule is list-driven.

### Fix Focus Areas
- py/private/generate_bidi_protocol.bzl[8-39]
- py/generate_bidi_protocol.py[1026-1058]

### Suggested fix approach
Pick one single source of truth:
1) **Preferred**: change the Bazel rule to declare a single output directory (tree artifact) and have the generator write all modules into it; return that directory in `DefaultInfo`.
2) **Alternative**: pass the expected module list from the Bazel rule into the generator (e.g., a `--modules` arg), and make the generator:
  - generate *only* those modules, and
  - error out if the schema domains don’t exactly match the expected list (so drift is caught deterministically without writing undeclared files).

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



Remediation recommended

3. Transport mis-detects error replies ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description
Transport.execute() uses if reply.get("error"): which only triggers on truthy error values; an
error envelope with a falsy error value will fall through and then raise KeyError on
reply["result"]. This is inconsistent with WebSocketConnection.execute() which treats the
presence of the error key as an error (if "error" in response).
Code

py/selenium/webdriver/common/_bidi/transport.py[R52-58]

+    def execute(self, cmd: str, params: Any = None, result: Any = None) -> Any:
+        reply = self._connection.send_cmd(cmd, params.as_json() if params is not None else {})
+        if reply.get("error"):
+            message = reply.get("message")
+            raise WebDriverException(f"{reply['error']}: {message}" if message else reply["error"])
+        value = reply["result"]
+        return result.from_json(value) if result is not None else value
Evidence
Transport’s error detection is currently based on truthiness, while the websocket connection’s own
execute path treats any presence of the error key as an error; this mismatch means Transport can
miss some error envelopes and then crash on missing result.

py/selenium/webdriver/common/_bidi/transport.py[52-58]
py/selenium/webdriver/remote/websocket_connection.py[148-160]

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

## Issue description
`Transport.execute()` currently uses `if reply.get("error"):` to detect error replies. This can miss replies that include an `error` key whose value is falsy (e.g., empty string / None), and then it will attempt `reply["result"]`, raising a `KeyError` rather than a `WebDriverException`.

## Issue Context
The underlying websocket implementation uses presence-based semantics (`if "error" in response:`), so Transport should match that contract.

## Fix Focus Areas
- py/selenium/webdriver/common/_bidi/transport.py[52-58]
- py/selenium/webdriver/remote/websocket_connection.py[148-160]

### Suggested fix
Change the condition to `if "error" in reply:` (and ideally also guard for missing `result` with a clearer exception). Keep using `message = reply.get("message")` for formatting.

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


4. Bazel picks local generated files ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New
Description
The bidi_protocol py_library globs selenium/webdriver/common/_bidi/*.py in the workspace in
addition to depending on the generated outputs from :create-bidi-protocol-src, so after running
the documented local generator/copy workflows it can see both workspace copies and Bazel-generated
copies of the same domain modules. This can cause Bazel analysis/build conflicts (competing
artifacts for the same module paths) or inconsistent packaging inputs for _bidi.
Code

py/BUILD.bazel[R786-790]

+py_library(
+    name = "bidi_protocol",
+    # Hand-written runtime from source + the generated domain modules from the rule.
+    srcs = [":create-bidi-protocol-src"] + glob(["selenium/webdriver/common/_bidi/*.py"]),
+    imports = ["."],
Evidence
The build rule currently adds workspace _bidi/*.py files via glob, while the generator defaults to
writing generated domain modules into that same workspace directory under bazel run (and local_dev
copies _bidi into the tree). This makes it possible for the same domain module filenames to appear
both as workspace sources and as Bazel-generated outputs for :create-bidi-protocol-src.

py/BUILD.bazel[755-793]
py/generate_bidi_protocol.py[45-48]
py/generate_bidi_protocol.py[1051-1072]
rake_tasks/python.rake[72-90]

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

## Issue description
`py/BUILD.bazel` defines `:bidi_protocol` as `srcs = [":create-bidi-protocol-src"] + glob(["selenium/webdriver/common/_bidi/*.py"])`. The generator’s `bazel run //py:generate-bidi-protocol` default writes generated domain modules into the *workspace* under `py/selenium/webdriver/common/_bidi/`, and `rake local_dev` also copies `common/_bidi` into the source tree. Once those files exist locally, the glob will start including generated domain modules from the workspace *in addition to* the Bazel-generated outputs, creating conflicting/duplicated artifacts for the same import paths.

## Issue Context
The intention (per comments) is: runtime files are checked in, domain modules are generated by Bazel. Globbing `*.py` in that directory defeats this separation once local generation is performed.

## Fix Focus Areas
- py/BUILD.bazel[755-793]
- py/generate_bidi_protocol.py[1051-1072]
- rake_tasks/python.rake[72-90]

### Suggested fix
Replace the glob with an explicit list of the four hand-written runtime files (and keep `:create-bidi-protocol-src` for generated domain modules), e.g.:
- `srcs = [":create-bidi-protocol-src", "selenium/webdriver/common/_bidi/__init__.py", "selenium/webdriver/common/_bidi/domain.py", "selenium/webdriver/common/_bidi/serialization.py", "selenium/webdriver/common/_bidi/transport.py"]`
This prevents locally generated domain modules from being picked up as additional sources.

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


5. No outbound primitive/list validation 🐞 Bug ≡ Correctness
Description
Record.__post_init__() validates nullability and enums only, so wrong primitive types,
list-vs-scalar mismatches, and invalid nested ref values can be serialized by as_json() and sent
on the wire unchanged. This allows constructing protocol objects that violate the schema and only
fail later as remote/protocol errors.
Code

py/selenium/webdriver/common/_bidi/serialization.py[R178-194]

+    def __post_init__(self) -> None:
+        for f in _fields(self):
+            if f.name == "extensions":
+                continue
+            w = _wire_of(f)
+            if w.fixed is not UNSET:
+                continue
+            value = getattr(self, f.name)
+            if value is UNSET:
+                continue
+            if value is None:
+                if not w.nullable:
+                    raise BiDiSerializationError(f"{type(self).__name__}.{f.name} cannot be None")
+                continue
+            if w.enum is not None:
+                self._validate_enum(f.name, w, value)
+
Evidence
Record.__post_init__ only checks nullability and enums; it does not use
primitive/is_list/ref metadata for outbound validation. Meanwhile inbound
_read_field/_read_scalar do enforce list shape and primitive types using _PRIMITIVE_CHECKS,
demonstrating the intended strictness is currently one-sided.

py/selenium/webdriver/common/_bidi/serialization.py[178-194]
py/selenium/webdriver/common/_bidi/serialization.py[238-255]
py/selenium/webdriver/common/_bidi/serialization.py[289-329]

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 outbound construction path for generated Records does not validate primitives (`w.primitive`), list shape (`w.is_list`), or referenced nested types (`w.ref`). As a result, invalid values can be serialized and sent to the browser without any local `BiDiSerializationError`.

### Issue Context
Inbound parsing is strict (checks list-ness and primitive types), but outbound construction is not, despite having the schema facts (`primitive`, `is_list`, `ref`) and `_PRIMITIVE_CHECKS` available.

### Fix Focus Areas
- py/selenium/webdriver/common/_bidi/serialization.py[178-205]
- py/selenium/webdriver/common/_bidi/serialization.py[238-255]
- py/selenium/webdriver/common/_bidi/serialization.py[289-329]
- py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py[1-453]

### Suggested fix
1. Extend `Record.__post_init__` to validate, for each non-UNSET/non-None field:
  - If `w.is_list` is True, assert `isinstance(value, list)`.
  - If `w.is_list` is False, assert `not isinstance(value, list)` when the field is typed (`primitive`/`enum`/`ref`) to avoid emitting arrays accidentally.
  - If `w.primitive` is set, validate using `_PRIMITIVE_CHECKS[w.primitive]` (and for list fields validate each element).
  - If `w.ref` is set, require values to be `Record` instances (or list of them) rather than arbitrary dict/scalars (unless explicitly intended).
2. Add unit tests showing that constructing a Record with:
  - a wrong primitive (e.g., `int` field passed a `str`), and
  - a list field passed a scalar,
  raises `BiDiSerializationError` before serialization.

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


View more (3)
6. Transport.execute() error branch untested ✓ Resolved 📘 Rule violation ☼ Reliability
Description
Transport.execute() introduces new error-handling logic when the websocket reply contains error,
but the added unit tests only cover successful result replies. This risks regressions in the
failure path going unnoticed and weakens confidence in the new transport seam’s correctness.
Code

py/selenium/webdriver/common/_bidi/transport.py[R52-56]

+    def execute(self, cmd: str, params: Any = None, result: Any = None) -> Any:
+        reply = self._connection.send_cmd(cmd, params.as_json() if params is not None else {})
+        if reply.get("error"):
+            message = reply.get("message")
+            raise WebDriverException(f"{reply['error']}: {message}" if message else reply["error"])
Evidence
PR Compliance ID 5 requires new behavior to be covered by automated tests. The new
Transport.execute() error-raising logic is present in the transport implementation, while the
added unit tests’ stand-in connection only returns {"result": ...} and therefore never exercises
the error path.

AGENTS.md: Add/Prefer Automated Tests; Prefer Small Unit Tests and Avoid Mocks
py/selenium/webdriver/common/_bidi/transport.py[52-56]
py/test/unit/selenium/webdriver/common/bidi_transport_tests.py[49-107]

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

## Issue description
`Transport.execute()` has an error-handling branch (when the reply contains `error`) that is not covered by the new unit tests.

## Issue Context
The new internal BiDi transport seam is a core piece that will be shared by generated domains; its error path should be unit-tested to prevent silent regressions.

## Fix Focus Areas
- py/selenium/webdriver/common/_bidi/transport.py[52-56]
- py/test/unit/selenium/webdriver/common/bidi_transport_tests.py[49-107]

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


7. Extensions overwrite known fields ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Record.as_json() merges extensions into the serialized payload via payload.update(...), so a
caller-supplied extension key can overwrite a known wire field and silently change what is sent on
the wire.
Code

py/selenium/webdriver/common/_bidi/serialization.py[R207-221]

+    def as_json(self) -> dict:
+        payload: dict = {}
+        for f in _fields(self):
+            if f.name == "extensions":
+                continue
+            w = _wire_of(f)
+            value = getattr(self, f.name)
+            if value is UNSET:
+                continue
+            if value is None and not w.nullable:
+                continue
+            payload[w.wire] = _as_json(value)
+        if self._EXTENSIBLE:
+            payload.update(getattr(self, "extensions", None) or {})
+        return payload
Evidence
The serialization code writes known wire keys first, then unconditionally updates with extensions,
which gives extensions precedence over declared fields.

py/selenium/webdriver/common/_bidi/serialization.py[207-235]

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

### Issue description
For extensible records, `Record.as_json()` does `payload.update(extensions)`. If `extensions` contains a key that matches a known wire field, it overwrites the serialized value from the actual dataclass field, producing an incorrect wire payload.

### Issue Context
`from_json()` only captures unknown keys into `extensions`, so round-trips won’t collide. But callers can still construct a record with arbitrary `extensions` and accidentally (or intentionally) override real fields.

### Fix Focus Areas
- py/selenium/webdriver/common/_bidi/serialization.py[207-221]

### Suggested fix approach
In `Record.as_json()` when `_EXTENSIBLE`:
- compute `extras = getattr(self, "extensions", None) or {}`
- if `set(extras) & set(payload)` is non-empty, raise `BiDiSerializationError` identifying the colliding keys (or, at minimum, ensure declared fields win by merging as `{**extras, **payload}` so extras cannot override).

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


8. send_cmd() lacks direct tests 📘 Rule violation ☼ Reliability
Description
WebSocketConnection.send_cmd() is newly added but no unit test exercises the real
WebSocketConnection send/reply path; existing BiDi tests use stand-in connections instead. This
increases the risk of regressions in the core BiDi command transport path going undetected.
Code

py/selenium/webdriver/remote/websocket_connection.py[R140-146]

+    def send_cmd(self, method, params):
+        """Send one command and return its raw reply (``result`` or ``error`` left intact).
+
+        The dumb-send counterpart to ``execute``: no coroutine, no error raising. The
+        caller parses the reply into its declared type and raises on ``error`` itself.
+        """
+        return self._round_trip({"method": method, "params": params})
Evidence
PR Compliance ID 3 requires new behavior to be covered by tests. The PR adds
WebSocketConnection.send_cmd() (used by the new BiDi transport), while the added tests only use
stand-in DrivingConnection.send_cmd() implementations, so the real
WebSocketConnection.send_cmd() path is not exercised by unit tests.

AGENTS.md: Add Tests for Fixes/Features and Prefer Small Unit Tests Over Browser Tests
py/selenium/webdriver/remote/websocket_connection.py[118-160]
py/test/unit/selenium/webdriver/common/bidi_transport_tests.py[49-63]
py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py[44-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
`WebSocketConnection.send_cmd()` is a new API used by the new internal BiDi `Transport`, but there is no unit test that exercises `send_cmd()` on the real `WebSocketConnection` implementation.

## Issue Context
Current BiDi unit tests validate `Transport` and generated domains using local `DrivingConnection` stand-ins, so they don’t cover the actual frame/envelope/id correlation behavior implemented in `WebSocketConnection._round_trip()`/`send_cmd()`.

## Fix Focus Areas
- py/selenium/webdriver/remote/websocket_connection.py[118-160]
- py/test/unit/selenium/webdriver/common/bidi_transport_tests.py[49-63]
- py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py[44-58]

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



Informational

9. Generator output not reproducible ✓ Resolved 🐞 Bug ☼ Reliability
Description
The generator writes generated modules with Path.write_text() without an explicit encoding (and
without controlling newline handling), which can produce platform-dependent output and hinder stable
regeneration/verification across environments.
Code

py/generate_bidi_protocol.py[R1054-1058]

+    out = Path(out_dir)
+    out.mkdir(parents=True, exist_ok=True)
+    for name, contents in render_all(args.schema).items():
+        (out / name).write_text(contents)
+        print(f"bidi-generate: wrote {name}")
Evidence
The generator loop uses write_text(contents) directly, which depends on environment defaults for
encoding/newline handling.

py/generate_bidi_protocol.py[1026-1059]

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

### Issue description
`Path.write_text(contents)` uses the platform default encoding and doesn’t let you force newline handling; this makes generated output less deterministic across environments.

### Issue Context
This PR’s workflow relies on regenerating modules and comparing output; deterministic file writing reduces cross-platform diffs.

### Fix Focus Areas
- py/generate_bidi_protocol.py[1037-1058]

### Suggested fix approach
- Read schema with an explicit encoding: `Path(schema_path).read_text(encoding="utf-8")`.
- Write output with explicit UTF-8 and LF newlines, e.g.:
 ```py
 p = out / name
 with p.open("w", encoding="utf-8", newline="\n") as f:
     f.write(contents)
 ```

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


Grey Divider

Previous review results

Review updated until commit c73c938

Results up to commit 2b06fe9 ⚖️ Balanced


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


Action required
1. Schema drift breaks Bazel ✓ Resolved 🐞 Bug ☼ Reliability
Description
The Bazel rule declares a fixed set of generated domain module outputs, but the generator renders
modules dynamically from the schema’s domains; a schema domain add/remove/rename can cause missing
declared outputs or undeclared outputs, breaking builds.
Code

py/private/generate_bidi_protocol.bzl[R8-36]

+# Generated domain modules, snake_case (one per BiDi domain in the schema). The
+# package ``__init__.py`` is hand-written and checked in, so it is not generated here.
+_MODULES = [
+    "bluetooth",
+    "browser",
+    "browsing_context",
+    "emulation",
+    "input",
+    "log",
+    "network",
+    "permissions",
+    "script",
+    "session",
+    "speculation",
+    "storage",
+    "user_agent_client_hints",
+    "web_extension",
+]
+
+def _generate_bidi_protocol_impl(ctx):
+    outputs = [ctx.actions.declare_file(ctx.attr.package + "/" + name + ".py") for name in _MODULES]
+
+    ctx.actions.run(
+        inputs = [ctx.file.schema],
+        outputs = outputs,
+        executable = ctx.executable.generator,
+        # The generator writes one file per domain into the output dir.
+        arguments = [ctx.file.schema.path, outputs[-1].dirname],
+        use_default_shell_env = True,
Evidence
The Bazel rule hard-codes _MODULES and uses it to declare outputs, but the generator builds
modules from schema.domains() and writes all of them to the output directory, making the output
set schema-dependent rather than _MODULES-dependent.

py/private/generate_bidi_protocol.bzl[8-39]
py/generate_bidi_protocol.py[262-283]
py/generate_bidi_protocol.py[1026-1035]

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

### Issue description
`generate_bidi_protocol.bzl` declares a fixed `_MODULES` output list, but `generate_bidi_protocol.py` generates one module per domain discovered in the schema at runtime. When the schema changes, Bazel can either:
- fail because a declared output wasn’t created, or
- fail/sandbox-violate by writing extra (undeclared) outputs.

### Issue Context
Bazel actions should only write declared outputs; right now the generator is schema-driven while the rule is list-driven.

### Fix Focus Areas
- py/private/generate_bidi_protocol.bzl[8-39]
- py/generate_bidi_protocol.py[1026-1058]

### Suggested fix approach
Pick one single source of truth:
1) **Preferred**: change the Bazel rule to declare a single output directory (tree artifact) and have the generator write all modules into it; return that directory in `DefaultInfo`.
2) **Alternative**: pass the expected module list from the Bazel rule into the generator (e.g., a `--modules` arg), and make the generator:
  - generate *only* those modules, and
  - error out if the schema domains don’t exactly match the expected list (so drift is caught deterministically without writing undeclared files).

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



Remediation recommended
2. send_cmd() lacks direct tests 📘 Rule violation ☼ Reliability
Description
WebSocketConnection.send_cmd() is newly added but no unit test exercises the real
WebSocketConnection send/reply path; existing BiDi tests use stand-in connections instead. This
increases the risk of regressions in the core BiDi command transport path going undetected.
Code

py/selenium/webdriver/remote/websocket_connection.py[R140-146]

+    def send_cmd(self, method, params):
+        """Send one command and return its raw reply (``result`` or ``error`` left intact).
+
+        The dumb-send counterpart to ``execute``: no coroutine, no error raising. The
+        caller parses the reply into its declared type and raises on ``error`` itself.
+        """
+        return self._round_trip({"method": method, "params": params})
Evidence
PR Compliance ID 3 requires new behavior to be covered by tests. The PR adds
WebSocketConnection.send_cmd() (used by the new BiDi transport), while the added tests only use
stand-in DrivingConnection.send_cmd() implementations, so the real
WebSocketConnection.send_cmd() path is not exercised by unit tests.

AGENTS.md: Add Tests for Fixes/Features and Prefer Small Unit Tests Over Browser Tests
py/selenium/webdriver/remote/websocket_connection.py[118-160]
py/test/unit/selenium/webdriver/common/bidi_transport_tests.py[49-63]
py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py[44-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
`WebSocketConnection.send_cmd()` is a new API used by the new internal BiDi `Transport`, but there is no unit test that exercises `send_cmd()` on the real `WebSocketConnection` implementation.

## Issue Context
Current BiDi unit tests validate `Transport` and generated domains using local `DrivingConnection` stand-ins, so they don’t cover the actual frame/envelope/id correlation behavior implemented in `WebSocketConnection._round_trip()`/`send_cmd()`.

## Fix Focus Areas
- py/selenium/webdriver/remote/websocket_connection.py[118-160]
- py/test/unit/selenium/webdriver/common/bidi_transport_tests.py[49-63]
- py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py[44-58]

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


3. Extensions overwrite known fields ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Record.as_json() merges extensions into the serialized payload via payload.update(...), so a
caller-supplied extension key can overwrite a known wire field and silently change what is sent on
the wire.
Code

py/selenium/webdriver/common/_bidi/serialization.py[R207-221]

+    def as_json(self) -> dict:
+        payload: dict = {}
+        for f in _fields(self):
+            if f.name == "extensions":
+                continue
+            w = _wire_of(f)
+            value = getattr(self, f.name)
+            if value is UNSET:
+                continue
+            if value is None and not w.nullable:
+                continue
+            payload[w.wire] = _as_json(value)
+        if self._EXTENSIBLE:
+            payload.update(getattr(self, "extensions", None) or {})
+        return payload
Evidence
The serialization code writes known wire keys first, then unconditionally updates with extensions,
which gives extensions precedence over declared fields.

py/selenium/webdriver/common/_bidi/serialization.py[207-235]

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

### Issue description
For extensible records, `Record.as_json()` does `payload.update(extensions)`. If `extensions` contains a key that matches a known wire field, it overwrites the serialized value from the actual dataclass field, producing an incorrect wire payload.

### Issue Context
`from_json()` only captures unknown keys into `extensions`, so round-trips won’t collide. But callers can still construct a record with arbitrary `extensions` and accidentally (or intentionally) override real fields.

### Fix Focus Areas
- py/selenium/webdriver/common/_bidi/serialization.py[207-221]

### Suggested fix approach
In `Record.as_json()` when `_EXTENSIBLE`:
- compute `extras = getattr(self, "extensions", None) or {}`
- if `set(extras) & set(payload)` is non-empty, raise `BiDiSerializationError` identifying the colliding keys (or, at minimum, ensure declared fields win by merging as `{**extras, **payload}` so extras cannot override).

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



Informational
4. Generator output not reproducible ✓ Resolved 🐞 Bug ☼ Reliability
Description
The generator writes generated modules with Path.write_text() without an explicit encoding (and
without controlling newline handling), which can produce platform-dependent output and hinder stable
regeneration/verification across environments.
Code

py/generate_bidi_protocol.py[R1054-1058]

+    out = Path(out_dir)
+    out.mkdir(parents=True, exist_ok=True)
+    for name, contents in render_all(args.schema).items():
+        (out / name).write_text(contents)
+        print(f"bidi-generate: wrote {name}")
Evidence
The generator loop uses write_text(contents) directly, which depends on environment defaults for
encoding/newline handling.

py/generate_bidi_protocol.py[1026-1059]

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

### Issue description
`Path.write_text(contents)` uses the platform default encoding and doesn’t let you force newline handling; this makes generated output less deterministic across environments.

### Issue Context
This PR’s workflow relies on regenerating modules and comparing output; deterministic file writing reduces cross-platform diffs.

### Fix Focus Areas
- py/generate_bidi_protocol.py[1037-1058]

### Suggested fix approach
- Read schema with an explicit encoding: `Path(schema_path).read_text(encoding="utf-8")`.
- Write output with explicit UTF-8 and LF newlines, e.g.:
 ```py
 p = out / name
 with p.open("w", encoding="utf-8", newline="\n") as f:
     f.write(contents)
 ```

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


Results up to commit 52abe0c ⚖️ Balanced


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


Action required
1. Record.from_json non-dict crash 🐞 Bug ☼ Reliability
Description
Record.from_json() assumes payload is a dict and can raise raw TypeError/AttributeError when
the wire payload is not an object, leaking non-WebDriverException errors to callers. This can
surface via Transport.execute() which calls result.from_json(...) directly on websocket replies
without normalizing failures to BiDiSerializationError.
Code

py/selenium/webdriver/common/_bidi/serialization.py[R223-235]

+    @classmethod
+    def from_json(cls, payload: dict) -> Any:
+        kwargs: dict = {}
+        known: set[str] = set()
+        for f in _fields(cls):
+            w = _wire_of(f)
+            known.add(w.wire)
+            if w.fixed is not UNSET or f.name == "extensions":
+                continue
+            kwargs[f.name] = _read_field(cls, f.name, w, payload)
+        if cls._EXTENSIBLE:
+            kwargs["extensions"] = {k: v for k, v in payload.items() if k not in known}
+        return cls(**kwargs)
Evidence
Record.from_json uses dict operations (_read_field(..., payload) and payload.items()) without
a runtime type check; if payload is not a dict this will raise raw Python exceptions.
Transport.execute calls result.from_json(value) on the reply payload, so these raw exceptions
propagate instead of becoming BiDiSerializationError (a WebDriverException).

py/selenium/webdriver/common/_bidi/serialization.py[223-235]
py/selenium/webdriver/common/_bidi/transport.py[52-58]
py/selenium/webdriver/common/_bidi/serialization.py[44-45]

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

### Issue description
`Record.from_json` assumes the payload is a `dict` and will throw raw Python exceptions if it receives a non-dict payload (e.g., browser returns a scalar/array where an object is expected). This breaks the intended error boundary: deserialization failures should raise `BiDiSerializationError` (a `WebDriverException`), not `TypeError`/`AttributeError`.

### Issue Context
`Transport.execute` calls `result.from_json(value)` directly on the raw websocket reply result, so any non-`BiDiSerializationError` exceptions escape to callers.

### Fix Focus Areas
- py/selenium/webdriver/common/_bidi/serialization.py[223-235]
- py/selenium/webdriver/common/_bidi/transport.py[52-58]
- py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py[1-453]

### Suggested fix
1. Change `Record.from_json` signature to accept `payload: Any` (or keep `dict` but still validate).
2. At the top of `Record.from_json`, add:
  - `if not isinstance(payload, dict): raise BiDiSerializationError(f"{cls.__name__} expected an object on the wire, got {type(payload).__name__} {payload!r}")`
3. Add a unit test that calls a small Record type’s `from_json` with a non-dict (e.g. `[]` or `"x"`) and asserts `BiDiSerializationError` is raised.

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



Remediation recommended
2. No outbound primitive/list validation 🐞 Bug ≡ Correctness
Description
Record.__post_init__() validates nullability and enums only, so wrong primitive types,
list-vs-scalar mismatches, and invalid nested ref values can be serialized by as_json() and sent
on the wire unchanged. This allows constructing protocol objects that violate the schema and only
fail later as remote/protocol errors.
Code

py/selenium/webdriver/common/_bidi/serialization.py[R178-194]

+    def __post_init__(self) -> None:
+        for f in _fields(self):
+            if f.name == "extensions":
+                continue
+            w = _wire_of(f)
+            if w.fixed is not UNSET:
+                continue
+            value = getattr(self, f.name)
+            if value is UNSET:
+                continue
+            if value is None:
+                if not w.nullable:
+                    raise BiDiSerializationError(f"{type(self).__name__}.{f.name} cannot be None")
+                continue
+            if w.enum is not None:
+                self._validate_enum(f.name, w, value)
+
Evidence
Record.__post_init__ only checks nullability and enums; it does not use
primitive/is_list/ref metadata for outbound validation. Meanwhile inbound
_read_field/_read_scalar do enforce list shape and primitive types using _PRIMITIVE_CHECKS,
demonstrating the intended strictness is currently one-sided.

py/selenium/webdriver/common/_bidi/serialization.py[178-194]
py/selenium/webdriver/common/_bidi/serialization.py[238-255]
py/selenium/webdriver/common/_bidi/serialization.py[289-329]

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 outbound construction path for generated Records does not validate primitives (`w.primitive`), list shape (`w.is_list`), or referenced nested types (`w.ref`). As a result, invalid values can be serialized and sent to the browser without any local `BiDiSerializationError`.

### Issue Context
Inbound parsing is strict (checks list-ness and primitive types), but outbound construction is not, despite having the schema facts (`primitive`, `is_list`, `ref`) and `_PRIMITIVE_CHECKS` available.

### Fix Focus Areas
- py/selenium/webdriver/common/_bidi/serialization.py[178-205]
- py/selenium/webdriver/common/_bidi/serialization.py[238-255]
- py/selenium/webdriver/common/_bidi/serialization.py[289-329]
- py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py[1-453]

### Suggested fix
1. Extend `Record.__post_init__` to validate, for each non-UNSET/non-None field:
  - If `w.is_list` is True, assert `isinstance(value, list)`.
  - If `w.is_list` is False, assert `not isinstance(value, list)` when the field is typed (`primitive`/`enum`/`ref`) to avoid emitting arrays accidentally.
  - If `w.primitive` is set, validate using `_PRIMITIVE_CHECKS[w.primitive]` (and for list fields validate each element).
  - If `w.ref` is set, require values to be `Record` instances (or list of them) rather than arbitrary dict/scalars (unless explicitly intended).
2. Add unit tests showing that constructing a Record with:
  - a wrong primitive (e.g., `int` field passed a `str`), and
  - a list field passed a scalar,
  raises `BiDiSerializationError` before serialization.

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


3. Transport.execute() error branch untested ✓ Resolved 📘 Rule violation ☼ Reliability
Description
Transport.execute() introduces new error-handling logic when the websocket reply contains error,
but the added unit tests only cover successful result replies. This risks regressions in the
failure path going unnoticed and weakens confidence in the new transport seam’s correctness.
Code

py/selenium/webdriver/common/_bidi/transport.py[R52-56]

+    def execute(self, cmd: str, params: Any = None, result: Any = None) -> Any:
+        reply = self._connection.send_cmd(cmd, params.as_json() if params is not None else {})
+        if reply.get("error"):
+            message = reply.get("message")
+            raise WebDriverException(f"{reply['error']}: {message}" if message else reply["error"])
Evidence
PR Compliance ID 5 requires new behavior to be covered by automated tests. The new
Transport.execute() error-raising logic is present in the transport implementation, while the
added unit tests’ stand-in connection only returns {"result": ...} and therefore never exercises
the error path.

AGENTS.md: Add/Prefer Automated Tests; Prefer Small Unit Tests and Avoid Mocks
py/selenium/webdriver/common/_bidi/transport.py[52-56]
py/test/unit/selenium/webdriver/common/bidi_transport_tests.py[49-107]

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

## Issue description
`Transport.execute()` has an error-handling branch (when the reply contains `error`) that is not covered by the new unit tests.

## Issue Context
The new internal BiDi transport seam is a core piece that will be shared by generated domains; its error path should be unit-tested to prevent silent regressions.

## Fix Focus Areas
- py/selenium/webdriver/common/_bidi/transport.py[52-56]
- py/test/unit/selenium/webdriver/common/bidi_transport_tests.py[49-107]

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


Qodo Logo

Comment thread py/selenium/webdriver/remote/websocket_connection.py
Comment thread py/private/generate_bidi_protocol.bzl
Comment thread py/selenium/webdriver/common/_bidi/serialization.py
Comment thread py/generate_bidi_protocol.py
Comment thread py/selenium/webdriver/common/_bidi/transport.py
Comment thread py/selenium/webdriver/common/_bidi/serialization.py
Comment thread py/selenium/webdriver/common/_bidi/serialization.py
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 52abe0c

Comment thread py/BUILD.bazel
Comment thread py/selenium/webdriver/common/_bidi/transport.py
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 41 changed files in this pull request and generated 1 comment.

Comment thread py/test/selenium/webdriver/common/_bidi/webextension_tests.py
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-py Python Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants