Actionable comments posted: 0

crates/rustpython_doc_db/src/lib.rs (1)

10-19: Consider expanding test coverage.

The single test validates basic DB functionality, but additional tests would strengthen confidence in the generated database. Consider adding tests for:

• Multiple module paths (builtins, stdlib modules)
• Non-existent keys returning None
• Documentation content format validation

crates/rustpython_doc_db/generate.py (1)

36-36: Replace Python 3.12+ type statement with backwards-compatible syntax.

The type statement for type aliases was introduced in Python 3.12. While the workflow specifies Python 3.13.9, using a more compatible syntax ensures the script works with older Python versions.

Apply this diff:

-type Parts = tuple[str, ...]
+Parts = tuple[str, ...]

Alternatively, if type hints are important, use typing.TypeAlias:

+from typing import TypeAlias
+
-type Parts = tuple[str, ...]
+Parts: TypeAlias = tuple[str, ...]

.github/workflows/update-doc-db.yml (1)

56-56: Intentionally disabled PR job with confusing condition.

The condition if: false && ${{ inputs.open_pr }} is always false due to the leading false &&. While this effectively disables the job, it's confusing. Consider using a clearer approach:

-  if: false && ${{ inputs.open_pr }}
+  if: false  # TODO: Re-enable when PR automation is ready

Or remove the job entirely until it's ready to be used.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 5cad66c and 83ab568.

