fix(sports): make the schedule-window settings actually take effect - #293
Merged
Conversation
Reported by a user: Manchester United never appeared even though their next fixture was 22 August, and with favourites turned off the board showed exactly one Premier League game, Arsenal v Coventry. _get_weeks_data() is the partial that serves the display until the background fetch lands. It looked ahead seven days; _fetch_soccer_api_data(), the fetch it substitutes for, looks ahead fourteen. So a fixture inside the real window was simply missing from the board. That gap is invisible in a league that plays daily and severe in one that plays weekly, where a whole matchweek can fall inside it. Reproduced against ESPN on 2026-08-14: -2w..+1w 20260731-20260821 -> 1 event (COV @ ARS, the 21st) -2w..+4w 20260731-20260911 -> 30 events (MAN @ HUL on the 22nd, ...) The Premier League's opening matchweek was 21-24 August, so a +7d horizon caught the Friday opener and hid the other nine fixtures -- exactly the single game the user described. Both horizons now come from one pair of module constants, so the partial cannot silently end up narrower than the fetch it stands in for again. The test reads the full fetch's span out of soccer_managers.py and requires the partial to be at least as wide, rather than hard-coding either number. Mutation-checked: reverting the horizon, re-hardcoding the window in _get_weeks_data, and narrowing the back-window are all caught. Harness clean; soccer's other tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
How far back and forward games are fetched was hard-coded at 14 days back and 7 forward, in all nine scoreboards. Days is the wrong unit for a league that plays weekly: a whole matchweek can fall just past a short horizon and never reach the board, which is how a user's favourite team went missing while other clubs in the same league appeared. Both sides are now advanced settings -- schedule_lookback_days and schedule_lookahead_days -- bounded 1 to 60, with a non-numeric or out-of-range value falling back to the default rather than raising. Defaults keep today's behaviour, so nobody's board changes on upgrade. The exception is soccer, whose lookahead moves 7 -> 14 to match the fetch its partial stands in for; that is the reported bug. This is an escape hatch, not the final answer. The right primitive is a game count rather than a day count -- "the next five fixtures" spans five days for a daily league and five weeks for a weekly one, so any fixed number of days is wrong for one of them. That change wants gating on whether favourite teams are configured, so a user browsing a whole league does not pull a season of fixtures to show five. Harness clean on a four-plugin spot check; the two failures across the nine plugins' test suites are the pre-existing missing-src ones also present on main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 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 | 46 |
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.
ChuckBuilds
force-pushed
the
fix/schedule-window-plumbing
branch
2 times, most recently
from
August 18, 2026 22:17
570fcaa to
2da6c51
Compare
Builds on #286, which added schedule_lookback_days / schedule_lookahead_days across nine scoreboard plugins. The settings never reached the code. Every plugin hands its managers a config built by _adapt_config_for_manager, and each of those adapters constructs its output key by key. Neither window key was named in any of them, so both were dropped on the way through. SportsCore then did config.get(), got None, and _clamp_window fell back -- every user pinned to the defaults, nothing logged. Verified by running all nine adapters for real: with both keys set in the plugin config, the adapted config contained neither, at the root or nested. They are now carried across, and the same probe shows both landing at the root for all nine. Three further reasons the feature did not work end to end: - The authoritative fetch ignored the setting. #286 parameterised _get_weeks_data(), which is the stand-in that serves the display while the background fetch is in flight. For soccer, afl and nrl the real fetch lives in *_managers.py and carried its own hard-coded fortnight, so a widened window showed briefly and then vanished when the real data replaced it. Those three now read the same configured values -- they already inherit SportsCore, so the attributes were there. - Four of the nine could not show the setting at all. afl, basketball, nrl and soccer declare a top-level x-propertyOrder, and the Jinja partial that serves the settings page iterates only that list, so a property missing from it never renders. Both keys added to all four. - schedule_lookback_days above 21 was inert. The Recent screen filtered on a fixed 21-day cutoff while the schema allows 60 and tells the user to "raise it if finished games disappear sooner than you want". The cutoff now follows the setting in all ten files that carried it. Note the fixed 21 was already unreachable in practice, since the fetch itself only went back a fortnight -- so this is not a behaviour change so much as the two numbers finally agreeing. The cutoff reads through getattr with the module default, because managers are also built without __init__ -- the plugin test suites do exactly that, and a bare attribute access raised into the surrounding except and silently skipped the spoiler filter. Two existing tests caught that, which is the only reason it is not in this commit as a regression. _clamp_window also catches OverflowError now: json parses bare Infinity by default and int(inf) raises, which the existing TypeError/ValueError guard did not cover. scripts/test_schedule_window_plumbing.py runs each of the nine adapters and asserts both keys survive. Grepping for the key names would not have caught this -- one plugin forwards its config with **self.config in a different method, which reads like a passthrough. Mutation-checked: removing the plumbing from the nine adapters fails all 18 checks. soccer's test_schedule_horizon.py derived the full fetch's horizon from timedelta literals to prove the partial was not narrower. Those literals are gone, so it now asserts the stronger property that both fetches read the same configured values and neither carries a hard-coded window. All 95 test files across the nine plugins pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
ChuckBuilds
force-pushed
the
fix/schedule-window-plumbing
branch
from
August 18, 2026 22:32
2da6c51 to
aa8bae6
Compare
ChuckBuilds
pushed a commit
that referenced
this pull request
Aug 19, 2026
This was referenced Aug 19, 2026
ChuckBuilds
added a commit
that referenced
this pull request
Aug 19, 2026
* feat(sports): back off the live poll when a league has nothing on Rebases #287 onto main and fixes the three things that stopped it working. #287 stacked on #286, whose content reached main through #293 in a fixed form, so only its own commit is carried here; the code applied cleanly and the only conflicts were manifest version numbers and the generated registry. The two settings could not reach the code. Each plugin's _adapt_config_for_manager builds its output key by key, so a key it does not name is dropped -- the same defect that made the schedule window inert, and #293 fixed that by naming those two keys in a loop. Rather than adding two more names to nine copies of a hard-coded tuple, the loop now walks a module-level _ROOT_CONFIG_KEYS listing every plugin-root setting SportsCore reads, so the next one only has to be declared once. Verified by running all nine adapters: with all four keys set, all four now arrive at the config root. The ceiling did not bound the base interval. _idle_live_interval ended with a bare `return base`, and the two settings are independent integers with no cross-validation, so base > ceiling is a reachable config -- base=3600 with the default 900 ceiling waited 3600s at streak 0 and 900s at streak 24. The interval shrank as the streak grew, which is the opposite of what a setting named "maximum" promises. Now min(base, ceiling), and checked across three base/ceiling combinations that the sequence never decreases. The test for that escalation could not fail: it compared long_wait to a second call of the same method with the same state. Compared against the short-streak value now. Mutation-checked -- making the long branch behave like the short one is caught, where before it was not. The plumbing test from #293 covers all four keys rather than two: removing the loop from the nine adapters fails 36 of 36 checks. All 104 test files across the nine plugins pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(sports): read the idle-poll settings from where they are written Review follow-up on the back-off change. Five findings, four real. The settings were read from self.mode_config -- the per-league {sport_key}_scoreboard block -- while the schema declares them, and the web UI writes them, at the config root. Nothing ever wrote them where the code looked, so a user who changed either value silently kept the default and the whole setting was inert. The plumbing added alongside it carried the keys to the root, and the test asserted they arrived there, but neither established that the root was where they were read from: the wrong invariant, verified thoroughly. Now read from the root with mode_config kept as a fallback, so a hand-placed per-league value still works. Also: - _clamp_seconds now catches OverflowError, the gap _clamp_window in the same file already covered. json accepts bare Infinity by default and int(inf) raises, so a hand-edited config could take down the update loop. - soccer's custom leagues go through a second whitelist adapter that was left out of the forwarding, so these settings applied to every built-in league but silently not to custom ones. Same defect class, second instance. - the back-off test built a stand-in object and set the intervals on it by hand, so it could not see the config lookup at all -- which is exactly how the defect above survived it. It now constructs a real SportsLive from a real config dict and asserts on what __init__ resolved. Only SportsCore.__init__ is stubbed (it pulls in logo downloading, fonts and an ESPN data source); the lookup lines themselves run for real, and the stub's two assumptions are asserted against the real SportsCore AST so they cannot drift unnoticed. That guard earned itself immediately: ufc-scoreboard keys its league block on the bare sport_key rather than {sport_key}_scoreboard, so a hardcoded key made the test assert fiction there. The key is now read out of the source instead. Verified by mutation, per plugin: reverting the read to mode_config fails 2 checks, letting a stale per-league value shadow the root fails 1, dropping OverflowError raises, removing the custom-league forwarding fails 4, and either stub assumption breaking fails its own check. The two pre-existing baseball failures (test_config_reload, test_odds_placement) are byte-identical on main and untouched here. --------- 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.
Builds on #286 (rebased onto
main, so this includes that work and supersedes it — #286 is currently conflicted anyway).The settings never reached the code
Every plugin hands its managers a config built by
_adapt_config_for_manager, and each of those adapters constructs its output key by key. Neither window key was named in any of the nine, so both were dropped in transit.SportsCorethen didconfig.get(...)→None→_clamp_windowfell back. Every user was pinned to the defaults, silently, with nothing logged.Verified by running all nine adapters for real, with both keys set:
Three more reasons it didn't work end to end
The authoritative fetch ignored the setting. #286 parameterised
_get_weeks_data(), which is the stand-in serving the display while the background fetch is in flight. For soccer, afl and nrl the real fetch lives in*_managers.pywith its own hard-coded fortnight — so a widened window appeared briefly and then vanished when the real data replaced it. Those three now read the same configured values (they already inheritSportsCore).Four plugins couldn't show the setting at all. afl, basketball, nrl and soccer declare a top-level
x-propertyOrder, and the Jinja partial that serves the settings page iterates only that list — a property missing from it never renders. Added to all four.schedule_lookback_daysabove 21 was inert. The Recent screen filtered on a fixed 21-day cutoff while the schema allows 60 and tells the user to "raise it if finished games disappear sooner than you want". Now follows the setting, in all ten files that carried it. Worth noting the fixed 21 was already unreachable in practice — the fetch only went back a fortnight — so this is less a behaviour change than the two numbers finally agreeing.A bug my own change introduced, and how it surfaced
The cutoff reads through
getattr(self, "schedule_lookback_days", _DEFAULT_LOOKBACK_DAYS)because managers are also constructed without__init__— the plugin test suites do exactly that. A bare attribute access raisedAttributeErrorinto the surroundingexceptand silently skipped the spoiler filter for excluded teams. Two existing tests (football and lacrossetest_favorite_live_boost.py) caught it, which is the only reason it isn't in this PR as a regression._clamp_windowalso catchesOverflowErrornow:jsonparses bareInfinityby default andint(inf)raises, which the existingTypeError/ValueErrorguard missed.Testing
scripts/test_schedule_window_plumbing.pyruns each of the nine adapters and asserts both keys survive. Grepping for the key names would not have caught this — one plugin forwards its config with**self.configin a different method, which reads like a passthrough.Mutation-checked: removing the plumbing from the nine adapters fails all 18 checks; restoring it passes all 18.
soccer/test_schedule_horizon.pyderived the full fetch's horizon fromtimedeltaliterals to prove the partial wasn't narrower. Those literals are gone, so it now asserts the stronger property: both fetches read the same configured values and neither carries a hard-coded window.All 95 test files across the nine plugins pass.
Nine manifests bumped,
plugins.jsonregenerated.Note on #287
#287 stacks on #286 and its two settings are dropped by the same adapters. The plumbing here is per-key; #287 will need its own two keys added to the same loop, or the loop generalised to forward a list of root-level keys.
🤖 Generated with Claude Code
https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW