doc db to include types.* #6391
Conversation
WalkthroughThis PR updates the documentation generation workflow, expanding builtin type discovery in the Python generation script, and adding comprehensive documentation entries for Python built-in types and related constructs in the static data file. The workflow now supports branch-specific updates with updated file paths under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/doc/generate.py (1)
212-217:isinstance(entry.raw_doc, str)guard is good; consider aligning the type annotationTightening the filter to
entry.raw_doc is not None and isinstance(entry.raw_doc, str)is a solid defensive fix to avoid feeding non‑string docs intoinspect.cleandoc. Given this, you might optionally relaxDocEntry.raw_doc’s annotation fromstr | Noneto something likeobject | None(ortyping.Any) so the type hints better reflect that non‑string values can flow through and are then narrowed at the comprehension..github/workflows/update-doc-db.yml (1)
104-107: Add error handling for git operations and explicit remote specification.The git push on line 107 uses no explicit remote or branch reference. While this typically works with the current checkout, consider:
- Adding explicit error handling if the push fails
- Using
git push origin HEAD:${{ inputs.ref || github.ref_name }}for clarity and robustnessApply this diff to improve reliability and clarity:
if [ -n "$(git status --porcelain)" ]; then git add crates/doc/src/data.inc.rs git commit -m "Update doc DB for CPython ${{ inputs.python-version }}" - git push + git push origin HEAD:${{ inputs.ref || github.ref_name }} fi
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/update-doc-db.yml(5 hunks)crates/doc/generate.py(2 hunks)crates/doc/src/data.inc.rs(17 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: In most cases, Python code should not be edited; bug fixes should be made through Rust code modifications only
Follow PEP 8 style for custom Python code
Use ruff for linting Python code
Files:
crates/doc/generate.py
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/doc/src/data.inc.rs
🔇 Additional comments (4)
crates/doc/generate.py (1)
180-208: Including publictypes.*inbuiltin_typeslooks correct; confirm how keys are consumed downstreamThe
dir(types)+isinstance(obj, type)+ leading-underscore filter is a reasonable way to pull inModuleType,FunctionType, etc., without dragging in non-type objects. One thing to double‑check: these entries are still keyed as("builtins", typ.__name__)(Lines 206‑208), so the docs end up underbuiltins.FunctionType, nottypes.FunctionType. If the goal is to exposetypes.*docs, make sure the Rust side (or later processing) aliasestypes.<Name>to thesebuiltins.<Name>entries rather than expecting this script to emittypes.*keys directly.crates/doc/src/data.inc.rs (1)
4053-4074: Documentation additions look good overall.The new documentation entries for Python built-in types (
DynamicClassAttribute,GenericAlias,NotImplementedType,SimpleNamespace,UnionType,async_generator,coroutine,generator,frame,cell,code,module,traceback, various descriptor types, etc.) are comprehensive and consistent with CPython's documentation.The entries correctly document:
- PEP 585 (
GenericAlias) and PEP 604 (UnionType) type annotations- Generator/coroutine protocol methods (
send,throw,close)- Descriptor protocol methods (
__get__,__set__,__delete__)- Internal type attributes (e.g.,
gi_yieldfrom,cr_await,ag_await)Also applies to: 4343-4369, 4805-4826, 5117-5138, 5553-5578, 5750-5778, 5852-5872, 6131-6177, 6219-6246, 6424-6444, 6553-6574, 6642-6695, 6872-6931, 7004-7094, 7490-7511, 7622-7643
.github/workflows/update-doc-db.yml (2)
72-75: Clarify JSON merge strategy for multi-platform artifacts.The workflow merges JSON files from three different platforms (Ubuntu, Windows, macOS) using
jq -s "add". This operator merges JSON objects by key, with later values overriding earlier ones. Verify that:
- Generated docs are identical or compatible across all platforms, or
- Platform-specific overrides are intentional and expected
- Key collisions are handled correctly
Consider adding a brief comment documenting the expected merge behavior, or verify that CI validates output consistency across platforms.
100-100: Verify the condition logic prevents commits on intended branches.The condition
github.ref != 'refs/heads/main' && inputs.ref != 'main'prevents commits when either the current branch ismainOR wheninputs.refis explicitly set tomain. Sinceinputs.refdefaults to an empty string, the step will run on non-main branches unless explicitly overridden withref: main.Confirm this logic matches your intent—particularly that commits should not run on the main branch even if explicitly triggered via workflow dispatch.
| - name: Commit and push (non-main branches only) | ||
| if: github.ref != 'refs/heads/main' && inputs.ref != 'main' | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| if [ -n "$(git status --porcelain)" ]; then | ||
| git add crates/doc/src/data.inc.rs | ||
| git commit -m "Update doc DB for CPython ${{ inputs.python-version }}" | ||
| git push | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find .github -name "update-doc-db.yml" -type fRepository: RustPython/RustPython
Length of output: 100
🏁 Script executed:
cat -n .github/workflows/update-doc-db.yml | head -120Repository: RustPython/RustPython
Length of output: 4365
🏁 Script executed:
ls -la .github/workflows/ | grep -i updateRepository: RustPython/RustPython
Length of output: 132
🏁 Script executed:
find . -name "*.yml" -o -name "*.yaml" | grep -i updateRepository: RustPython/RustPython
Length of output: 102
🏁 Script executed:
git ls-files .github/workflows/ | head -20Repository: RustPython/RustPython
Length of output: 264
🏁 Script executed:
ls -la .github/ 2>/dev/null || echo "Checking git tracked files"; git ls-files .github/ | head -20Repository: RustPython/RustPython
Length of output: 887
🏁 Script executed:
git ls-files | grep -E "\.github.*\.(yml|yaml)$" | sortRepository: RustPython/RustPython
Length of output: 287
🏁 Script executed:
rg -l "Commit and push (non-main branches only)" --type yamlRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
rg "AUTO_COMMIT_PAT" --type yamlRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
rg "Update doc DB for CPython" --type yamlRepository: RustPython/RustPython
Length of output: 47
🌐 Web query:
GitHub Actions sparse checkout git operations authentication
💡 Result:
Summary + quick examples
- Use actions/checkout sparse options to do sparse-checkout in a workflow:
- inputs: sparse-checkout (newline-separated patterns), sparse-checkout-cone-mode (true/false), fetch-depth, filter (partial clone).
Example:- uses: actions/checkout@v6
with:
fetch-depth: 0
sparse-checkout: |
.github
src
sparse-checkout-cone-mode: true
(cite: actions/checkout). [1]
- uses: actions/checkout@v6
- Authenticated git operations in the job
- By default actions/checkout persists an auth token into local git config so subsequent git commands (fetch, push) run authenticated. That token is the job-scoped GITHUB_TOKEN. Use persist-credentials: true to keep it (default). To push from a workflow you also need appropriate token permissions (contents: write) — set repository permissions in the workflow or use a PAT (secret) if you need broader scopes or create new workflow runs. GITHUB_TOKEN expires at job end and actions-triggered events from it don’t re-run workflows. [2]
Example push with GITHUB_TOKEN:
permissions:
contents: write
jobs:
push:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
with:
persist-credentials: true
fetch-depth: 0
sparse-checkout: |
README.md
- run: |
git config user.name "github-actions[bot]"
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
# make changes...
git add .
git commit -m "Update"
git push origin HEAD:main
If you need to use a PAT (for cross-repo write, finer scopes, or to trigger new workflows), store it as a secret and set actions/checkout.token or configure git remote HTTPS with that PAT. [1][2]
- Caveats & troubleshooting
- Sparse/partial-clone behavior depends on Git and runner versions; some git versions and combinations of partial-clone/filter vs sparse-checkout have known issues (see issues where sparse may fall back to full checkout). If you see all files fetched, try changing cone-mode, fetch-depth, or use full fetch-depth: 0 and explicit git sparse-checkout commands on the runner. Test runner git version and update if needed. [1][3]
References
[1] actions/checkout README (sparse-checkout inputs & usage).
[2] GitHub Docs — GITHUB_TOKEN and using it for authentication in workflows.
[3] actions/checkout GitHub issues (sparse-checkout / git-version caveats).
🏁 Script executed:
git ls-files .github/ 2>/dev/null | head -50Repository: RustPython/RustPython
Length of output: 509
🏁 Script executed:
test -d .github && find .github -type f | head -30 || echo ".github directory not found in current repo"Repository: RustPython/RustPython
Length of output: 509
Ensure the AUTO_COMMIT_PAT secret is configured in repository settings with write access.
The merge job requires the AUTO_COMMIT_PAT secret to authenticate git push operations. Configure this secret in your repository settings with appropriate permissions to push commits to non-main branches. The workflow correctly uses the full repository checkout (without sparse checkout) for the merge and commit step, so git operations will work as expected.
🤖 Prompt for AI Agents
.github/workflows/update-doc-db.yml lines 99-108: the workflow assumes a
push-able token but doesn't ensure one is provided; configure the
AUTO_COMMIT_PAT secret in the repository settings with repo write permissions
and update the workflow to use it when pushing (for example export a GIT_TOKEN
env from secrets.AUTO_COMMIT_PAT and run git remote set-url origin
https://x-access-token:${GIT_TOKEN}@github.com/${{ github.repository }}.git
before git push); also ensure the workflow has repository write permissions
enabled so the push succeeds.
| "builtins.bytes_iterator.__str__" => "Return str(self).", | ||
| "builtins.bytes_iterator.__subclasshook__" => "Abstract classes can override this to customize issubclass().\n\nThis is invoked early on by abc.ABCMeta.__subclasscheck__().\nIt should return True, False or NotImplemented. If it returns\nNotImplemented, the normal algorithm is used. Otherwise, it\noverrides the normal algorithm (and the outcome is cached).", | ||
| "builtins.callable" => "Return whether the object is callable (i.e., some kind of function).\n\nNote that classes are callable, as are instances of classes with a\n__call__() method.", | ||
| "builtins.cell" => "Create a new cell object.\n\n contents\n the contents of the cell. If not specified, the cell will be empty,\n and \nfurther attempts to access its cell_contents attribute will\n raise a ValueError.", |
There was a problem hiding this comment.
Minor formatting issue in the cell docstring.
There's a stray newline in the documentation string: "and \nfurther" breaks the sentence mid-flow. This likely originates from the source docstring or the generation process in generate.py.
Since this is a generated file, consider investigating whether this malformation comes from CPython's docstring or if there's a processing issue in the generator script.
🤖 Prompt for AI Agents
In crates/doc/src/data.inc.rs around line 6086, the generated docstring for
"builtins.cell" contains a stray newline and space breaking "and \nfurther" into
two lines; update the generator (generate.py) to normalize docstring whitespace
for generated output—either fix the source docstring if it's malformed in
CPython, or in the generator collapse internal newlines and multiple spaces into
single spaces (e.g., join wrapped lines with a single space while preserving
paragraph breaks), then regenerate the file so the doc becomes "and further
attempts..." without the stray "\n" and extraneous space.
ShaharNaveh
left a comment
There was a problem hiding this comment.
Great! Thanks for fixing the workflow as well!
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.