Give FormatsConf.FORMATS AST parsing a strategy layer#162
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCore Weblate format path resolution is extracted into a dedicated resolver module. ChangesFormatsConf resolution extraction
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/formats/test_weblate_formats_source.py (1)
51-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winEscape version string before using as regex pattern.
match=weblate_versiontreats the version string as a regex. If the installed Weblate version contains regex metacharacters (e.g.+in a dev/pre-release tag), the match could silently behave differently than intended. Usere.escape()for a precise, robust check.🔧 Proposed fix
+import re + weblate_version = importlib.metadata.version("Weblate") - with pytest.raises(RuntimeError, match=weblate_version) as exc_info: + with pytest.raises(RuntimeError, match=re.escape(weblate_version)) as exc_info: resolve_core_weblate_formats()🤖 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/formats/test_weblate_formats_source.py` around lines 51 - 53, The test in resolve_core_weblate_formats uses the Weblate version string directly as the pytest.raises match pattern, which can be interpreted as a regex. Update the assertion to escape the imported version from importlib.metadata.version("Weblate") before passing it to match, so the check is exact and robust even when the version contains regex metacharacters. Use the existing resolve_core_weblate_formats and weblate_version symbols to locate the change.
🤖 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 `@src/boost_weblate/formats/weblate_formats_source.py`:
- Line 121: The source selection in the formats resolver is treating any falsy
value as missing, so explicitly provided strategy objects that implement
__bool__ or __len__ can be incorrectly replaced. Update the source
initialization in the relevant resolver (where resolved is assigned from source
and AstFormatsConfSource) to default only when source is None, preserving any
caller-provided falsy strategy instance.
---
Nitpick comments:
In `@tests/formats/test_weblate_formats_source.py`:
- Around line 51-53: The test in resolve_core_weblate_formats uses the Weblate
version string directly as the pytest.raises match pattern, which can be
interpreted as a regex. Update the assertion to escape the imported version from
importlib.metadata.version("Weblate") before passing it to match, so the check
is exact and robust even when the version contains regex metacharacters. Use the
existing resolve_core_weblate_formats and weblate_version symbols to locate the
change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b68a8f05-f843-47c0-b3c7-8c5371a0ed46
📒 Files selected for processing (4)
src/boost_weblate/formats/weblate_formats_source.pysrc/boost_weblate/settings_override.pytests/formats/test_weblate_formats_source.pytests/test_weblate_internal_contract.py
henry0816191
reviewed
Jul 7, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/formats/test_weblate_formats_source.py`:
- Around line 43-56: Update the contract test in test_weblate_formats_source.py
so test_weblate_contract_formatsconf_ast handles the same parse failures as the
source helper, not just RuntimeError. Expand the except block around
_parse_formatsconf_formats_ast(_load_weblate_formats_models_source()) to also
catch SyntaxError and ValueError, then wrap them in the existing AssertionError
message using _CONTRACT_PREFIX_FORMATSCONF so any parsing failure is reported
consistently.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 923a4e67-4036-4b7c-a1d5-f2850caac34a
📒 Files selected for processing (6)
scripts/check-weblate-internal-contract.shsrc/boost_weblate/formats/formats_source.pysrc/boost_weblate/settings_override.pytests/formats/test_weblate_formats_source.pytests/test_settings_override.pytests/test_weblate_internal_contract.py
💤 Files with no reviewable changes (1)
- tests/test_weblate_internal_contract.py
✅ Files skipped from review due to trivial changes (1)
- tests/test_settings_override.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/boost_weblate/settings_override.py
wpak-ai
approved these changes
Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #158.
Summary by CodeRabbit
New Features
Bug Fixes
Tests