refactor(display): run() stage 1 - golden traces and extracted helpers, no behaviour change by ChuckBuilds · Pull Request #704 · ChuckBuilds/LEDMatrix

ChuckBuilds · 2026-10-01T01:18:52Z

Stage 1 of restructuring DisplayController.run() into an Arbiter / ScreenRunner / Sources (roadmap theme T1, item 8). No behaviour change. The plan for the rest is in docs/RUN_LOOP_REDESIGN.md.

What's in it

1. Golden trace tests

test/_run_loop_harness.py builds a real DisplayController through __init__ and runs the real run() on fakes:

fake plugins, cache, config service, plugin manager, sync manager and display manager;
one fake clock patched in for the module's time and datetime. Sleeping only advances it, and scripted events (requests, WiFi files, live windows) fire as it passes them.

The first frame of each screen still goes through the real PluginExecutor and the per-plugin locks. The run stops at a horizon via a BaseException, so run()'s except Exception can't swallow it, and the harness fails if run() returns early or spins.

Each trace row is [start, mode, duration, exit_reason, frames, force_clear], and notable events (requests, live switches, schedule, brightness) are listed with their time. The traces are in test/fixtures/run_loop_golden/. There are 15 scenarios:

Scenario	Covers
`plain_rotation`	`display_durations` override, a plugin's own duration, a high-FPS scroller (1252 frames / 10 s), a legacy `display()` without `display_mode`
`empty_modes`	a mode with no content skipped at once, a mode with no plugin, a plugin whose later frames return False (the 1 s loop breaks and the dwell is made up)
`all_empty`	one empty rotation, then the 1 s pause per pass
`plugin_error`	a dispatch that raises skips all of that plugin's modes, and two failures open the breaker; `display()` raising inside the executor counts as an empty pass
`dynamic_duration`	cycle complete after the minimum plus grace, plugin cap vs `get_cycle_duration`, global cap, cycle resets
`live_priority`	live content takes over and hands back to the saved rotation index
`live_round_robin`	two live games taking turns
`on_demand`	start mid-screen, stop by request, a timed request that expires (on-demand clamp)
`on_demand_pinned`	a pinned mode only; an on-demand mode with no content
`on_demand_restored`	a session resumed after a restart
`schedule`	dim at 23:00, off at 23:02, an on-demand override during downtime, back on at 23:05
`wifi_notice`	the notice dwell; a notice held back while on-demand is active
`follower`	the sync-follower branch at 60 fps
`vegas`	Vegas iterations, preempted by live, by on-demand and by a WiFi notice
`vegas_live_in_ticker`	Vegas keeping live content in the ticker

Vegas uses FakeVegas, a contract fake. It returns True after its cycle and False when the interrupt or live check asks it to yield, checking at the real coordinator's cadence. Driving the real coordinator on the fake clock is stage 4 work, as the doc explains.

All goldens were generated from main's run() before any code moved. They pass unchanged on this branch.

2. Helpers extracted from run(), verbatim

Helper	Block	Future home
`_blank_while_scheduled_off`	the schedule-off pass	ScheduledOff gate
`_run_follower_frame`	the follower dead-reckoning frame	Follower source
`_show_wifi_notice`	the WiFi status pass	Wifi source
`_resolve_active_mode`	on-demand mode vs rotation mode	OnDemand source
`_plugin_for_mode`	mode to plugin, with the circuit-breaker skip	Arbiter
`_dispatch_first_frame`	the executor-bound first frame and lock release	ScreenRunner
`_skip_failed_plugin_modes`	skipping a failed plugin's other modes	Rotation
`_track_dynamic_cycle`	cycle reset on a new dynamic mode	ScreenRunner
`_resolve_durations`	min/max duration, caps, validation (changes no state)	ScreenPlan bounds
`_clamp_to_on_demand`	the on-demand remaining-time clamp	ScreenPlan bounds
`_needs_high_fps`	the 125 Hz / 1 Hz decision	`frame_policy` (stage 5)
`_advance_after_screen`	next mode, or the live hold	Rotation

