Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.7k
Comments
Conversation
There was a problem hiding this comment.
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 introduces a CDDL (Concise Data Definition Language) to Python generator for WebDriver BiDi modules. It generates 9 BiDi protocol modules from the W3C specification, replacing hand-written implementations with auto-generated code.
Changes:
- Adds
py/generate_bidi.py- CDDL parser and Python code generator (623 lines) - Adds Bazel build integration for code generation
- Generates 9 BiDi modules (browser, browsing_context, emulation, input, network, script, session, storage, webextension) with 146 type definitions and 52 commands
- Adds validation tooling and documentation
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| py/generate_bidi.py | Core CDDL parser and Python code generator |
| py/private/generate_bidi.bzl | Bazel rule for BiDi code generation |
| py/BUILD.bazel | Integration of generation target |
| py/requirements.txt | Added pycddl dependency |
| py/selenium/webdriver/common/bidi/*.py | Generated BiDi module replacements |
| py/validate_bidi_modules.py | Validation tooling for comparing generated vs hand-written code |
| common/bidi/spec/local.cddl | CDDL specification (1331 lines) |
| Various .md files | Documentation and findings |
| class ChannelValue: | ||
| """ChannelValue type type.""" |
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The docstring incorrectly refers to "type type" which appears to be a duplication error. This pattern appears throughout all generated dataclasses. The description should be more meaningful, such as "ChannelValue type definition" or "ChannelValue parameter type".
| # DO NOT EDIT THIS FILE! | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # This file is generated from the WebDriver BiDi specification. If you need to make | ||
| # changes, edit the generator and regenerate all of the modules. | ||
| # |
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The generated code contains "DO NOT EDIT THIS FILE!" warnings but the hand-written code is being completely replaced. This creates a maintainability issue where developers might be confused about whether they should edit these files or regenerate them. The approach should be clarified: either these files should live only in bazel-bin and be imported from there, or the warning should be removed if manual edits are expected in the source tree.
| attrs==25.4.0 | ||
| backports.tarfile==1.2.0 | ||
| cachetools==7.0.1 | ||
| pycddl |
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The dependencypycddl was added to requirements.txt but it's not actually being used in the generator code. The generator inpy/generate_bidi.py implements its own CDDL parser using regex instead of using the pycddl library. Either the dependency should be removed as unused, or the generator should be updated to actually use pycddl for proper CDDL parsing instead of fragile regex-based parsing.
| pycddl |
| def __init__(self, driver) -> None: | ||
| self._driver = driver |
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The__init__ method signature changed from__init__(self, conn) to__init__(self, driver). This is a breaking API change that will affect all code using these BiDi modules. The parameter was renamed fromconn (connection) todriver, and the stored attribute changed fromself.conn toself._driver. This breaking change needs to be clearly documented and coordinated across all language bindings.
| def__init__(self,driver)->None: | |
| self._driver=driver | |
| def__init__(self,conn)->None: | |
| self.conn=conn |
| # via | ||
| # -r py/requirements.txt | ||
| # keyring | ||
| # secretstorage |
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The removal of entry pointjeepney from keyring dependencies in requirements_lock.txt appears unrelated to the CDDL generator changes. This should either be explained in the PR description or moved to a separate commit/PR to keep changes focused.
| # secretstorage | |
| # secretstorage | |
| # keyring |
| # DO NOT EDIT THIS FILE! | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # This file is generated from the WebDriver BiDi specification. If you need to make | ||
| # changes, edit the generator and regenerate all of the modules. | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
| # WebDriver BiDi module: storage |
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The generated code removes the Apache 2.0 license header that was present in the hand-written code. According to the Selenium project conventions, all source files should include the Apache 2.0 license header. The generator should be updated to include the full license text.
| class WebExtension: | ||
| """BiDi implementation of the webExtension module.""" | ||
| def __init__(self, conn): | ||
| self.conn = conn | ||
| def install(self, path=None, archive_path=None, base64_value=None) -> dict: | ||
| """Installs a web extension in the remote end. | ||
| You must provide exactly one of the parameters. | ||
| Args: | ||
| path: Path to an extension directory. | ||
| archive_path: Path to an extension archive file. | ||
| base64_value: Base64 encoded string of the extension archive. | ||
| Returns: | ||
| A dictionary containing the extension ID. | ||
| """ | ||
| if sum(x is not None for x in (path, archive_path, base64_value)) != 1: | ||
| raise ValueError("Exactly one of path, archive_path, or base64_value must be provided") | ||
| if path is not None: | ||
| extension_data = {"type": "path", "path": path} | ||
| elif archive_path is not None: | ||
| extension_data = {"type": "archivePath", "path": archive_path} | ||
| elif base64_value is not None: | ||
| extension_data = {"type": "base64", "value": base64_value} | ||
| params = {"extensionData": extension_data} | ||
| try: | ||
| result = self.conn.execute(command_builder("webExtension.install", params)) | ||
| return result | ||
| except WebDriverException as e: | ||
| if "Method not available" in str(e): | ||
| raise WebDriverException( | ||
| f"{e!s}. If you are using Chrome or Edge, add '--enable-unsafe-extension-debugging' " | ||
| "and '--remote-debugging-pipe' arguments or set options.enable_webextensions = True" | ||
| ) from e | ||
| raise | ||
| def uninstall(self, extension_id_or_result: str | dict) -> None: | ||
| """Uninstalls a web extension from the remote end. | ||
| Args: | ||
| extension_id_or_result: Either the extension ID as a string or the result dictionary | ||
| from a previous install() call containing the extension ID. | ||
| """ | ||
| if isinstance(extension_id_or_result, dict): | ||
| extension_id = extension_id_or_result.get("extension") | ||
| else: | ||
| extension_id = extension_id_or_result | ||
| params = {"extension": extension_id} | ||
| self.conn.execute(command_builder("webExtension.uninstall", params)) | ||
| """WebDriver BiDi webExtension module.""" | ||
| def __init__(self, driver) -> None: | ||
| self._driver = driver | ||
| def install(self, extension_data: Any = None) -> Generator[dict, dict, dict]: | ||
| """Execute webExtension.install.""" | ||
| params = { | ||
| "extensionData": extension_data, | ||
| } | ||
| params = {k: v for k, v in params.items() if v is not None} | ||
| return command_builder("webExtension.install", params) | ||
| def uninstall(self, extension: Any = None) -> Generator[dict, dict, dict]: | ||
| """Execute webExtension.uninstall.""" | ||
| params = { | ||
| "extension": extension, | ||
| } | ||
| params = {k: v for k, v in params.items() if v is not None} | ||
| return command_builder("webExtension.uninstall", params) | ||
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The hand-writtenwebextension.py had important error handling that provides helpful messages to users (e.g., "If you are using Chrome or Edge, add '--enable-unsafe-extension-debugging'..."). The generated code removes all of this error handling logic. This is a regression in user experience. The generator should preserve error handling patterns or the generated code should be manually augmented with error handling after generation.
| def install(self, extension_data: Any = None) -> Generator[dict, dict, dict]: | ||
| """Execute webExtension.install.""" | ||
| params = { | ||
| "extensionData": extension_data, | ||
| } | ||
| params = {k: v for k, v in params.items() if v is not None} | ||
| return command_builder("webExtension.install", params) | ||
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The hand-written code had validation logic (e.g., checking that exactly one parameter is provided forinstall(), validating coordinate ranges for GeolocationCoordinates, etc.). The generated code removes all this validation. This could lead to runtime errors that would have been caught earlier. Consider either generating basic validation logic or documenting that validation must be added manually to generated code.
| @dataclass | ||
| class CookieFilter: | ||
| """CookieFilter type type.""" | ||
| Returns: | ||
| A new instance of PartitionKey. | ||
| """ | ||
| return cls( | ||
| user_context=data.get("userContext"), | ||
| source_origin=data.get("sourceOrigin"), | ||
| ) | ||
| name: Optional[str] = None | ||
| value: Optional[Any] = None | ||
| domain: Optional[str] = None | ||
| path: Optional[str] = None | ||
| size: Optional[Any] = None | ||
| http_only: Optional[bool] = None | ||
| secure: Optional[bool] = None | ||
| same_site: Optional[Any] = None | ||
| expiry: Optional[Any] = None | ||
| @dataclass | ||
| class BrowsingContextPartitionDescriptor: | ||
| """Represents a browsing context partition descriptor.""" | ||
| def __init__(self, context: str): | ||
| self.type = "context" | ||
| self.context = context | ||
| def to_dict(self) -> dict[str, str]: | ||
| """Converts the BrowsingContextPartitionDescriptor to a dictionary. | ||
| """BrowsingContextPartitionDescriptor type type.""" | ||
| Returns: | ||
| Dict: A dictionary representation of the BrowsingContextPartitionDescriptor. | ||
| """ | ||
| return {"type": self.type, "context": self.context} | ||
| type: Optional[Any] = None | ||
| context: Optional[Any] = None | ||
| @dataclass | ||
| class StorageKeyPartitionDescriptor: | ||
| """Represents a storage key partition descriptor.""" | ||
| """StorageKeyPartitionDescriptor type type.""" | ||
| def __init__(self, user_context: str | None = None, source_origin: str | None = None): | ||
| self.type = "storageKey" | ||
| self.user_context = user_context | ||
| self.source_origin = source_origin | ||
| type: Optional[Any] = None | ||
| user_context: Optional[str] = None | ||
| source_origin: Optional[str] = None | ||
| def to_dict(self) -> dict[str, str]: | ||
| """Converts the StorageKeyPartitionDescriptor to a dictionary. | ||
| Returns: | ||
| Dict: A dictionary representation of the StorageKeyPartitionDescriptor. | ||
| """ | ||
| result = {"type": self.type} | ||
| if self.user_context is not None: | ||
| result["userContext"] = self.user_context | ||
| if self.source_origin is not None: | ||
| result["sourceOrigin"] = self.source_origin | ||
| return result | ||
| class PartialCookie: | ||
| """Represents a partial cookie for setting.""" | ||
| def __init__( | ||
| self, | ||
| name: str, | ||
| value: BytesValue, | ||
| domain: str, | ||
| path: str | None = None, | ||
| http_only: bool | None = None, | ||
| secure: bool | None = None, | ||
| same_site: str | None = None, | ||
| expiry: int | None = None, | ||
| ): | ||
| self.name = name | ||
| self.value = value | ||
| self.domain = domain | ||
| self.path = path | ||
| self.http_only = http_only | ||
| self.secure = secure | ||
| self.same_site = same_site | ||
| self.expiry = expiry | ||
| def to_dict(self) -> dict[str, Any]: | ||
| """Converts the PartialCookie to a dictionary. | ||
| Returns: | ||
| ------- | ||
| Dict: A dictionary representation of the PartialCookie. | ||
| """ | ||
| result: dict[str, Any] = { | ||
| "name": self.name, | ||
| "value": self.value.to_dict(), | ||
| "domain": self.domain, | ||
| } | ||
| if self.path is not None: | ||
| result["path"] = self.path | ||
| if self.http_only is not None: | ||
| result["httpOnly"] = self.http_only | ||
| if self.secure is not None: | ||
| result["secure"] = self.secure | ||
| if self.same_site is not None: | ||
| result["sameSite"] = self.same_site | ||
| if self.expiry is not None: | ||
| result["expiry"] = self.expiry | ||
| return result | ||
| @dataclass | ||
| class GetCookiesParameters: | ||
| """GetCookiesParameters type type.""" | ||
| class GetCookiesResult: | ||
| """Represents the result of a getCookies command.""" | ||
| filter: Optional[Any] = None | ||
| partition: Optional[Any] = None | ||
| def __init__(self, cookies: list[Cookie], partition_key: PartitionKey): | ||
| self.cookies = cookies | ||
| self.partition_key = partition_key | ||
| @classmethod | ||
| def from_dict(cls, data: dict[str, Any]) -> GetCookiesResult: | ||
| """Creates a GetCookiesResult instance from a dictionary. | ||
| Args: | ||
| data: A dictionary containing the get cookies result information. | ||
| Returns: | ||
| A new instance of GetCookiesResult. | ||
| """ | ||
| cookies = [Cookie.from_dict(cookie) for cookie in data.get("cookies", [])] | ||
| partition_key = PartitionKey.from_dict(data.get("partitionKey", {})) | ||
| return cls(cookies=cookies, partition_key=partition_key) | ||
| class SetCookieResult: | ||
| """Represents the result of a setCookie command.""" | ||
| @dataclass | ||
| class PartialCookie: | ||
| """PartialCookie type type.""" | ||
| def __init__(self, partition_key: PartitionKey): | ||
| self.partition_key = partition_key | ||
| name: Optional[str] = None | ||
| value: Optional[Any] = None | ||
| domain: Optional[str] = None | ||
| path: Optional[str] = None | ||
| http_only: Optional[bool] = None | ||
| secure: Optional[bool] = None | ||
| same_site: Optional[Any] = None | ||
| expiry: Optional[Any] = None | ||
| @classmethod | ||
| def from_dict(cls, data: dict[str, Any]) -> SetCookieResult: | ||
| """Creates a SetCookieResult instance from a dictionary. | ||
| Args: | ||
| data: A dictionary containing the set cookie result information. | ||
| @dataclass | ||
| class SetCookieParameters: | ||
| """SetCookieParameters type type.""" | ||
| Returns: | ||
| A new instance of SetCookieResult. | ||
| """ | ||
| partition_key = PartitionKey.from_dict(data.get("partitionKey", {})) | ||
| return cls(partition_key=partition_key) | ||
| cookie: Optional[Any] = None | ||
| partition: Optional[Any] = None | ||
| class DeleteCookiesResult: | ||
| """Represents the result of a deleteCookies command.""" | ||
| @dataclass | ||
| class DeleteCookiesParameters: | ||
| """DeleteCookiesParameters type type.""" | ||
| def __init__(self, partition_key: PartitionKey): | ||
| self.partition_key = partition_key | ||
| filter: Optional[Any] = None | ||
| partition: Optional[Any] = None |
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The generated code loses important type information. The hand-written code had specific types likeBytesValue,Cookie,CookieFilter,PartialCookie with proper methods liketo_dict() andfrom_dict(). The generated code replaces all of these with genericOptional[Any] which loses type safety. This makes the code less maintainable and removes IDE autocomplete support. The generator should create proper dataclass types with conversion methods.
| def get_cookies(self, filter: Any = None, partition: Any = None) -> Generator[dict, dict, dict]: | ||
| """Execute storage.getCookies.""" | ||
| params = { | ||
| "filter": filter, | ||
| "partition": partition, | ||
| } | ||
| params = {k: v for k, v in params.items() if v is not None} | ||
| return command_builder("storage.getCookies", params) |
CopilotAIFeb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The hand-written code had detailed docstrings explaining parameters, return values, exceptions, and examples. The generated code has minimal docstrings that just say "Execute {module}.{command}". This is a significant loss of documentation that will make the API harder to use. Consider enhancing the generator to extract documentation from CDDL comments or maintaining a separate documentation layer.
There was a problem hiding this comment.
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 23 out of 24 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
py/validate_bidi_modules.py:1
- Corrected spelling of 'Analyze' to match class name convention.
#!/usr/bin/env python3py/generate_bidi.py Outdated
| description: str = "" | ||
| def to_python_dataclass(self) -> str: | ||
| """Generate Python dataclass code for this type.""" |
CopilotAIFeb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The description says 'type type' in several dataclass docstrings (e.g., lines 18, 25, 33, etc. in generated files). This should be 'type definition' or similar.
| attrs==25.4.0 | ||
| backports.tarfile==1.2.0 | ||
| cachetools==7.0.1 | ||
| pycddl |
CopilotAIFeb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The pycddl dependency is added without a version pin. For production stability, specify a version (e.g., 'pycddl==0.6.4').
| pycddl | |
| pycddl==0.6.4 |
| connectStart: float, | ||
| connectEnd: float, | ||
| tlsStart: float, | ||
CopilotAIFeb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Line 620 contains trailing whitespace that should be removed.
Tests were failing with 'unrecognized arguments: --browser=chrome'Added --browser as an alias for --driver that populates the same 'drivers' destBoth arguments now work interchangeably
- Changed BUILD.bazel cddl_file from local.cddl to all.cddl- Regenerated all 9 BiDi modules with full command coverage- all.cddl includes all 90 commands vs 26 from local.cddl- WebExtension now has install() and uninstall() methods- Browser has full user context and client window management- Session has complete lifecycle and subscription methods- All modules now have complete W3C BiDi specification coverage
Log module in all.cddl only contains event types, not commandsRemoved import and export of non-existent Log class
This tells the agent to favour union types instead of notation. Also telling it to explicitly check the python version locally when running things in the terminal
There was a problem hiding this comment.
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 33 out of 34 changed files in this pull request and generated 10 comments.
| def _add_intercept(self) -> dict[str, Any]: | ||
| """Internal method to add an intercept. | ||
| class Request: | ||
| """Represents an intercepted network request.""" | ||
| def __init__( | ||
| self, | ||
| network: Network, | ||
| request_id: Any, | ||
| body_size: int | None = None, | ||
| cookies: Any = None, | ||
| resource_type: str | None = None, | ||
| headers: Any = None, | ||
| headers_size: int | None = None, | ||
| method: str | None = None, | ||
| timings: Any = None, | ||
| url: str | None = None, | ||
| ) -> None: | ||
| self.network = network | ||
| self.request_id = request_id | ||
| self.body_size = body_size | ||
| self.cookies = cookies | ||
| self.resource_type = resource_type | ||
| self.headers = headers | ||
| self.headers_size = headers_size | ||
| self.method = method | ||
| self.timings = timings | ||
| self.url = url | ||
| def fail_request(self) -> None: | ||
| """Fail this request.""" | ||
| if not self.request_id: | ||
| raise ValueError("Request not found.") | ||
| params: dict[str, Any] = {"request": self.request_id} | ||
| self.network.conn.execute(command_builder("network.failRequest", params)) | ||
| Returns: | ||
| Dictionary with 'intercept' key containing the intercept ID. | ||
| """ | ||
| # Simplified implementation for testing | ||
| intercept_id = f"intercept_{self._handler_id_counter}" | ||
| self._handler_id_counter += 1 | ||
| self.intercepts.append(intercept_id) | ||
| return {"intercept": intercept_id} |
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Network._add_intercept() is currently a stub that fabricates intercept IDs locally instead of issuing thenetwork.addIntercept BiDi command and tracking the returned intercept ID. This makes interception/non-deterministic behavior non-functional. Restore the previous behavior: execute the add/remove intercept commands on the WebSocket connection and keepintercepts in sync with remote state.
| @@ -27,7 +27,10 @@ class WebDriverException(Exception): | |||
| """Base webdriver exception.""" | |||
| def __init__( | |||
| self, msg: Any | None = None, screen: str | None = None, stacktrace: Sequence[str] | None = None | |||
| self, | |||
| msg: Optional[Any] = None, | |||
| screen: Optional[str] = None, | |||
| stacktrace: Optional[Sequence[str]] = None, | |||
| ) -> None: | |||
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This file switches toOptional[...] annotations (and importsOptional/Union), but the repository’s Python conventions explicitly preferX | None overOptional[X] (seepy/AGENTS.md). Please revert these signatures to use| None and drop theOptional/Union imports to keep typing style consistent across the codebase.
| @dataclass | ||
| class disownDataParameters: | ||
| """disownDataParameters type type.""" | ||
| data_type: Any | None = None | ||
| collector: Any | None = None | ||
| request: Any | None = None |
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Generated dataclass/type names likedisownDataParameters start with a lowercase letter, which is inconsistent with Python class naming and the surrounding generated types. This is likely a generator bug in the CDDL-to-Python name mapping; please ensure all generated class/type names use PascalCase (e.g.,DisownDataParameters).
| def script(self) -> Script: | ||
| if not self._websocket_connection: | ||
| self._start_bidi() | ||
| if not self._script: | ||
| self._script = Script(self._websocket_connection, self) | ||
| return self._script | ||
| def _start_bidi(self) -> None: | ||
| if self.caps.get("webSocketUrl"): | ||
| ws_url = self.caps.get("webSocketUrl") | ||
| else: | ||
| raise WebDriverException("Unable to find url to connect to from capabilities") | ||
| raise WebDriverException( | ||
| "Unable to find url to connect to from capabilities" | ||
| ) | ||
| if not isinstance(self.command_executor, RemoteConnection): | ||
| raise WebDriverException("command_executor must be a RemoteConnection instance for BiDi support") | ||
| raise WebDriverException( | ||
| "command_executor must be a RemoteConnection instance for BiDi support" | ||
| ) | ||
| self._websocket_connection = WebSocketConnection( | ||
| ws_url, | ||
| self.command_executor.client_config.websocket_timeout, | ||
| self.command_executor.client_config.websocket_interval, | ||
| ) | ||
| @property | ||
| def network(self) -> Network: | ||
| if not self._websocket_connection: | ||
| self._start_bidi() | ||
| assert self._websocket_connection is not None | ||
| if not hasattr(self, "_network") or self._network is None: | ||
| assert self._websocket_connection is not None | ||
| self._network = Network(self._websocket_connection) | ||
| return self._network |
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The BiDi module properties still instantiate modules with the old constructor signatures (e.g.,Script(self._websocket_connection, self)), but the updated/generated BiDi modules now take a singledriver argument. This will raiseTypeError when accessingdriver.script/driver.network/driver.browser/etc. Either keep the BiDi module constructors compatible withWebSocketConnection, or update these property implementations to pass the expecteddriver and have the modules calldriver._websocket_connection.execute(...) internally.
| @dataclass | ||
| class setNetworkConditionsParameters: | ||
| """setNetworkConditionsParameters type type.""" | ||
| network_conditions: Any | None = None | ||
| contexts: list[Any | None] | None = field(default_factory=list) | ||
| user_contexts: list[Any | None] | None = field(default_factory=list) |
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Generated dataclass/type names likesetNetworkConditionsParameters start with a lowercase letter, which is inconsistent with Python class naming and the surrounding generated types. This is likely a generator bug in the CDDL-to-Python name mapping; please ensure all generated class/type names use PascalCase (e.g.,SetNetworkConditionsParameters).
| import argparse | ||
| import importlib.util | ||
| import logging | ||
| import re | ||
| import sys | ||
| from collections import defaultdict | ||
| from dataclasses import dataclass, field | ||
| from enum import Enum | ||
| from pathlib import Path | ||
| from textwrap import dedent, indent as tw_indent | ||
| from typing import Any, Dict, List, Optional, Set, Tuple | ||
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
New code in the generator usesOptional[...]/Union[...] imports and annotations (e.g.,Optional[str]), but the repo’s conventions inpy/AGENTS.md preferX | None and avoidingOptional. Please update the generator to follow the same typing style to keep generated code and tooling consistent.
| def execute( | ||
| self, driver_command: str, params: dict[str, Any] | None = None | ||
| ) -> dict[str, Any]: | ||
| """Sends a command to be executed by a command.CommandExecutor. | ||
| Args: | ||
| driver_command: The name of the command to execute as a string. | ||
| driver_command: The name of the command to execute as a string. Can also be a generator | ||
| for BiDi protocol commands. | ||
| params: A dictionary of named parameters to send with the command. | ||
| Returns: | ||
| The command's JSON response loaded into a dictionary object. | ||
| """ | ||
| # Handle BiDi generator commands | ||
| if inspect.isgenerator(driver_command): | ||
| # BiDi command: use WebSocketConnection directly | ||
| return self.command_executor.execute(driver_command) | ||
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
execute() now treats a generatordriver_command as a BiDi command and callsself.command_executor.execute(driver_command). In this class,command_executor is typically aRemoteConnection whoseexecute() requires(command, params), so this will raiseTypeError at runtime. BiDi commands should be routed throughself._websocket_connection.execute(...) (after ensuring_start_bidi() has initialized it), and theexecute() signature/type should be updated accordingly (e.g., acceptGenerator[dict, dict, dict] in addition tostr).
| class UserPromptHandler: | ||
| """UserPromptHandler type.""" | ||
| pass | ||
| @dataclass | ||
| class CapabilitiesRequest: | ||
| """CapabilitiesRequest type type.""" | ||
| always_match: Any | None = None | ||
| first_match: list[Any | None] | None = field(default_factory=list) | ||
| @dataclass | ||
| class CapabilityRequest: | ||
| """CapabilityRequest type type.""" | ||
| accept_insecure_certs: bool | None = None | ||
| browser_name: str | None = None | ||
| browser_version: str | None = None | ||
| platform_name: str | None = None | ||
| proxy: Any | None = None | ||
| unhandled_prompt_behavior: Any | None = None | ||
| @dataclass | ||
| class AutodetectProxyConfiguration: | ||
| """AutodetectProxyConfiguration type type.""" | ||
| proxy_type: Any | None = None | ||
| VALID_TYPES = {ACCEPT, DISMISS, IGNORE} | ||
| @dataclass | ||
| class DirectProxyConfiguration: | ||
| """DirectProxyConfiguration type type.""" | ||
| proxy_type: Any | None = None | ||
| @dataclass | ||
| class ManualProxyConfiguration: | ||
| """ManualProxyConfiguration type type.""" | ||
| proxy_type: Any | None = None | ||
| http_proxy: str | None = None | ||
| ssl_proxy: str | None = None | ||
| no_proxy: list[Any | None] | None = field(default_factory=list) | ||
| @dataclass | ||
| class SocksProxyConfiguration: | ||
| """SocksProxyConfiguration type type.""" | ||
| socks_proxy: str | None = None | ||
| socks_version: Any | None = None | ||
| @dataclass | ||
| class PacProxyConfiguration: | ||
| """PacProxyConfiguration type type.""" | ||
| proxy_type: Any | None = None | ||
| proxy_autoconfig_url: str | None = None | ||
| @dataclass | ||
| class SystemProxyConfiguration: | ||
| """SystemProxyConfiguration type type.""" | ||
| proxy_type: Any | None = None | ||
| @dataclass | ||
| class UserPromptHandler: | ||
| """Represents the configuration of the user prompt handler.""" | ||
| """UserPromptHandler type type.""" | ||
| def __init__( | ||
| self, | ||
| alert: str | None = None, | ||
| before_unload: str | None = None, | ||
| confirm: str | None = None, | ||
| default: str | None = None, | ||
| file: str | None = None, | ||
| prompt: str | None = None, | ||
| ): | ||
| """Initialize UserPromptHandler. | ||
| Args: | ||
| alert: Handler type for alert prompts. | ||
| before_unload: Handler type for beforeUnload prompts. | ||
| confirm: Handler type for confirm prompts. | ||
| default: Default handler type for all prompts. | ||
| file: Handler type for file picker prompts. | ||
| prompt: Handler type for prompt dialogs. | ||
| Raises: | ||
| ValueError: If any handler type is not valid. | ||
| """ | ||
| for field_name, value in [ | ||
| ("alert", alert), | ||
| ("before_unload", before_unload), | ||
| ("confirm", confirm), | ||
| ("default", default), | ||
| ("file", file), | ||
| ("prompt", prompt), | ||
| ]: | ||
| if value is not None and value not in UserPromptHandlerType.VALID_TYPES: | ||
| raise ValueError( | ||
| f"Invalid {field_name} handler type: {value}. Must be one of {UserPromptHandlerType.VALID_TYPES}" | ||
| ) | ||
| self.alert = alert | ||
| self.before_unload = before_unload | ||
| self.confirm = confirm | ||
| self.default = default | ||
| self.file = file | ||
| self.prompt = prompt | ||
| def to_dict(self) -> dict[str, str]: | ||
| """Convert the UserPromptHandler to a dictionary for BiDi protocol. | ||
| Returns: | ||
| Dictionary representation suitable for BiDi protocol. | ||
| """ | ||
| field_mapping = { | ||
| "alert": "alert", | ||
| "before_unload": "beforeUnload", | ||
| "confirm": "confirm", | ||
| "default": "default", | ||
| "file": "file", | ||
| "prompt": "prompt", | ||
| } | ||
| alert: Any | None = None | ||
| before_unload: Any | None = None | ||
| confirm: Any | None = None | ||
| default: Any | None = None | ||
| file: Any | None = None | ||
| prompt: Any | None = None |
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
UserPromptHandler is defined twice: once as an empty class (lines 25-28) and again as a@dataclass (lines 98-107). The first definition is immediately overwritten, which is confusing and suggests a generator issue. Remove the placeholder definition (or rename one of them) so there is a single canonicalUserPromptHandler type.
| def file_dialog_opened( | ||
| self, context: Any = None, element: Any = None, multiple: bool = None | ||
| ) -> Generator[dict, dict, dict]: | ||
| """Execute input.fileDialogOpened.""" | ||
| params = { | ||
| "context": context, | ||
| "element": element, | ||
| "multiple": multiple, | ||
| } | ||
| params = {k: v for k, v in params.items() if v is not None} | ||
| return command_builder("input.fileDialogOpened", params) |
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
file_dialog_opened() builds an executable command forinput.fileDialogOpened, but in the previous implementation this was an event (input.fileDialogOpened) that clients subscribe to and receive, not a command to execute. Generating it as a command will produce invalid protocol traffic. Consider generating an event class (withevent_class = "input.fileDialogOpened" andfrom_json) and keep subscription management in the higher-level API, rather than exposing it throughcommand_builder().
| """Execute network.beforeRequestSent.""" | ||
| params = { | ||
| "initiator": initiator, | ||
| "method": method, | ||
| "params": params, | ||
| } | ||
| params = {k: v for k, v in params.items() if v is not None} | ||
| return command_builder("network.beforeRequestSent", params) | ||
| def fetch_error( | ||
| self, error_text: Any = None, method: Any = None, params: Any = None | ||
| ) -> Generator[dict, dict, dict]: | ||
| """Execute network.fetchError.""" | ||
| params = { | ||
| "errorText": error_text, | ||
| "method": method, | ||
| "params": params, | ||
| } | ||
| params = {k: v for k, v in params.items() if v is not None} | ||
| return command_builder("network.fetchError", params) | ||
| def response_completed( | ||
| self, response: Any = None, method: Any = None, params: Any = None | ||
| ) -> Generator[dict, dict, dict]: | ||
| """Execute network.responseCompleted.""" | ||
| params = { | ||
| "response": response, | ||
| "method": method, | ||
| "params": params, | ||
| } | ||
| params = {k: v for k, v in params.items() if v is not None} | ||
| return command_builder("network.responseCompleted", params) | ||
| def response_started(self, response: Any = None) -> Generator[dict, dict, dict]: | ||
| """Execute network.responseStarted.""" | ||
| params = { | ||
| "response": response, | ||
| } | ||
| params = {k: v for k, v in params.items() if v is not None} | ||
| return command_builder("network.responseStarted", params) | ||
CopilotAIFeb 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Several methods are generated as executable commands even though they represent BiDi events (e.g.,network.beforeRequestSent,network.responseStarted,network.fetchError). These should not be sent viacommand_builder(); they should be modeled as event classes used withsession.subscribe and callback dispatch. Treating events as commands will break consumers relying on request interception/event subscription.
| """Execute network.beforeRequestSent.""" | |
| params= { | |
| "initiator":initiator, | |
| "method":method, | |
| "params":params, | |
| } | |
| params= {k:vfork,vinparams.items()ifvisnotNone} | |
| returncommand_builder("network.beforeRequestSent",params) | |
| deffetch_error( | |
| self,error_text:Any=None,method:Any=None,params:Any=None | |
| )->Generator[dict,dict,dict]: | |
| """Execute network.fetchError.""" | |
| params= { | |
| "errorText":error_text, | |
| "method":method, | |
| "params":params, | |
| } | |
| params= {k:vfork,vinparams.items()ifvisnotNone} | |
| returncommand_builder("network.fetchError",params) | |
| defresponse_completed( | |
| self,response:Any=None,method:Any=None,params:Any=None | |
| )->Generator[dict,dict,dict]: | |
| """Execute network.responseCompleted.""" | |
| params= { | |
| "response":response, | |
| "method":method, | |
| "params":params, | |
| } | |
| params= {k:vfork,vinparams.items()ifvisnotNone} | |
| returncommand_builder("network.responseCompleted",params) | |
| defresponse_started(self,response:Any=None)->Generator[dict,dict,dict]: | |
| """Execute network.responseStarted.""" | |
| params= { | |
| "response":response, | |
| } | |
| params= {k:vfork,vinparams.items()ifvisnotNone} | |
| returncommand_builder("network.responseStarted",params) | |
| """network.beforeRequestSentisaBiDieventandcannotbeexecutedasacommand. | |
| Use`session.subscribe("network.beforeRequestSent", ...)`toreceivethisevent. | |
| """ | |
| raiseRuntimeError( | |
| "network.beforeRequestSent is a BiDi event and cannot be executed as a command. " | |
| "Use session.subscribe('network.beforeRequestSent', ...) instead." | |
| ) | |
| deffetch_error( | |
| self,error_text:Any=None,method:Any=None,params:Any=None | |
| )->Generator[dict,dict,dict]: | |
| """network.fetchErrorisaBiDieventandcannotbeexecutedasacommand. | |
| Use`session.subscribe("network.fetchError", ...)`toreceivethisevent. | |
| """ | |
| raiseRuntimeError( | |
| "network.fetchError is a BiDi event and cannot be executed as a command. " | |
| "Use session.subscribe('network.fetchError', ...) instead." | |
| ) | |
| defresponse_completed( | |
| self,response:Any=None,method:Any=None,params:Any=None | |
| )->Generator[dict,dict,dict]: | |
| """network.responseCompletedisaBiDieventandcannotbeexecutedasacommand. | |
| Use`session.subscribe("network.responseCompleted", ...)`toreceivethisevent. | |
| """ | |
| raiseRuntimeError( | |
| "network.responseCompleted is a BiDi event and cannot be executed as a command. " | |
| "Use session.subscribe('network.responseCompleted', ...) instead." | |
| ) | |
| defresponse_started(self,response:Any=None)->Generator[dict,dict,dict]: | |
| """network.responseStartedisaBiDieventandcannotbeexecutedasacommand. | |
| Use`session.subscribe("network.responseStarted", ...)`toreceivethisevent. | |
| """ | |
| raiseRuntimeError( | |
| "network.responseStarted is a BiDi event and cannot be executed as a command. " | |
| "Use session.subscribe('network.responseStarted', ...) instead." | |
| ) |
This is my scratch pad so far and not worth merging yet