feat: dynamic PyPI version resolution + add clang-include-cleaner & clang-apply-replacements#242
Conversation
…lacements - Add version lists for clang-include-cleaner (22.1.7) and clang-apply-replacements (16.0.0, 17.0.6) in versions.py - Extend _versions_for_tool and _default_version_for_tool in util.py to handle the new tools via lookup maps - Add comprehensive tests for version resolution, default versions, install dispatch, and diagnostic messages
|
Warning Review limit reached
More reviews will be available in 39 minutes and 4 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughReplaces the static ChangesDynamic PyPI version resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #242 +/- ##
==========================================
+ Coverage 97.07% 97.51% +0.43%
==========================================
Files 4 3 -1
Lines 239 241 +2
==========================================
+ Hits 232 235 +3
+ Misses 7 6 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will degrade performance by 23.12%
|
Replace the hardcoded version lists (versions.py, update_versions.yml) with dynamic resolution via PyPI's JSON API at hook runtime. Why: - Eliminates the need for weekly cron jobs + manual PR reviews to keep version lists current - New PyPI releases are instantly available — no lag - Adding a new tool (e.g. clang-include-cleaner) requires zero changes to version infrastructure - The hardcoded approach was a shadow copy of PyPI's ground truth, creating inconsistency risks Changes: - util.py: Add _get_pypi_versions() (PyPI JSON + stable filtering + lru_cache) and _resolve_version_from_pypi() for dynamic prefix matching - Delete versions.py — no more hardcoded lists - Delete scripts/update_versions.py and update-versions.yml — obsolete - Update release-drafter.yml — versions are now always latest from PyPI - Update copilot-instructions.md to reflect new architecture - tests/test_util.py: 45 tests covering dynamic resolution, caching, network failure, prefix matching, and all four tools
shenxianpeng
force-pushed
the
feature/support-include-cleaner-apply-replacements
branch
from
a845e43 to
5027134
Compare
for more information, see https://pre-commit.ci
shenxianpeng
changed the title
github-actions
Bot
added
the
documentation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp_linter_hooks/util.py`:
- Around line 76-80: The prefix matching logic using startswith() can
incorrectly match unintended versions because it performs a simple string prefix
match rather than matching on dot-separated version segments (e.g., "21.1" would
match both "21.1.8" and "21.10.x"). Replace the startswith() check in the list
comprehension with logic that splits both the user_input and each version string
by dots, then verifies that the version segments match the user_input segments
at the beginning. This ensures proper dot-separated segment matching so that
"21.1" only matches versions starting with "21.1" and not "21.10" or higher.
In `@tests/test_util.py`:
- Line 1: Replace all EN DASH characters (`–`) with ASCII hyphens (`-`) in the
docstrings throughout the file. The Ruff RUF002 rule flags these characters as
linting violations. Search for docstrings containing the EN DASH character and
replace each occurrence with a standard ASCII hyphen to resolve the lint
failures.
- Around line 55-68: The cache verification calls are made outside the urlopen
patch context, which means if the cache is broken, the test will attempt a real
network call and become flaky. Move the second _get_pypi_versions call and its
subsequent assertions (latest2, versions2) inside the patch context manager to
ensure the cache test remains deterministic and does not require network access.
- Around line 101-119: The parametrized test cases do not cover all major
versions from 16 through 21 as required. Review the current test cases in the
parametrize decorator and identify which major versions are missing
(specifically check for version 18). Add additional parametrized test cases to
ensure every major version from 16 to 21 is represented, including both exact
version matches and prefix matches where applicable for the tools being tested
in this parametrize block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5cca0afa-ecef-4ec2-b782-9886d0d4d0aa
📒 Files selected for processing (7)
.github/copilot-instructions.md.github/workflows/release-drafter.yml.github/workflows/update-versions.ymlcpp_linter_hooks/util.pycpp_linter_hooks/versions.pyscripts/update_versions.pytests/test_util.py
💤 Files with no reviewable changes (3)
- scripts/update_versions.py
- cpp_linter_hooks/versions.py
- .github/workflows/update-versions.yml
| # Prefix match (e.g. "20" → "20.1.8"). Versions are newest-first, | ||
| # so the first matching entry is the latest for that prefix. | ||
| matched = [v for v in versions if v.startswith(user_input)] | ||
| if matched: | ||
| return matched[0], None |
There was a problem hiding this comment.
Prefix matching can resolve the wrong major/minor line.
Using startswith() for partial version matching can select unintended versions (e.g., 21.1 also matches 21.10.x). Match on dot-separated segments instead.
Suggested fix
- matched = [v for v in versions if v.startswith(user_input)]
+ requested_parts = user_input.split(".")
+ matched = [
+ v
+ for v in versions
+ if v.split(".")[: len(requested_parts)] == requested_parts
+ ]
if matched:
return matched[0], None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp_linter_hooks/util.py` around lines 76 - 80, The prefix matching logic
using startswith() can incorrectly match unintended versions because it performs
a simple string prefix match rather than matching on dot-separated version
segments (e.g., "21.1" would match both "21.1.8" and "21.10.x"). Replace the
startswith() check in the list comprehension with logic that splits both the
user_input and each version string by dots, then verifies that the version
segments match the user_input segments at the beginning. This ensures proper
dot-separated segment matching so that "21.1" only matches versions starting
with "21.1" and not "21.10" or higher.
| with patch("urllib.request.urlopen") as mock_urlopen: | ||
| mock_urlopen.return_value.__enter__.return_value.read.return_value = ( | ||
| __import__("json").dumps(mock_data).encode() | ||
| ) | ||
| latest, versions = _get_pypi_versions("clang-format") | ||
|
|
||
| assert latest == "22.1.5" | ||
| assert "22.1.0-rc1" not in versions | ||
| assert "22.1.0a3" not in versions | ||
| assert versions == ["22.1.5", "22.1.4", "20.1.8"] | ||
| # Verify cache works | ||
| latest2, versions2 = _get_pypi_versions("clang-format") | ||
| assert latest2 == latest | ||
| assert versions2 == versions |
There was a problem hiding this comment.
Keep the cache test deterministic and offline-safe.
The cache verification makes the second call after the urlopen patch context exits, so a cache regression would trigger a real network call and make this unit test flaky.
Suggested fix
with patch("urllib.request.urlopen") as mock_urlopen:
mock_urlopen.return_value.__enter__.return_value.read.return_value = (
__import__("json").dumps(mock_data).encode()
)
latest, versions = _get_pypi_versions("clang-format")
+ # Verify cache works without leaving the mocked network context
+ latest2, versions2 = _get_pypi_versions("clang-format")
assert latest == "22.1.5"
assert "22.1.0-rc1" not in versions
assert "22.1.0a3" not in versions
assert versions == ["22.1.5", "22.1.4", "20.1.8"]
-# Verify cache works
-latest2, versions2 = _get_pypi_versions("clang-format")
assert latest2 == latest
assert versions2 == versions
+assert mock_urlopen.call_count == 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_util.py` around lines 55 - 68, The cache verification calls are
made outside the urlopen patch context, which means if the cache is broken, the
test will attempt a real network call and become flaky. Move the second
_get_pypi_versions call and its subsequent assertions (latest2, versions2)
inside the patch context manager to ensure the cache test remains deterministic
and does not require network access.
Resolves Ruff RUF002 lint violation.
When no explicit --version is requested and PyPI is unreachable, fall back to whatever version of the tool is already installed locally instead of failing immediately. This preserves the pre-existing behavior where pre-installed tools worked offline. Only applies when version=None (auto-detect). When the user specifies a specific --version, PyPI must be reachable to verify it exists and resolve prefix matches. Also fix EN DASH characters in test docstrings (Ruff RUF002). New: - _detect_installed_version() helper with version regex extraction - 5 new tests: offline fallback (happy path, no tool, with version), detect installed version tests
for more information, see https://pre-commit.ci
|
shenxianpeng
deleted the
feature/support-include-cleaner-apply-replacements
branch
shenxianpeng
removed
the
documentation
2 participants
Summary
Replace the hardcoded version list approach with dynamic PyPI resolution and add wheel installation support for two new clang tools.
Changes
🔄 Dynamic version resolution (
util.py)_get_pypi_versions()– queries PyPI JSON API at runtime, filters pre-releases, returns(latest, [stable_versions_descending]). Results cached vialru_cache._resolve_version_from_pypi()– resolves user input (exact match → prefix match → friendly error) against live PyPI data.versions.py(130 lines of hardcoded versions) — no longer needed.scripts/update_versions.py+.github/workflows/update-versions.yml— the weekly cron job, auto-PR, and manual review process are all obsolete.🆕 New tools support
clang-include-cleanerandclang-apply-replacementsare now fully supported through the same dynamic resolution pipeline.resolve_install_with_diagnostics().🧹 Cleanup
release-drafter.yml: simplified — versions are always "latest from PyPI", no more extracting fromversions.py..github/copilot-instructions.md: updated to reflect the new architecture.✅ Tests
Why
versions.py)Net diff: -383 lines (690 deleted, 307 added).
Depends on: cpp-linter/clang-tools-pip#185
Summary by CodeRabbit
Release Notes
New Features
Chores