The two frame loops, the Vegas branch and every early exit stay in run(). That keeps frame timing untouched and leaves the rest of the file alone. The only non-mechanical edit drops two and not wifi_status_data conditions. Past the WiFi block that variable was always None: a shown message ends the pass, and a failed one is reset.

3. docs/RUN_LOOP_REDESIGN.md

The doc covers the target design and stages 2-5, each with how it is verified. It also lists seven current behaviours the traces pin down that look wrong. They are left as they are here, and each should get its own PR with a golden diff:

a WiFi notice posted mid-screen is never shown;
Vegas yields to a WiFi notice, then shows a rotation screen instead of it;
Vegas yields to live content, then shows one rotation screen first;
live content only takes over between screens;
an on-demand session that expires during scheduled-off keeps the panel on until the next minute;
a schedule window's end minute is inclusive;
a plugin whose display() raises inside the executor is recorded as a breaker success.

CHANGELOG has a Tooling entry, and docs/ARCHITECTURE.md has a one-line pointer to the doc.

Stage-2 plan

Add ScreenPlan and Arbiter.decide(state, inputs, now). It is pure, with the ScheduledOff gate and the Follower and Wifi Sources only. Everything else returns a LEGACY plan.
run() gathers inputs after its bookkeeping, calls decide(), and dispatches on plan.source. A LEGACY plan takes the existing path.
Table tests for decide(). The golden traces must not change, including the missed WiFi notice, which gets its own fix PR.

Stage 3 adds the ScreenRunner, FrameClock and the PREEMPTED exit reason, which replaces the five remaining current_display_mode != active_mode re-checks. Stage 4 makes Vegas a Source driven by run_frame(). Stage 5 adds frame_policy.

Verification

Golden tests: 16 passed in each of 5 consecutive runs, 0.8-1.3 s per run.
Goldens against main's code: with main's src/display_controller.py swapped into this branch, all 16 pass.

Mutation check: I broke each extracted helper once, and every mutation failed at least one trace:

Mutation	Failing traces
`_resolve_durations` uses max of the caps	dynamic_duration
`_advance_after_screen` drops the live hold	live_priority, live_round_robin
`_dispatch_first_frame` never clears force_change	live_priority, live_round_robin
`_needs_high_fps` ignores enable_scrolling	plain_rotation
`_resolve_active_mode` skips the forced clear	on_demand_restored
`_clamp_to_on_demand` doesn't clamp	on_demand, on_demand_restored, vegas
`_skip_failed_plugin_modes` never skips	plugin_error
`_show_wifi_notice` dwells 1 s	wifi_notice
`_run_follower_frame` at 30 fps	follower
`_track_dynamic_cycle` never resets	dynamic_duration
`_plugin_for_mode` ignores the breaker	plugin_error
`_blank_while_scheduled_off` dwells 30 s	schedule

Full suite (python -m pytest -m "not hardware" test/ --ignore=test/plugins) on this Windows host, compared with a baseline run of main (f4bda507) in a separate worktree. Both runs had the same 65 FAILED/ERROR ids (59 failed + 6 errors, the known Windows set), so nothing is new. Passes went from 6934 to 6950, which is the 16 new tests.
scripts/check_types.py: clean (82 modules). display_controller.py is not on the ratchet. mypy on it reports 42 errors here and 43 on main, with none new.

ledpi soak needed before merge

The frame loops did not move, but every screen now goes through extra method calls, so check on ledpi:

python3 scripts/frame_soak.py --preview for 10 minutes on a scrolling rotation (no live game holding a static screen), main vs this branch. Alternate which build goes first. Compare the late-frame % and freezes.
The same with Vegas enabled.
By hand: an on-demand start, stop and expiry from the web UI; a live game taking over and handing back if one is on; a schedule boundary turning the panel off and on; and a WiFi notice, if convenient.

