feat(nfl-draft): post-draft multi-round display + off-season silence (v1.3.5) - #107
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a verified "NFL Draft" plugin under Changes
Sequence DiagramsequenceDiagram
participant LEDMatrix as LEDMatrix Framework
participant Plugin as NFLDraftPlugin
participant ESPN as ESPN API
participant Tankathon as Tankathon
participant Display as LED Display
LEDMatrix->>Plugin: update()
activate Plugin
alt live draft or simulate_live
Plugin->>ESPN: Fetch draft status, current picks, prospects/historical rounds
else pre-draft projections
Plugin->>Tankathon: Fetch projection picks
alt Tankathon fails / empty
Plugin->>ESPN: Fallback fetch (Round 1 order / prospects)
end
end
Plugin->>Plugin: Assemble picks, mark "On the Clock", apply favorites
deactivate Plugin
LEDMatrix->>Plugin: display()
activate Plugin
Plugin->>Plugin: Render scrolling stream or vegas items
Plugin->>Display: Push frame(s) / item images
deactivate Plugin
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
plugins/nfl-draft/manager.py (1)
635-650: Position supplementation fans out up to 150 parallel HTTP fetches every refresh.
_fetch_all_prospectsis called whenever any pick has a blank position (line 639). Early in a live draft that's essentially every refresh, and it issues up to 150 concurrent requests with 10 worker threads. Beyond the startup concern, this also runs on every 10-minute live tick until the cache warms.Two improvements worth considering:
- Short-circuit when the prospects cache is present (already partially handled, but gate the fan-out behind a single "cache miss" check rather than per-call).
- Back off to a smaller cap (e.g. top 64) since only picks appearing on-screen need positions supplemented, and the site API already carries athlete IDs.
Also,
_fetch_all_prospectsbypassesAPIHelperand talks directly tocache_manager(lines 229-232) while_fetch_draft_datausesapi_helper.get. Consider unifying both throughAPIHelperfor consistent caching/retry semantics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/nfl-draft/manager.py` around lines 635 - 650, The position supplementation currently calls _fetch_all_prospects whenever any pick is missing a position which fans out to many concurrent HTTP requests; change the logic so you first check the prospects cache (short-circuit if present) before calling _fetch_all_prospects, and when a fetch is required limit the scope to only the top N athletes (e.g., 64) instead of all prospects so you don’t spawn 150 concurrent requests for every refresh; also update _fetch_all_prospects (or the caller) to use APIHelper (the same helper used by _fetch_draft_data) instead of talking directly to cache_manager so caching/retry behavior is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 99-102: The constructor currently calls self.update() which blocks
startup; remove that immediate call from __init__ and instead kick off the
initial fetch asynchronously (or defer to the first scheduled tick): either
spawn a background Thread/Executor to run self.update() (so __init__ returns
quickly) or simply rely on the scheduler to call update() later; ensure
display() continues to call _display_no_data() until the async update completes,
and protect shared state accessed by update() and display() as needed; the
relevant symbols to change are the __init__ method (remove the self.update()
invocation), the update() function, and consider using _fetch_all_prospects and
display()/_display_no_data() to coordinate readiness.
- Around line 1016-1030: When simulate_live is true the code sets
self.draft_status and calls _fetch_historical_picks but does not reset live
state; update the simulate branch (where simulate_live is checked) to explicitly
set self.is_draft_live = False and reset self.current_round to a sane default
(e.g., 1 or None) before assigning self.draft_picks so the UI and downstream
logic (get_info, on-the-clock markers) don't report stale live state; modify the
block that sets self.draft_status = "simulate" and calls _fetch_historical_picks
to include these resets.
- Around line 15-16: The module-level docstring currently contains a stale line
"API Version: 1.0.0"; update that line in the top-of-file module docstring in
plugins/nfl-draft/manager.py to match the version in manifest.json (change to
"API Version: 1.3.2") or remove the "API Version" line entirely so the docstring
cannot drift out of sync with manifest.json.
- Around line 554-577: The status-handling logic sets self.is_draft_live = True
when state == "in" but never clears it, so update(), has_live_content(), and the
on-the-clock loop can behave incorrectly after draft ends; fix by explicitly
setting self.is_draft_live = False in the non-"in" branches (the "post" branch
and the else branch that sets draft_status = "pre") and also ensure the fallback
path that defaults draft_status to "pre" when unknown likewise clears
self.is_draft_live; adjust the code around the status parsing (refer to the
status variable, the state check, and the attributes self.draft_status and
self.is_draft_live) so the live flag always reflects the current draft_status.
- Around line 652-680: Delete the unused _check_draft_live_status method
entirely; it duplicates logic already present in _fetch_draft_picks and only
mutates draft_status without updating is_draft_live. Remove the method
definition for _check_draft_live_status and ensure that _fetch_draft_picks
remains the single source of truth for draft state (it should continue to set
both draft_status and is_draft_live consistently). Also search for any
references to _check_draft_live_status and remove or replace them to avoid dead
references.
In `@plugins/nfl-draft/manifest.json`:
- Around line 1-46: Add a top-level "versions" array to manifest.json and
populate it with the current release as the first entry: include "released" (ISO
date or YYYY-MM-DD), "version" matching the existing "version" field ("1.3.2"),
and "ledmatrix_min" set to the minimum compatible LEDMatrix version (e.g.,
"2.0.0"); ensure the array is placed alongside existing top-level keys and
future releases are always prepended so the most recent version is first.
- Around line 5-25: Update the manifest to use the canonical metadata values:
set "author" to "ChuckBuilds" (to match plugins.json) and set "icon" to a valid
Font Awesome class with a style prefix, e.g. "fas fa-football-ball"; locate
these keys in the manifest (look for "author" and "icon" near
"entry_point"/"class_name") and replace the current values so the registry sync
and UI icon rendering remain consistent.
In `@plugins/nfl-draft/README.md`:
- Around line 19-25: Update the README.md to point to the monorepo URL and add
code-block language hints: replace the incorrect install URL
"https://github.com/sarjent/ledmatrix-nfl-draft" with the registry/monorepo URL
used in plugins.json (the ChuckBuilds/ledmatrix-plugins repo and indicate the
plugin path "plugins/nfl-draft" or "(plugin: nfl-draft)"), change the
Contributions link to the same ChuckBuilds/ledmatrix-plugins repo, and add
"text" as the language for the fenced code blocks shown (so the install URL and
any similar blocks use ```text) to satisfy markdownlint MD040; refer to
plugins.json and the README sections around the install and contributing text to
locate the edits.
---
Nitpick comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 635-650: The position supplementation currently calls
_fetch_all_prospects whenever any pick is missing a position which fans out to
many concurrent HTTP requests; change the logic so you first check the prospects
cache (short-circuit if present) before calling _fetch_all_prospects, and when a
fetch is required limit the scope to only the top N athletes (e.g., 64) instead
of all prospects so you don’t spawn 150 concurrent requests for every refresh;
also update _fetch_all_prospects (or the caller) to use APIHelper (the same
helper used by _fetch_draft_data) instead of talking directly to cache_manager
so caching/retry behavior is consistent.
🪄 Autofix (Beta)
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
Run ID: d708d7c4-352e-4e2b-9aa0-99218aa93250
⛔ Files ignored due to path filters (1)
plugins/nfl-draft/nfl_draft_logo.pngis excluded by!**/*.png
📒 Files selected for processing (6)
plugins.jsonplugins/nfl-draft/README.mdplugins/nfl-draft/config_schema.jsonplugins/nfl-draft/manager.pyplugins/nfl-draft/manifest.jsonplugins/nfl-draft/requirements.txt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
plugins/nfl-draft/manager.py (1)
200-215:⚠️ Potential issue | 🟠 MajorGate cache TTL on draft window to detect live status transitions.
During the April 20–27 draft window,
self.is_draft_liveremainsFalseuntil ESPN transitionsstatus.stateto"in". However, the cache TTL is determined before that response is parsed:cache_ttlis set based on the current (stale) value ofself.is_draft_live, so the first_fetch_draft_data()call caches the ESPN response withprojection_refresh_interval(86400 s). Subsequentupdate()ticks every 10 minutes then hit cached data and never observe the state transition until the 24-hour TTL organically expires.Apply the proposed fix to align cache TTL with the draft window logic already used in
update():Proposed fix
- cache_ttl = self.live_refresh_interval if self.is_draft_live else self.projection_refresh_interval + cache_ttl = ( + self.live_refresh_interval + if (self.is_draft_live or self._is_draft_date()) + else self.projection_refresh_interval + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/nfl-draft/manager.py` around lines 200 - 215, The cache TTL is chosen from self.is_draft_live before the ESPN response can flip that flag, causing long projection TTLs to be cached and missing live transitions; modify _fetch_draft_data to first fetch the ESPN_DRAFT_SITE response in a non-cached (or very short TTL) manner, parse it to determine draft live status (same logic used in update() that sets self.is_draft_live), then compute cache_ttl = live_refresh_interval if now live else projection_refresh_interval and re-cache the parsed data (or call api_helper.get again with the correct cache_ttl); ensure you reference _fetch_draft_data, self.is_draft_live, update(), live_refresh_interval, projection_refresh_interval, ESPN_DRAFT_SITE, and api_helper.get when implementing.
🧹 Nitpick comments (1)
plugins/nfl-draft/manager.py (1)
100-103: Backgroundupdate()has unsynchronized access to shared state.Spawning
self.update()on a daemon thread resolves the blocking-init concern from the prior review, butupdate()mutatesself.draft_picks,self.is_draft_live,self.current_round, andself.scroll_helperstate whiledisplay()andget_vegas_content()read the same attributes from the main/render thread. A mid-updateself.draft_picks.sort(...)(line 1050) or the stale-flag clearing loop (line 1055) can be observed in a partially mutated state by the renderer. The practical risk is low given Python's GIL and infrequent updates, but athreading.Lockguarding the mutation block inupdate()(and a snapshot indisplay()/get_vegas_content()) would make this race-free.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/nfl-draft/manager.py` around lines 100 - 103, The background update thread mutates shared attributes (self.draft_picks, self.is_draft_live, self.current_round, self.scroll_helper) while display() and get_vegas_content() read them, creating a race; introduce a threading.Lock (e.g., self._state_lock) initialized in __init__ and use it to guard the mutation block inside update() (acquire before mutating and release after), and also take the lock in display() and get_vegas_content() only long enough to make a shallow snapshot/copy of the needed state (e.g., copy draft_picks list and scalar fields) and release the lock before doing any expensive rendering so the renderer observes a consistent snapshot without blocking long-running operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 697-704: The _is_draft_date function currently uses draft_end =
datetime(self.draft_year, 4, 27) which equals midnight at the start of 4/27 and
excludes most of that day; update the check so the full April 27 is included by
either comparing dates (use now.date() between date(self.draft_year,4,20) and
date(self.draft_year,4,27)) or extend draft_end to end-of-day (e.g.,
datetime(self.draft_year,4,27,23,59,59,999999)), and keep references to
draft_start, draft_end and now in the updated logic in _is_draft_date.
- Around line 611-615: The code assigns self.current_round from status["round"]
without rejecting 0, which lets self.current_round become 0 and breaks
_get_display_round() and the "On the Clock" logic in update(); change the
assignment logic around current_round (the local variable and
self.current_round) to validate and clamp to a minimum of 1 before setting
self.current_round (e.g., only accept ints >= 1 or use max(1, current_round)) so
downstream functions like _get_display_round() and update() always see rounds in
the valid 1–7 range.
---
Duplicate comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 200-215: The cache TTL is chosen from self.is_draft_live before
the ESPN response can flip that flag, causing long projection TTLs to be cached
and missing live transitions; modify _fetch_draft_data to first fetch the
ESPN_DRAFT_SITE response in a non-cached (or very short TTL) manner, parse it to
determine draft live status (same logic used in update() that sets
self.is_draft_live), then compute cache_ttl = live_refresh_interval if now live
else projection_refresh_interval and re-cache the parsed data (or call
api_helper.get again with the correct cache_ttl); ensure you reference
_fetch_draft_data, self.is_draft_live, update(), live_refresh_interval,
projection_refresh_interval, ESPN_DRAFT_SITE, and api_helper.get when
implementing.
---
Nitpick comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 100-103: The background update thread mutates shared attributes
(self.draft_picks, self.is_draft_live, self.current_round, self.scroll_helper)
while display() and get_vegas_content() read them, creating a race; introduce a
threading.Lock (e.g., self._state_lock) initialized in __init__ and use it to
guard the mutation block inside update() (acquire before mutating and release
after), and also take the lock in display() and get_vegas_content() only long
enough to make a shallow snapshot/copy of the needed state (e.g., copy
draft_picks list and scalar fields) and release the lock before doing any
expensive rendering so the renderer observes a consistent snapshot without
blocking long-running operations.
🪄 Autofix (Beta)
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
Run ID: 7c833d40-e8fc-4094-8f3a-f75356fd0dc1
📒 Files selected for processing (4)
plugins.jsonplugins/nfl-draft/README.mdplugins/nfl-draft/manager.pyplugins/nfl-draft/manifest.json
✅ Files skipped from review due to trivial changes (1)
- plugins/nfl-draft/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/nfl-draft/manifest.json
sarjent
force-pushed
the
feat/nfl-draft-plugin
branch
from
April 23, 2026 16:46
0562bd2 to
3e3ba87
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins.json (1)
661-683:⚠️ Potential issue | 🔴 CriticalDuplicate
nfl-draftregistry entries — old standalone-repo entry should be removed.
plugins.jsonnow has two entries with"id": "nfl-draft":
- Lines 661-683: the pre-existing entry pointing at the standalone fork (
repo: https://github.com/sarjent/ledmatrix-nfl-draft,plugin_path: "",verified: false). This PR even bumped itslast_updated(Line 678) andlatest_version(Line 681) to mirror the new release.- Lines 777-800: the new monorepo entry (
repo: https://github.com/ChuckBuilds/ledmatrix-plugins,plugin_path: "plugins/nfl-draft",verified: true).The registry is the source of truth for the Plugin Store and IDs must be unique, so consumers will either render duplicate cards or pick one non-deterministically — and the un-verified standalone entry advertising
1.3.5is misleading now that the code lives in the monorepo. Delete the lines 661-683 block and keep only the monorepo entry at lines 777-800.As per coding guidelines: "Registry entries for monorepo plugins must use:
repoashttps://github.com/ChuckBuilds/ledmatrix-plugins,plugin_pathasplugins/<plugin-id>,branchasmain, andlatest_versionsynced from manifest" — only the new entry satisfies this; the old one cannot coexist under the sameid.🐛 Proposed fix (delete the superseded entry)
- { - "id": "nfl-draft", - "name": "NFL Draft", - "description": "Displays projected NFL draft picks from ESPN with live draft tracking support during the annual NFL Draft event. Shows team logos, player names, positions, and pick numbers in a scrolling display.", - "author": "sarjent", - "category": "sports", - "tags": [ - "nfl", - "draft", - "football", - "sports", - "espn" - ], - "repo": "https://github.com/sarjent/ledmatrix-nfl-draft", - "branch": "main", - "plugin_path": "", - "stars": 0, - "downloads": 0, - "last_updated": "2026-04-26", - "verified": false, - "screenshot": "", - "latest_version": "1.3.5", - "icon": "fas fa-football-ball" - }, { "id": "pga-tour-leaderboard",Also applies to: 777-800
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins.json` around lines 661 - 683, Remove the duplicated standalone registry entry with "id": "nfl-draft" that points to repo "https://github.com/sarjent/ledmatrix-nfl-draft" (the block containing plugin_path: "", verified: false, latest_version: "1.3.5") and keep only the monorepo entry that uses repo "https://github.com/ChuckBuilds/ledmatrix-plugins" and plugin_path "plugins/nfl-draft"; ensure the remaining entry follows the monorepo convention (repo set to ChuckBuilds/ledmatrix-plugins, plugin_path "plugins/nfl-draft", branch "main", and latest_version synced from the plugin manifest) so there is a single unique "nfl-draft" id in plugins.json.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 1110-1119: The update() logic leaks inconsistent state because
draft_picks is set before acquiring _state_lock and _create_draft_scroll_image()
runs while draft_status, is_draft_live, and current_round are still stale; move
the assignments for draft_picks, draft_status, is_draft_live, and current_round
so they all occur atomically inside the with self._state_lock block (remove the
unlocked write at the earlier assignment and the redundant re-assignment), then
release the lock and call _create_draft_scroll_image() so rendering sees a
consistent state; references to display() and get_vegas_content() show why the
lock across the cheap reference swap is appropriate.
---
Outside diff comments:
In `@plugins.json`:
- Around line 661-683: Remove the duplicated standalone registry entry with
"id": "nfl-draft" that points to repo
"https://github.com/sarjent/ledmatrix-nfl-draft" (the block containing
plugin_path: "", verified: false, latest_version: "1.3.5") and keep only the
monorepo entry that uses repo "https://github.com/ChuckBuilds/ledmatrix-plugins"
and plugin_path "plugins/nfl-draft"; ensure the remaining entry follows the
monorepo convention (repo set to ChuckBuilds/ledmatrix-plugins, plugin_path
"plugins/nfl-draft", branch "main", and latest_version synced from the plugin
manifest) so there is a single unique "nfl-draft" id in plugins.json.
🪄 Autofix (Beta)
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
Run ID: 9e58648a-6393-4649-a5ef-801ec0422327
⛔ Files ignored due to path filters (1)
plugins/nfl-draft/nfl_draft_logo.pngis excluded by!**/*.png
📒 Files selected for processing (6)
plugins.jsonplugins/nfl-draft/README.mdplugins/nfl-draft/config_schema.jsonplugins/nfl-draft/manager.pyplugins/nfl-draft/manifest.jsonplugins/nfl-draft/requirements.txt
✅ Files skipped from review due to trivial changes (3)
- plugins/nfl-draft/requirements.txt
- plugins/nfl-draft/README.md
- plugins/nfl-draft/config_schema.json
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/nfl-draft/manifest.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
plugins/nfl-draft/manager.py (1)
1120-1129:⚠️ Potential issue | 🟠 MajorInconsistent state still leaks to renderer / readers.
self.draft_picks = new_picksat Line 1122 and_create_draft_scroll_image()at Line 1123 run before the lock block at Lines 1125-1129, so:
_create_draft_scroll_image()reads new picks but staleself.is_draft_live,self.draft_status, andself.current_round— on apre→liveorlive→completetransition it will pick the wrong rendering branch (or wrongcurrent_roundin_get_display_round()).- A reader holding
_state_lockbetween Lines 1122 and 1125 (display(),get_vegas_content()) sees new picks alongside stale status/round.Move all four assignments inside the lock and render afterwards (
update()is the sole writer, so rendering can run unlocked).🐛 Proposed fix
- # Build the scroll image before acquiring the lock so rendering - # doesn't block display() for longer than a list swap. - self.draft_picks = new_picks - self._create_draft_scroll_image() - - with self._state_lock: - self.draft_status = new_status - self.is_draft_live = new_live - self.current_round = new_round - self.draft_picks = new_picks + # Atomically swap state so display()/get_vegas_content() never + # observe new picks alongside stale status/round. + with self._state_lock: + self.draft_status = new_status + self.is_draft_live = new_live + self.current_round = new_round + self.draft_picks = new_picks + + # Render after the swap; update() is the sole writer. + self._create_draft_scroll_image()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/nfl-draft/manager.py` around lines 1120 - 1129, The state update leaks because self.draft_picks is set and _create_draft_scroll_image() is called before acquiring _state_lock, allowing readers (display(), get_vegas_content()) or the renderer to observe mixed old/new state; fix by moving all four assignments (self.draft_picks, self.draft_status, self.is_draft_live, self.current_round) into the with self._state_lock block inside update(), and then call _create_draft_scroll_image() after releasing the lock so rendering uses a consistent snapshot while writes remain protected.
🧹 Nitpick comments (1)
plugins/nfl-draft/manager.py (1)
1097-1108:simulate_liveonly ever renders Round 1.
_fetch_historical_picks()pulls all 7 rounds, but herenew_round = 1andnew_live = Falseare set so:
_create_draft_scroll_image()enters theis_draft_live or simulate_livebranch (Line 818) and calls_get_display_round()which filtersdraft_pickstoround == self.current_round == 1only.display_rounds/post_draft_showare ignored in simulate mode.If the intent is to replay an entire completed draft, consider routing simulate through the post-draft branch (e.g., set
new_status = "complete"and bypass the post-draft window check whensimulate_live), or render all rounds explicitly whensimulate_live. As-is, rounds 2–7 of the historical fetch are wasted work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/nfl-draft/manager.py` around lines 1097 - 1108, The simulate_live branch currently fetches all rounds via _fetch_historical_picks() but forces new_status="simulate" and new_round=1 so only round 1 is rendered; change the simulate branch to route through the post-draft rendering path by setting new_status="complete" (leave new_live=False) and set new_round appropriately (or make downstream logic respect simulate_live) so _create_draft_scroll_image will use display_rounds/post_draft_show instead of _get_display_round filtering to round==1; alternatively, explicitly render all rounds when simulate_live by iterating the fetched picks rather than forcing round 1. Ensure you update references to simulate_live, new_status, new_live, new_round, _create_draft_scroll_image, _get_display_round, display_rounds, and post_draft_show accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 1153-1159: The off-season guard incorrectly hides the plugin when
draft_status == "simulate" by treating non-"live"/"complete" statuses as
off-season; in update() detect simulate_live sets draft_status = "simulate", so
modify the guard in the display branch (around the check using status and
self._is_off_season()) to allow status == "simulate" to bypass _display_blank()
(i.e., only blank when status not in ("live","complete","simulate") and
self._is_off_season()); make the identical change in get_vegas_content() so
simulate mode renders during off-season as intended.
---
Duplicate comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 1120-1129: The state update leaks because self.draft_picks is set
and _create_draft_scroll_image() is called before acquiring _state_lock,
allowing readers (display(), get_vegas_content()) or the renderer to observe
mixed old/new state; fix by moving all four assignments (self.draft_picks,
self.draft_status, self.is_draft_live, self.current_round) into the with
self._state_lock block inside update(), and then call
_create_draft_scroll_image() after releasing the lock so rendering uses a
consistent snapshot while writes remain protected.
---
Nitpick comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 1097-1108: The simulate_live branch currently fetches all rounds
via _fetch_historical_picks() but forces new_status="simulate" and new_round=1
so only round 1 is rendered; change the simulate branch to route through the
post-draft rendering path by setting new_status="complete" (leave
new_live=False) and set new_round appropriately (or make downstream logic
respect simulate_live) so _create_draft_scroll_image will use
display_rounds/post_draft_show instead of _get_display_round filtering to
round==1; alternatively, explicitly render all rounds when simulate_live by
iterating the fetched picks rather than forcing round 1. Ensure you update
references to simulate_live, new_status, new_live, new_round,
_create_draft_scroll_image, _get_display_round, display_rounds, and
post_draft_show accordingly.
🪄 Autofix (Beta)
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
Run ID: d7620410-6458-4200-8b09-d02424dfd5e9
📒 Files selected for processing (4)
plugins.jsonplugins/nfl-draft/config_schema.jsonplugins/nfl-draft/manager.pyplugins/nfl-draft/manifest.json
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/nfl-draft/manifest.json
New plugin that displays projected and live NFL draft picks from ESPN
on LED matrix displays.
Features:
- Pre-draft: Tankathon mock draft picks (Round 1) with team logos,
player name, position, and college
- Live: Auto-detects draft start via ESPN status API; switches to real
picks, shows current round, marks next pick "On the Clock" in green
- Auto-poll: update_interval 300s; internal throttle uses 10-min interval
automatically during April 20-27 draft window — no config change needed
- Position fix: ESPN site API returns position as {id} only; supplements
from core API prospects cache keyed by athlete ID
- College: sourced from ESPN site API athlete.team.shortDisplayName inline
- Favorite teams: up to 3 team abbreviations pinned to scroll front
- Vegas scroll: returns individual pick cards as List[Image] for
continuous stream integration
- Simulate mode: replay any completed draft year via ESPN core API
- NFL Draft logo auto-installed to core assets on startup
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Background initial fetch: replace blocking self.update() in __init__ with threading.Thread(daemon=True) so startup returns immediately; display() already guards with `if not self.draft_picks` - is_draft_live never cleared: add explicit False assignments in the "post", "pre", and unknown-fallback branches of _fetch_draft_picks so the live flag always reflects current draft state - simulate_live stale state: reset is_draft_live=False and current_round=1 before assigning draft_picks in the simulate branch - Delete unused _check_draft_live_status: duplicated _fetch_draft_picks logic and never updated is_draft_live; _fetch_draft_picks is the single source of truth for draft state - Stale docstring: remove "API Version: 1.0.0" line - Prospects fetch scope: limit athlete URL fetch to 64 (from 150) to reduce concurrent HTTP requests on cache-miss - manifest: set author="ChuckBuilds", icon="fas fa-football-ball", add versions array - README: point install URL and contributing link to ChuckBuilds monorepo, add "text" language hint to fenced code blocks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add full browser headers to Tankathon request to prevent bot blocking on embedded Linux devices (Pi/similar) where minimal UA gets rejected - Add _fetch_espn_predraft_order() fallback: if Tankathon is unreachable, display ESPN Round 1 draft order with team logos (player names TBD) so the display is never empty pre-draft Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… clamp, draft_end (v1.3.4) - Add _state_lock (threading.Lock) to guard draft_picks/is_draft_live/ current_round mutations in update(); display() and get_vegas_content() take a shallow snapshot under the lock before rendering - _fetch_draft_data: cache TTL now uses live_refresh_interval during draft week (April 20-27) regardless of is_draft_live so live transition is detected within 10 min instead of waiting for a 24-hour cache expiry - _fetch_draft_picks: clamp current_round to max(1, round) so ESPN sending 0 never breaks _get_display_round or on-the-clock logic - _is_draft_date: extend draft_end to 23:59:59 so all of April 27 is included (midnight comparison excluded the rest of the final draft day) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Upstream merged nfl-draft as external plugin at v1.3.2 (ChuckBuilds#108). Update latest_version to 1.3.4 to reflect fixes landed since. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e (v1.3.5) Three new post-draft lifecycle phases: - Post-draft window (post_draft_days, default 7 days after April 27): shows all favorite-team picks in pick order, then rounds 1-N (display_rounds, default 3) each with a round label. - Off-season (May through January): get_vegas_content returns None and display() renders a blank black frame — completely silent, no messages. - Pre-draft (February onward, after Super Bowl): resumes Tankathon mock picks as before. New config keys: display_rounds (int, default 3), post_draft_days (int, default 7). Adds timedelta import, _is_post_draft_window(), _is_off_season(), _display_blank(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Exposes the two new post-draft config keys in the web UI: - display_rounds (1–7, default 3): rounds shown during post-draft window - post_draft_days (1–30, default 7): days after April 27 before silence Also clarifies favorite_teams description to cover post-draft behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Lets the user choose what to display during the post-draft window: - 'favorites' — only picks for their configured favorite teams - 'rounds' — per-round results for all teams (rounds 1–display_rounds) - 'both' — favorite team picks first, then per-round results (default) Adds post_draft_show enum field to config_schema.json so the web UI exposes a dropdown. display_rounds description updated to clarify it only applies when rounds are shown. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…1.3.7) ESPN does not expose the draft end date. Previously _is_draft_date() and _is_post_draft_window() hardcoded April 27, which was wrong for 2026 (draft ended April 26) and any year the last day falls before the 27th. New _get_draft_end_date() computes the last Saturday of April for the draft year by walking back from April 30. Verified against 2022–2026. Also removes the stale "April 27" reference from the schema description. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The sarjent/ledmatrix-nfl-draft external entry (plugin_path: "", verified: false, latest_version: 1.3.5) was a leftover from before the plugin was added to the monorepo. The canonical monorepo entry (plugin_path: plugins/nfl-draft, verified: true, latest_version: 1.3.7) is the single source of truth going forward. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ate, simulate routing (v1.3.8) 1. Off-season guard blocked simulate_live: added "simulate" to the allowed- status set in display() and get_vegas_content() so simulate_live always renders regardless of month. 2. State update leaked across lock boundary: removed the early self.draft_picks assignment outside the lock; all four state fields (draft_picks, draft_status, is_draft_live, current_round) are now written atomically inside _state_lock, then _create_draft_scroll_image() runs after the lock is released against a fully consistent snapshot. 3. simulate_live showed only round 1: split the is_draft_live/simulate_live branch so simulate_live routes through the post-draft rendering path (display_rounds / post_draft_show) instead of _get_display_round(). The post-draft-window early-return is guarded by draft_status=="complete" so it never suppresses simulate. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sarjent
force-pushed
the
feat/nfl-draft-plugin
branch
from
April 26, 2026 20:53
ed5c293 to
f9b966d
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/nfl-draft/manager.py (1)
810-878: Consider extracting shared content-building logic between_create_draft_scroll_imageandget_vegas_content.Both methods implement the same three-branch flow (live → round label + favorites + round picks; complete/simulate → favorites then rounds 1..N; pre-draft → favorites + current display round) and only differ in their output sink (scroll helper vs. returned list). Future tweaks to ordering, off-season behavior, or
post_draft_showsemantics will need to be made in two places — easy to drift.A shared helper that returns
List[Image.Image]would let_create_draft_scroll_imagesimply pass the result toscroll_helper.create_scrolling_image(...)andget_vegas_contentreturn it directly.♻️ Sketch
def _build_content_items(self, status: str, is_live: bool) -> List[Image.Image]: items: List[Image.Image] = [] if self.nfl_draft_logo: items.append(self.nfl_draft_logo) if is_live: display_round, round_picks = self._get_display_round() items.append(self._create_round_label_item(display_round)) for pick in self._get_favorite_team_picks(): if (img := self._create_pick_item(pick)): items.append(img) for pick in round_picks: if (img := self._create_pick_item(pick)): items.append(img) elif status == "complete" or self.simulate_live: show = self.post_draft_show if show in ("favorites", "both"): for pick in self._get_favorite_team_picks(limit=None, ascending=True): if (img := self._create_pick_item(pick)): items.append(img) if show in ("rounds", "both"): for rnd in range(1, self.display_rounds + 1): round_picks = [ p for p in self.draft_picks if p.get("round") == rnd and p.get("player_name", "TBD") != "TBD" ] if round_picks: items.append(self._create_round_label_item(rnd)) for pick in round_picks: if (img := self._create_pick_item(pick)): items.append(img) else: _, round_picks = self._get_display_round() for pick in self._get_favorite_team_picks(): if (img := self._create_pick_item(pick)): items.append(img) for pick in round_picks: if (img := self._create_pick_item(pick)): items.append(img) return itemsThen both call sites become a thin wrapper that handles their respective off-season/empty-window early-return and either feeds
scroll_helperor returns the list.Also applies to: 1252-1325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/nfl-draft/manager.py` around lines 810 - 878, Extract the shared content-building logic into a new helper (e.g. _build_content_items) that returns List[Image.Image] and encapsulates the three-branch flow currently duplicated in _create_draft_scroll_image and get_vegas_content (live path: include nfl_draft_logo, _get_display_round + _create_round_label_item, favorite picks via _get_favorite_team_picks, then round_picks; complete/simulate path: respect post_draft_show and iterate favorites and/or rounds 1..display_rounds filtering "TBD"; pre-draft path: skip if _is_off_season() else include favorites + current display round), and ensure callers handle their early-return conditions (_is_post_draft_window or _is_off_season) before/after calling the helper; then change _create_draft_scroll_image to call _build_content_items and pass the result to scroll_helper.create_scrolling_image, and change get_vegas_content to return the same list from _build_content_items.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/nfl-draft/manager.py`:
- Around line 810-878: Extract the shared content-building logic into a new
helper (e.g. _build_content_items) that returns List[Image.Image] and
encapsulates the three-branch flow currently duplicated in
_create_draft_scroll_image and get_vegas_content (live path: include
nfl_draft_logo, _get_display_round + _create_round_label_item, favorite picks
via _get_favorite_team_picks, then round_picks; complete/simulate path: respect
post_draft_show and iterate favorites and/or rounds 1..display_rounds filtering
"TBD"; pre-draft path: skip if _is_off_season() else include favorites + current
display round), and ensure callers handle their early-return conditions
(_is_post_draft_window or _is_off_season) before/after calling the helper; then
change _create_draft_scroll_image to call _build_content_items and pass the
result to scroll_helper.create_scrolling_image, and change get_vegas_content to
return the same list from _build_content_items.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1461b349-7052-4f2a-98a6-aa32e04a0e5c
⛔ Files ignored due to path filters (1)
plugins/nfl-draft/nfl_draft_logo.pngis excluded by!**/*.png
📒 Files selected for processing (6)
plugins.jsonplugins/nfl-draft/README.mdplugins/nfl-draft/config_schema.jsonplugins/nfl-draft/manager.pyplugins/nfl-draft/manifest.jsonplugins/nfl-draft/requirements.txt
✅ Files skipped from review due to trivial changes (3)
- plugins/nfl-draft/requirements.txt
- plugins/nfl-draft/config_schema.json
- plugins/nfl-draft/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins.json
…/Vegas duplication (v1.3.9) The three-branch content-building logic (live / complete+simulate / pre-draft) was duplicated verbatim across _create_draft_scroll_image and get_vegas_content. New _build_content_items(picks=None) encapsulates all three branches and returns List[Image.Image]. Callers retain their own early-return guards: - _create_draft_scroll_image: checks window-expired and off-season before calling, then passes the result to scroll_helper.create_scrolling_image. - get_vegas_content: keeps its existing None-return guards, then delegates to _build_content_items(picks=picks) passing the lock snapshot. Removed the now-unused is_live snapshot variable from get_vegas_content. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4 tasks
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.
Summary
post_draft_days, default 7): after the draft concludes, displays all favorite-team picks in pick-number order followed by rounds 1–N (display_rounds, default 3), each with a round labelget_vegas_contentreturnsNoneanddisplay()renders a solid black frame — no messages, no errors, plugin drops out of rotation entirely until FebruaryNew optional config keys (backward-compatible defaults):
Also adds
timedeltaimport (was missing),_is_post_draft_window(),_is_off_season(), and_display_blank()helper.Test plan
draft_status = "complete"(current state) — confirm rounds 1–3 scroll with round labelsfavorite_teamsis configuredpost_draft_dayswindow — confirm plugin goes fully silent (no display, no Vegas entry)display_rounds: 2limits display to rounds 1–2 onlyis_draft_live = True)🤖 Generated with Claude Code
Summary by CodeRabbit