fix: derive __version__ from dist metadata instead of hardcoding it - #25
Open
robinjhuang wants to merge 2 commits into
Open
fix: derive __version__ from dist metadata instead of hardcoding it#25robinjhuang wants to merge 2 commits into
robinjhuang wants to merge 2 commits into
Conversation
publish.yml stamps the release tag into pyproject.toml at build time and nowhere else, so the hardcoded __version__ = "0.1.0" would have reported the placeholder on every published release -- while the wheel metadata and the User-Agent (which already reads dist metadata) said the real version. Verified by building with an injected 9.9.9: __version__ and the User-Agent now agree.
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Runtime version lookup and validation src/comfy_sdk/__init__.py, tests/test_version.py |
__version__ uses importlib.metadata.version("comfy-sdk"), falls back to "0+unknown" when metadata is unavailable, and tests cover both paths. |
Suggested reviewers: alexisrolland, wei-hai
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/version-single-source
✨ Simplify code
- Create PR with simplified code
- Commit simplified code in branch
fix/version-single-source
Comment @coderabbitai help to get the list of available commands.
![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
🤖 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/test_version.py`:
- Around line 12-18: Update test_version_matches_installed_distribution_metadata
to mock the importlib.metadata version lookup, return "9.9.9", and assert
comfy_sdk.__version__ resolves to that injected value. Remove the dependency on
the ambient installed "comfy-sdk" distribution while preserving the test’s focus
on version resolution.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 0464cc0e-85a3-4364-9575-c4f731c0d0f5
📒 Files selected for processing (2)
src/comfy_sdk/__init__.pytests/test_version.py
wei-hai
previously approved these changes
Jul 30, 2026
…tall The previous assertion compared __version__ against a live importlib.metadata lookup, so both sides resolved through the same call and it passed even when __version__ was re-hardcoded to a literal that happened to match the installed placeholder -- confirmed by mutation: re-hardcoding __version__ = "0.1.0" left the old assertion green. __version__ is bound once at import, so patching the lookup alone asserts against the value from the first import. The tests now patch and reload, which fails on that regression, and cover the PackageNotFoundError fallback that had no test at all. The patched import is undone and reloaded on the way out; verified no ordering pollution and that __version__ is intact afterwards.
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/test_version.py`:
- Around line 43-45: Update test_version_comes_from_the_distribution_metadata to
spy on the injected metadata lookup, assert the returned version remains
"9.9.9", and verify the lookup was called with the exact distribution name
"comfy-sdk".
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: d628cbdf-4faa-46c3-886d-ddf6246cb1e5
📒 Files selected for processing (1)
tests/test_version.py
wei-hai
approved these changes
Jul 30, 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.
publish.ymlstamps the release tag intopyproject.tomlat build time and nowhere else, so the hardcoded__version__ = "0.1.0"incomfy_sdk/__init__.pywould have reported the placeholder on every published release — while the wheel metadata and the User-Agent (which already reads dist metadata) reported the real version.Verified by building with an injected
9.9.9:__version__and the User-Agent now agree, where before__version__stayed0.1.0. Adds a regression test.