I didn't touch either Pi.

Coordination

The control-socket PR (claude/ipc-control-socket) adds an on-demand hook. This PR only touches run() and the new helpers placed just above it. _poll_on_demand_requests and the rest of the on-demand code are unchanged.
Frame pacing (the 8 ms deadline sleep, the 1 ms yield, the 1 s loop) is unchanged, and so is the Vegas branch.

Do not merge until the soak is done.

🤖 Generated with Claude Code

Summary by CodeRabbit

Documentation
- Clarified the display priority order and documented the planned run-loop redesign.
Tests
- Added deterministic coverage for 15 display-loop scenarios, including rotation, schedules, Wi-Fi notices, on-demand requests, and plugin errors.
Refactor
- Restructured display-loop logic while preserving existing screen behavior and traces.

Characterisation tests for DisplayController.run() before it is restructured. test/_run_loop_harness.py builds a real controller on fake plugins, cache, config, sync and plugin manager, swaps the module's time and datetime for one fake clock, and runs the real loop until a horizon. The event log is folded into screens (mode, start, duration, exit reason, frames, force_clear) and compared with test/fixtures/run_loop_golden/. Fourteen scenarios: plain rotation, empty and missing modes, an all-empty rotation, plugin errors and the circuit breaker, dynamic duration, live priority and live round-robin, on-demand start/stop/expiry, pinned on-demand, the on/off and dim schedules, a WiFi notice, sync follower, and Vegas (a contract fake of the coordinator) with and without live_in_ticker. All 15 tests run in under a second. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…change Stage 1 of restructuring DisplayController.run() (docs/RUN_LOOP_REDESIGN.md). Each block moves verbatim into a method named for what the later Arbiter / ScreenRunner needs from it: - _blank_while_scheduled_off: the schedule-off pass - _run_follower_frame: the sync-follower frame (Follower source) - _show_wifi_notice: the WiFi status message pass (Wifi source) - _resolve_active_mode: on-demand mode vs rotation mode (OnDemand source) - _plugin_for_mode: mode -> plugin, with the circuit-breaker skip - _dispatch_first_frame: the executor-bound first frame and lock release - _skip_failed_plugin_modes: skipping a failed plugin's other modes - _track_dynamic_cycle / _resolve_durations / _clamp_to_on_demand: the screen's min/max duration (the plan's bounds) - _needs_high_fps: the frame-rate decision (future frame_policy) - _advance_after_screen: next mode after a screen (rotation / live hold) The frame loops, the Vegas branch and every early exit stay in run(). The two `not wifi_status_data` tests are dropped: past the WiFi block it was always None (a shown message ends the pass, a failed one is reset). The golden traces in test/fixtures/run_loop_golden/ are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…-off stretch Two gaps a mutation check of the extracted helpers found: nothing exercised _resolve_active_mode moving the panel onto an on-demand mode (it only happens when a session is resumed after a restart), and the schedule scenario's off stretch was always cut short before the 60 s dwell ended. Both goldens were generated from main's run(), before the extraction, and pass unchanged after it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…s stages docs/RUN_LOOP_REDESIGN.md: what run() does today, the target design, stages 1-5 with how each is verified (golden traces, mutation check, suite diff, ledpi soak), and seven current behaviours the traces pin down that look wrong and should each get their own PR. CHANGELOG Tooling entry; a pointer from ARCHITECTURE.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… new _resolve_active_mode can return None (no current mode), and annotating that made every screen-path helper call an Optional-to-str error that run() never had. It is left unannotated, as the inline code was, and _advance_after_screen takes Optional[str]. mypy on display_controller.py (not yet on the ratchet): 42 errors, main has 43. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

coderabbitai · 2026-10-01T01:19:01Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 57a3144c-167c-4a9e-a095-af9f7f4f300a

