chore(sports): drop the dead second copy of the fallback scroll classes - #252
Merged
Conversation
hockey, basketball and lacrosse shipped as the pre-adoption file and the
adopted file CONCATENATED rather than one replacing the other. Each carried a
full copy of the bundled classes at module level that nothing referenced --
the fallback branch imports the real ones from `scroll_display_legacy`. The
concatenation also duplicated the GameRenderer import and the logger
assignment.
hockey 991 -> 334 (-657)
basketball 1058 -> 363 (-695)
lacrosse 959 -> 321 (-638)
-1990
The dead copy was stale as well: it still annotated `Dict[str, ScrollDisplay]`
where the live file had been corrected to `Dict[str, 'LegacyScrollDisplay']`.
That one line is the only meaningful difference between them, which is what
makes removing the inline block safe.
Not cosmetic
------------
The separator-icon constants whose absence broke scroll mode on a 3.2.0 core
were sitting in this dead block. That is why the file read as correct -- to a
reviewer, and to an AST checker that only asked whether the names were defined
somewhere in the module. One implementation per file is what makes the next
such miss visible rather than camouflaged.
`scripts/check_scroll_adoption.py` now fails any plugin whose
`scroll_display.py` defines a module-level `Legacy*` class, and runs in CI
beside the collision check. It fails on the three files as they were and passes
on all ten now.
Verification
------------
* 8/8 fallback suites pass; the 5 remaining failures are the known
pre-existing stale-test tranche, unchanged.
* 168/168 harness renders byte-identical to before the strip.
* Scroll strips rendered from identical games are pixel-identical between the
core and fallback paths in all 8 plugins, with the same SHAs as before.
* Both structural guards pass across all 42 plugins.
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: 39 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (7)
📝 WalkthroughWalkthroughThree scoreboard plugins now use shared sports scroll classes with guarded legacy fallbacks. A new AST checker detects duplicate legacy classes. CI runs the checker independently from module-collision checks. Plugin versions and release histories are updated. ChangesSports scroll adoption
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 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 | -246 |
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: 4
🤖 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 @.github/workflows/module-collisions.yml:
- Around line 21-24: Add .github/workflows/module-collisions.yml to the
pull_request paths list in the workflow trigger so pull requests modifying the
workflow itself also run validation.
In `@plugins.json`:
- Line 104: Regenerate all three registry entries with update_registry.py from
their manifests: plugins.json lines 104-104 for basketball-scoreboard, 338-338
for hockey-scoreboard, and 362-362 for lacrosse-scoreboard. Update each entry’s
derived metadata, including last_updated, to reflect the manifest releases dated
2026-08-05 and versions 1.10.2, 1.7.2, and 1.7.2 respectively.
In `@scripts/check_scroll_adoption.py`:
- Around line 41-45: Update the SyntaxError handling in the parser used by
main() so a failed ast.parse is recorded as a validation problem or propagated,
causing validation to exit with code 1. Do not return an empty result that
allows malformed files to be reported as clean.
- Around line 46-47: Update the Legacy class discovery logic in the module AST
scan to recursively traverse bodies of module-scope control-flow statements,
including if, try, with, loops, and match blocks, while detecting Legacy*-named
ClassDef nodes. Do not descend into function or class bodies, and preserve the
existing list-of-class-names result.
🪄 Autofix
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: 4c936537-0b30-4038-afc2-5a6d9b9f31df
📒 Files selected for processing (9)
.github/workflows/module-collisions.ymlplugins.jsonplugins/basketball-scoreboard/manifest.jsonplugins/basketball-scoreboard/scroll_display.pyplugins/hockey-scoreboard/manifest.jsonplugins/hockey-scoreboard/scroll_display.pyplugins/lacrosse-scoreboard/manifest.jsonplugins/lacrosse-scoreboard/scroll_display.pyscripts/check_scroll_adoption.py
💤 Files with no reviewable changes (3)
- plugins/lacrosse-scoreboard/scroll_display.py
- plugins/basketball-scoreboard/scroll_display.py
- plugins/hockey-scoreboard/scroll_display.py
`module-collisions.yml` did not list itself in `pull_request.paths`, so a PR that only edited the workflow matched no path and the workflow never ran against its own change. `check_scroll_adoption.py` turned a `SyntaxError` into an empty result, so `main()` reported the plugin clean and exited 0 — a malformed `scroll_display.py` could skip the check entirely. Parse and read failures now propagate and fail the run with their own error line. The check also only inspected direct members of `tree.body`. A `Legacy*` class inside a module-level `if`/`try`/`with`/loop/`match` still binds a module global, and the guarded import in these very files is an `if/else` — so the likeliest hiding place was the one place not being looked at. It now walks every statement that executes in module scope, without descending into function or class bodies, which stay legitimate. Adds the gate's own regression suite, following `test_check_manifest_version_fields.py`. This gate reports by absence — "no legacy classes found" and "could not look" are otherwise the same answer — so a gate that quietly stopped detecting would still exit 0. Twelve cases cover each module-scope block type, the allowed nestings, the fallback branch's legitimate import, and the malformed file. The three bumped manifests kept `last_updated: 2026-07-31` while their new `versions[0]` entries are released `2026-08-05`, and `plugins.json` mirrored the stale date rather than causing it. Fixed at the source and regenerated the registry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
ChuckBuilds
pushed a commit
that referenced
this pull request
Aug 5, 2026
Around 11:00 EDT on 2026-08-04 ESPN's site.api began rejecting the agents
these plugins send, and every ESPN-backed scoreboard started returning
`403 Client Error: Forbidden`. A device that had been running fine logged
287 ESPN errors in a day.
The filter is not the familiar one. Probing site.api across agents, using
requests as the plugins do:
bare 'LEDMatrix/1.0' 403 (with or without Accept)
browser string 403 (no header rescues it)
'LEDMatrix/1.0 (+https://github.com/...)' 200
requests / urllib / curl defaults 200
ESPN rejects browser-style strings outright and bare custom tokens, and
accepts honest client tokens or an agent that identifies the client and
links to it. The instinct to "just send a browser User-Agent" is now
exactly backwards — that is the one thing guaranteed to stay blocked.
Sweeping every User-Agent literal in the repo and probing each turned up
ten blocked strings across 24 files in 14 plugins: the `LEDMatrix/1.0`
family in the data_sources/dynamic_team_resolver files, per-plugin tokens
like `LEDMatrix-F1/1.0` and `LEDMatrix Masters Plugin/2.1`, and browser
strings in nfl-draft and masters-tournament. All now send one agent
carrying the project URL — one string, so the next ESPN change is one
grep rather than ten.
Callers that reach other services are deliberately untouched:
ledmatrix-flights, ledmatrix-stocks and stock-news send a browser agent
to hosts ESPN's change never involved.
Also fixes the probe this branch added, which reported "ESPN is not the
problem" throughout the outage. It only tested for the old shape —
browser works, ours does not — so an inverted filter read as healthy, and
the fix it printed (send a browser agent) was the change that would have
kept everything broken. The verdict is direction-agnostic now, names
which agents were accepted and refused rather than assuming, and
separates what the plugins ship from controls kept to characterise the
filter.
basketball, hockey and lacrosse skip a patch number so this cannot
collide with the versions PR #252 already claims.
Verified on a live device: ESPN errors went from a steady stream to zero
across a restart, with live MLB games fetching again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
ChuckBuilds
pushed a commit
that referenced
this pull request
Aug 5, 2026
Around 11:00 EDT on 2026-08-04 ESPN's site.api began rejecting the agents
these plugins send, and every ESPN-backed scoreboard started returning
`403 Client Error: Forbidden`. A device that had been running fine logged
287 ESPN errors in a day.
The filter is not the familiar one. Probing site.api across agents, using
requests as the plugins do:
bare 'LEDMatrix/1.0' 403 (with or without Accept)
browser string 403 (no header rescues it)
'LEDMatrix/1.0 (+https://github.com/...)' 200
requests / urllib / curl defaults 200
ESPN rejects browser-style strings outright and bare custom tokens, and
accepts honest client tokens or an agent that identifies the client and
links to it. The instinct to "just send a browser User-Agent" is now
exactly backwards — that is the one thing guaranteed to stay blocked.
Sweeping every User-Agent literal in the repo and probing each turned up
ten blocked strings across 24 files in 14 plugins: the `LEDMatrix/1.0`
family in the data_sources/dynamic_team_resolver files, per-plugin tokens
like `LEDMatrix-F1/1.0` and `LEDMatrix Masters Plugin/2.1`, and browser
strings in nfl-draft and masters-tournament. All now send one agent
carrying the project URL — one string, so the next ESPN change is one
grep rather than ten.
Callers that reach other services are deliberately untouched:
ledmatrix-flights, ledmatrix-stocks and stock-news send a browser agent
to hosts ESPN's change never involved.
Also fixes the probe this branch added, which reported "ESPN is not the
problem" throughout the outage. It only tested for the old shape —
browser works, ours does not — so an inverted filter read as healthy, and
the fix it printed (send a browser agent) was the change that would have
kept everything broken. The verdict is direction-agnostic now, names
which agents were accepted and refused rather than assuming, and
separates what the plugins ship from controls kept to characterise the
filter.
basketball, hockey and lacrosse skip a patch number so this cannot
collide with the versions PR #252 already claims.
Verified on a live device: ESPN errors went from a steady stream to zero
across a restart, with live MLB games fetching again.
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 5, 2026
…#254) * feat(scripts): add an ESPN endpoint probe that names the failure "The ESPN plugins went blank" arrives as a symptom and rarely as a diagnosis. The plugins call ESPN's undocumented API from ~15 places and, critically, do not speak with one voice: some callers send LEDMatrix/1.0, many send nothing at all and get the stdlib/requests default, and a few already send a browser string. Anti-bot filtering discriminates on exactly that header, so the same outage can hit some plugins and spare others, which makes the reports hard to read. check_espn_api.py probes each endpoint family under all three User-Agent profiles and turns the result into a diagnosis: a browser-works/ours-fail split means header filtering and a header fix; failure under every agent means the URL moved or the response shape changed. It also separates "never got an HTTP response" from "ESPN returned an error". A denied proxy or a DNS failure otherwise looks identical to a withdrawn endpoint, and sends whoever runs it off rewriting URLs that were never broken. That case now exits 2 and says so. Endpoint checks assert an expected key rather than a bare 200, since ESPN can serve a cheerful 200 whose body no longer holds the field the renderer reads. An empty events list stays a note, not a failure, so it does not cry wolf on an off-day. No plugin code or versions touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017mSAZ7o55QKTP8KQm3cv5c * fix(scripts): pin the probe's URL scheme to https Codacy flagged the urlopen call (bandit B310, medium): urlopen honours file:// and custom schemes, so a URL reaching it unchecked is a path traversal waiting for a careless edit. The endpoints here are module constants today, but probe() is generic and the next person to add a row gets the guard for free. Follows the convention already set in check_team_pickers.py, with one difference: probe() documents that it never raises, so a bad scheme comes back as an ordinary failure result rather than an exception that would abort the remaining probes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017mSAZ7o55QKTP8KQm3cv5c * fix(espn): send an identifying User-Agent so ESPN stops returning 403 Around 11:00 EDT on 2026-08-04 ESPN's site.api began rejecting the agents these plugins send, and every ESPN-backed scoreboard started returning `403 Client Error: Forbidden`. A device that had been running fine logged 287 ESPN errors in a day. The filter is not the familiar one. Probing site.api across agents, using requests as the plugins do: bare 'LEDMatrix/1.0' 403 (with or without Accept) browser string 403 (no header rescues it) 'LEDMatrix/1.0 (+https://github.com/...)' 200 requests / urllib / curl defaults 200 ESPN rejects browser-style strings outright and bare custom tokens, and accepts honest client tokens or an agent that identifies the client and links to it. The instinct to "just send a browser User-Agent" is now exactly backwards — that is the one thing guaranteed to stay blocked. Sweeping every User-Agent literal in the repo and probing each turned up ten blocked strings across 24 files in 14 plugins: the `LEDMatrix/1.0` family in the data_sources/dynamic_team_resolver files, per-plugin tokens like `LEDMatrix-F1/1.0` and `LEDMatrix Masters Plugin/2.1`, and browser strings in nfl-draft and masters-tournament. All now send one agent carrying the project URL — one string, so the next ESPN change is one grep rather than ten. Callers that reach other services are deliberately untouched: ledmatrix-flights, ledmatrix-stocks and stock-news send a browser agent to hosts ESPN's change never involved. Also fixes the probe this branch added, which reported "ESPN is not the problem" throughout the outage. It only tested for the old shape — browser works, ours does not — so an inverted filter read as healthy, and the fix it printed (send a browser agent) was the change that would have kept everything broken. The verdict is direction-agnostic now, names which agents were accepted and refused rather than assuming, and separates what the plugins ship from controls kept to characterise the filter. basketball, hockey and lacrosse skip a patch number so this cannot collide with the versions PR #252 already claims. Verified on a live device: ESPN errors went from a steady stream to zero across a restart, with live MLB games fetching again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --------- Co-authored-by: Claude <noreply@anthropic.com>
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
2 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.
Follow-up to #251. In one sentence: three files contain two copies of the same code, one copy is never used by anything, and this deletes the unused copy.
What was there
hockey,basketballandlacrosseshipped as the pre-adoption file and the adopted file concatenated, rather than one replacing the other. Each carried a full copy of the bundled classes at module level that nothing referenced — the fallback branch imports the real ones fromscroll_display_legacy.py. The same concatenation duplicated theGameRendererimport and theloggerassignment.The dead copy was also stale — it still annotated
Dict[str, ScrollDisplay]where the live file had been corrected toDict[str, 'LegacyScrollDisplay']. That single line is the only meaningful difference between the two, which is what makes removing the inline block safe.Why this isn't cosmetic
The separator-icon constants whose absence broke scroll mode in #251 were sitting in this dead block. That is why the file read as correct — to a reviewer, and to an AST checker that only asked whether the names were defined somewhere in the module. One implementation per file is what makes the next such miss visible instead of camouflaged.
scripts/check_scroll_adoption.pynow fails any plugin whosescroll_display.pydefines a module-levelLegacy*class, and runs in CI beside the collision check. It fails on the three files as they were, and passes on all ten now.Verification
Patch bumps only, no behaviour change: hockey 1.7.2, basketball 1.10.2, lacrosse 1.7.2.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Summary by CodeRabbit
New Features
Bug Fixes
Documentation