Skip to content

perf(sports): drop get_cycle_duration's diagnostics to debug level - #309

Closed
ChuckBuilds wants to merge 1 commit into
mainfrom
perf/cycle-duration-log-level
Closed

perf(sports): drop get_cycle_duration's diagnostics to debug level#309
ChuckBuilds wants to merge 1 commit into
mainfrom
perf/cycle-duration-log-level

Conversation

@ChuckBuilds

Copy link
Copy Markdown
Owner

get_cycle_duration() opens with an unconditional INFO line naming its own arguments, and five sports plugins carry 8–12 more through the body:

self.logger.info(f"get_cycle_duration() called with display_mode={display_mode}, is_enabled={self.is_enabled}")
self.logger.info(f"get_cycle_duration() returning None: ...")
self.logger.info(f"get_cycle_duration: extracted mode_type={mode_type}, league={league} ...")

All of it describes the function's own control flow, not anything that happened to the display.

Why the level matters here

The core calls this from the main display loop. In display_controller.run():

while True:
    ...
    if not display_result:   # nothing rendered
        ...
    else:                    # content was shown
        base_duration = self._get_display_duration(active_mode)
        if dynamic_enabled:
            plugin_cycle_duration = self._plugin_cycle_duration(manager_to_display, active_mode)

That loop is paced at frame rate, so each of these INFO lines lands at frame rate for as long as the mode is on screen — and every line is a journal write to an SD card.

Honest scoping

I could not measure this firing, and I want to be clear about that. It is gated behind dynamic_duration being enabled; the rig I have access to has that unset, and its journal shows zero such lines in six hours. So this is a latent flood, not one I caught in the act.

It is not an obscure setting though — football ships DYNAMIC_DURATION.md explaining how to turn it on, so users are actively encouraged into the path.

Why demote rather than throttle

Same shape as the has_live_content flood fixed in #241 — a per-frame caller meeting logging written as if it ran once. But unlike the live-content summary, none of this is worth an operator's attention even once. It is what you turn debug on to see, so debug is where it belongs. No throttle state to maintain.

Verification

48 lines demoted across five plugins, zero logger.info remaining in those functions, no behaviour change. Safety harness: 104 pass, 0 fail.

Merge order

Versions land above the ones in #308 (which touches three of the same plugins), so the two merge cleanly in ascending version order: #308 first, then this.

get_cycle_duration() opens with an unconditional INFO line naming its
arguments, and five sports plugins carry between eight and twelve more
through the body -- "returning None", "extracted mode_type=", and so on.
All of it is diagnostic: it describes the function's own control flow, not
anything that happened to the display.

The core calls this from the main display loop. In display_controller.run(),
once a plugin has rendered content, the else branch reads the base duration
and then, if dynamic duration is enabled for the mode, queries the plugin for
its cycle duration. That loop is paced at frame rate, so every one of those
INFO lines lands at frame rate for as long as the mode is on screen.

It is gated behind dynamic_duration being enabled, which is why it is not
visible on the rig I can measure -- that config has the feature off, and the
journal shows zero such lines in six hours. It is not an obscure setting
though; football ships DYNAMIC_DURATION.md telling users how to turn it on.

Same shape as the has_live_content flood fixed in #241: a per-frame caller
meeting logging written as if it ran once. Demoted rather than throttled,
because unlike the live-content summary none of this is worth an operator's
attention even once -- it is what you turn debug on to see.

48 lines across five plugins. No behaviour change.

Safety harness: 104 pass, 0 fail across all five.

Versions land above the ones in #308 so the two merge in ascending order.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d5a9d45a-0167-4b3c-8376-4e5144dd8482


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.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric Results
Complexity 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.

@ChuckBuilds

Copy link
Copy Markdown
Owner Author

Correction to the merge-order note in the description: I wrote that these "merge cleanly in ascending version order". They do not merge cleanly. I test-merged them and they conflict.

Ascending order decides the correct resolution; it does not avoid the conflict. Both PRs edit the same "version" line from the same base (1.13.1 → 1.13.2 here, 1.13.1 → 1.13.3 there), so git sees two edits to one line regardless of which lands first.

What I verified, merging #308 then #309 onto main:

conflicted files:
   plugins.json
   plugins/basketball-scoreboard/manifest.json
   plugins/hockey-scoreboard/manifest.json
   plugins/lacrosse-scoreboard/manifest.json

manager.py conflicts: 0

The code composes cleanly. Only version bookkeeping collides, and plugins.json is generated rather than authored.

Resolution, which I ran through end to end:

  1. Take the higher version in each manifest — basketball 1.16.4, hockey 1.13.3, lacrosse 1.12.3.
  2. python update_registry.py to regenerate plugins.json rather than resolving it by hand.

The result carries both changes: the throttle fix's _last_live_content_state is present, and zero logger.info remain in get_cycle_duration. Safety harness on the merged tree: 56 pass, 0 fail across the three.

So the ordering advice stands — #308 first, then this — but expect to resolve the version lines rather than fast-forward through them.

@ChuckBuilds

Copy link
Copy Markdown
Owner Author

Superseded by #315, which combines this with the other five sports PRs so each plugin gets one version bump instead of up to four competing ones. Every change from this PR is verified present on that branch; the branch here is untouched if you want to compare.

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.

2 participants