• Cargo.lock is excluded by !**/*.lock

• .gitattributes (1 hunks)
• .github/workflows/update-doc-db.yml (1 hunks)
• Cargo.toml (3 hunks)
• crates/rustpython_doc_db/Cargo.toml (1 hunks)
• crates/rustpython_doc_db/LICENSE (1 hunks)
• crates/rustpython_doc_db/generate.py (1 hunks)
• crates/rustpython_doc_db/src/lib.rs (1 hunks)
• derive-impl/Cargo.toml (2 hunks)
• derive-impl/src/lib.rs (0 hunks)
• derive-impl/src/pyclass.rs (2 hunks)
• derive-impl/src/pymodule.rs (3 hunks)

• derive-impl/src/lib.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Format Rust code with the default rustfmt style (run cargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management

Files:

• derive-impl/src/pyclass.rs
• derive-impl/src/pymodule.rs
• crates/rustpython_doc_db/src/lib.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files; only make minimal changes to work around RustPython limitations

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

• derive-impl/src/pyclass.rs
• derive-impl/src/pymodule.rs
• crates/rustpython_doc_db/src/lib.rs
• crates/rustpython_doc_db/Cargo.toml
• Cargo.toml
• crates/rustpython_doc_db/generate.py
• derive-impl/Cargo.toml
• .gitattributes

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files; only make minimal changes to work around RustPython limitations

Applied to files:

• derive-impl/src/pyclass.rs
• derive-impl/src/pymodule.rs
• crates/rustpython_doc_db/src/lib.rs
• crates/rustpython_doc_db/Cargo.toml
• Cargo.toml
• crates/rustpython_doc_db/generate.py
• derive-impl/Cargo.toml
• .gitattributes

Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.

Applied to files:

• crates/rustpython_doc_db/src/lib.rs

Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.

Applied to files:

• crates/rustpython_doc_db/src/lib.rs

Learnt from: jackoconnordev
Repo: RustPython/RustPython PR: 6086
File: benches/microbenchmarks/sort.py:1-3
Timestamp: 2025-08-09T22:56:24.527Z
Learning: In RustPython's microbenchmarks (benches/microbenchmarks/*.py), the variable `ITERATIONS` is intentionally used without being defined in the Python files. It is injected by the cargo bench harness at runtime. This pattern should be maintained for consistency across all microbenchmarks, and F821 lint warnings for undefined `ITERATIONS` are expected and acceptable in this context.

Applied to files:

• Cargo.toml

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Run clippy and fix any warnings or lints introduced by your changes

Applied to files:

• derive-impl/Cargo.toml
• .gitattributes

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Format Rust code with the default rustfmt style (run `cargo fmt`)

Applied to files:

• .gitattributes

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Follow Rust best practices for error handling and memory management

Applied to files:

• .gitattributes

Lib/platform.py (2)

• platform (1193-1265)
• python_version (1125-1133)

Lib/importlib/_bootstrap_external.py (1)

• ExtensionFileLoader (1267-1317)

Lib/warnings.py (1)

• catch_warnings (442-515)

56-56: if: condition "false && ${{ inputs.open_pr }}" is always evaluated to true because extra characters are around ${{ }}

(if-cond)

[error] 36-36: SyntaxError: invalid syntax

(E999)

.gitattributes (1)

8-8: LGTM!

Correctly marks the generated PHF map files as linguist-generated, which is appropriate for code-generated artifacts.

crates/rustpython_doc_db/LICENSE (1)

1-277: LGTM!

The Python Software Foundation License is appropriate for a crate containing documentation derived from Python's standard library.

crates/rustpython_doc_db/src/lib.rs (1)

1-8: LGTM!

Platform-specific conditional compilation is correctly structured. The approach ensures the appropriate documentation database is included for each target OS.

Cargo.toml (3)

76-78: LGTM!

Setting codegen-units = 1 for the doc database in release builds is a good optimization. Since this crate changes infrequently and contains a large static PHF map, maximizing optimization at the cost of longer compile times is appropriate.

---

139-139: LGTM!

Using the wildcard pattern crates/* for workspace members is cleaner than listing individual crates and allows for easier future additions.

---

164-164: LGTM!

The rustpython_doc_db and phf dependencies are correctly configured. PHF 0.13.1 with macros feature is appropriate for the perfect hash function implementation.

Also applies to: 171-171

crates/rustpython_doc_db/generate.py (3)

44-49: Verify UNICODE_ESCAPE regex pattern behavior.

The regex replacement at line 45 appears to convert \uNNNN to \u{NNNN} format, but the pattern r"\\u([0-9]+)" will match any sequence of digits (not limited to 4). This could cause issues if the input contains sequences like \u123456.

Consider whether the pattern should be more specific:

-UNICODE_ESCAPE = re.compile(r"\\u([0-9]+)")
+UNICODE_ESCAPE = re.compile(r"\\u([0-9a-fA-F]{4})")

Or if extended unicode is intended:

-UNICODE_ESCAPE = re.compile(r"\\u([0-9]+)")
+UNICODE_ESCAPE = re.compile(r"\\u([0-9a-fA-F]+)")

---

93-109: LGTM!

Properly suppresses DeprecationWarning during module imports and handles import failures gracefully with warnings. This is appropriate for scanning stdlib modules where some may be deprecated or platform-specific.

---

200-226: LGTM!

The main function correctly generates a Rust PHF map with proper formatting, headers, and platform-specific output paths. The sorted output ensures deterministic generation.

.github/workflows/update-doc-db.yml (2)

31-53: LGTM!

The generation job correctly runs on all major platforms (Windows, Linux, macOS), executes the Python generator script, detects output changes, and uploads artifacts with appropriate retention settings.

---

19-19: The review comment is incorrect—Python 3.13.9 is valid.

Python 3.13.9 is the latest Python 3.13 release, released October 14, 2025. The specified version in the workflow is correct and no changes are needed.

Likely an incorrect or invalid review comment.

derive-impl/src/pyclass.rs (2)

9-9: LGTM!

Correctly imports the centralized documentation database.

---

317-322: LGTM!

The migration to the centralized documentation database is correctly implemented. The DB.get(&format!(...)).copied().map(str::to_owned) pattern properly handles the lookup and ownership transfer.

crates/rustpython_doc_db/Cargo.toml (1)

13-14: Remove or update the unnecessary doctest disable.

The crate contains no documentation comments or doctests—the doctest = false setting serves no purpose. The comment "Crashes when true" is unsubstantiated. Either remove the setting or replace the comment with accurate context if there's a specific reason for keeping it disabled.

Likely an incorrect or invalid review comment.

derive-impl/Cargo.toml (2)

14-14: LGTM!

The dependency migration from rustpython-doc to rustpython_doc_db correctly reflects the PR's objective to move the documentation database into the monorepo.

---

26-26: LGTM!

The lint configuration is now correctly inheriting from the workspace.

derive-impl/src/pymodule.rs (3)

9-9: LGTM!

The import of DB from rustpython_doc_db correctly provides access to the static documentation database.

---

104-104: LGTM!

The migration to the static DB lookup is correctly implemented. The pattern DB.get(module_name).copied().map(str::to_owned) appropriately converts the PHF map's Option<&&'static str> to Option<String>, and the fallback chain ensures code-level documentation takes precedence.

---

461-466: LGTM!

The item-level documentation lookup correctly migrates to the DB-based approach. The key format "{module}.{py_name}" follows Python's standard naming convention, and the fallback logic preserves the precedence of code-level documentation.

The format! allocation on each lookup is a minor cost during proc-macro expansion, which is acceptable given this code runs at compile time rather than runtime.

Actionable comments posted: 2

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 83ab568 and 6140114.

• .github/workflows/update-doc-db.yml (1 hunks)

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files; only make minimal changes to work around RustPython limitations

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.

Applied to files:

• .github/workflows/update-doc-db.yml

• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run tests under miri
• GitHub Check: Check Rust code with rustfmt and clippy
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Check the WASM package and demo

.github/workflows/update-doc-db.yml (1)

31-32: Sparse-checkout is sufficient for generate.py.

Verification confirms that generate.py is completely self-contained within crates/rustpython_doc_db and does not require access to files outside this directory. The script:

• Anchors all operations to CRATE_DIR (the script's parent directory)
• Writes output exclusively to CRATE_DIR / "src" / *.inc.rs
• Contains no file read operations, config file loading, or external data dependencies
• Uses only standard library imports with no workspace or sibling crate references

The workflow's sparse-checkout configuration is correct and sufficient.

Actionable comments posted: 3

crates/rustpython_doc_db/src/lib.rs (1)

7-11: Consider testing DB existence more generally.

The test currently hardcodes a specific key ("array._array_reconstructor"). If the generated data changes or this entry is removed, the test will break. Consider either:

• Testing that the DB is not empty: assert!(!DB.is_empty());
• Testing multiple well-known entries to be more robust

     #[test]
     fn test_db() {
-        let doc = DB.get("array._array_reconstructor");
-        assert!(doc.is_some());
+        // Verify DB contains documentation entries
+        assert!(!DB.is_empty(), "DB should contain documentation entries");
+        
+        // Verify at least one known entry exists
+        assert!(
+            DB.get("builtins.dict").is_some() || DB.get("builtins.list").is_some(),
+            "DB should contain common builtin types"
+        );
     }

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 7a432ec and 25cf146.

• .github/workflows/update-doc-db.yml (1 hunks)
• crates/rustpython_doc_db/.gitignore (1 hunks)
• crates/rustpython_doc_db/generate.py (1 hunks)
• crates/rustpython_doc_db/src/lib.rs (1 hunks)

• crates/rustpython_doc_db/.gitignore

• .github/workflows/update-doc-db.yml

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Format Rust code with the default rustfmt style (run cargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management

Files:

• crates/rustpython_doc_db/src/lib.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files; only make minimal changes to work around RustPython limitations

Applied to files:

• crates/rustpython_doc_db/src/lib.rs
• crates/rustpython_doc_db/generate.py

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

• crates/rustpython_doc_db/src/lib.rs

Lib/importlib/_bootstrap_external.py (1)

• ExtensionFileLoader (1267-1317)

Lib/warnings.py (1)

• catch_warnings (442-515)

[error] 37-37: SyntaxError: invalid syntax

(E999)

• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Check Rust code with rustfmt and clippy
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Check the WASM package and demo

crates/rustpython_doc_db/src/lib.rs (1)

1-1: LGTM: Appropriate use of include! for generated code.

The include! macro is the standard approach for incorporating generated code in Rust.

crates/rustpython_doc_db/generate.py (5)

1-34: LGTM: Imports and constants are well-structured.

Good use of pathlib and proper handling of the output directory creation.

---

53-92: LGTM: Robust C extension detection logic.

The function uses multiple fallback approaches to reliably detect C extensions.

---

94-121: LGTM: Proper warning handling and module iteration.

Good use of warnings.catch_warnings() to suppress deprecation warnings while still reporting import failures.

---

176-176: Verify builtins usage across different contexts.

Using __builtins__ directly can be fragile as it's a dict in __main__ and a module in other contexts. While traverse() handles both cases, it's worth verifying this works correctly.

Consider using the builtins module explicitly:

+import builtins
+
 def find_doc_entires() -> "Iterable[DocEntry]":
     yield from (
         doc_entry
         for module in iter_c_modules()
         for doc_entry in traverse(module, module)
     )
-    yield from (doc_entry for doc_entry in traverse(__builtins__, __builtins__))
+    yield from (doc_entry for doc_entry in traverse(builtins, builtins))
 
     builtin_types = [
         # ...
     ]
     for typ in builtin_types:
         parts = ("builtins", typ.__name__)
         yield DocEntry(parts, pydoc._getowndoc(typ))
-        yield from traverse(typ, __builtins__, parts)
+        yield from traverse(typ, builtins, parts)

---

37-37: ****

The type statement syntax is appropriate for this codebase. RustPython explicitly targets Python 3.13.0+ (as stated in README.md: "A Python-3 (CPython >= 3.13.0) Interpreter written in Rust"), and the Python 3.12+ type syntax (PEP 695) is consistent with multiple other Python files in the project (scripts/lib_updater.py, Lib/typing.py, Lib/test/typinganndata/mod_generics_cache.py, and Lib/test/test_symtable.py). No compatibility issue exists.

Likely an incorrect or invalid review comment.

Actionable comments posted: 3

crates/rustpython_doc_db/generate.py (1)

150-161: Guard against type-checking edge cases.

Two potential runtime errors remain from the previous review:

1. Line 151: type(__builtins__) may not be consistent—__builtins__ can be a dict in __main__ or a module elsewhere.
2. Line 159: issubclass(attr_typ, type) raises TypeError if attr_typ is not a class (e.g., a descriptor or special object).

Consider this safer approach:

-    attr_typ = type(attr)
-    is_type_or_builtin = any(attr_typ is x for x in (type, type(__builtins__)))
+    # Check if attr itself is a type (class)
+    is_type_or_builtin = isinstance(attr, type)
 
     if is_type_or_builtin:
         yield from traverse(attr, module, new_parts)
         continue
 
     is_callable = (
         callable(attr)
-        or not issubclass(attr_typ, type)
+        or not isinstance(attr, type)
         or attr_typ.__name__ in ("getset_descriptor", "member_descriptor")
     )

This avoids the __builtins__ type inconsistency and uses isinstance(attr, type) instead of issubclass, which won't raise TypeError.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 25cf146 and 4e21b6f.

• crates/rustpython_doc_db/generate.py (1 hunks)

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files; only make minimal changes to work around RustPython limitations

Applied to files:

• crates/rustpython_doc_db/generate.py

Lib/importlib/_bootstrap_external.py (1)

• ExtensionFileLoader (1267-1317)

Lib/warnings.py (1)

• catch_warnings (442-515)

[error] 37-37: SyntaxError: invalid syntax

(E999)

• GitHub Check: Check the WASM package and demo
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Check Rust code with rustfmt and clippy
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Ensure compilation on various targets

crates/rustpython_doc_db/generate.py (2)

48-52: Good addition of assertion to document precondition.

The assert self.raw_doc is not None on line 50 clearly documents that this property should only be called when raw_doc is not None. Since main() filters entries (lines 206-207) before accessing .doc, this precondition is satisfied in practice.

---

203-211: LGTM! Clean filtering and output logic.

The filtering on line 207 (if entry.raw_doc is not None) correctly ensures that only entries with documentation are included, and the subsequent .doc property access is safe. The JSON output formatting is appropriate.

Thank you so much!

@coolreader18 you have to review crate naming policy not to block these changes

Actionable comments posted: 0

crates/rustpython-doc/generate.py (3)

37-37: Critical: Use backwards-compatible type alias syntax.

The PEP 695 type statement requires Python 3.12+. Since no Python 3.12+ requirement is documented, use the backwards-compatible form to ensure the script works on earlier Python versions.

Apply this diff:

-type Parts = tuple[str, ...]
+Parts: typing.TypeAlias = tuple[str, ...]

---

178-201: Critical: Fix __builtins__ handling for __main__ context.

When this script runs as __main__, __builtins__ is a dict, not a module. Lines 178 and 200 pass __builtins__ to traverse(), which expects a types.ModuleType as the second parameter. This causes is_child_of() to fail since inspect.getmodule(obj) is module will compare a module/None against a dict.

Apply this diff:

+import builtins
+
 def find_doc_entries() -> "Iterable[DocEntry]":
     yield from (
         doc_entry
         for module in iter_c_modules()
         for doc_entry in traverse(module, module)
     )
-    yield from (doc_entry for doc_entry in traverse(__builtins__, __builtins__))
+    yield from (doc_entry for doc_entry in traverse(builtins, builtins))

     builtin_types = [
         type(None),
         type(bytearray().__iter__()),
         ...
     ]
     for typ in builtin_types:
         parts = ("builtins", typ.__name__)
         yield DocEntry(parts, pydoc._getowndoc(typ))
-        yield from traverse(typ, __builtins__, parts)
+        yield from traverse(typ, builtins, parts)

---

150-161: Guard against TypeError in type checking logic.

Line 159's issubclass(attr_typ, type) can raise TypeError if attr_typ is not a class (e.g., special descriptors, generic aliases). The callable(attr) check doesn't fully protect against this.

Additionally, line 151's type(__builtins__) is fragile since __builtins__ can be a dict in some contexts.

Apply this diff to make the checks more robust:

 attr_typ = type(attr)
-is_type_or_builtin = any(attr_typ is x for x in (type, type(__builtins__)))
+# Check if attr itself is a type/class
+is_type_or_builtin = isinstance(attr, type)

 if is_type_or_builtin:
     yield from traverse(attr, module, new_parts)
     continue

 is_callable = (
     callable(attr)
-    or not issubclass(attr_typ, type)
+    or not (inspect.isclass(attr_typ) and issubclass(attr_typ, type))
     or attr_typ.__name__ in ("getset_descriptor", "member_descriptor")
 )

crates/rustpython-doc/generate.py (1)

48-52: Consider a more defensive approach for the doc property.

The assert at line 50 assumes raw_doc is not None, which is currently safe because main() (line 207) filters out entries with None raw_doc before calling .doc. However, adding a more descriptive error or returning an empty string would make this more defensive.

Optional improvement:

 @property
 def doc(self) -> str:
-    assert self.raw_doc is not None
-
+    if self.raw_doc is None:
+        return ""
     return re.sub(UNICODE_ESCAPE, r"\\u{\1}", inspect.cleandoc(self.raw_doc))

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 4e21b6f and 4544be3.

• Cargo.lock is excluded by !**/*.lock

