test(sports): pin the four-case sunset matrix before B6 runs - #505
Merged
Conversation
B5 and B6 make different promises about an adopted scoreboard, and only the
second is obvious. The table:
| bundled copy present | bundled copy removed
pinned old core | loads (the fallback) | ERROR, naming the module
current core | loads, using core code | loads, using core code
The top-left cell is the one worth having. Nothing in the suite proved that an
adopted plugin still runs on a core predating src.common.sports_scroll, and
that claim is the entire basis for having shipped B5 ahead of B6's gate.
The bottom-left cell is asserted through PluginManager.load_plugin rather than
a bare import, deliberately. The manager catches the ModuleNotFoundError, so a
test written around pytest.raises would pass against a core where the module is
merely broken rather than absent, and would say nothing about what the user
meets: a plugin parked in ERROR and one log line. The assertion is the ERROR
state plus an error naming the module, which is what makes the log actionable.
Modelling notes, both of which were wrong first time and matter:
- The copy-removed shape is an UNGUARDED import, not the guarded one with the
legacy file deleted. Keeping the guard while removing its fallback only
mislabels the failure -- the plugin reports a missing scroll_display_legacy
and never mentions the core module that is actually absent.
- Only the leaf module is hidden. A pre-3.2.0 core still ships src/common/;
hiding the package would be a harsher core than any that shipped, and would
make the failure name the package instead of the module.
Ablation: disabling the old-core simulation fails three cells, and dropping the
error object from load_plugin's set_state fails the fourth, so none of it
passes vacuously. The install gate -- the other half of the guarantee -- stays
in test_plugin_compatibility_gate.py rather than being duplicated here.
Refs docs/SPORTS_UNIFICATION.md, "B6 -- why the sunset needs more than a
version floor".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
ChuckBuilds
added a commit
that referenced
this pull request
Aug 26, 2026
… done The remaining-work list had two entries that finished without the doc noticing, which is the failure mode this file exists to prevent. - The stale plugin-test tranche is gone. run_plugin_tests.py --all now reports 174 passed, 2 skipped, 0 failed across the whole fleet. Recorded how to re-check it too: these are standalone scripts, not a pytest suite, and one calls sys.exit(1) at import, so pointing pytest at a plugin directory collapses into an INTERNALERROR that looks nothing like the real state. - CLAUDE.md already says eight panel sizes. That leaves the hardware soaks as the only open item needing work rather than calendar time. B6's prerequisite is now built -- core test/test_sports_sunset_matrix.py (#505) -- so the phase table and the regression-test section say so, and the two modelling traps it had to work through are recorded for whoever touches it next: the copy-removed shape must be an unguarded import or the failure names scroll_display_legacy instead of the core module, and only the leaf module may be hidden because a pre-3.2.0 core still ships src/common/. The hold itself is re-checked and unchanged: v3.2.0 is still latest, __version__ is still 3.2.0, no 3.3.0, 23 days rather than the few months the gate asks for. Also worth stating plainly -- the core updates by git pull, not by downloading a release, so release-asset counts would not measure uptake even if we had them. Whatever unblocks this has to come from the store side. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughAdds a compatibility matrix test suite for sports plugins. The suite simulates core versions and bundled-copy states, loads isolated plugins through ChangesSports sunset compatibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds a localized regression test for the sports plugin sunset matrix without changing production behavior; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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
|
ChuckBuilds
added a commit
that referenced
this pull request
Sep 2, 2026
* docs(sports): record where B6 stands, and why it is waiting
The phase table had B4 as "next" and B5 as "after B4" while both had shipped,
and described B6 as blocked on B4's gate — which is now merged and released. A
plan that misreports which phase it is in is worse than no plan: the next
person reads it and repeats finished work.
Corrected, and three things that were only ever decided in conversation are now
written down:
* **B6 is deliberately held.** 3.2.0 published 2026-08-03; 3.1.0 ran nine
months before it. B6's premise is that cores without the module are gone,
and there is no release-asset count or install telemetry to show that.
Running it now strands users on their current plugin versions. The gate
that makes it safe is already built and tested — it is the calendar that is
missing, and no amount of further code changes that.
* **Stop adopting further shared modules** (data_sources, game_renderer,
base_odds_manager) until B6 closes. Each adoption adds a copy to keep in
step against a payoff contingent on B6.
* **A B5 retrospective**, because "the adoption went fine" is not what
happened: four of eight shipped with scroll mode broken on a 3.2.0 core.
The bundled fallback did not protect against it — the break was on the
modern path — which is an argument for the sunset, not against it. Records
the ledger too: net negative on disk until B6 runs.
Also replaces the "what's next" list, whose first five items were all done,
with what actually remains.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
* fix: apply CodeRabbit auto-fixes
Fixed 1 file(s) based on 2 unresolved review comments.
Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
* docs(sports): stop a wrapped PR reference reading as a heading
A line wrapped onto "#433), the newest manifest entry ...", which
markdownlint reads as a malformed ATX heading (MD018). Reflowed so the
line starts with "(#431, #433)" instead.
Not the suggested fix: adding a space after the hash would have turned
the PR reference into "# 433". The B5 safety claim raised alongside this
was already corrected in ac44b5a.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
* docs(sports): scope the B5 safety claim to the fallback
The heading read "B5 — adoption is safe by construction", which this same
document disproves two sections later: four of the eight adopted plugins
shipped with scroll mode broken on a 3.2.0 core and were repaired in
plugins #251.
The body was already careful -- it says fallback compatibility is what is
guaranteed, and that correctness on a core which *does* ship the module
needs object-level and scroll-mode validation. The heading was not, and a
heading is what a reader scanning the plan actually takes away.
Retitled to name both halves, with a sentence up front saying why the
unqualified claim is false and pointing at the retrospective that shows
it. The phase intro said "one of them is safe by construction and the
other is not"; that now says what it actually means -- one cannot break a
user on an old core, the other can.
The second review point, MD018 on the ATX heading at line 409, does not
reproduce: that line now begins "(#431, #433)" rather than "#433)", so
there is no bare-hash heading. `grep -cE '^#+[^ #]'` returns 0 for the
whole file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
* docs(sports): re-check the hold, and close two items that are already done
The remaining-work list had two entries that finished without the doc noticing,
which is the failure mode this file exists to prevent.
- The stale plugin-test tranche is gone. run_plugin_tests.py --all now reports
174 passed, 2 skipped, 0 failed across the whole fleet. Recorded how to
re-check it too: these are standalone scripts, not a pytest suite, and one
calls sys.exit(1) at import, so pointing pytest at a plugin directory
collapses into an INTERNALERROR that looks nothing like the real state.
- CLAUDE.md already says eight panel sizes.
That leaves the hardware soaks as the only open item needing work rather than
calendar time.
B6's prerequisite is now built -- core test/test_sports_sunset_matrix.py
(#505) -- so the phase table and the regression-test section say so, and the
two modelling traps it had to work through are recorded for whoever touches it
next: the copy-removed shape must be an unguarded import or the failure names
scroll_display_legacy instead of the core module, and only the leaf module may
be hidden because a pre-3.2.0 core still ships src/common/.
The hold itself is re-checked and unchanged: v3.2.0 is still latest,
__version__ is still 3.2.0, no 3.3.0, 23 days rather than the few months the
gate asks for. Also worth stating plainly -- the core updates by git pull, not
by downloading a release, so release-asset counts would not measure uptake even
if we had them. Whatever unblocks this has to come from the store side.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
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.
docs/SPORTS_UNIFICATION.mdnames this as B6's prerequisite — "then, when the evidence supports it, B6 — with the four-case compatibility regression test above in CI first." This is that test. B6 itself stays parked; its gate is 3.2.0 uptake and v3.2.0 is 23 days old.The table
PluginState.ERROR, naming the moduleThe top-left cell is the one worth having. Nothing in the suite proved an adopted plugin still runs on a core predating
src.common.sports_scroll, and that claim is the entire basis for having shipped B5 ahead of B6's gate.Why it goes through
load_plugin, notpytest.raisesThe doc is explicit about this and it's the easiest thing to get wrong.
PluginManager.load_plugincatches theModuleNotFoundError, so nothing propagates to a caller. A test built onpytest.raiseswould pass against a core where the module is merely broken rather than absent, and would say nothing about what the user actually meets — a plugin parked inERRORand one log line. So the assertion is the ERROR state plus an error naming the module, which is what makes that log line actionable.Two modelling details I got wrong first, both load-bearing
scroll_display_legacyand never mentions the core module that is actually absent. My first version asserted the wrong module name and passed.src/common/—scroll_helperand friends live there. Hiding the package is a harsher core than any that shipped, and makes the failure name the package rather than the module.A sixth test covers the guard's other accepted names (
src,src.common), branches that exist in all eight shipped plugins and that nothing else exercises.Verification
Ablated rather than assumed:
set_state(..., error=e)→set_state(...)inload_pluginFull core suite with the file: 1 failed, 3696 passed, 60 skipped — the one failure is
test_install_lowmem, pre-existing and environment-specific (it assumes a disk-backed/tmp; this box's is tmpfs). It fails identically on unmodifiedmain.Also verified while here: the whole plugin fleet is green —
run_plugin_tests.py --allgives 174 passed, 2 skipped, 0 failed, so the "5 stale failures across baseball, hockey and basketball" on the doc's remaining-work list are already fixed. Doc correction to follow on #435.No production code changed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
Summary by CodeRabbit