refactor(display): run() stage 2 - an Arbiter decides the scheduled-off blank, follower and WiFi notice (no behaviour change)

[ChuckBuilds]

Stage 2 of the run() restructure in docs/RUN_LOOP_REDESIGN.md. No behaviour change.

Design

New module src/display_arbiter.py, added to the mypy ratchet:

• Source: SCHEDULED_OFF, FOLLOWER, WIFI, LEGACY
• ArbiterInputs (frozen): schedule_on, on_demand_active, follower_active, wifi_notice: Optional[WifiNotice]
• ArbiterState (frozen, empty for now: none of the stage-2 Sources remembers anything between passes)
• ScreenPlan (frozen): source, max_duration (60 s for the blank and 0.5 s for the notice, the constants run() used to hard-code), notice
• Arbiter.decide(state, inputs, now) -> ScreenPlan. It is pure: no I/O, no clock reads, no mutation. The order is:
  1. Gate: blank when the schedule is off and no on-demand session overrides it. This is fix(display): schedule windows end at the end time; on-demand ending in off hours blanks at once #714's rule; a session that ends in off hours blanks on the next pass.
  2. Follower comes before on-demand.
  3. OnDemand returns LEGACY, which is still run()'s code, and it outranks the notice.
  4. Wifi: a pending notice wins.
  5. Live, Vegas and Rotation all return LEGACY.
• wifi_notice_preempts(notice, on_demand, now) is the Wifi Source's mid-screen rule. _wifi_notice_pending calls it.

now is passed but not read. Before this change the top-of-pass WiFi check never compared the notice's expiry, and a table row stops decide() from starting to.

What changed in run()

• After the schedule and brightness bookkeeping, run() calls Arbiter.decide(ArbiterState(), self._arbiter_inputs(), time.time()) and dispatches on plan.source:
  • SCHEDULED_OFF → _blank_while_scheduled_off(plan.max_duration)
  • FOLLOWER → _run_follower_frame()
  • WIFI → _show_wifi_notice(plan.notice, plan.max_duration)
  • LEGACY, or a notice that fails to draw → the existing live / Vegas / rotation code, unchanged
• _arbiter_inputs():
  • Sets schedule_on = is_display_active and not on_demand_schedule_override. With this, the gate blanks exactly when is_display_active is False, as before. A test walks this through an on-demand override, the session ending in the same minute, and the schedule turning on during a session.
  • Reads the WiFi notice only when it could win (panel on, no follower, no on-demand). _check_wifi_status_message has side effects (its 1 Hz throttle, deleting an expired file) that those passes never had.
• _publish_current_mode_state_if_changed, _apply_pending_vegas_init and process_deferred_updates run at the same points relative to the branches, so side effects within a pass keep their order.
• All edits are confined to run(), the WiFi, blank and follower helpers, and the new module. Nothing touches state publishing or the command drain, which the control socket stage 3 branch edits.

Tests

• New test/test_display_arbiter.py (56 tests):
  • a written-out 16-row table covering every combination of the four stage-2 inputs, checking both the Source and the whole plan
  • a mid-screen preemption table
  • purity checks: no clock reads, the same inputs give the same plan, nothing is mutated, frozen dataclasses, and no I/O or clock imports
  • the controller's snapshot: when it reads the notice and when it must not, and the gate matching is_display_active through an on-demand session
• Four existing tests were updated for the new helper signatures:
  • three in test_handover_scroll_state.py::TestTheControllersOwnScreens
  • the source-order test in test_display_controller_shutdown_and_state.py, which now checks that _apply_pending_vegas_init() comes before _run_follower_frame() instead of before is_follower_active(); that read moved into _arbiter_inputs
• Mutation check: I broke 23 pieces one at a time: each gate variant, each priority swap, each Source removed, both dwells, the expiry comparison, the snapshot's three reads, and each dispatch in run(). Every mutant failed at least one test.

Verification

• Golden traces: LEDMATRIX_REGEN_GOLDEN=1 regenerates all 15 fixtures byte-identical to the committed blobs (compared with git cat-file). test_run_loop_golden.py, test_run_loop_wifi_and_live.py and test_wifi_notice_preemption.py pass unchanged.
• Full suite: I diffed it against an origin/main (08469734) baseline run in a separate worktree. Both runs have the same 61 failed and 6 errors (the pre-existing Windows set), with identical FAILED/ERROR IDs. The branch has +56 passed, which are the new tests. plugin-repos/ is clean.
• mypy: scripts/check_types.py with mypy 1.20.2 reports no issues in 94 modules, including src/display_arbiter.py.

Still to do (not run: these need the rigs)

• ledpi frame soak A/B: with the service running this branch, python3 scripts/frame_soak.py --preview for 10 minutes on a scrolling rotation, against main, alternating which build goes first. Compare the late-frame rate and freezes. Note that plain main already fails the default gate on ledpi (~0.11–0.16% late), so compare against main's arms rather than the absolute gate.
• By hand on ledpi: the schedule turning the panel off and on; an on-demand session started, stopped and expiring during off hours; a WiFi notice (posted, and with on-demand active); and follower mode, if a leader is available.