• .github/workflows/update-doc-db.yml (1 hunks)
• Cargo.toml (3 hunks)
• crates/rustpython-doc/.gitignore (1 hunks)
• crates/rustpython-doc/Cargo.toml (1 hunks)
• crates/rustpython-doc/LICENSE (1 hunks)
• crates/rustpython-doc/generate.py (1 hunks)
• crates/rustpython-doc/src/lib.rs (1 hunks)
• derive-impl/Cargo.toml (1 hunks)
• derive-impl/src/pyclass.rs (2 hunks)
• derive-impl/src/pymodule.rs (3 hunks)

• crates/rustpython-doc/Cargo.toml
• crates/rustpython-doc/src/lib.rs
• crates/rustpython-doc/.gitignore

• derive-impl/src/pymodule.rs
• derive-impl/src/pyclass.rs
• .github/workflows/update-doc-db.yml
• derive-impl/Cargo.toml

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files; only make minimal changes to work around RustPython limitations

Applied to files:

• crates/rustpython-doc/LICENSE
• crates/rustpython-doc/generate.py
• Cargo.toml

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to Lib/test/**/*.py : When modifying CPython tests, add a '# TODO: RUSTPYTHON' comment documenting the reason

Applied to files:

• crates/rustpython-doc/generate.py

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

