test: guard package version fallback#7
Conversation
Change-Id: I5dea4df3b40e1d18c286f8bc4c098e14989a0c08
Reviewer's GuideAdds a regression test ensuring the fallback version string embedded in src/agy_mcp/init.py stays in sync with the canonical project version in pyproject.toml, guarding against release-version drift in editable/source installs and skill bridge fallbacks. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The AST walk currently treats any string constant with exactly two dots as a candidate version, which is brittle; consider narrowing this to version-like constants in a specific assignment (e.g., the fallback version variable or
__version__) to avoid false positives as the module grows. - You may want to assert that the parsed
pyproject.tomlactually has the expectedprojectandversionkeys and raise a clear failure message if not, so missing or reorganized metadata produces an explicit, actionable test failure rather than a generic KeyError.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The AST walk currently treats any string constant with exactly two dots as a candidate version, which is brittle; consider narrowing this to version-like constants in a specific assignment (e.g., the fallback version variable or `__version__`) to avoid false positives as the module grows.
- You may want to assert that the parsed `pyproject.toml` actually has the expected `project` and `version` keys and raise a clear failure message if not, so missing or reorganized metadata produces an explicit, actionable test failure rather than a generic KeyError.
## Individual Comments
### Comment 1
<location path="tests/test_version.py" line_range="31" />
<code_context>
+ and node.value.count(".") == 2
+ ]
+
+ assert expected in fallback_versions
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the assertion by checking for a single, exact fallback version instead of membership in a list.
`expected in fallback_versions` will still pass if extra version-like literals are present. To make this a stricter regression guard, assert that there is exactly one fallback version and that it equals `expected`, e.g. `assert fallback_versions == [expected]` or `assert len(fallback_versions) == 1 and fallback_versions[0] == expected`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| and node.value.count(".") == 2 | ||
| ] | ||
|
|
||
| assert expected in fallback_versions |
There was a problem hiding this comment.
suggestion (testing): Strengthen the assertion by checking for a single, exact fallback version instead of membership in a list.
expected in fallback_versions will still pass if extra version-like literals are present. To make this a stricter regression guard, assert that there is exactly one fallback version and that it equals expected, e.g. assert fallback_versions == [expected] or assert len(fallback_versions) == 1 and fallback_versions[0] == expected.
|
Warning Review limit reached
More reviews will be available in 41 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 527d824728
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| and node.value.count(".") == 2 | ||
| ] | ||
|
|
||
| assert expected in fallback_versions |
There was a problem hiding this comment.
If src/agy_mcp/__init__.py later contains any other string constant matching the package version, this test will still pass even when the actual __version__ fallback has drifted, because it accepts every string with exactly two dots rather than the value assigned in the PackageNotFoundError handler. That leaves the release-drift regression this test is meant to catch unguarded in that scenario; parse the __version__ assignment in the fallback block instead of scanning all constants.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a regression test to ensure the hard-coded __version__ fallback used for editable/source checkouts stays in sync with pyproject.toml, preventing release-version drift.
Changes:
- Introduce
tests/test_version.pyto comparepyproject.toml’sproject.versionwith the fallback version embedded insrc/agy_mcp/__init__.py.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fallback_versions = [ | ||
| node.value | ||
| for node in ast.walk(tree) | ||
| if isinstance(node, ast.Constant) | ||
| and isinstance(node.value, str) | ||
| and node.value.count(".") == 2 | ||
| ] | ||
|
|
||
| assert expected in fallback_versions |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Change-Id: I09b3766819ba598859329728a25742400bff18bb
Summary
Test plan
Reference: packaging/release comparison against gemini-plugin-cc and codex-plugin-cc highlighted release-drift checks as a small useful hardening item.
Summary by Sourcery
Tests:
Summary by cubic
Add a precise regression test to ensure the version fallback in
src/agy_mcp/__init__.pyexactly matches thepyproject.tomlversion. The test parses the file’s AST and asserts a single fallback literal, preventing release-version drift for editable/source installs.Written for commit e297728. Summary will update on new commits. Review in cubic