📥 Commits

Reviewing files that changed from the base of the PR and between 16b566e and 62e2e2b.

📒 Files selected for processing (21)

CHANGELOG.md
docs/ARCHITECTURE.md
docs/RUN_LOOP_REDESIGN.md
src/display_controller.py
test/_run_loop_harness.py
test/fixtures/run_loop_golden/all_empty.json
test/fixtures/run_loop_golden/dynamic_duration.json
test/fixtures/run_loop_golden/empty_modes.json
test/fixtures/run_loop_golden/follower.json
test/fixtures/run_loop_golden/live_priority.json
test/fixtures/run_loop_golden/live_round_robin.json
test/fixtures/run_loop_golden/on_demand.json
test/fixtures/run_loop_golden/on_demand_pinned.json
test/fixtures/run_loop_golden/on_demand_restored.json
test/fixtures/run_loop_golden/plain_rotation.json
test/fixtures/run_loop_golden/plugin_error.json
test/fixtures/run_loop_golden/schedule.json
test/fixtures/run_loop_golden/vegas.json
test/fixtures/run_loop_golden/vegas_live_in_ticker.json
test/fixtures/run_loop_golden/wifi_notice.json
test/test_run_loop_golden.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.

📝 Walkthrough

Walkthrough

DisplayController.run() now delegates several existing operations to private helpers. A fake-clock harness runs the real loop and checks its traces against fixtures for 15 scenarios. Documentation describes the current loop and a staged redesign.

Changes

Display run-loop restructuring

Layer / File(s)	Summary
Capture run-loop traces `test/_run_loop_harness.py`, `test/test_run_loop_golden.py`, `test/fixtures/run_loop_golden/*`, `docs/RUN_LOOP_REDESIGN.md`, `CHANGELOG.md`	A fake-clock harness records and reduces controller events. Tests compare 15 scenario traces with JSON fixtures. The documentation describes the stage 1 harness and its verification checks.
Extract pass handling and dispatch `src/display_controller.py`, `docs/RUN_LOOP_REDESIGN.md`, `docs/ARCHITECTURE.md`	Private helpers handle scheduled-off blanking, follower frames, Wi-Fi notices, mode and plugin lookup, and first-frame dispatch. The run-loop description lists normal rotation after Vegas mode. The redesign document describes the current flow and planned stages.
Extract screen timing and advancement `src/display_controller.py`	Private helpers handle failed-plugin mode skipping, dynamic durations, on-demand duration limits, FPS selection, and post-screen mode advancement.

Priority: ⬇️ Low

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to 62e2e

