镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(football): a two-point conversion is not a safety (3.28.1) - #665

Merged
ChuckBuilds merged 3 commits into
mainfrom
fix/football-2pt-conversion-celebration
Nov 4, 2026
Merged

ChuckBuilds merged 3 commits into
mainfrom
fix/football-2pt-conversion-celebration

Conversation

@ChuckBuilds

@ChuckBuilds ChuckBuilds commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

Problem

User report: the football plugin celebrates a two-point conversion as a SAFETY.

The celebration phrase was inferred purely from the size of the score increment (+2 → SAFETY). The follow-up after a touchdown was only suppressed while the TD takeover was on screen (8 s), but in a real game the try lands after a review or TV break — long after that — so it arrived as a lone +2.

Fix

Each score is now classified into a kind (td, fg, safety, pat, two_pt, def_two_pt). Only +2 is ambiguous:

  1. ESPN scoringPlays (game summary endpoint) — the play whose running score matches the new score. Safeties and defensive returns are typed (safety, defensive-two-point-conversion); a conversion is folded into its touchdown entry's text, e.g. (Cam Ward Run for Two-Point Conversion). Fetched only for a +2, plain requests.get with a 3 s timeout (the session's retry adapter backs off for tens of seconds).
  2. Fallback: score sequence — a pending try is recorded after a +6. The TD side's next +2 is its conversion, the other side's is a defensive return, anything else is a safety. A pending try lasts one score change and is cleared on first sighting and at the final.

A try landing after the TD takeover closed now gets its own: KC 2PT GOOD! (goal line), KC EXTRA PT! (goalposts), DAL DEF 2PT!. One landing during the TD takeover stays part of it, as before.

Tests

  • 16 new cases in test_score_celebration.py (conversion / PAT / defensive return after the window, pending-try lifetime, suppressed TD still tracked, +8 in one poll, real ESPN play texts from Oct 2026, lagging summary fallback, fetch only for +2, URL + failure handling).
  • run_plugin_tests.py football-scoreboard: 66 passed, 1 skipped, 0 failed.
  • scripts/test_celebration_renders.py: 135 frames match goldens. Version-bump / manifest checks pass.

Not exercised against a live game (none on when built).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved classification of ambiguous two-point scoring plays, using current play information when available and the score sequence as a fallback.
    • An extra point scored after a touchdown celebration ends now gets its own “EXTRA PT!” celebration with goalposts; one scored during the celebration remains part of it.

The try after a touchdown usually lands after the touchdown's takeover
has closed, so it arrived as a lone +2 and was named from its size
alone: SAFETY. An ambiguous +2 is now named from ESPN's scoring plays
(game summary, fetched only for a +2, 3 s timeout) when they are up to
date, and otherwise from the score sequence: the touchdown side's next
+2 is its conversion, the other side's a defensive return, anything
else a safety. A late extra point gets its own takeover (EXTRA PT!).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codacy-production

codacy-production Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 32 complexity

Metric Results
Complexity 32

View in Codacy

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

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 32 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ba63b860-5918-4de1-afd3-87bdadbf0d64
Commits

Reviewing files that changed from the base of the PR and between 82937e7 and 6590c4b.

Files selected for processing: 5
  • plugins.json
  • plugins/football-scoreboard/CHANGELOG.md
  • plugins/football-scoreboard/manifest.json
  • plugins/football-scoreboard/sports.py
  • plugins/football-scoreboard/test_score_celebration.py
Walkthrough

The football scoreboard now classifies scoring changes by score kind, using ESPN scoring plays or pending-try state for ambiguous +2 changes. Celebration phrases and motifs use the classified kind. The plugin release version is 3.28.1.

Changes: Football scoring celebrations

Layer / File(s) Summary
Classify score changes and track tries
plugins/football-scoreboard/sports.py, plugins/football-scoreboard/test_score_celebration.py
SportsLive uses ESPN scoring plays and pending-try state to classify ambiguous +2 changes. Tests cover score classification, lookup behavior, and pending-try tracking.
Map score kinds to celebrations
plugins/football-scoreboard/sports.py, plugins/football-scoreboard/test_score_celebration.py, plugins/football-scoreboard/CHANGELOG.md, plugins/football-scoreboard/manifest.json
Celebration phrase and motif mapping uses score kinds. Tests cover the mappings. The changelog and manifest record version 3.28.1 and its release notes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: Low · previous review 82937

