Skip to content

Introduce /upgrade-pylib-next #6827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 9 commits into from
Jan 30, 2026
Merged

Introduce /upgrade-pylib-next #6827

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6b3e342
Simplify upgrade-pylib.md by removing manual copy steps handled by qu…
moreal Jan 21, 2026
70e7a6d
Let upgrade-pylib run auto-mark instead marking manually
moreal Jan 21, 2026
37bdfd6
Let upgrade-pylib a separate commit from updating module
moreal Jan 21, 2026
35ceed8
Correct commands' permissions
moreal Jan 21, 2026
79babbc
Add open PR check to upgrade-pylib-next command
moreal Jan 21, 2026
cd1389b
Fix dependency pattern example in upgrade-pylib-next command
moreal Jan 21, 2026
b2c93ca
Let upgrade-pylib command to invetigate failing tests
moreal Jan 21, 2026
2712f2b
Add pre-commit review to invetigate-test-failure command
moreal Jan 23, 2026
01c614d
Use deps command to get dependent tests in upgrade-pylib workflow
moreal Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Let upgrade-pylib run auto-mark instead marking manually
  • Loading branch information
@moreal
moreal committed Jan 30, 2026
commit 70e7a6d03d56c06627019da373ac80fd423223d2
27 changes: 27 additions & 0 deletions .claude/commands/upgrade-pylib-next.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
allowed-tools: Bash(git add:*), Bash(git commit:*), Bash(python3 scripts/update_lib quick:*), Skill(upgrade-pylib)
---

# Upgrade Next Python Library

Find the next Python library module ready for upgrade and run `/upgrade-pylib` for it.

## Current TODO Status

!`cargo run --release -- scripts/update_lib todo 2>/dev/null`

## Instructions

From the TODO list above, find modules matching these patterns (in priority order):

1. `[ ] [no deps]` - Modules with no dependencies (can be upgraded immediately)
2. `[ ] [n/n]` - Modules where all dependencies are already upgraded (e.g., `[3/3]`, `[5/5]`)
Comment thread
moreal marked this conversation as resolved.
Outdated

These patterns indicate modules that are ready to upgrade without blocking dependencies.

**After identifying a suitable module**, run:
```
/upgrade-pylib <module_name>
```

If no modules match these criteria, inform the user that all eligible modules have dependencies that need to be upgraded first.
43 changes: 23 additions & 20 deletions .claude/commands/upgrade-pylib.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
---
allowed-tools: Bash(python3 scripts/update_lib auto-mark:*)
---
Comment on lines +1 to +3

@coderabbitai coderabbitai Bot Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add missing deps subcommand to allowed-tools.

The workflow in step 3 (line 54) uses cargo run --release -- scripts/update_lib deps, but the allowed-tools header only permits quick and auto-mark subcommands. This will cause the command to be blocked during execution.

🔧 Proposed fix 
 ---
-allowed-tools: Bash(git add:*), Bash(git commit:*), Bash(python3 scripts/update_lib quick:*), Bash(python3 scripts/update_lib auto-mark:*)
+allowed-tools: Bash(git add:*), Bash(git commit:*), Bash(python3 scripts/update_lib quick:*), Bash(python3 scripts/update_lib auto-mark:*), Bash(cargo run:*)
 ---

Note: This allows cargo run for both the deps command (line 54) and the test execution commands (lines 59, 63).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
---
allowed-tools: Bash(git add:*), Bash(git commit:*), Bash(python3 scripts/update_lib quick:*), Bash(python3 scripts/update_lib auto-mark:*)
---
---
allowed-tools: Bash(git add:*), Bash(git commit:*), Bash(python3 scripts/update_lib quick:*), Bash(python3 scripts/update_lib auto-mark:*), Bash(cargo run:*)
---
🤖 Prompt for AI Agents 
In @.claude/commands/upgrade-pylib.md around lines 1 - 3, The allowed-tools
header currently permits only Bash(git add:*), Bash(git commit:*), Bash(python3
scripts/update_lib quick:*), and Bash(python3 scripts/update_lib auto-mark:*),
but the workflow runs cargo run --release -- scripts/update_lib deps; update the
allowed-tools header to also include Bash(python3 scripts/update_lib deps:*) (or
the equivalent pattern used for quick/auto-mark) so the `deps` subcommand is
allowed; ensure the new entry matches the existing pattern for `quick` and
`auto-mark` to permit the deps invocation and related test commands.


# Upgrade Python Library from CPython

Upgrade a Python standard library module from CPython to RustPython.
Expand Down Expand Up @@ -44,13 +48,25 @@ This helps improve the tooling for future upgrades.
- **Do NOT restore other diff changes** - these are likely upstream CPython changes, not RustPython-specific modifications
- When restoring, preserve the original context and formatting

3. **Verify tests**
- Run: `cargo run --release -- -m test test_$ARGUMENTS -v`
- The `-v` flag shows detailed output to identify which tests fail and why
- For each new failure, add appropriate markers based on the failure type:
- **Test assertion failure** → `@unittest.expectedFailure` with `# TODO: RUSTPYTHON` comment
- **Panic/crash** → `@unittest.skip("TODO: RUSTPYTHON; <panic message>")`
- **Class-specific markers**: If a test fails only in the C implementation (TestCFoo) but passes in the Python implementation (TestPyFoo), or vice versa, add the marker to the specific subclass, not the base class:
3. **Mark remaining test failures with auto-mark**

@youknowone youknowone Jan 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick now auto-mark newly added failing tests. only regressions are remained failing.
Before step 3, let's add review the failing tests and analyze If that's justified to mark failure or not.
Lots of them will be better to report issue or worth to try fix them; otherwise upgrade the dependency lib first.

@moreal moreal Jan 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm dogfooding this command, and as your suggestion, it fixes a failed test automatically!

38ff638

(Other tests (snippets) seem to fail but anyway 😅)

@youknowone youknowone Jan 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, it is good as long as it doesn't blindly mark everything expectedFailure

- Run: `python3 scripts/update_lib auto-mark Lib/test/test_$ARGUMENTS.py --mark-failure`
- Or for directory: `python3 scripts/update_lib auto-mark Lib/test/test_$ARGUMENTS/ --mark-failure`
- This will:
- Run tests and mark ALL failing tests with `@unittest.expectedFailure`
- Remove `@unittest.expectedFailure` from tests that now pass
- **Note**: The `--mark-failure` flag marks all failures including regressions. Review the changes before committing.

4. **Handle panics manually**
- If any tests cause panics/crashes (not just assertion failures), they need `@unittest.skip` instead:
```python
@unittest.skip("TODO: RUSTPYTHON; panics with 'index out of bounds'")
def test_crashes(self):
...
```
- auto-mark cannot detect panics automatically - check the test output for crash messages

5. **Handle class-specific failures**
- If a test fails only in the C implementation (TestCFoo) but passes in the Python implementation (TestPyFoo), or vice versa, move the marker to the specific subclass:
```python
# Base class - no marker here
class TestFoo:
Expand All @@ -65,19 +81,6 @@ This helps improve the tooling for future upgrades.
def test_something(self):
return super().test_something()
```
- **New tests from CPython**: The upgrade may bring in entirely new tests that didn't exist before. These won't have any RUSTPYTHON markers in the diff - they just need to be tested and marked if they fail.
- Example markers:
```python
# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_something(self):
...

# TODO: RUSTPYTHON
@unittest.skip("TODO: RUSTPYTHON; panics with 'index out of bounds'")
def test_crashes(self):
...
```

## Example Usage
```
Expand Down