fix(sports): align the odds row, and honour the offsets that control it - #268
Merged
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 52 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 (4)
📝 WalkthroughWalkthroughThe PR adds configurable betting-odds X/Y offsets to several scoreboard plugins. Baseball odds now avoid measured overlap with top-row text. Regression scripts, plugin manifests, and catalog versions are updated. ChangesBetting Odds Layout
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ScoreboardCard
participant GameRenderer
participant TextMeasurement
ScoreboardCard->>GameRenderer: Pass game data and rendered top-row text
GameRenderer->>TextMeasurement: Measure top-row and odds text
TextMeasurement-->>GameRenderer: Return overlap result
GameRenderer->>ScoreboardCard: Draw odds at the top or one row lower
Possibly related PRs
🚥 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
|
| Metric | Results |
|---|---|
| Complexity | 78 |
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.
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 `@plugins/afl-scoreboard/game_renderer.py`:
- Around line 914-915: Move the odds_x_offset and odds_y_offset assignments in
the odds-rendering method before the if favored_spread is not None branch. Keep
the existing offset values and ensure the over/under path can use them when no
negative spread is present, including the corresponding logic around the
alternate referenced lines.
In `@plugins/baseball-scoreboard/game_renderer.py`:
- Around line 997-1010: Update the collision-check flow using _inning_text so it
measures the actual top-row text for each card type: recent cards should use
“Final” and upcoming cards should use their rendered time or date, with the
matching font. Pass this rendered text and font into the collision check or
provide card-specific obstacle text, then add placement coverage for recent and
upcoming cards across the required panel sizes.
In `@plugins/baseball-scoreboard/test_odds_placement.py`:
- Around line 104-109: Update the odds_y test case for the narrow panel and long
“FINAL” label to pass is_final=True alongside inning_half="top". Extend odds_y
with an is_final argument if needed, ensuring this case measures the FINAL label
rather than the default ▲3 text.
In `@plugins/nrl-scoreboard/game_renderer.py`:
- Around line 886-897: Move the odds_x_offset and odds_y_offset initialization
using _layout_offset before the favored_spread conditional so both
favorite-spread and over/under rendering can use them safely. Apply this change
in plugins/nrl-scoreboard/game_renderer.py at lines 886-897 and
plugins/soccer-scoreboard/game_renderer.py at lines 886-897; no other behavior
needs changing.
🪄 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: b06f1333-c61f-43b9-bec4-2cda360bf61e
📒 Files selected for processing (17)
plugins.jsonplugins/afl-scoreboard/config_schema.jsonplugins/afl-scoreboard/game_renderer.pyplugins/afl-scoreboard/manifest.jsonplugins/baseball-scoreboard/game_renderer.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/test_odds_placement.pyplugins/football-scoreboard/game_renderer.pyplugins/football-scoreboard/manifest.jsonplugins/nrl-scoreboard/config_schema.jsonplugins/nrl-scoreboard/game_renderer.pyplugins/nrl-scoreboard/manifest.jsonplugins/soccer-scoreboard/config_schema.jsonplugins/soccer-scoreboard/game_renderer.pyplugins/soccer-scoreboard/manifest.jsonplugins/ufc-scoreboard/manifest.jsonplugins/ufc-scoreboard/sports.py
Comment on lines
+104
to
+109
| print("\nthe step is decided by the text, not the panel size") | ||
| # A long inning label ("FINAL") is wider than a short one, so it can push | ||
| # the odds down at a width where a short label does not. | ||
| y_final, _ = odds_y(64, 32, 8.5, inning_half="top") | ||
| check("a narrow panel with short strings still uses the top edge", | ||
| y_final == 0, "y=%s" % y_final) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise the FINAL collision case.
Line 107 passes inning_half="top". The helper does not set is_final, so this test measures ▲3 instead of FINAL.
Add an is_final argument to odds_y() and set it for this case. This will test the long top-row label described by the comment.
🤖 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 `@plugins/baseball-scoreboard/test_odds_placement.py` around lines 104 - 109,
Update the odds_y test case for the narrow panel and long “FINAL” label to pass
is_final=True alongside inning_half="top". Extend odds_y with an is_final
argument if needed, ensuring this case measures the FINAL label rather than the
default ▲3 text.
The same element sat in three different places. Baseball drew the odds a whole text row below the top -- status_bbox[3] + 2, which measures 8px with the 4x6 font and 10px with PressStart2P -- while basketball drew them at the top edge with an offset applied, and football, soccer, afl, nrl and ufc drew them at a fixed 0,0. On a 32px card that put baseball's odds a third of the card lower than everyone else's. Baseball now uses the top edge too. Its lower row was not arbitrary: it is the only scoreboard with text centred on that row, and the odds are drawn hard left and hard right, so on a narrow panel they meet. That is now decided by measuring the actual strings rather than by assuming -- on a 64px panel "O/U: 8.5" clears the inning by 2px while "O/U: 12.5" overlaps it, and double-digit over/unders are ordinary in baseball. The odds start at the top edge and step down one row only when these particular strings would collide, so every panel 128px and wider is unconditionally top-aligned and a 64px panel keeps the top edge whenever the text fits. Football and ufc declare customization.layout.odds in their config schemas, so the web UI offered x_offset and y_offset, and both renderers ignored them entirely. A control that visibly does nothing is worse than no control: it tells the user the position cannot be fixed. Both now apply them. Soccer, afl and nrl had no odds control at all. They get the same schema block and the same wiring, so the element behaves the same way everywhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
The plugin modules load under bare names, so a static analyser resolving `game_renderer` across the repo can bind another plugin's copy, whose _draw_dynamic_odds takes one argument fewer. Naming the parameter makes the call unambiguous to both a reader and a checker. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Two defects this PR introduced, both found by CodeRabbit. The layout offsets were read inside `if favored_spread is not None`, while the over/under block below uses them unconditionally. A game priced with a total and no spread -- ordinary enough -- raised UnboundLocalError, and the bare except around the method swallowed it, so the card drew no odds at all rather than drawing the total. Affects afl, nrl and soccer; the offsets are now read before either branch. Baseball's collision check always measured the live card's inning indicator, but _draw_dynamic_odds is called from three renderers and they do not centre the same string: a recent card draws "Final" and an upcoming card a time or a date. On a narrow panel the odds were being placed against a string that was not on the screen. Each caller now passes what it actually draws, and the upcoming card's choice is derived in one place so it cannot drift from the code that draws it. The three new tests fail exactly the total-with-no-spread case against the previous commit; the baseball placement test gains recent and upcoming cases across panel sizes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Codacy resolves the bare `game_renderer` module name across plugins, so it can bind another scoreboard's copy -- whose _draw_dynamic_odds takes fewer arguments -- and reports the extra positional ones as an error. It flagged the two call sites this branch changed, the same complaint that was already settled once on the test file. Naming the optional parameters makes the calls unambiguous to a reader and to the checker. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
The afl test acquired a second module-level import block partway down the file, for the core-path shim its renderer needs, which reads as two misplaced imports. os moves up with the other standard-library imports; the plugin import has to stay after the sys.path work and now says so explicitly rather than leaving a checker to guess. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
ChuckBuilds
force-pushed
the
fix/odds-alignment-and-offsets
branch
from
August 12, 2026 17:30
076b3c0 to
f6e5ab2
Compare
Contributor
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 `@plugins/baseball-scoreboard/game_renderer.py`:
- Around line 827-838: Update _upcoming_top_row_text and the related
_odds_would_hit_top_row flow to measure the actual rendered top-row bounds,
including the selected font and X position from _draw_upcoming_game_status.
Preserve swap_date_time behavior so the date uses detail and
layout.date.x_offset rather than re-centering the time. Add coverage for 64×32
with swap_date_time enabled and a nonzero date X offset, while preserving
correct placement at 128×32, 128×64, and 256×32.
🪄 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: d1ad4fe7-2db6-4f52-b667-74cf6eb24b70
📒 Files selected for processing (6)
plugins.jsonplugins/afl-scoreboard/test_odds_without_spread.pyplugins/baseball-scoreboard/game_renderer.pyplugins/football-scoreboard/manifest.jsonplugins/nrl-scoreboard/test_odds_without_spread.pyplugins/soccer-scoreboard/test_odds_without_spread.py
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/afl-scoreboard/test_odds_without_spread.py
- plugins/soccer-scoreboard/test_odds_without_spread.py
The collision check took the top-row text and measured it itself, always with the `time` font and always perfectly centred. An upcoming card with swap_date_time draws the date in `detail` instead, shifted by that element's configured x_offset, so the check could move the odds down when nothing was in the way or leave them overlapping when something was. Reported by CodeRabbit. Callers now pass the span they actually drew. That also removes the last place where the measurement could drift from the drawing, which is the same defect this file already had once: the check used to measure a live card's inning on recent and upcoming cards too. "Nothing on the top row" and "the caller did not say" were both None, so a card that genuinely centres nothing fell back to measuring an inning it does not draw. A sentinel separates them. 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
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.
Reported as odds looking too low on some sports and too high on others. It's two separate problems.
1. The same element sat in three different places
ystatus_bbox[3] + 2 + offset0 + odds_y_offset0Measured with the real fonts: 8px with
4x6, 10px withPressStart2P. On a 32px card that put baseball's odds a third of the card lower than everyone else's.Baseball now uses the top edge too — but its lower row was not arbitrary. It's the only scoreboard with text centred on that row (the inning), and the odds are drawn hard left and hard right, so on a narrow panel they meet:
Double-digit over/unders are ordinary in baseball, so this is a real collision, not a corner case.
So the odds start at the top edge and step down one row only when the actual strings would collide — measured, not keyed to a panel size, because it's the text widths that decide. Verified:
A 64px panel still uses the top edge whenever the text fits; everything 128px and wider is unconditionally top-aligned.
2. Controls that did nothing
Football and ufc both declare
customization.layout.oddsin their config schemas, so the web UI rendersx_offsetandy_offsetfields — and both renderers drew at a fixed0,0and never read them.grepfor_layout_offset('odds'…)returned 0 matches in football, in a file that uses the same helper forscore. A control that visibly does nothing is worse than no control: it tells the user the position can't be fixed.Soccer, afl and nrl had no odds control at all. They get the same schema block and wiring, so the element behaves the same way everywhere.
A static check confirms every odds coordinate in all seven renderers now involves an offset (6–10 coordinates each, all covered).
Verification
test_odds_placement.pyin baseball: 20 checks — top edge across four panel sizes with one- and two-digit over/unders, the step-down triggering only on a genuine collision, the step being about one text row, both manual offsets still applying, and empty odds drawing nothing.Versions: baseball, football, ufc are PATCH (fixes); soccer, afl, nrl are MINOR (new config option).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Summary by CodeRabbit