Skip to content

[py] Include generated BiDi modules in API docs#17375

Open
cgoldberg wants to merge 1 commit intoSeleniumHQ:trunkfrom
cgoldberg:py-include-bidi-docs
Open

[py] Include generated BiDi modules in API docs#17375
cgoldberg wants to merge 1 commit intoSeleniumHQ:trunkfrom
cgoldberg:py-include-bidi-docs

Conversation

@cgoldberg
Copy link
Copy Markdown
Member

💥 What does this PR do?

This PR updates the ./go py:docs task so it runs ./go py:local_dev before creating the API docs. This ensures generated BiDi modules are copied into the source tree from Bazel output first. Without this, API docs for generated BiDi modules are not included.

🔧 Implementation Notes

🤖 AI assistance

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

💡 Additional Considerations

🔄 Types of changes

  • Docs
  • Bug fix (backwards compatible)

@cgoldberg cgoldberg added the C-py Python Bindings label Apr 23, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Include generated BiDi modules in Python API documentation

📝 Documentation 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Ensures generated BiDi modules are included in API docs
• Invokes py:local_dev task before generating docs
• Adds missing BiDi modules to documentation index
• Includes api_request_context and _event_manager modules

Grey Divider

File Changes

1. rake_tasks/python.rake 🐞 Bug fix +2/-0

Invoke local_dev task before generating docs

• Added invocation of py:local_dev task before py:docs_generate
• Ensures generated BiDi modules are copied into source tree before docs generation
• Includes explanatory comment about the task ordering

rake_tasks/python.rake


2. py/docs/source/api.rst 📝 Documentation +2/-0

Add missing BiDi modules to API documentation

• Added selenium.webdriver.common.api_request_context to webdriver.common module docs
• Added selenium.webdriver.common.bidi._event_manager to webdriver.common.bidi module docs
• Updates documentation index to include previously missing generated modules

py/docs/source/api.rst


Grey Divider

Qodo Logo

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Apr 23, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 23, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. BiDi docs missing in RTD 🐞 Bug ≡ Correctness
Description
Only the Rake py:docs path now generates/copies BiDi sources; ReadTheDocs and tox doc builds still
regenerate api.rst by walking the source tree without generating/copying BiDi outputs, so BiDi
modules will remain absent from those builds. This prevents generated BiDi modules from being
included in the published/non-Bazel API docs.
Code

rake_tasks/python.rake[R101-103]

+  # Run this task so generated modules are copied into the source tree before generating docs
+  Rake::Task['py:local_dev'].invoke
  Rake::Task['py:docs_generate'].invoke
Evidence
ReadTheDocs/tox both run generate_api_module_listing.py directly (not via Bazel) and then run
sphinx-autogen/sphinx-build with PYTHONPATH=py. The listing script walks the on-disk
selenium/ directory and only finds .py files present in the checkout; but the BiDi package is
Bazel-generated (generate_bidi to selenium/webdriver/common/bidi) and the directory is
gitignored, so those pipelines won’t discover/document it.

rake_tasks/python.rake[95-121]
py/docs/.readthedocs.yaml[10-19]
py/tox.ini[15-28]
py/generate_api_module_listing.py[32-46]
.gitignore[66-75]
py/BUILD.bazel[622-634]

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

## Issue description
ReadTheDocs/tox docs builds regenerate `api.rst` and run Sphinx against the source tree, but BiDi modules live in a Bazel-generated (and gitignored) directory. As a result, those builds cannot include generated BiDi modules in API docs.

## Issue Context
- RTD runs: `cd py && python3 generate_api_module_listing.py` then `PYTHONPATH=py sphinx-autogen`/`sphinx-build`.
- The BiDi sources are produced by Bazel `generate_bidi` into `selenium/webdriver/common/bidi` and are ignored by git.

## Fix Focus Areas
- py/docs/.readthedocs.yaml[14-18]
- py/tox.ini[15-28]
- py/BUILD.bazel[622-634]

## Suggested fix options (pick one that works in your infra)
1) **Make RTD/tox generate the BiDi package before listing/autogen**:
  - Install Bazel/Bazelisk in RTD, run a Bazel target that produces the BiDi outputs, then run a copy/sync step equivalent to `py:local_dev`.
  - For tox, document/automate the same prerequisite.

2) **Add a non-Bazel generation path** for docs builds:
  - Vendor/obtain the BiDi CDDL spec in a way RTD can access, then run `py/generate_bidi.py` to emit `selenium/webdriver/common/bidi` before running `generate_api_module_listing.py`.

3) If published docs are intended to be built only via the Bazel/Rake pipeline, update RTD/tox configs to use that pipeline (or remove the regeneration steps) to avoid permanently missing BiDi docs.

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



Remediation recommended

2. Stale BiDi files persist 🐞 Bug ☼ Reliability
Description
py:docs now invokes py:local_dev, but py:local_dev copies BiDi files into
py/selenium/webdriver/common/bidi without clearing the destination, so removed/renamed generated
modules can remain and get documented/imported. This can produce incorrect API docs (and potentially
stale runtime imports when using the source tree).
Code

rake_tasks/python.rake[R101-103]

+  # Run this task so generated modules are copied into the source tree before generating docs
+  Rake::Task['py:local_dev'].invoke
  Rake::Task['py:docs_generate'].invoke
Evidence
The newly added py:docs call to py:local_dev means docs generation depends on the local copy
logic. That logic for BiDi only overwrites files it sees (no destination cleanup), unlike other
generated dirs where rm_rf(dest) is used, so stale files can accumulate in the ignored output
directory.

rake_tasks/python.rake[95-107]
rake_tasks/python.rake[51-76]
.gitignore[66-75]

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:local_dev` syncs `bazel-bin/py/selenium/webdriver/common/bidi` into `py/selenium/webdriver/common/bidi` by copying individual files, but it never removes destination files that no longer exist in Bazel output. Now that `py:docs` invokes `py:local_dev`, stale generated modules can persist and be picked up by docs generation.

## Issue Context
Other generated dirs in `py:local_dev` (e.g., devtools/platform dirs) are cleaned via `rm_rf(dest)` before copy; BiDi is not.

## Fix Focus Areas
- rake_tasks/python.rake[63-76]

## Suggested fix
- Before copying BiDi files, delete the destination directory (or at least delete any `*.py`/`py.typed` not present in `bidi_src`) and recreate it:
 - `FileUtils.rm_rf(bidi_dest)`
 - `FileUtils.mkdir_p(bidi_dest)`
- Optionally extend the sync to handle directories if the generator ever emits subpackages.

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


3. Private module in docs 🐞 Bug ⚙ Maintainability
Description
The generated API listing now includes selenium.webdriver.common.bidi._event_manager, which is an
internal helper module (underscore-prefixed) copied into generated outputs. Publishing it in API
docs exposes internal implementation details and encourages unsupported imports.
Code

py/docs/source/api.rst[95]

+   selenium.webdriver.common.bidi._event_manager
Evidence
api.rst explicitly lists the underscore-prefixed module. The BiDi build target copies
py/private/_event_manager.py into the generated selenium/webdriver/common/bidi package (via
extra_srcs), and the module itself documents that it is shared helper logic emitted into generated
modules; the listing script currently includes any .py file not starting with __, so underscore
modules are not filtered out.

py/docs/source/api.rst[88-99]
py/BUILD.bazel[622-634]
py/private/_event_manager.py[18-24]
py/generate_api_module_listing.py[32-46]

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_api_module_listing.py` includes underscore-prefixed modules (it only excludes `__*`), which causes internal modules like `_event_manager` to appear in public API docs.

## Issue Context
`_event_manager` is a shared internal helper copied into generated BiDi outputs via Bazel `extra_srcs`, and its docstring indicates it exists to avoid duplication in generated modules.

## Fix Focus Areas
- py/generate_api_module_listing.py[32-46]
- py/docs/source/api.rst[88-99]

## Suggested fix
- In `find_modules()`, exclude filenames starting with `_` in addition to `__`:
 - change the condition to something like `filename.endswith('.py') and not filename.startswith('_')` (or `and not filename.startswith('__') and not filename.startswith('_')`).
- Regenerate `py/docs/source/api.rst` after the filter change so `_event_manager` is removed from autosummary.

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


Grey Divider

Qodo Logo

Comment thread rake_tasks/python.rake
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.

2 participants