No actionable defect is established in this behavior-preserving extraction. Complete the planned hardware soak before merging, as requested by the author.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 121 functions across 3 files. (18 skipped…	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 clearly summarizes the main changes: stage 1 refactoring of run(), golden-trace tests, and helper extraction with intended behavior preservation.
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 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 121 functions across 3 files. (18 skipped: 18 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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 · 2026-10-01T01:20:12Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

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 · 2026-10-01T13:50:29Z

Hardware check on ledpi (Pi 4, 192x48, 2026-10-01): partial, the Vegas half is still to do

Branch f5c348e9, compared against its merge-base f4bda507. The batch stopped during the first Vegas soak: an outside web-UI config change and display restart landed mid-run. The frame-cost A/B on a scrolling screen has not been done yet.

Normal rotation (football and baseball scoreboards, all static switch screens). frame_soak.py --preview, 5 min each, in the order M B B M:

frame_soak gives no verdict: 0 scrolling frames in all four runs, because this rotation has nothing that scrolls.
Rotation sequence and dwell times are the same on both builds. Every run went ncaa_fb_recent/upcoming 20/20 s, mlb_recent/upcoming 30/30-31 s, nfl_recent/upcoming 20-21/20-22 s.
Journal: 0 ERROR lines on either build. One WARNING appeared, in a main run only: No game data available ... MLBLiveManager.

On-demand, on the branch:

start→active 0.18-0.90 s and stop→idle 0.39 s (3 rounds).
A 15 s timed request expired on time: expired (duration: 15.0 seconds), then the rotation resumed.

Not tested yet:

the Vegas soak A/B;
live takeover and handback;
a schedule boundary;
a WiFi notice.

🤖 Generated with Claude Code

# Conflicts: # src/display_controller.py

ChuckBuilds · 2026-10-01T19:13:28Z

Hardware check on hdpi: PASS (no regression)

Rig: Pi 4, 512x64 (4x 128x64, chain 4), pwm_bits 8, rebuilt binding that releases the GIL, ~90 Hz. Vegas was on (it already was in this rig's config). Each phase ran scripts/frame_soak.py --preview --duration 300 after a restart of both services and 120 s to settle. The phases ran in ABBA order.

phase	build	scrolling frames	late %	missed refreshes	held Hz	blit p99 ms
1	merge-base `f4bda50`	26179	0.569	165	86.0	8.25
2	branch `f5c348e`	26279	0.571	163	86.0	7.25
3	branch `f5c348e`	26145	0.555	154	86.0	7.75
4	merge-base `f4bda50`	25211	0.718	193	84.2	10.25

Merge-base mean: 0.64 % late. Branch mean: 0.56 % late.
frame_soak reports FAIL against its 0.1 % limit on both builds. This rig's Vegas late rate was already ~0.6-0.8 % on main before this run, so the comparison is about parity, not the absolute limit.
Journal: the same error set on both builds, with no new errors or tracebacks:
- calendar credentials and the youtube key are not set;
- football-scoreboard update() timed out at boot (the startup budget);
- one ESPN odds retry.
Vegas rotation: 25 of 26 plugins and the same 90 px/s pacing line on both builds. Each phase had 2 Vegas iterations.
On-demand on the branch during Vegas (clock-simple, then stop):
- the start was picked up and activated within 10 ms;
- the stop cleared on-demand and started a new Vegas iteration within 8 ms;
- neither the start nor the stop restarted a service.

🤖 Generated with Claude Code

ChuckBuilds and others added 6 commits September 30, 2026 21:10

docs: rewrap the run-loop CHANGELOG entry

f5c348e

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

ChuckBuilds mentioned this pull request Oct 1, 2026

fix(display): a raising display() counts as a circuit-breaker failure #707

Merged

ChuckBuilds added 2 commits October 1, 2026 11:11

Merge remote-tracking branch 'origin/main' into HEAD

ada1d81

# Conflicts: # src/display_controller.py

Merge remote-tracking branch 'origin/main' into HEAD

62e2e2b

ChuckBuilds merged commit a21650e into main Oct 1, 2026
13 checks passed

ChuckBuilds deleted the claude/run-loop-stage1 branch October 1, 2026 18:41

Navigation Menu

Repository navigation

refactor(display): run() stage 1 - golden traces and extracted helpers, no behaviour change - #704

refactor(display): run() stage 1 - golden traces and extracted helpers, no behaviour change#704
ChuckBuilds merged 8 commits into
mainfrom
claude/run-loop-stage1

ChuckBuilds commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codacy-production Bot commented Oct 1, 2026

Uh oh!

ChuckBuilds commented Oct 1, 2026

Uh oh!

Uh oh!

ChuckBuilds commented Oct 1, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ChuckBuilds commented Oct 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's in it

Stage-2 plan

Verification

ledpi soak needed before merge

Coordination

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codacy-production Bot commented Oct 1, 2026

Up to standards ✅

Uh oh!

ChuckBuilds commented Oct 1, 2026

Hardware check on ledpi (Pi 4, 192x48, 2026-10-01): partial, the Vegas half is still to do

Uh oh!

Uh oh!

ChuckBuilds commented Oct 1, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ChuckBuilds commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading