fix(leaderboard): move missing-logo downloads off the render thread - #271
Merged
ChuckBuilds merged 2 commits intoAug 12, 2026
Merged
Conversation
_get_team_logo ran from layout construction inside display(), and on a cache miss it invoked download_missing_logo -- a network call on the render thread the Vegas scroll depends on. Missing logos are now backfilled by a new download_missing_logos(), called from update() instead; _get_team_logo only ever reads local files, and a backfilled logo becomes visible on the next rebuild via the existing mtime-keyed cache. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRsTMmLUcpd8tpN1qciEgC
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 | 7 |
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.
Codacy flagged an unused local in the new test coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRsTMmLUcpd8tpN1qciEgC
ChuckBuilds
added a commit
that referenced
this pull request
Aug 12, 2026
* perf(leaderboard): decode each logo once, not once per rebuild Image.open defers the PNG decode to first access, so the decode landed inside _prepare_logo's convert(), with a LANCZOS resize behind it -- for every team, on every rebuild, for files that had not changed. Measured on the real asset directory: 25 logos cost 276ms cold and 0.3ms warm. It matters more than a normal cache miss because leaderboard scroll content is generated on the render thread. The adapter's capture path needs the shared canvas, so it cannot be prepared in the background (render_pipeline.drain_deferred explains why), and the cost lands directly on the Vegas scroll as stutter. Keyed on path and mtime, so a logo the downloader backfills is picked up rather than served stale. A missing file is deliberately not cached: remembering it as absent would keep a logo downloaded moments later invisible until restart. Honest scope: this does not fix the 3.2s freeze that led here. That one is still present with this change in place, and the watchdog work on the core side now shows why it was never caught -- it holds the GIL. This removes a real and separate cost on the same path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(leaderboard): move missing-logo downloads off the render thread (#271) * fix(leaderboard): move missing-logo downloads off the render thread _get_team_logo ran from layout construction inside display(), and on a cache miss it invoked download_missing_logo -- a network call on the render thread the Vegas scroll depends on. Missing logos are now backfilled by a new download_missing_logos(), called from update() instead; _get_team_logo only ever reads local files, and a backfilled logo becomes visible on the next rebuild via the existing mtime-keyed cache. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRsTMmLUcpd8tpN1qciEgC * test(leaderboard): drop unused variable in logo cache test Codacy flagged an unused local in the new test coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRsTMmLUcpd8tpN1qciEgC --------- Co-authored-by: Claude <noreply@anthropic.com> --------- 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.
What
Addresses the CodeRabbit review comment on #270:
_get_team_logoran fromlayout construction inside
display()(the render thread the Vegas scrolldepends on), and on a cache miss it called
download_missing_logo— anetwork request that could stall the scroll. This targets that PR's branch
directly.
_get_team_logonow only ever reads a local file; it never invokes thedownloader.
ImageRenderer.download_missing_logos(leaderboard_data)backfills anymissing team logo files from the network.
manager.update()calls itafter fetching standings — off the render thread, alongside the plugin's
other network I/O.
after the fact, so a logo downloaded in
update()becomes visible on thenext rebuild with no further wiring needed.
Skipped
The review comment also asked to update
test_logo_cache.py#L97-115. Thoselines test
_cached_prepared_logo's generic cache-miss-is-retried behaviorvia a synthetic
loadcallback — they never call_get_team_logoor thedownloader, so they were not exercising the code path this fix changes and
didn't need updating. Instead, added new coverage that directly targets the
fixed behavior:
_get_team_logonever invokes the downloader on a miss, anddownload_missing_logos(theupdate()path) does, with a subsequentlywritten local file becoming visible on a later render.
Tests
plugins/ledmatrix-leaderboard/test_logo_cache.py— all checks pass,including the two new sections covering this fix.
Bumped
manifest.jsonto 1.3.2 per repo convention; pre-commit hook syncedplugins.json.Generated by Claude Code