Follow-up needed right after merge

src/display_arbiter.py is a new core src module. The ledmatrix-plugins scripts/check_min_core_version.py MODULE_FIRST_VERSION table needs a follow-up PR adding "src.display_arbiter": None; until then, every plugins PR's safety job goes red. It has to come after this merges, because a None entry must already exist on core main.

Stage 3 (next)

ScreenRunner with FrameClock, ExitReason and PREEMPTED, plus the OnDemand, Live and Rotation Sources, so that LEGACY means only Vegas:

• ArbiterState gains the rotation index, the on-demand list, index, expiry and pin, and the live resume point.
• ArbiterInputs gains the live modes and the Vegas flags.
• ScreenPlan gains mode, plugin, the min and max durations, dynamic, frame_policy and preemptible_by.
• The mid-screen checks (_screen_preempted, _check_live_takeover, _wifi_notice_pending) become a single decide() call at the runner's service points.

Stage 3 touches frame pacing, so it needs the ledpi soak A/B. The details are in docs/RUN_LOOP_REDESIGN.md.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Improvements
  • Display handling now more consistently prioritizes scheduled-off screens, follower frames, and pending WiFi notices.
  • WiFi notices can preempt a display during playback when eligible, while active on-demand sessions retain priority.
  • Scheduled-off screens and WiFi notices use defined display durations. Existing handling for other display modes remains unchanged.

[coderabbitai]

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: 5dc8de3c-1eb9-41bb-950b-7a2f42aeb8c1

📥 Commits

Reviewing files that changed from the base of the PR and between 0846973 and 55b4069.

📒 Files selected for processing (8)

• CHANGELOG.md
• docs/RUN_LOOP_REDESIGN.md
• mypy-clean.txt
• src/display_arbiter.py
• src/display_controller.py
• test/test_display_arbiter.py
• test/test_display_controller_shutdown_and_state.py
• test/test_handover_scroll_state.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

The display loop now builds an input snapshot and uses a stateless arbiter to select scheduled-off, follower, WiFi-notice, or legacy handling. The controller applies plan dwell durations, and WiFi notice preemption uses a shared expiry-aware rule. Tests cover the decision table and controller snapshot behavior.

Changes

Display Loop Arbitration

Layer / File(s)	Summary
Arbiter inputs and decisions src/display_arbiter.py, test/test_display_arbiter.py, docs/RUN_LOOP_REDESIGN.md	Adds immutable inputs and plans, source-selection rules, dwell constants, and WiFi notice preemption. Tests cover all 16 input combinations, deterministic decisions, and notice expiry.
Controller integration and validation src/display_controller.py, test/test_display_arbiter.py, test/test_display_controller_shutdown_and_state.py, test/test_handover_scroll_state.py, CHANGELOG.md, mypy-clean.txt, docs/RUN_LOOP_REDESIGN.md	The controller snapshots arbitration inputs and dispatches scheduled-off, follower, and WiFi plans. On-demand, live, Vegas, and rotation handling remains on the legacy path. The controller tests and project records are updated; the design document also records a stage-3 plan.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant DisplayController
  participant SyncManager
  participant Arbiter
  DisplayController->>SyncManager: Read follower state
  SyncManager-->>DisplayController: Return follower state
  DisplayController->>Arbiter: decide with the pass input snapshot
  Arbiter-->>DisplayController: Return ScreenPlan

Loading

Merge Risk: ⚪ Minimal · up to 55b40

No actionable regression remains identified for this change. It is ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 55b40

The reviewed display priorities and rendering operations are preserved, and no new security exposure was demonstrated. Concurrent handovers and the trust provenance of external inputs remain incompletely established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated effect remains selection of content on the controller's existing display. The inspected integration does not add an independently addressable privileged sink or expand controller authority; external producer authentication and wider deployment exposure were not established.

Trust Boundaries and Controls

• observed — Schedule and on-demand controls remain selection gates, with follower handling ahead of WiFi. Notice data still reaches the existing WiFi renderer through controller-owned code; the arbiter neither validates producer identity nor grants additional authority.

Resilience and Maintainability Implications

• inferred — Freezing inputs does not make collection atomic. Follower state can change independently after selection, and selection now precedes additional controller work. The former direct check also raced with rendering, so a newly introduced security failure is not established; acceptance of stale handover frames remains unresolved.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 45.16% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 5 files. (3 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 describes the stage-2 display refactor and identifies the scheduled-off blank, follower, and WiFi notice decisions handled by the arbiter. It is longer than necessary but remains spe…
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 45.16% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 5 files. (3 skipped: 3 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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 9 complexity

Metric	Results
Complexity	9

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.}