-
-
Notifications
You must be signed in to change notification settings - Fork 29
Python 3.13 Compatibility VALIDATED (#272) #348
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.
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds Python 3.13 to CI workflows and project metadata/tooling, and introduces a new benchmark script Changes
Sequence Diagram(s)(omitted — changes are configuration/benchmarks without multi-component control-flow requiring visualization) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/automation.yml (1)
18-18: Fix spacing in YAML array for consistency.There's a missing space after the comma between
'3.12'and'3.13'. For consistency with the other elements, add a space.🔎 Proposed fix
- python-version: ['3.10', '3.11', '3.12','3.13'] + python-version: ['3.10', '3.11', '3.12', '3.13'].github/workflows/ci.yml (1)
48-48: Fix spacing in YAML array for consistency.There's a missing space after the comma between
"3.12"and"3.13". For consistency with the other elements, add a space.🔎 Proposed fix
- python-version: ["3.10", "3.11", "3.12","3.13"] + python-version: ["3.10", "3.11", "3.12", "3.13"]pyproject.toml (1)
97-97: Fix spacing in array for consistency.There's a missing space after the comma between
"py312"and"py313". For consistency with the other elements, add a space.🔎 Proposed fix
-target-version = ["py310", "py311", "py312","py313"] +target-version = ["py310", "py311", "py312", "py313"]tests/benchmark_313.py (1)
1-8: Fix typo in docstring.Line 2 has a typo: "TO run" should be "To run" (lowercase 'o').
🔎 Proposed fix
"""Cortex Python 3.13 Benchmark - Issue #272 -TO run benchmarks: +To run benchmarks: # Install PyYAML (only dep needed) pip install pyyaml # Test both versions
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/automation.yml.github/workflows/ci.ymlpyproject.tomlsetup.pytests/benchmark_313.py
🧰 Additional context used
📓 Path-based instructions (3)
{setup.py,setup.cfg,pyproject.toml,**/__init__.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use Python 3.10 or higher as the minimum supported version
Files:
pyproject.tomlsetup.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
tests/benchmark_313.pysetup.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/benchmark_313.py
🧠 Learnings (1)
📚 Learning: 2025-12-11T12:03:24.071Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T12:03:24.071Z
Learning: Applies to {setup.py,setup.cfg,pyproject.toml,**/__init__.py} : Use Python 3.10 or higher as the minimum supported version
Applied to files:
.github/workflows/ci.yml.github/workflows/automation.ymlpyproject.tomlsetup.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Package
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.10)
🔇 Additional comments (3)
setup.py (1)
44-44: LGTM!The Python 3.13 classifier addition is correct and consistent with the existing classifier pattern and the
python_requires=">=3.10"constraint on line 47.pyproject.toml (1)
40-40: LGTM!The Python 3.13 classifier addition is correct and consistent with the
requires-python = ">=3.10"constraint on line 44.tests/benchmark_313.py (1)
10-33: LGTM!The benchmark script correctly uses
timeitfor performance measurement and provides a simple, effective way to compare Python 3.11 vs 3.13 performance on relevant operations (YAML serialization and dict comprehensions). The approach is sound and aligns with the PR objectives to produce performance benchmarks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
48-48: Python 3.13 successfully added to test matrix.The addition of Python 3.13 to the CI test matrix aligns with the PR objectives and is well-validated by the 647 passing tests.
Minor formatting nitpick: missing space after
"3.12"before the comma for consistency with other entries.🔎 Proposed formatting fix
- python-version: ["3.10", "3.11", "3.12","3.13"] + python-version: ["3.10", "3.11", "3.12", "3.13"]
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/automation.yml.github/workflows/ci.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/automation.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T12:03:24.071Z
Learning: Applies to {setup.py,setup.cfg,pyproject.toml,**/__init__.py} : Use Python 3.10 or higher as the minimum supported version
📚 Learning: 2025-12-11T12:03:24.071Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T12:03:24.071Z
Learning: Applies to {setup.py,setup.cfg,pyproject.toml,**/__init__.py} : Use Python 3.10 or higher as the minimum supported version
Applied to files:
.github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Package
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.10)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/automation.yml.github/workflows/ci.ymlpyproject.tomltests/benchmark_313.py
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/ci.yml
- .github/workflows/automation.yml
- pyproject.toml
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
tests/benchmark_313.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/benchmark_313.py
🧬 Code graph analysis (1)
tests/benchmark_313.py (1)
cortex/semantic_cache.py (1)
total(30-32)
🔇 Additional comments (3)
tests/benchmark_313.py (3)
1-8: Clear documentation with usage instructions.The module docstring effectively explains the benchmark's purpose and provides actionable setup/run commands.
24-29: Dict operations logic is correct.The dict comprehension and filtering operations accurately test common patterns with proper logic.
31-33: Output logic is correct.The total calculation and result presentation are appropriate for the benchmark's purpose.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/benchmark_313.py (1)
15-21: Previous issue resolved; dict comprehension now correct.The dict comprehension on line 17 now correctly creates 100 unique key-value pairs using
{f'test_{i}': i for i in range(100)}, addressing the concern from the previous review.One optional enhancement: consider adding an early check for PyYAML availability to provide clearer error messages if the dependency is missing, though the docstring already documents this requirement.
🔎 Optional: Add early dependency check
import sys import timeit + +try: + import yaml +except ImportError: + print("Error: PyYAML is required. Install with: pip install pyyaml") + sys.exit(1) print(f"=== Cortex Python {sys.version_info.major}.{sys.version_info.minor} Benchmark ===\n")
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/benchmark_313.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
tests/benchmark_313.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/benchmark_313.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
🔇 Additional comments (4)
tests/benchmark_313.py (4)
1-8: Clear documentation for benchmark usage.The module docstring effectively documents the benchmark's purpose, dependency requirements, and usage instructions.
10-13: LGTM!Standard library imports are appropriate, and the benchmark header clearly identifies the Python version being tested.
23-29: LGTM!The dict operations benchmark correctly creates 100 key-value pairs and filters values greater than 50. The structure is consistent with the YAML benchmark section.
31-33: LGTM!The summary correctly totals the benchmark times and provides clear output confirming Python version compatibility.



Related Issue
Closes #272
Summary
Performance benchmarks 3.11 vs 3.13
AVERAGE: 3.11=0.1427s | 3.13=0.1156s → 3.13 = 19% FASTER!
Run :
python3.11 benchmark_313.pyandpython3.13 benchmark_313.pyRecordings
benchmark_313.py execution :
Screen.Recording.2025-12-23.170834.mp4
647 tests passing and benchmark test:
Screen.Recording.2025-12-22.220446.1.mp4
other tests :
Screen.Recording.2025-12-22.223159.mp4
Checklist
pytest tests/)[#XX] DescriptionSummary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.