Some uncommon scoring sequences can still display incorrect celebration labels, and malformed scoring data can interrupt one refresh. The impact is bounded, but these cases warrant follow-up.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the primary change: correcting the classification of a two-point conversion as a safety. It matches the main objective and includes the release version.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 14 functions across 2 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

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.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 62bb80da-9583-4281-b3c2-138f90c46589
Commits

Reviewing files that changed from the base of the PR and between b4bdda9 and 82937e7.

Files selected for processing: 4
  • plugins/football-scoreboard/CHANGELOG.md
  • plugins/football-scoreboard/manifest.json
  • plugins/football-scoreboard/sports.py
  • plugins/football-scoreboard/test_score_celebration.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

Comment thread plugins/football-scoreboard/sports.py Resolved
Comment thread plugins/football-scoreboard/sports.py Resolved
Comment thread plugins/football-scoreboard/sports.py Resolved
Comment thread plugins/football-scoreboard/sports.py Resolved
Comment thread plugins/football-scoreboard/sports.py Resolved
ChuckBuilds and others added 2 commits November 3, 2026 14:51
…l returns

Review follow-ups for #665:
- A failed try changes no score, so the owed try survived until the next
  score; a same-side safety a drive later read as 2PT GOOD!. The try is
  untimed, so it now ends once the period changes or the clock has run
  (5 s slack). Unreadable clocks keep the one-score-change limit.
- A score correction (any decrease) clears the owed try.
- A touchdown and the other side's +2 in one poll: the +2 is the
  defensive return on that try, and no try is left owed.
- A non-object scoringType no longer raises out of classification; it
  falls back to the score sequence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ChuckBuilds
ChuckBuilds merged commit 9c9a225 into main Nov 4, 2026
5 checks passed
@ChuckBuilds
ChuckBuilds deleted the fix/football-2pt-conversion-celebration branch November 4, 2026 17:24
ChuckBuilds added a commit that referenced this pull request Nov 4, 2026
Football's metadata conflicted with #665 (3.28.1); code merged cleanly.
Resolution: football 3.28.2 stays the newest version, with the 3.28.1
entry kept beneath it in manifest.json versions[] and CHANGELOG.md, and
plugins.json regenerated with update_registry.py.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ChuckBuilds added a commit that referenced this pull request Nov 4, 2026
* fix(scoreboards): switching screens never flashes the panel black

display_manager.clear() writes black straight to the matrix. All nine
scoreboards called it before drawing a card whenever force_clear was
set -- a mode change, a block boundary, or a >5 s gap between display()
calls (a slow update() holding the plugin lock) -- and rendering the
card took long enough for the black to show. With one favourite team
the same card came back, so the board read as flashing. Empty modes
also blanked the panel as the controller tried them on its way past;
the recent screen did so on every frame while its list was empty.

Every draw path already builds a full frame and assigns it, so the
clears are simply removed: the panel keeps the previous frame until the
new one is pushed, and the core's dirty-frame check skips an unchanged
card. Empty modes return False and leave the panel to the controller,
which blanks it once when the whole rotation is empty. Same fix as
calendar's #328.

Guarded by scripts/test_scoreboards_never_blank_the_panel.py (no
display_manager.clear() in any of the nine) and football's
test_switching_never_flashes_black.py (real display() paths).

afl 1.44.1, baseball 1.66.1, basketball 1.52.1, football 3.28.2
(3.28.1 is #665), hockey 1.54.1, lacrosse 1.45.1, nrl 1.43.1,
soccer 2.50.1, ufc 1.27.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(registry): sync the nine scoreboards

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(scoreboards): require core 3.9.0 for the empty-rotation blank

Review follow-ups for #666:
- The panel is now blanked only by the controller when every mode is
  empty, and that blank first shipped in core 3.9.0 (#799). On 3.8.7 the
  last card would stay up, so all nine scoreboards' newest version entry
  and compatible_versions now require 3.9.0.
- test_switching_never_flashes_black: a forced redraw must show the card,
  not only avoid black; bind the draw stub with types.MethodType (Codacy
  misread the descriptor call).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant