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

CHANGELOG.md

@@ -208,6 +208,9 @@ policies are unchanged.
   this release: plugins reach it through `APIHelper` and `espn_dates`, and
   should not import it directly until a plugin-facing API ships (stage 3), so
   it sets no `ledmatrix_min_version` floor.
+- `src/display_arbiter.py` -- the display loop's Arbiter (see Tooling).
+  Core-internal: plugins have no reason to import it, so it sets no
+  `ledmatrix_min_version` floor.
 
 ### Tooling
 
@@ -225,6 +228,17 @@ policies are unchanged.
   (`_dispatch_first_frame`, `_resolve_durations`, `_resolve_active_mode`,
   `_needs_high_fps`, `_advance_after_screen` and others), and the traces are
   identical before and after the move.
+- Display loop stage 2: an Arbiter decides who gets the panel. Each pass,
+  `run()` gathers a small snapshot (the schedule, the on-demand flag, the
+  sync follower, the pending WiFi notice) and calls
+  `Arbiter.decide(state, inputs, now)` in `src/display_arbiter.py`, a pure
+  function, which returns a `ScreenPlan`. It decides the scheduled-off
+  blank, the follower frame and the WiFi notice; on-demand, live priority,
+  Vegas and the rotation return a `LEGACY` plan and run the existing code.
+  The WiFi notice's mid-screen rule (`wifi_notice_preempts`) moves there
+  too. No behaviour change: the golden traces regenerate byte-identical.
+  `test/test_display_arbiter.py` tests `decide()` with a table of all 16
+  combinations of its inputs.
 
 ### Fixes
 

docs/RUN_LOOP_REDESIGN.md

@@ -44,7 +44,9 @@ Each pass, in order:
 3. Poll on-demand requests and expiry, release plugins loaded only for
    on-demand, tick plugin updates, drop an expired WiFi notice, evaluate
    the schedule (an on-demand session overrides scheduled-off), apply the
-   brightness target.
+   brightness target. Then gather the Arbiter's inputs
+   (`_arbiter_inputs`) and call `Arbiter.decide()`, which picks one of
+   steps 4-6 or returns `LEGACY` for steps 7-9 (stage 2).
 4. **Scheduled off:** blank, dwell up to 60 s. `_blank_while_scheduled_off`
 5. **Follower:** render one frame from the leader. `_run_follower_frame`
 6. **WiFi notice** (unless on-demand): draw it, dwell 0.5 s. `_show_wifi_notice`.
@@ -70,8 +72,9 @@ Each pass, in order:
    next mode (`_advance_after_screen`).
 
 The helpers named above were extracted in stage 1 without changing
-behaviour. The frame loops, the Vegas branch and every early exit are still
-inline in `run()`.
+behaviour. Since stage 2 the choice between steps 4, 5, 6 and the rest is
+made by `Arbiter.decide()` in `src/display_arbiter.py`. The frame loops, the
+Vegas branch and every early exit are still inline in `run()`.
 
 ## Target design
 
@@ -160,7 +163,7 @@ that the harness patches in today.
 | 4 | Vegas as a Source driven by `run_frame()` | none intended | traces against the real coordinator; ledpi Vegas soak A/B |
 | 5 | Plugins declare `frame_policy` | DEBUG instead of INFO for the FPS line | traces; soak on a static-heavy rotation |
 
-### Stage 1 (this PR)
+### Stage 1 (#704)
 
 - `test/_run_loop_harness.py` builds a real `DisplayController` through
   `__init__` on in-memory fakes (plugins, cache, config service, plugin
@@ -194,7 +197,7 @@ that the harness patches in today.
 - Twelve helpers were extracted from `run()` (listed under "What `run()`
   does today"). Breaking any one of them fails at least one golden trace.
 
-### Stage 2: Arbiter, starting with Follower and Wifi
+### Stage 2: Arbiter, starting with Follower and Wifi (done)
 
 1. Add `ScreenPlan` and an `Arbiter` with the ScheduledOff gate, Follower
    and Wifi. Every other case returns a `LEGACY` plan, which means "carry on
@@ -211,13 +214,69 @@ that the harness patches in today.
 Follower and Wifi go first because each is one self-contained branch that
 ends the pass. They prove the plumbing without touching the frame loops.
 
+What shipped:
+
+- `src/display_arbiter.py` (on the mypy ratchet) holds `Source`
+  (`SCHEDULED_OFF`, `FOLLOWER`, `WIFI`, `LEGACY`), `ArbiterInputs`,
+  `ArbiterState`, `WifiNotice`, `ScreenPlan` and `Arbiter.decide`.
+  `ScreenPlan` has only the fields stage 2 uses: `source`, `max_duration`
+  (60 s for the blank, 0.5 s for the notice, the constants `run()` used to
+  hard-code) and `notice`. `mode`, `plugin`, the other durations,
+  `frame_policy` and `preemptible_by` arrive with the Sources that need them.
+- `ArbiterState` is empty: no stage-2 Source remembers anything between
+  passes. `now` is passed but not read, because the top-of-pass WiFi check
+  never compared the expiry and must not start (the table pins this).
+- `ArbiterInputs` holds `schedule_on`, `on_demand_active`,
+  `follower_active` and `wifi_notice`. `_arbiter_inputs` derives
+  `schedule_on` as `is_display_active and not on_demand_schedule_override`,
+  so the gate (blank when the schedule is off and no on-demand session
+  overrides it) blanks exactly when `is_display_active` is False, as before,
+  including #714's on-demand ending in off hours. It reads the WiFi notice
+  only when the notice could win, because `_check_wifi_status_message` has
+  side effects (its 1 Hz throttle, deleting an expired file) that those
+  passes never had.
+- The mid-screen rule is `wifi_notice_preempts(notice, on_demand, now)`,
+  which `_wifi_notice_pending` calls; it does compare the expiry.
+- `run()` still calls `_publish_current_mode_state_if_changed`,
+  `_apply_pending_vegas_init` and `process_deferred_updates` at the same
+  points relative to the branches, so the order of side effects in a pass
+  is unchanged.
+- `test/test_display_arbiter.py`: the 16-row table (every combination of
+  the four inputs, written out), the mid-screen table, purity checks (no
+  clock reads, nothing mutated, no I/O imports), and the controller's
+  snapshot through an on-demand session that overrides the schedule and
+  ends. A mutation run broke 23 pieces once each (the gate, the order, each
+  Source, the dwells, the expiry comparison, the snapshot's reads, each
+  dispatch in `run()`); every one failed a test.
+
 ### Stage 3: ScreenRunner and `PREEMPTED`
 
 Move the two frame loops, the make-up dwell and the dynamic-duration exit
 into `ScreenRunner.run(plan)` with an injected `FrameClock`. Replace the
 five re-checks with `PREEMPTED`. Add the OnDemand, Live and Rotation Sources
 so `LEGACY` is left meaning only Vegas.
 
+Concretely, from where stage 2 left off:
+
+1. `ArbiterState` gains the rotation index, the on-demand mode list, index,
+   expiry and pin, and the live resume point (today `current_mode_index`,
+   `on_demand_*` and the live-priority stash). `ArbiterInputs` gains the
+   live modes (`_collect_live_modes`) and whether Vegas is enabled and keeps
+   live content in the ticker.
+2. OnDemand returns its current mode with `_clamp_to_on_demand`'s bound,
+   reading `now` for the expiry. Live returns the next live mode
+   (round-robin). Rotation returns `available_modes[current_mode_index]`.
+   `ScreenPlan` gains `mode`, `plugin`, `min_duration`, `max_duration`,
+   `dynamic`, `frame_policy` and `preemptible_by`.
+3. `ScreenRunner.run(plan)` returns an `ExitReason`; `state.after(plan,
+   outcome)` replaces `_advance_after_screen` and the live-resume
+   bookkeeping. Each mid-screen check asks `decide()` whether a Source in
+   `plan.preemptible_by` now wins, so `_screen_preempted`,
+   `_check_live_takeover` and `_wifi_notice_pending` become one call.
+4. The control socket (`_drain_control_commands`, `_wait_for_control`) and
+   state publishing stay where they are; the runner calls them at its
+   service points.
+
 This stage touches frame pacing (the 8 ms deadline sleep, the 1 ms yield),
 so it needs a frame soak on ledpi, A/B against main. Coordinate with
 whoever owns scroll performance (`docs/SCROLL_PERFORMANCE.md`).

mypy-clean.txt

@@ -47,6 +47,7 @@ src/config_service.py
 src/core_config_keys.py
 src/deprecation.py
 src/device_location.py
+src/display_arbiter.py
 src/display_geometry.py
 src/dynamic_team_resolver.py
 src/exceptions.py

src/display_arbiter.py

@@ -0,0 +1,185 @@
+"""What the panel shows next: the Arbiter of docs/RUN_LOOP_REDESIGN.md.
+
+``Arbiter.decide(state, inputs, now)`` takes a snapshot that
+``DisplayController.run()`` gathers once per pass and returns a
+:class:`ScreenPlan` naming the Source that gets the panel. It is a pure
+function: no I/O, no clock reads (``now`` is passed in), no locks, and it
+changes nothing it is given. That is what lets a plain table of cases test
+the priority order, which used to exist only as the order of ``if`` blocks
+in ``run()``.
+
+The full order is
+
+    ScheduledOff (a gate), Follower, OnDemand, Wifi, Live, Vegas, Rotation
+
+Stage 2 decides the gate, Follower and Wifi. Every other case returns a
+``LEGACY`` plan, meaning "carry on with run()'s existing code" (live
+priority, Vegas, then one rotation screen). OnDemand is in the order already
+because it outranks the WiFi notice: an active session is a ``LEGACY`` plan
+even when a notice is pending.
+
+The Wifi Source's mid-screen rule, :func:`wifi_notice_preempts`, lives here
+too, so both of its answers -- at the top of a pass and between frames --
+come from one module.
+"""
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Optional
+
+__all__ = [
+    "Arbiter",
+    "ArbiterInputs",
+    "ArbiterState",
+    "SCHEDULED_OFF_DWELL",
+    "ScreenPlan",
+    "Source",
+    "WIFI_NOTICE_DWELL",
+    "WifiNotice",
+    "wifi_notice_preempts",
+]
+
+# How long one scheduled-off pass blanks the panel. The dwell ends early when
+# on-demand starts or the schedule turns the panel back on.
+SCHEDULED_OFF_DWELL = 60.0
+
+# How long one WiFi-notice pass holds the notice before the next pass looks
+# again; the notice stays up, pass after pass, until it expires.
+WIFI_NOTICE_DWELL = 0.5
+
+
+class Source(Enum):
+    """Who gets the panel this pass."""
+
+    SCHEDULED_OFF = "scheduled-off"
+    FOLLOWER = "follower"
+    WIFI = "wifi"
+    # Not decided by the Arbiter yet: on-demand, live priority, Vegas and the
+    # rotation are still chosen by run()'s own code. Stage 3 adds the
+    # OnDemand, Live and Rotation Sources; stage 4 adds Vegas.
+    LEGACY = "legacy"
+
+
+@dataclass(frozen=True)
+class WifiNotice:
+    """A WiFi status message waiting to be drawn.
+
+    ``expires_at`` is wall-clock time (``time.time()``), as written by the
+    WiFi manager.
+    """
+
+    message: str
+    expires_at: float
+
+
+@dataclass(frozen=True)
+class ArbiterState:
+    """What the Arbiter remembers between passes.
+
+    Nothing yet: the stage-2 Sources decide from the inputs alone. The
+    on-demand index, the rotation index and the live resume point move here
+    with their Sources in stage 3.
+    """
+
+
+@dataclass(frozen=True)
+class ArbiterInputs:
+    """One pass's snapshot, gathered by run() before it calls decide().
+
+    Attributes:
+        schedule_on: The display schedule has the panel on, not counting an
+            on-demand override of a scheduled-off window.
+        on_demand_active: An on-demand session is running.
+        follower_active: A sync leader is driving this panel.
+        wifi_notice: The pending WiFi notice, or None. run() reads it only
+            when it could win (the panel is on, and neither a follower nor
+            on-demand outranks it), because reading it has side effects: a
+            1 Hz throttle and deleting an expired file.
+    """
+
+    schedule_on: bool
+    on_demand_active: bool
+    follower_active: bool
+    wifi_notice: Optional[WifiNotice] = None
+
+
+@dataclass(frozen=True)
+class ScreenPlan:
+    """The Arbiter's answer for one pass.
+
+    Attributes:
+        source: The Source that gets the panel.
+        max_duration: How long the plan holds the panel, in seconds, at most
+            (its dwell ends early when what the panel should show changes).
+            None when the Source paces itself: a follower frame, or LEGACY.
+        notice: The WiFi notice to draw, for a WIFI plan.
+    """
+
+    source: Source
+    max_duration: Optional[float] = None
+    notice: Optional[WifiNotice] = None
+
+
+SCHEDULED_OFF_PLAN = ScreenPlan(Source.SCHEDULED_OFF, max_duration=SCHEDULED_OFF_DWELL)
+FOLLOWER_PLAN = ScreenPlan(Source.FOLLOWER)
+LEGACY_PLAN = ScreenPlan(Source.LEGACY)
+
+
+class Arbiter:
+    """Decides which Source gets the panel. Stateless; see the module docstring."""
+
+    @staticmethod
+    def decide(state: ArbiterState, inputs: ArbiterInputs, now: float) -> ScreenPlan:
+        """The plan for this pass, from the Sources in priority order.
+
+        Args:
+            state: What the Arbiter remembers between passes (nothing yet).
+            inputs: This pass's snapshot.
+            now: Wall-clock time of the snapshot. No stage-2 Source reads it:
+                the top-of-pass WiFi check takes the notice as read, and only
+                the mid-screen check (:func:`wifi_notice_preempts`) compares
+                it with the expiry. It is in the signature for the Sources
+                stage 3 adds (on-demand expiry, durations).
+
+        Returns:
+            The winning Source's plan, or LEGACY_PLAN when the winner is one
+            run() still decides itself.
+        """
+        del state, now  # not read by the stage-2 Sources; see the docstring
+
+        # ScheduledOff is a gate, not a Source: a scheduled-off panel stays
+        # blank even for a follower, and only an on-demand session overrides
+        # it (#714 -- one ending in off hours blanks at the next pass).
+        if not inputs.schedule_on and not inputs.on_demand_active:
+            return SCHEDULED_OFF_PLAN
+
+        # 1. Follower: a sync leader drives this panel, ahead of on-demand.
+        if inputs.follower_active:
+            return FOLLOWER_PLAN
+
+        # 2. OnDemand: decided by run() until stage 3. It outranks the notice.
+        if inputs.on_demand_active:
+            return LEGACY_PLAN
+
+        # 3. Wifi: a pending notice, held for one short dwell per pass.
+        if inputs.wifi_notice is not None:
+            return ScreenPlan(Source.WIFI, max_duration=WIFI_NOTICE_DWELL,
+                              notice=inputs.wifi_notice)
+
+        # 4-6. Live, Vegas, Rotation: still run()'s own code.
+        return LEGACY_PLAN
+
+
+def wifi_notice_preempts(notice: Optional[WifiNotice], on_demand_active: bool,
+                         now: float) -> bool:
+    """Whether a WiFi notice should end the current screen early.
+
+    The Wifi Source's mid-screen rule, polled between frames, during dwells
+    and when a Vegas iteration yields. On-demand outranks the notice, as in
+    :meth:`Arbiter.decide`. Unlike the top-of-pass check it also compares
+    ``now`` with the expiry, because the 1 Hz read throttle can hand back a
+    notice that has expired since it was read.
+    """
+    if on_demand_active or notice is None:
+        return False
+    return now < notice.expires_at