fix(version): make the core version have exactly one answer - #428
Merged
Conversation
Plugin compatibility floors compare against src.__version__, so that string
has to be trustworthy. It has not been. v3.1.0 was tagged 2026-05-31 while
src/__init__.py still said "1.0.0"; the bump did not land until 2026-07-12.
Every device installed from that release reports 1.0.0, which is below the
(2, 0, 0) floor in PluginLoader._warn_if_incompatible -- so those users are
silently exempt from every plugin compatibility warning.
web_interface carried a third answer, a hardcoded "3.0.0" that nothing read
and that had drifted two majors from the core. It now re-exports the
canonical value, so it cannot disagree again.
Adds:
- test/test_version_consistency.py (enrolled in the core unit CI job):
src.__version__ is parseable semver, matches the newest CHANGELOG heading,
the CHANGELOG's headings are unique and descending, and web_interface
tracks the core. src.plugin_system.__version__ is deliberately excluded --
it versions the plugin API and moves independently.
- scripts/check_release_version.py + a release-version-check workflow that
asserts the tag, the CHANGELOG and src.__version__ agree. Runs on pushed
v* tags and published releases, and via workflow_dispatch so a tag can be
checked *before* it is created:
python scripts/check_release_version.py v3.2.0
Verified: 757 core unit tests pass including the four new ones; the script
exits 0 for v3.2.0 and non-zero for both a mismatched tag (v3.1.0) and a
non-semver one (v2.5); web_interface and web_interface.app still import.
Prerequisite for cutting v3.2.0 -- phase B4 in docs/SPORTS_UNIFICATION.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Contributor
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR centralizes the web-interface version, adds version-consistency tests, and introduces a CLI that validates release tags against the core version and changelog. ChangesVersion consistency and release validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseVersionScript as scripts/check_release_version.py
participant SourceVersion as src.__version__
participant Changelog as CHANGELOG.md
participant ErrorOutput as stderr
ReleaseVersionScript->>SourceVersion: read core version
ReleaseVersionScript->>Changelog: read newest semantic-version heading
ReleaseVersionScript->>ErrorOutput: report mismatches
ReleaseVersionScript-->>ReleaseVersionScript: return exit code
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 13 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
test/test_version_consistency.py (2)
40-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd return type annotations to the test functions.
Add
-> Noneto each test function. This follows the required type-hint convention.As per coding guidelines, “Use type hints for function parameters and return values.”
🤖 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 `@test/test_version_consistency.py` around lines 40 - 91, Add the return type annotation -> None to each test function in test_version_consistency.py: test_core_version_is_semver, test_changelog_documents_the_current_version, test_changelog_versions_are_ordered_and_unique, and test_web_interface_version_tracks_the_core. Do not alter their assertions or behavior.Source: Coding guidelines
67-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the uniqueness and ordering checks.
test_changelog_versions_are_ordered_and_uniqueverifies two independent responsibilities. Create one test for duplicates and one test for descending order. This gives each failure a precise test name.As per coding guidelines, “Each test should verify a single responsibility.”
🤖 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 `@test/test_version_consistency.py` around lines 67 - 79, Split test_changelog_versions_are_ordered_and_unique into separate tests: one that checks duplicate version headings and one that checks descending version order. Keep the existing CHANGELOG parsing and assertions, assigning each assertion to the appropriately named single-responsibility test.Source: Coding guidelines
🤖 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 `@scripts/check_release_version.py`:
- Around line 54-55: Wrap the CHANGELOG read performed by
newest_changelog_version in the release-check flow with OSError handling. On
failure, write a clear message including the affected CHANGELOG path and a
recovery action to stderr, then return a nonzero status from the surrounding
command entry point instead of emitting a traceback.
In `@test/test_version_consistency.py`:
- Around line 16-17: Remove unsupported workflow-execution claims: in
test/test_version_consistency.py lines 16-17, revise or remove the module
documentation describing the deferred workflow; in
scripts/check_release_version.py lines 8-10, remove the claim that CI runs the
script on pushed tags and published releases.
- Around line 34-37: Restrict version and heading regexes to ASCII digits and
horizontal whitespace: update SEMVER and HEADING in
test/test_version_consistency.py, and the corresponding patterns in
scripts/check_release_version.py, replacing \d with [0-9] and heading \s usage
with [ \t].
---
Nitpick comments:
In `@test/test_version_consistency.py`:
- Around line 40-91: Add the return type annotation -> None to each test
function in test_version_consistency.py: test_core_version_is_semver,
test_changelog_documents_the_current_version,
test_changelog_versions_are_ordered_and_unique, and
test_web_interface_version_tracks_the_core. Do not alter their assertions or
behavior.
- Around line 67-79: Split test_changelog_versions_are_ordered_and_unique into
separate tests: one that checks duplicate version headings and one that checks
descending version order. Keep the existing CHANGELOG parsing and assertions,
assigning each assertion to the appropriately named single-responsibility test.
🪄 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 Plus
Run ID: 296dd7ed-5c66-4180-aa0b-b7ced2cab79b
📒 Files selected for processing (3)
scripts/check_release_version.pytest/test_version_consistency.pyweb_interface/__init__.py
…laim From CodeRabbit on #428, all three valid: - The module docstring claimed the tag check "runs at release time in .github/workflows/release-version-check.yml". That workflow is held back to a follow-up PR (the pushing token lacks the `workflow` scope), so the claim was false as written. Both files now describe the script as a manual pre-flight and say the CI wiring is still to come. - `\d` also matches non-ASCII decimal digits, which int() happily parses, and `\s` matches newlines -- so "##\n3.2.0" read as a version heading. Patterns now use [0-9] and [ \t], kept in step across the test and the script, with a regression test pinning both behaviours. - A missing or unreadable CHANGELOG.md raised OSError out of read_text() and printed a traceback. In a release gate that reads as "the tooling is broken"; it now reports the path and a recovery action and exits 1. Verified: v3.2.0 passes, a mismatched tag exits 1, and a missing CHANGELOG exits 1 with the new message instead of a traceback. 5 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
ChuckBuilds
added a commit
that referenced
this pull request
Aug 3, 2026
These were split out of #428/#429 because the token pushing them lacked the `workflow` scope. Folding them in here rather than opening a stacked PR -- #429 was merged into its stacked base after that base had already been squash-merged, so its content never reached main, and one such near-miss is enough. All three enrolled suites exist on this branch: test_version_consistency.py came with #428 and is on main; the other two arrive with the commits above. Enrolling them in a separate PR would have either raced with this one on test.yml or briefly pointed CI at files main did not have. - test.yml: enroll test_version_consistency, test_plugin_compatibility_gate and test_install_preserves_existing in the core unit job. Until now these 32 tests existed but nothing ran them automatically. - release-version-check.yml: run scripts/check_release_version.py on pushed v* tags and published releases, plus workflow_dispatch so a tag can be checked *before* it is created. No dependencies -- it reads src/__init__.py and CHANGELOG.md only. Verified: both workflow files parse, and the release check still passes for v3.2.0 against this tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
ChuckBuilds
added a commit
that referenced
this pull request
Aug 3, 2026
…arget of #429) (#431) * feat(store): refuse to install a plugin that needs a newer core `ledmatrix_min_version` was decoration. The loader logged an advisory warning and continued; the store never compared the core version at all, so a routine "update" delivered a plugin that could not run. That is the gap phase B6 (the sports-unification sunset) cannot be done over: deleting a plugin's bundled fallback while nothing enforces the floor hands un-updated users a scoreboard that raises ModuleNotFoundError at load and is reported only as one line in the journal. The gate lives in install_plugin, after the manifest is on disk and before dependencies are installed. That is the earliest knowable point -- the registry carries no compatibility field, so the floor is not visible until the files are down -- and it is also the chokepoint: _reinstall_with_rollback calls install_plugin, so a refused *update* restores the version the user already had, for free. Floor resolution and the comparison move to src/plugin_system/compatibility.py, shared with the loader so the two cannot drift. Both read all four spellings published manifests use, including the deprecated `ledmatrix_min`. Refusal requires evidence. An undeclared floor, an unparseable version on either side, or a core below TRUSTWORTHY_FLOOR (2.0.0) all allow the install. That last one is deliberate and load-bearing: the v3.1.0 release reports __version__ = "1.0.0" while nearly every published manifest floors at 2.0.0, so a strict gate would lock those users out of the plugin store entirely -- much worse than the problem being solved. They stay unprotected until they update the core, which is also what fixes their version string. Verified: 782 core unit tests pass, including 25 new ones and the existing loader-warning suite unchanged (the refactor is behavior-preserving). The install tests drive the real install_plugin path with the download stubbed -- the allow and refuse cases differ only in the declared floor, so the refusal is demonstrably the gate and not an earlier bail-out. Follow-ups, deliberately not in this PR: surfacing the reason in the store UI rather than only the log, and publishing the floor in plugins.json so the store can refuse before downloading. Phase B4 in docs/SPORTS_UNIFICATION.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(store): a failed install must not destroy the plugin it replaced Found while validating the compatibility gate. `_install_plugin_impl` deletes the existing plugin directory *before* downloading, so any failure after that point leaves the user with nothing. `_reinstall_with_rollback` protects the update path exactly this way; a direct `install_plugin` had no equivalent. The gate made this reachable in a new way: a plugin whose declared floor exceeds the running core is now refused *after* the old copy is already gone. Floors are hand-written and can be over-declared, so the refusal could remove a plugin that had been working fine on that core. install_plugin is now a thin wrapper that renames any existing install aside, delegates to _install_plugin_impl, and restores it on failure -- including when the implementation raises, which is re-raised after the restore. It is a pass-through when nothing is installed and when called from _reinstall_with_rollback, which has already moved the old copy aside; a test pins that so the two mechanisms cannot start nesting. The aside name embeds '.standalone-backup-' because plugin_manager._scan_directory_for_plugins keys on exactly that substring to skip backups. A different name would have made the backup discoverable as a duplicate plugin; a test pins that too. 789 core unit tests pass, including 7 new ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(store): serialize concurrent installs, and make the lock reentrant Second bug found while validating the previous commit on hardware. install_plugin's new set-aside/restore had no lock. The web UI runs Flask threaded, so a double-clicked Install button gives two threads the same plugin_id; interleaved, one thread's restore deletes the other's freshly installed copy. _reinstall_with_rollback already guards exactly this with a per-plugin lock, and install_plugin needs the same one. Taking that lock naively deadlocks. _reinstall_with_rollback holds it across its call to install_plugin, and threading.Lock is not reentrant -- so the request thread hangs forever on the standard monorepo update path (update_plugin -> _reinstall_with_rollback -> install_plugin), which is to say on every plugin update. Verified by reverting to a plain Lock: the regression test times out after 10s instead of passing. The per-plugin locks are now RLocks, and install_plugin holds one for its whole set-aside/install/restore sequence. Verified on devpi (Pi, Python 3.13.5, real registry and network): - update_plugin on an up-to-date plugin: True in 5.4s - update_plugin forced through the full reinstall-with-rollback path: True in 13.1s, correct version restored, old copy replaced, no backup directories left behind - install -> reinstall-over-existing -> failed-reinstall-restores: all pass against real downloads - 22 plugins load, no tracebacks, web API and UI 200, steady-state journal 50 lines/min 791 core unit tests pass, including 2 new concurrency tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * ci: run the new suites, and check tag/version agreement at release time These were split out of #428/#429 because the token pushing them lacked the `workflow` scope. Folding them in here rather than opening a stacked PR -- #429 was merged into its stacked base after that base had already been squash-merged, so its content never reached main, and one such near-miss is enough. All three enrolled suites exist on this branch: test_version_consistency.py came with #428 and is on main; the other two arrive with the commits above. Enrolling them in a separate PR would have either raced with this one on test.yml or briefly pointed CI at files main did not have. - test.yml: enroll test_version_consistency, test_plugin_compatibility_gate and test_install_preserves_existing in the core unit job. Until now these 32 tests existed but nothing ran them automatically. - release-version-check.yml: run scripts/check_release_version.py on pushed v* tags and published releases, plus workflow_dispatch so a tag can be checked *before* it is created. No dependencies -- it reads src/__init__.py and CHANGELOG.md only. Verified: both workflow files parse, and the release check still passes for v3.2.0 against this tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
evansalter
pushed a commit
to evansalter/LEDMatrix
that referenced
this pull request
Aug 5, 2026
The 3.2.0 section described the unified sports library but none of the install-path work that landed in ChuckBuilds#428 and ChuckBuilds#431 -- which matters more than a normal changelog omission, because the sunset rule keys on this section to tell plugin authors what a given floor buys them. The headline addition: 3.2.0 is the first release that *enforces* ledmatrix_min_version. Before it the floor was advisory, so a plugin could declare one and still be delivered to a core that could not run it. That is the property B6 waits on, and it is now stated where a plugin author will look for it -- along with the caveat that a core reporting below 2.0.0 is treated as unknown rather than old and is never blocked. Also records compatibility.py (and that it does not yet read compatible_versions), check_release_version.py and its workflow, the install-preservation fix, the reentrant-lock deadlock fix, and the web_interface version re-export. No version bump: 3.2.0 is unreleased, so this describes the release being cut rather than a new one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
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
1 participant
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.
Prerequisite for cutting v3.2.0 — phase B4 in
docs/SPORTS_UNIFICATION.md(see #427).Why
Plugin compatibility floors compare against
src.__version__, so that string has to be trustworthy. It has not been:v3.1.0was tagged 2026-05-31 whilesrc/__init__.pystill said"1.0.0". The bump to"3.1.0"didn't land until 2026-07-12 (7f7f0d64), six weeks later. Every device installed from that release reports1.0.0— below the(2, 0, 0)floor inPluginLoader._warn_if_incompatible, so those users are silently exempt from every plugin compatibility warning.web_interfacecarried a third answer: a hardcoded"3.0.0", two majors adrift, that nothing read.What changes
web_interface/__init__.pyre-exportssrc.__version__instead of hardcoding, so it cannot disagree again. (src.plugin_system.__version__is deliberately left alone — it versions the plugin API, sits beside__api_version__, and moves independently.)test/test_version_consistency.py—src.__version__is parseable semver, matches the newest CHANGELOG heading, CHANGELOG headings are unique and descending, andweb_interfacetracks the core.scripts/check_release_version.py— asserts a tag, the CHANGELOG, andsrc.__version__all agree. Usable as a pre-flight before creating a tag:Verified
v3.2.0, non-zero for a mismatched tag (v3.1.0) and a non-semver one (v2.5).web_interfaceandweb_interface.appstill import;src/__init__.pyis a docstring and a string, so the new import costs nothing.srcandweb_interfaceboth report3.2.0, and the release check passes on-device.Held back
A
release-version-checkworkflow (running the script on pushedv*tags, published releases, and viaworkflow_dispatch) and the CI enrollment of the new test are not in this PR — the token pushing it lacks theworkflowscope. They will follow in a small separate PR. Until then the new test is not executed by CI.🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Summary by CodeRabbit
New Features
Bug Fixes
Tests