• Cargo.toml

Learnt from: jackoconnordev
Repo: RustPython/RustPython PR: 6086
File: benches/microbenchmarks/sort.py:1-3
Timestamp: 2025-08-09T22:56:24.527Z
Learning: In RustPython's microbenchmarks (benches/microbenchmarks/*.py), the variable `ITERATIONS` is intentionally used without being defined in the Python files. It is injected by the cargo bench harness at runtime. This pattern should be maintained for consistency across all microbenchmarks, and F821 lint warnings for undefined `ITERATIONS` are expected and acceptable in this context.

Applied to files:

• Cargo.toml

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: In most cases, avoid editing Python code; prefer fixing bugs via Rust changes

Applied to files:

• Cargo.toml

Lib/importlib/_bootstrap_external.py (1)

• ExtensionFileLoader (1267-1317)

Lib/warnings.py (1)

• catch_warnings (442-515)

[error] 37-37: SyntaxError: invalid syntax

(E999)

• GitHub Check: auto_format

crates/rustpython-doc/LICENSE (1)

1-277: LGTM: Standard Python license file.

The Python Software Foundation license text is appropriate for a crate that handles Python standard library documentation.

Cargo.toml (4)

76-78: LGTM: Appropriate optimization for a data-heavy crate.

Setting codegen-units = 1 for the doc database crate trades longer compile time for better optimization, which is reasonable for data that "doesn't change often."

---

139-139: LGTM: Standard workspace glob pattern.

Adding crates/* appropriately includes the new rustpython-doc crate and any future crates in that directory.

---

164-164: LGTM: Crate successfully migrated to workspace.

The path-based dependency correctly integrates the formerly external rustpython-doc crate into the workspace. The naming convention (dash separator) is consistent with other RustPython crates.

---

171-171: LGTM: phf dependency for compile-time hash maps.

The phf dependency configuration is appropriate for generating compile-time perfect hash maps, addressing the HashMap issues mentioned in the PR objectives when handling ~9,000 documentation entries.

Looks good to me.

I will wait for @coolreader18 a few more days about the crate relocation issue.

Actionable comments posted: 0

crates/doc/generate.py (4)

37-37: Critical: Use backwards-compatible type alias syntax.

Line 37 uses Python 3.12+ type statement syntax (PEP 695), which will cause a SyntaxError on Python 3.11 or earlier. Since no explicit Python 3.12+ requirement is documented, use the backwards-compatible form.

Apply this diff:

-type Parts = tuple[str, ...]
+Parts: typing.TypeAlias = tuple[str, ...]

---

150-161: Fix type checking logic to avoid errors and false positives.

Two issues in this type-checking code:

1. Line 151: type(__builtins__) is unreliable because __builtins__ can be a dict in __main__ context or a module elsewhere. Comparing attr_typ to type(__builtins__) will give inconsistent results.

2. Line 159: issubclass(attr_typ, type) can raise TypeError if attr_typ is not a class (e.g., if it's a descriptor or special object).

Consider this approach:

-        attr_typ = type(attr)
-        is_type_or_builtin = any(attr_typ is x for x in (type, type(__builtins__)))
+        # Check if attr itself is a type/class
+        is_type_or_builtin = isinstance(attr, type)
 
         if is_type_or_builtin:
             yield from traverse(attr, module, new_parts)
             continue
 
-        is_callable = (
-            callable(attr)
-            or not issubclass(attr_typ, type)
-            or attr_typ.__name__ in ("getset_descriptor", "member_descriptor")
-        )
+        attr_typ = type(attr)
+        try:
+            is_subclass_of_type = issubclass(attr_typ, type)
+        except TypeError:
+            is_subclass_of_type = False
+        
+        is_callable = (
+            callable(attr)
+            or not is_subclass_of_type
+            or attr_typ.__name__ in ("getset_descriptor", "member_descriptor")
+        )

---

178-178: Critical: Import and use the builtins module instead of builtins.

Using __builtins__ directly is problematic because it's a dict when the script runs as __main__ but a module in other contexts. When it's a dict, traverse() and is_child_of() will fail because they expect types.ModuleType.

Apply this fix:

+import builtins
+
 def find_doc_entries() -> "Iterable[DocEntry]":
     yield from (
         doc_entry
         for module in iter_c_modules()
         for doc_entry in traverse(module, module)
     )
-    yield from (doc_entry for doc_entry in traverse(__builtins__, __builtins__))
+    yield from (doc_entry for doc_entry in traverse(builtins, builtins))

Also update line 200 to use builtins instead of __builtins__.

---

197-200: Apply the same builtins fix here.

Line 200 also uses __builtins__ and needs the same fix as line 178. After importing builtins at the top:

     for typ in builtin_types:
         parts = ("builtins", typ.__name__)
         yield DocEntry(parts, pydoc._getowndoc(typ))
-        yield from traverse(typ, __builtins__, parts)
+        yield from traverse(typ, builtins, parts)

crates/doc/Cargo.toml (1)

13-14: Clarify the doctest crash and consider alternatives.

The comment "Crashes when true" is vague and doesn't explain the root cause. Please:

1. Document the specific error message or panic that occurs
2. Consider whether this is a fundamental limitation or a solvable issue
3. If it's due to the included generated data, add a more descriptive comment

Understanding the underlying issue will help future maintainers decide whether this is a permanent workaround or something that can be fixed.

crates/doc/generate.py (1)

48-52: Consider handling None more gracefully in the doc property.

The assert at line 50 will prevent crashes when raw_doc is None, but assertions can be disabled with Python's -O flag. Since main() already filters for raw_doc is not None at line 207, this shouldn't trigger in practice. However, for robustness, consider either:

1. Returning an empty string when raw_doc is None
2. Changing the return type to str | None and returning None

This makes the API more defensive and self-documenting.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 4544be3 and 4bee896.

• Cargo.toml (3 hunks)
• crates/doc/.gitignore (1 hunks)
• crates/doc/Cargo.toml (1 hunks)
• crates/doc/LICENSE (1 hunks)
• crates/doc/generate.py (1 hunks)
• crates/doc/src/lib.rs (1 hunks)

• crates/doc/.gitignore
• crates/doc/LICENSE

Learnt from: jackoconnordev
Repo: RustPython/RustPython PR: 6086
File: benches/microbenchmarks/sort.py:1-3
Timestamp: 2025-08-09T22:56:24.527Z
Learning: In RustPython's microbenchmarks (benches/microbenchmarks/*.py), the variable `ITERATIONS` is intentionally used without being defined in the Python files. It is injected by the cargo bench harness at runtime. This pattern should be maintained for consistency across all microbenchmarks, and F821 lint warnings for undefined `ITERATIONS` are expected and acceptable in this context.

Applied to files:

• Cargo.toml

[error] 37-37: SyntaxError: invalid syntax

(E999)

• GitHub Check: Check the WASM package and demo
• GitHub Check: Run tests under miri
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Check Rust code with clippy
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Ensure compilation on various targets
• GitHub Check: auto_format

Cargo.toml (4)

76-78: LGTM! Appropriate optimization for a data-heavy crate.

Setting codegen-units = 1 for the rustpython-doc package enables maximum optimization, which is appropriate given that this crate contains static documentation data that changes infrequently.

---

139-139: LGTM! Clean workspace organization.

Using the "crates/*" glob pattern is a good practice that simplifies adding new crates to the workspace.

---

164-164: Verify the crate path matches the PR objectives.

The PR title mentions moving to crates/rustpython_doc_db, but the actual path here is crates/doc. Please confirm this is intentional, as the discrepancy could cause confusion for future maintainers.

---

171-171: phf 0.13.1 is confirmed as the latest stable version.

The dependency configuration is correct and up-to-date. The explicit default-features = false with the "macros" feature is an appropriate choice for this use case.

crates/doc/Cargo.toml (1)

1-9: LGTM! Standard crate configuration.

The package metadata properly inherits workspace values and is well-configured.

crates/doc/src/lib.rs (2)

1-1: Verify the build process generates data.inc.rs.

The included file data.inc.rs is not present in the PR (likely gitignored). Please ensure that:

1. The build process (or a pre-build step) generates this file
2. Documentation explains how to regenerate it
3. CI validates that the generated file is up-to-date

---

3-12: LGTM! Adequate smoke test for the generated documentation database.

The test validates that the generated DB contains expected entries, which is appropriate for ensuring the data generation pipeline works correctly.

crates/doc/generate.py (3)

1-20: LGTM! Clean script setup with appropriate OS-specific output paths.

The imports and output file configuration are well-structured.

---

55-83: LGTM! Well-implemented C extension detection.

The function uses multiple fallback checks and is well-documented.

---

203-214: LGTM! Clean data generation and output.

The main function properly filters entries and produces well-formatted JSON output.

Looks great! And thank you so much for your patience