Skip to content

Sports unification phase 0: safety net, cwd-independent fonts, element_style - #425

Merged
ChuckBuilds merged 7 commits into
mainfrom
claude/sports-unification-safety-net
Aug 2, 2026
Merged

Sports unification phase 0: safety net, cwd-independent fonts, element_style#425
ChuckBuilds merged 7 commits into
mainfrom
claude/sports-unification-safety-net

Conversation

@ChuckBuilds

@ChuckBuilds ChuckBuilds commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Pull Request

Summary

Phase 0 of the shared-sports-code unification planned in ledmatrix-plugins#239 (see its docs/plugin-development/08-shared-sports-code.md): build the safety net before any refactor of src/base_classes/sports.py, and close two core gaps that PR surfaced — cwd-dependent font resolution and the missing src/element_style.py module three plugins already guard-import.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Refactor (no functional change)
  • Build / CI
  • Plugin work (link to the plugin)

Related issues

Refs ledmatrix-plugins#239 and complements #424 (global_config on BasePlugin).

What's in here (5 commits)

  1. fix(fonts): FontManager resolved assets/fonts relative to the process cwd, so any process started outside the install root (the plugin safety harness on CI being the recurring case) silently lost every font and degraded plugins to PIL's default face — three plugins grew per-plugin workarounds for exactly this. Catalog population now falls back to the install root derived from the module's own location; behavior from the install root is unchanged.
  2. docs: seed CHANGELOG.md with the module-availability release discipline the plugins repo's bundled-copy sunset rule depends on.
  3. ci: a dedicated unit-tests job enrolling the suites that existed but never ran in CI (skin system, font manager, data sources, extractors, scroll helper, adaptive layout, loader compat) plus the two new suites below — 338 tests.
  4. feat: src/element_style.py — the per-element style resolver of-the-day, ledmatrix-music, and football-scoreboard already import behind guarded fallbacks. Untouched configs resolve to exactly the classic values (byte-identical rendering, proven by of-the-day's committed goldens passing unchanged); schema_manager.load_schema() expands x-style-elements declarations so the config form shows the richer UI. of-the-day's previously-skipped 9-test spec suite now runs and passes.
  5. test: a 41-test characterization suite pinning src/base_classes/sports.py behavior (extractor key contract per sport, update() flows against cache-seeded fixtures under frozen time, rendering smoke, skin-seam guard rails) ahead of the phase-1 merge of the nine drifted plugin copies. Five surprising behaviors are pinned AS-IS and flagged in comments — notably hockey dropping events whose competitors lack statistics, and baseball reading the event-level status for innings — so the merge changes them knowingly or not at all.

Test plan

  • Ran on a real Raspberry Pi with hardware
  • Ran in emulator mode (EMULATOR=true python3 run.py)
  • Ran the dev preview server (scripts/dev_server.py)
  • Ran the test suite (pytest)
  • Manually verified the affected code path in the web UI
  • N/A — documentation-only change

All 338 tests in the new unit job pass; the existing plugin-safety job's 60 tests pass; FontManager verified resolving real faces from a foreign cwd; the of-the-day plugin harness passes with every committed golden unchanged (classic-identity proof for element_style); football's 27 adaptive/resolver tests and music's 38 plugin tests pass against the new module.

Documentation

  • I updated README.md if user-facing behavior changed
  • I updated the relevant doc in docs/ if developer behavior changed
  • I added/updated docstrings on new public functions
  • N/A — no docs needed

(CHANGELOG.md documents both the fix and the new module against the release discipline; the plugins repo's 08-shared-sports-code.md is the cross-repo companion.)

Plugin compatibility

  • No plugin breakage expected
  • Some plugins will need updates — listed below
  • N/A — change doesn't touch the plugin system

Existing consumers of src.element_style activate their already-shipped code paths; plugins without it are untouched. The font fix only adds a fallback path.

Checklist

  • My commits follow the message convention in CONTRIBUTING.md
  • I read CONTRIBUTING.md and CODE_OF_CONDUCT.md
  • I've not committed any secrets or hardcoded API keys
  • If this adds a new config key, the form in the web UI was
    verified (the form is generated from config_schema.json)

(No new config keys — x-style-elements expansion only affects plugins that already declare it.)

Notes for reviewer

The characterization suite deliberately pins two behaviors that look like real bugs (hockey's statistics requirement, baseball's event-level status read). Fixing them belongs to phase 1 where the plugin copies' corrected versions get merged back — this PR only makes the current behavior visible and diff-able. Phase 1 (merging the universal plugin-copy deltas into src/base_classes/sports.py) and phase 2 (opt-in celebrations/rotation capabilities) build on this safety net.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4


Generated by Claude Code

Summary by CodeRabbit

  • New Features
    • Added per-element style customization, including fonts, colors, sizes, and layout offsets.
    • Supports compact and manually defined style configurations with safe fallback handling.
  • Bug Fixes
    • Improved font discovery when the application is launched outside the project directory.
    • Invalid style settings and unavailable fonts now degrade gracefully to classic styling.
  • Documentation
    • Added a changelog covering compatibility requirements, plugin behavior, and recent improvements.
  • Tests
    • Expanded coverage for style customization, font loading, schema handling, and sports data flows.

claude added 5 commits August 1, 2026 14:08
FontManager built its catalog from cwd-relative paths ('assets/fonts'),
so any process started outside the install root — the plugin safety
harness on CI being the recurring case — found no fonts and silently
degraded every plugin to PIL's default face. Several plugins grew
per-plugin workarounds for exactly this (countdown, text-display,
tide-display in the plugins monorepo).

Catalog population now falls back to the install root derived from this
module's location when the cwd-relative path is missing; behavior when
running from the install root is unchanged. Verified: resolve_font
returns the real FreeType face from a foreign cwd, and the full unit
suites (266 tests) pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
The plugins monorepo's sunset rule ('delete a bundled fallback copy only
when the manifest floors on the first core release shipping the module')
needs core module additions recorded against version numbers. Seeds the
changelog at 3.1.0 and documents the discipline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
The existing workflow ran only the three plugin-harness suites; the
skin-system, font-manager, data-source, extractor, scroll-helper,
adaptive-layout, and loader-compat suites (266 tests) existed but never
ran in CI, so a refactor of src/base_classes or src/common could regress
them silently. Also enrolls the new sports characterization and
element-style suites landing in this branch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
… already expect

Three plugins (of-the-day, ledmatrix-music, football-scoreboard) import
src.element_style behind guarded try/except with classic fallbacks, but
the module never existed in core, so the richer per-element styling UI
those code paths implement has been dormant. This lands it:

- ElementStyleResolver.style() resolves per-element font/size/color with
  the key semantic the consumers encode: a config value counts as
  user-forced only when it differs from the schema default (the web UI
  bakes defaults into config.json on save), and untouched configs
  resolve to exactly the caller's classic values — byte-identical
  rendering, proven by of-the-day's committed goldens passing unchanged.
- defaults_from_schema_file parses both declaration forms (the compact
  x-style-elements map and hand-written customization blocks).
- expand_style_elements() expands x-style-elements into full config
  blocks; schema_manager.load_schema() applies it (guarded, no-op for
  schemas without the declaration) so the config form and defaults
  merging see the expanded UI.
- Fonts resolve cwd-independently with (path, size) caching; .bdf loads
  via freetype like FontManager; nothing in the module raises out of
  style().

Verified: 31 new unit tests; of-the-day's previously-skipped 9-test
spec suite now runs and passes; football's resolver tests pass (27);
music's 38 plugin tests pass; schema-manager suites pass (43).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
…unification

Pins current behavior before the planned merge of the nine drifted
plugin copies back into this ancestor: the _extract_game_details_common
key contract per sport (reusing GUARANTEED_KEYS from the skin tests),
update() flows for upcoming/recent/live against cache-seeded fixtures
under frozen time, rendering smoke per mode class, and guard rails on
the skin-system seam.

Five surprising behaviors are pinned AS-IS and flagged in comments so
the merge changes them knowingly or not at all: is_upcoming also
matching status.type.name; hockey dropping events whose competitors
lack 'statistics'; baseball reading the event-level status for innings;
no past-date filter in upcoming; and favorites-only mode with an empty
favorites list showing nothing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@ChuckBuilds, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3d4cfa63-c1d2-4f82-a9ce-823f2d0d769b

📥 Commits

Reviewing files that changed from the base of the PR and between 64e8f87 and 81a8dcd.

📒 Files selected for processing (3)
  • src/element_style.py
  • test/test_element_style.py
  • test/test_sports_base_characterization.py
📝 Walkthrough

Walkthrough

The pull request adds schema-driven element styling, cwd-independent font loading, sports characterization tests, a dedicated core-test workflow, and changelog entries.

Changes

Element styling

Layer / File(s) Summary
Element style resolution
src/element_style.py
Adds schema expansion, default extraction, style overrides, layout offsets, cached font loading, and safe fallback handling.
Schema and font integration
src/plugin_system/schema_manager.py, src/font_manager.py, test/test_element_style.py
Expands style declarations during schema loading, resolves font assets independently of the working directory, and tests defaults, overrides, malformed input, fonts, and integration behavior.

Sports characterization

Layer / File(s) Summary
Cross-sport extraction coverage
test/test_sports_base_characterization.py
Tests shared ESPN extraction behavior and sport-specific football, hockey, baseball, and basketball data.
Schedule update flows
test/test_sports_base_characterization.py
Tests upcoming, recent, and live update modes with filtering, limits, empty data, and disabled managers.
Rendering and seam validation
test/test_sports_base_characterization.py
Tests rendering output and required skin, display, update, and extraction interfaces.

CI and release records

Layer / File(s) Summary
Core test workflow
.github/workflows/test.yml
Adds read-only repository permissions and a Python 3.12 unit-tests job for selected headless suites.
Release changelog
CHANGELOG.md
Documents compatibility guidance, unreleased styling and test changes, the font fix, and the 3.1.0 baseline.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SchemaManager
  participant ElementStyleResolver
  participant load_font
  SchemaManager->>SchemaManager: load_schema
  SchemaManager->>SchemaManager: expand_style_elements
  SchemaManager-->>ElementStyleResolver: expanded schema and defaults
  ElementStyleResolver->>load_font: load_font(font_name, size)
  load_font-->>ElementStyleResolver: cached or fallback font
  ElementStyleResolver-->>SchemaManager: resolved style values
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 accurately summarizes the main changes: sports unification safety tests, cwd-independent fonts, and element-style support.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/sports-unification-safety-net

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

Comment @coderabbitai help to get the list of available commands.

Comment thread .github/workflows/test.yml Fixed
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 124 complexity · 0 duplication

Metric Results
Complexity 124
Duplication 0

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.

CodeQL flagged the new unit-tests job for running with the default
unrestricted token; the pre-existing job had the same exposure. Both
jobs only check out the repo and run pytest, so a workflow-level
contents:read is sufficient.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
@ChuckBuilds

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (6)
.github/workflows/test.yml (1)

52-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin RGBMatrixEmulator in CI.

.github/workflows/test.yml installs the package with an unbounded specifier in both jobs, and requirements-emulator.txt also allows any release. Add a tested exact version or constraints entry and remove the unbounded installs.

🤖 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 @.github/workflows/test.yml around lines 52 - 56, Pin RGBMatrixEmulator to
the tested exact version or shared constraints entry in both CI jobs and
requirements-emulator.txt. Update the dependency installation steps and remove
the unbounded RGBMatrixEmulator installs, while keeping the existing dependency
installation flow intact.
test/test_element_style.py (1)

236-243: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a fixture that clears the module-global _font_cache between tests.

test_same_font_object_from_cache asserts identity across two resolvers. The assertion depends on src.element_style._font_cache, which persists for the whole session. The current suite stays deterministic because every cache key uses an absolute resolved path. A future test that asserts a cache miss, or that patches font loading, would become order-dependent. Reset the global in setup.

As per coding guidelines: "Clean setup and teardown between tests to reset state".

♻️ Proposed fixture
 def _resolver(config, schema_path):
     return ElementStyleResolver(config, defaults_from_schema_file(schema_path))
+
+
+@pytest.fixture(autouse=True)
+def _clear_font_cache():
+    """Reset the module-global font cache so tests stay order-independent."""
+    from src import element_style
+    element_style._font_cache.clear()
+    yield
+    element_style._font_cache.clear()

Keep test_same_font_object_from_cache as is. Both style() calls run inside one test, so the cache is still populated by the first call.

🤖 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 `@test/test_element_style.py` around lines 236 - 243, Add an autouse fixture in
test/test_element_style.py that clears the module-global _font_cache before each
test, and optionally after each test if needed for teardown. Import or reference
the cache through its existing element_style module symbol without changing
test_same_font_object_from_cache, so both style() calls still share the cache
within that test.

Source: Coding guidelines

test/test_sports_base_characterization.py (4)

584-591: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Align the test name with its assertions.

The name states that the call does not raise. The body also asserts that the placeholder renderer draws ink. Rename the test so the name reports both checks.

♻️ Suggested rename
-    def test_draw_scorebug_layout_direct_call_does_not_raise(
+    def test_draw_scorebug_layout_direct_call_draws_ink(
             self, build_manager):

As per coding guidelines: "Use descriptive test names that explain what they test".

🤖 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 `@test/test_sports_base_characterization.py` around lines 584 - 591, Rename
test_draw_scorebug_layout_direct_call_does_not_raise to a descriptive name that
explicitly indicates both the direct call does not raise and the placeholder
renderer produces visible image content; leave the test assertions and
implementation unchanged.

Source: Coding guidelines


354-368: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add docstrings to the harness classes, and consider one shared _fetch_data.

_RecentHarness and _LiveHarness have no docstring, while _UpcomingHarness has one. The three classes also repeat the identical _fetch_data body. A small mixin removes the duplication and keeps the harness intent in one place.

♻️ Suggested consolidation
-class _UpcomingHarness(Hockey, SportsUpcoming):
-    """Cheapest concrete SportsUpcoming: hockey extractor + cache-fed data."""
-
-    def _fetch_data(self):
-        return self.cache_manager.get(f"{self.sport_key}_schedule")
-
-
-class _RecentHarness(Hockey, SportsRecent):
-
-    def _fetch_data(self):
-        return self.cache_manager.get(f"{self.sport_key}_schedule")
-
-
-class _LiveHarness(HockeyLive):
-
-    def _fetch_data(self):
-        return self.cache_manager.get(f"{self.sport_key}_schedule")
+class _CacheFedMixin:
+    """Replaces the network fetch with a read from the mocked cache."""
+
+    def _fetch_data(self):
+        return self.cache_manager.get(f"{self.sport_key}_schedule")
+
+
+class _UpcomingHarness(_CacheFedMixin, Hockey, SportsUpcoming):
+    """Cheapest concrete SportsUpcoming: hockey extractor + cache-fed data."""
+
+
+class _RecentHarness(_CacheFedMixin, Hockey, SportsRecent):
+    """Cheapest concrete SportsRecent: hockey extractor + cache-fed data."""
+
+
+class _LiveHarness(_CacheFedMixin, HockeyLive):
+    """Cheapest concrete SportsLive: hockey live extractor + cache-fed data."""

As per coding guidelines: "Use docstrings for classes and complex functions".

🤖 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 `@test/test_sports_base_characterization.py` around lines 354 - 368, Add
docstrings to _RecentHarness and _LiveHarness describing their test-harness
roles, and consolidate the identical _fetch_data implementation into a shared
mixin or base helper reused by _UpcomingHarness, _RecentHarness, and
_LiveHarness. Preserve each class’s existing inheritance and cache lookup
behavior.

Source: Coding guidelines


138-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add type hints to the module helpers.

make_probe, extract, make_event, and _competitor have no parameter or return annotations. Annotations make the fixture shapes explicit for future maintainers of these characterization tests.

♻️ Suggested annotations
-def make_probe(favorites=None):
+def make_probe(favorites: list[str] | None = None) -> MagicMock:
     """Bare-bones SportsCore stand-in for exercising the real extractors
     unbound (same pattern as TestViewModelContract in test_skin_system)."""
@@
-def extract(sport_cls, event, favorites=None):
+def extract(sport_cls: type, event: dict,
+            favorites: list[str] | None = None) -> dict | None:
     return sport_cls._extract_game_details(make_probe(favorites), event)

As per coding guidelines: "Use type hints for function parameters and return values".

🤖 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 `@test/test_sports_base_characterization.py` around lines 138 - 156, Add
parameter and return type annotations to the test helper functions make_probe,
extract, make_event, and _competitor, using types that accurately describe their
fixture inputs, generated objects, and optional favorites. Keep the helper
behavior unchanged and ensure all function parameters and return values are
explicitly typed.

Source: Coding guidelines


508-520: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Split the two favorite-team scenarios into separate tests.

This test covers a matching favorite and a non-matching favorite. Separate tests report which scenario regressed without further inspection.

♻️ Suggested split
     def test_filters_by_favorite_teams(self, build_manager):
         manager = build_manager(_RecentHarness, make_schedule(),
                                 show_favorite_teams_only=True,
                                 favorite_teams=["TOR"])
         manager.update()
         assert _ids(manager.games_list) == ["9001"]
 
+    def test_unknown_favorite_team_yields_no_games(self, build_manager):
         stranger = build_manager(_RecentHarness, make_schedule(),
                                  show_favorite_teams_only=True,
                                  favorite_teams=["XXX"])
         stranger.update()
         assert stranger.games_list == []
         assert stranger.current_game is None

As per coding guidelines: "Each test should verify a single responsibility".

🤖 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 `@test/test_sports_base_characterization.py` around lines 508 - 520, Split
test_filters_by_favorite_teams into two tests: one verifying matching favorite
teams returns game "9001", and another verifying a non-matching team produces an
empty games_list and no current_game. Keep each test’s existing setup and
assertions focused on its single scenario.

Source: Coding guidelines

🤖 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 `@src/element_style.py`:
- Around line 105-116: Update the relative-path handling in the font resolution
logic to accept only a bare filename, rejecting any font_name containing path
components before constructing candidates. Preserve absolute-path handling and
existing candidate lookup for valid filenames, returning None for invalid
relative values.

In `@test/test_sports_base_characterization.py`:
- Around line 423-430: Update the manager fixture around cls(...) to patch
requests.Session before constructing the manager, configuring every created
session’s get operation to raise requests.exceptions.ConnectionError; keep the
offline guard in place during construction so both SportsCore.session and
ESPNDataSource.session are covered, then return the constructed manager.

---

Nitpick comments:
In @.github/workflows/test.yml:
- Around line 52-56: Pin RGBMatrixEmulator to the tested exact version or shared
constraints entry in both CI jobs and requirements-emulator.txt. Update the
dependency installation steps and remove the unbounded RGBMatrixEmulator
installs, while keeping the existing dependency installation flow intact.

In `@test/test_element_style.py`:
- Around line 236-243: Add an autouse fixture in test/test_element_style.py that
clears the module-global _font_cache before each test, and optionally after each
test if needed for teardown. Import or reference the cache through its existing
element_style module symbol without changing test_same_font_object_from_cache,
so both style() calls still share the cache within that test.

In `@test/test_sports_base_characterization.py`:
- Around line 584-591: Rename
test_draw_scorebug_layout_direct_call_does_not_raise to a descriptive name that
explicitly indicates both the direct call does not raise and the placeholder
renderer produces visible image content; leave the test assertions and
implementation unchanged.
- Around line 354-368: Add docstrings to _RecentHarness and _LiveHarness
describing their test-harness roles, and consolidate the identical _fetch_data
implementation into a shared mixin or base helper reused by _UpcomingHarness,
_RecentHarness, and _LiveHarness. Preserve each class’s existing inheritance and
cache lookup behavior.
- Around line 138-156: Add parameter and return type annotations to the test
helper functions make_probe, extract, make_event, and _competitor, using types
that accurately describe their fixture inputs, generated objects, and optional
favorites. Keep the helper behavior unchanged and ensure all function parameters
and return values are explicitly typed.
- Around line 508-520: Split test_filters_by_favorite_teams into two tests: one
verifying matching favorite teams returns game "9001", and another verifying a
non-matching team produces an empty games_list and no current_game. Keep each
test’s existing setup and assertions focused on its single scenario.
🪄 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 Plus

Run ID: fdd3ec9e-1e83-4e4b-8598-62aebb19b1ee

📥 Commits

Reviewing files that changed from the base of the PR and between 5b45f35 and 64e8f87.

📒 Files selected for processing (7)
  • .github/workflows/test.yml
  • CHANGELOG.md
  • src/element_style.py
  • src/font_manager.py
  • src/plugin_system/schema_manager.py
  • test/test_element_style.py
  • test/test_sports_base_characterization.py

Comment thread src/element_style.py
Comment thread test/test_sports_base_characterization.py
Two Minor findings from CodeRabbit's first review of this PR.

- resolve_font_path: reject relative font names carrying path components.
  font_name comes from plugin config, which the web UI writes; a value like
  "../../config/config.json" escaped assets/fonts/ after os.path.join and let
  a config probe arbitrary paths for existence (disclosure unlikely, since
  Pillow/freetype reject non-font files, but the probe is real). Relative
  names must now be bare filenames (os.path.basename(name) == name); absolute
  paths keep their existing isfile() gate. Test confirms the traversal
  resolved the real config.json before the guard.

- build_manager fixture: patch requests.Session.get BEFORE constructing the
  manager. Construction creates both SportsCore.session and the
  ESPNDataSource.session; the old code only replaced manager.session after
  the fact, leaving data_source.session real and able to reach the network on
  an accidental fetch. Patching the class makes every session built in the
  fixture offline.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
@ChuckBuilds
ChuckBuilds merged commit 82a65ad into main Aug 2, 2026
17 checks passed
ChuckBuilds added a commit that referenced this pull request Aug 2, 2026
#425 was squash-merged, so #426's base content now lives in main under a
different SHA. Reconciled the three resulting conflicts:

- .github/workflows/test.yml: kept #426's superset test list (its unification
  suites plus test_element_style.py); main had only element_style from #425.
- CHANGELOG.md: kept #426's version, which promotes main's "Unreleased"
  section into the "3.2.0" release and is a verified superset of main's
  content (no lines dropped).
- test_sports_base_characterization.py: kept #426's side for all three blocks.
  Two are semantic, not cosmetic: #425 pinned the pre-fix behaviour
  (hockey/baseball events dropped -> *_returns_none), while #426 carries the
  actual fixes (2486bdb, 2eea7a7) and updated the tests to *_still_extracts.
  Taking main's side would fail against #426's fixed code. The third is the
  get_background_service import path, which is .core in #426 after the package
  split. Also refreshed a stale comment that still named the removed test.

#424's global_config plumbing merges in cleanly; verified the interlock end to
end on the merged tree (global_config target_fps -> SportsScrollDisplay 90.0).
435 tests pass across the affected suites; the lone failure
(test_get_system_status) is one of the four pre-existing on main.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
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.

3 participants