chore: generate typed protocol channels and use them in _impl (#3264)

Author: pavelfeldman

.claude/skills/playwright-roll/SKILL.md

@@ -174,9 +174,9 @@ Implement the change in `playwright/_impl/<module>.py`. Use the upstream JS impl
 |---|---|
 | `async foo(): Promise<X>` | `async def foo(self) -> X:` |
 | `foo(): X` (sync getter, no args, no body) | `@property def foo(self) -> X:` (the generator makes argument-less sync members properties — see `isPythonProperty` in `scripts/codegen/generate_api.js` — and reads `self._impl_obj.foo` without calling it. mypy does not catch a plain method here; the test does.) |
-| `await this._channel.foo({ a, b })` | `await self._channel.send("foo", None, locals_to_params(locals()))` |
-| `(await this._channel.foo()).value` | `await self._channel.send("foo", None)` (Python's `send()` auto-unwraps single-key responses; only call `send_return_as_dict` when the protocol returns multiple keys.) |
-| `(await this._channel.foo()).artifact` (multi-key, may be empty) | `result = await self._channel.send_return_as_dict("foo", None); (result or {}).get("artifact")` — `send_return_as_dict` returns **`None`** (not `{}`) when the protocol response carries no fields. |
+| `await this._channel.foo({ a, b }, this._timeout(options))` | `await self._channel.foo(a=a, b=b, timeout=self._timeout(timeout))`. The generated channel method takes every protocol parameter, with no defaults; an absent option is `None`. The implementation method's own parameters use the Python (snake_case) names, with no defaults. |
+| `(await this._channel.foo()).value` | `(await self._channel.foo())["value"]`. The result dict has every top-level field; a field that the driver left out is `None`. |
+| `this._channel.on('foo', ({ a }) => ...)` | `self._channel.onFoo(lambda event: ...)`; `event["a"]` is `None` when the driver left the optional field out. |
 | `try { ... } catch (e) { if (isTargetClosedError(e)) return; throw e; }` | `try: ...; except Exception as e: if is_target_closed_error(e): return; raise` (import from `playwright._impl._errors`) |
 | Inline `[Object]` return like `{endpoint: string}` | A `TypedDict` in `playwright/_impl/_api_structures.py`, which the generator writes. It names the struct from the docs: `` "Name" <[Object]> ``, or an `* alias-python:` line under it. See `RemoteAddr`, `BrowserBindResult`, `DebuggerPausedDetails`. |
 | `binary` event/return field | The Python channel layer hands you a base64 string. Decode with `base64.b64decode(value)` before exposing as `bytes`. See `Screencast._dispatch_frame`. |
@@ -216,7 +216,7 @@ The generator fails with "the entry … no longer matches anything; remove it" w
 **Do not suppress** these — they're PORTs in disguise:
 
 - "Internal tooling" classes/methods whose `langs` field is empty (`Screencast.*`, `Debugger.*`, `Page.pick_locator`, `BrowserContext.debugger`, `Browser.bind/unbind`, `Page.{clear,console}_*`). The 1.59 roll suppressed all of these initially, then had to walk every one back. Verify `langs` first.
-- A renamed positional parameter (mypy: unexpected keyword argument `newName` for the impl method). Just rename in `_impl/` and update the channel-send dict key.
+- A renamed positional parameter (mypy: unexpected keyword argument `newName` for the impl method). Just rename in `_impl/`; if the wire name changed too, mypy reports the generated channel call.
 - A struct return the generator cannot name ("No Python name for the struct at …"). Name it, so that the generator writes a TypedDict — don't type it `Dict`.
 
 #### N/A
@@ -241,7 +241,7 @@ The script, for `sync` and then `async`:
 1. runs `node scripts/codegen/generate_api.js --flavor <flavor>` (reads the docs from `$PW_SRC_DIR`) straight into `playwright/<flavor>_api/_generated.py`,
 2. invokes `pre-commit run --files` on the generated file.
 
-It then runs `python scripts/codegen/generate_protocol.py`, which reads `packages/protocol/spec/*.yml` from `$PW_SRC_DIR` and writes the command parameter tables to `playwright/_impl/_protocol.py`.
+It then runs `python scripts/codegen/generate_protocol.py`, which reads `packages/protocol/spec/*.yml` from `$PW_SRC_DIR` and writes the command parameter tables to `playwright/_impl/_protocol.py`, the message types to `playwright/_impl/_protocol_types.py` and the typed channels to `playwright/_impl/_protocol_channels.py`.
 
 Then run `mypy playwright` and work through the errors in `_generated.py` — each one is a call into `_impl/` that the implementation does not support yet.
 
@@ -261,7 +261,8 @@ Failure modes and fixes:
 | Symptom | Cause | Fix |
 |---|---|---|
 | mypy: `"XImpl" has no attribute "y"` in `_generated.py` | The docs have a new member `X.y`; `_impl/` does not. | PORT it. |
-| mypy: `Unexpected keyword argument "z" for "y" of "XImpl"` | New parameter or option on an existing method. | Add the kwarg (docs camelCase name) in `_impl/`. If Python must not have it, fix the docs: `* langs:` on the option. |
+| mypy: `Unexpected keyword argument "z" for "y" of "XImpl"` | New parameter or option on an existing method. | Add the parameter in `_impl/` under its Python (snake_case) name, as `Optional[X]` with no default, and pass it in the channel call. mypy then reports each caller that must pass it. If Python must not have it, fix the docs: `* langs:` on the option. |
+| mypy: `Missing named argument "z" for "y" of "XChannel"` | The protocol added a parameter to a command. | Pass the value in the channel call. If the implementation method needs a new parameter for it, add it as above. |
 | mypy: incompatible argument type in `_generated.py` | The docs type changed, or `_impl/` declares a narrower type. | Match the type in `_impl/`, or fix the docs with a Python-only declaration (`* langs: python`). |
 | generator: `No Python name for the struct at X.y.z` | A new inline `[Object]` with no name in the docs. | Name it in the docs: `` "Name" <[Object]> ``. |
 | generator: `The struct X has different fields at A and B` | Two places in the docs share one struct name, but their fields differ. | Give one place its own name in the docs. |
@@ -328,7 +329,7 @@ All keys use docs names: `Class.member` or `Class.member.arg`, with the upstream
 - **A cluster of rules on the same class is a smell.** If you're about to add five entries for `Foo.*`, you're almost certainly looking at a class that needs to be implemented. Implement the whole thing once and the entries disappear.
 - **Watch for revert pairs in the same range.** 1.59 added and reverted `Browser.isRemote` (#39613 / #39620) inside the same release. Walking chronologically lets you skip the add when you see the revert later.
 - **Watch for rename-revert pairs.** 1.59 had `Locator.normalize` → `Locator.toCode` (#39648) → `Locator.normalize` (#39754). Final state wins; only port the last.
-- **Doc renames almost always include a wire-protocol rename.** Whenever you see `### param: X.y.oldName` → `### param: X.y.newName` in a doc commit, also `git -C "$PW_SRC_DIR" show <sha> -- packages/protocol/spec/` and the corresponding `*Dispatcher.ts` file. If the wire field changed too, the channel-send dict key in `_impl/` must change. Suppressing the doc-side mismatch is hiding a real bug — the previous Python code is silently sending an unknown field that the new server ignores. The test suite reports such a field as a protocol violation, but only in tests that send it.
+- **Doc renames almost always include a wire-protocol rename.** Whenever you see `### param: X.y.oldName` → `### param: X.y.newName` in a doc commit, also `git -C "$PW_SRC_DIR" show <sha> -- packages/protocol/spec/` and the corresponding `*Dispatcher.ts` file. If the wire field changed too, the regenerated `_protocol_channels.py` renames the argument of the channel method, and mypy reports each call in `_impl/` that passes the old name.
 - **TypedDicts beat `Dict[str, X]` for any structured return.** When the docs describe a return as `[Object]` with named fields, name the struct for the generator, which writes its `TypedDict` into `_api_structures.py`, and re-export it from both public `__init__.py` files. Zero runtime cost (it's still a `dict`), and the docstrings expand its fields from the docs.
 - **Positional renames are free.** A param with no default before any `*` separator is positional-or-keyword in Python, but realistic call sites pass it positionally. Renaming such a param doesn't break callers.
 - **The "Backport changes" GitHub issue can be misleading.** In the 1.59 roll its checkboxes were all marked `[x]` with annotations like "✅ IMPLEMENTED", but several of those features had not actually been merged into the Python port. Trust the `docs/src/api/` walk over the issue.

AGENTS.md

@@ -12,7 +12,10 @@ Python bindings for [Playwright](https://playwright.dev). The Python client talk
 - `playwright/async_api/_generated.py`, `playwright/sync_api/_generated.py`, and the public TypedDicts in `playwright/_impl/_api_structures.py` — **auto-generated** from the upstream API docs. Never edit by hand; rerun `./scripts/update_api.sh` after a roll or after changing `scripts/codegen/`.
 - `scripts/codegen/generate_api.js` — the generator. A Node.js script that loads the upstream doc parser (`utils/doclint/api_parser.js`) from `$PW_SRC_DIR`, filters the docs for Python, and writes the whole public API: signatures, types, docstrings, event overloads, the calls into `_impl/`, and the TypedDicts. It does not read `_impl/`.
 - `scripts/codegen/python_rules.js` — the Python facts that the docs do not contain (positional options, struct `total=`, hidden members and options). Policy: the shipped Python API wins; each entry is a candidate for an upstream docs fix, and the generator fails on entries that no longer match anything. Every entry needs a one-line rationale comment above it.
-- `playwright/_impl/_protocol.py` — **auto-generated** by `scripts/codegen/generate_protocol.py` (run by `./scripts/update_api.sh`) from the upstream protocol spec: the parameters of every command. With `PW_CHECK_PROTOCOL=1`, `playwright/_impl/_protocol_check.py` checks each message that the client sends against it. The test suite sets this, and fails a test that sends an unknown command, an unknown or missing parameter, or a value of the wrong type.
+- `playwright/_impl/_protocol.py`, `_protocol_types.py` and `_protocol_channels.py` — **auto-generated** by `scripts/codegen/generate_protocol.py` (run by `./scripts/update_api.sh`) from the upstream protocol spec.
+  - `_protocol_channels.py` has a typed channel class for each interface (`PageChannel`, `FrameChannel`, …), like upstream's `channels.d.ts`. Each command is an async method with the protocol name and parameters (`await self._channel.goto(url=..., waitUntil=..., referer=..., timeout=...)`); each event has `on<Event>` / `off<Event>` methods. A result or event payload has every top-level field; a field that the driver left out is `None`.
+  - `_protocol_types.py` has the TypedDicts for initializers, results, events and nested types, named as upstream's `utils/generate_channels.js` names them (`PageInitializer`, `FrameGotoResult`, `PageRouteEvent`, …).
+  - `_protocol.py` has the parameters of every command. With `PW_CHECK_PROTOCOL=1`, `playwright/_impl/_protocol_check.py` checks each message that the client sends against it. The test suite sets this, and fails a test that sends an unknown command, an unknown or missing parameter, or a value of the wrong type.
 - `scripts/codegen/compare_api.py` — compares two generated files (signatures, annotations, call bodies, docstrings) and lists every difference. Use it to check that a generator change does not change the API.
 - `tests/async/`, `tests/sync/` — pytest suites, ported from the upstream tests. Most new tests are added to the async file with a sync mirror. See [Tests](#tests).
 - `DRIVER_VERSION` — the single source of truth for which Playwright release the driver is assembled from (one line, the `playwright-core` npm version, e.g. `1.61.0`, no `v` prefix). Read by `setup.py`, `scripts/build_driver.py`, and CI. The wheel build downloads `playwright-core` at this version from npm plus the matching Node.js binary and assembles the per-platform bundles — no source build. The version is baked into the staged bundle filenames (`driver/playwright-<version>-<suffix>.zip`), so it doubles as the build cache key.
@@ -80,7 +83,8 @@ It documents the full process: the upstream commit-range diff over `docs/src/api
 - Don't add `# type: ignore` or modify `_generated.py` to silence pyright; fix the source of the mismatch.
 - New public methods on impl classes need a sync test mirror under `tests/sync/`.
 - Keep `scripts/codegen/python_rules.js` minimal — every entry needs a one-line rationale comment above it. Prefer fixing the upstream docs.
-- Prefer `locals_to_params(locals())` for forwarding optional kwargs to channel sends, matching the rest of the codebase.
+- Implementation methods take the Python (snake_case) names that the generated API passes. Their options have no default: an option that can be absent is `Optional[X]` without `= None`. So mypy reports each caller that does not pass a new option.
+- Send messages through the generated channel methods, never by name: `await self._channel.click(selector=selector, ..., clickCount=click_count, timeout=self._timeout(timeout))`. Pass every parameter, with the protocol (camelCase) name; an absent option is `None`, and the connection drops it. Declare the channel type on each ChannelOwner subclass (`_channel: "PageChannel"`). Listen to events with `self._channel.on<Event>(handler)`.
 
 ## Commit Convention
 

playwright/_impl/_artifact.py

@@ -14,15 +14,20 @@
 
 import pathlib
 from pathlib import Path
-from typing import Dict, Optional, Union, cast
+from typing import TYPE_CHECKING, Dict, Optional, Union, cast
 
 from playwright._impl._connection import ChannelOwner, from_channel
 from playwright._impl._errors import Error, patch_error_message
 from playwright._impl._helper import make_dirs_for_file
 from playwright._impl._stream import Stream
 
+if TYPE_CHECKING:  # pragma: no cover
+    from playwright._impl._protocol_channels import ArtifactChannel
+
 
 class Artifact(ChannelOwner):
+    _channel: "ArtifactChannel"
+
     def __init__(
         self, parent: ChannelOwner, type: str, guid: str, initializer: Dict
     ) -> None:
@@ -34,55 +39,29 @@ async def path_after_finished(self) -> pathlib.Path:
             raise Error(
                 "Path is not available when using browser_type.connect(). Use save_as() to save a local copy."
             )
-        path = await self._channel.send(
-            "pathAfterFinished",
-            None,
-        )
+        path = (await self._channel.pathAfterFinished())["value"]
         return pathlib.Path(path)
 
     async def save_as(self, path: Union[str, Path]) -> None:
         stream = cast(
-            Stream,
-            from_channel(
-                await self._channel.send(
-                    "saveAsStream",
-                    None,
-                )
-            ),
+            Stream, from_channel((await self._channel.saveAsStream())["stream"])
         )
         make_dirs_for_file(path)
         await stream.save_as(path)
 
     async def failure(self) -> Optional[str]:
-        reason = await self._channel.send(
-            "failure",
-            None,
-        )
+        reason = (await self._channel.failure())["error"]
         if reason is None:
             return None
         return patch_error_message(reason)
 
     async def delete(self) -> None:
-        await self._channel.send(
-            "delete",
-            None,
-        )
+        await self._channel.delete()
 
     async def read_info_buffer(self) -> bytes:
-        stream = cast(
-            Stream,
-            from_channel(
-                await self._channel.send(
-                    "stream",
-                    None,
-                )
-            ),
-        )
+        stream = cast(Stream, from_channel((await self._channel.stream())["stream"]))
         buffer = await stream.read_all()
         return buffer
 
     async def cancel(self) -> None:  # pyright: ignore[reportIncompatibleMethodOverride]
-        await self._channel.send(
-            "cancel",
-            None,
-        )
+        await self._channel.cancel()