From 6de2f250b91903e7be76acfc64f90d3f5eeeb5e4 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Tue, 15 Sep 2026 13:10:09 -0700 Subject: [PATCH 1/2] test(daemon): accept either capture-demand startup order in gating tests Three render-thread tests asserted a demand-gated source sees exactly [false, true, false]. The leading false is a race, not a contract: start_all clears the input manager's cached demand, and the input publication worker reconciles capture demand on its first pass, before the first frame publishes the scene's authoritative demand. When the worker wins it applies "inactive" to an already-inactive source; when the first frame wins, the first application is already true. Fast Windows runners take the second path and failed with [true, false]. A shared helper now requires exactly one activation followed by the deactivation and tolerates the optional startup no-op. Twenty back to back runs of all three tests pass after the change. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013dXbUNEe4yxrGhQLXbzRou --- .../tests/render_thread_tests.rs | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/crates/hypercolor-daemon/tests/render_thread_tests.rs b/crates/hypercolor-daemon/tests/render_thread_tests.rs index f3cd4053f..3bd9cdf3f 100644 --- a/crates/hypercolor-daemon/tests/render_thread_tests.rs +++ b/crates/hypercolor-daemon/tests/render_thread_tests.rs @@ -1359,6 +1359,27 @@ impl SourceRoleBinding for EventOnlySource { impl InteractionSource for EventOnlySource {} +/// Assert a demand-gated source was switched on exactly once and left off. +/// +/// The input publication worker reconciles capture demand as soon as it +/// starts, before the first frame has published the scene's authoritative +/// demand. When that first reconcile wins the race it applies "inactive" to +/// a source whose cached demand was cleared by `start_all`, logging a leading +/// `false`; when the first frame wins, the first application is already +/// `true`. Both orderings are correct, so the contract is the shape of the +/// real transitions, not the presence of the startup no-op. +fn assert_capture_toggled_once(transitions: &[bool]) { + let real = match transitions { + [false, rest @ ..] => rest, + rest => rest, + }; + assert_eq!( + real, + [true, false], + "capture should activate once for the reactive scene and deactivate after it; saw {transitions:?}" + ); +} + async fn wait_for_audio_capture_transition(transitions: &Arc>>, expected: bool) { tokio::time::timeout(WAIT_DEADLINE, async { loop { @@ -2067,7 +2088,7 @@ async fn render_thread_gates_audio_capture_to_audio_reactive_effects() { .lock() .expect("transition log should lock") .clone(); - assert_eq!(transitions, vec![false, true, false]); + assert_capture_toggled_once(&transitions); } #[tokio::test] @@ -2729,7 +2750,7 @@ async fn audio_capture_enabled_when_any_active_zone_is_reactive() { .lock() .expect("transition log should lock") .clone(); - assert_eq!(transitions, vec![false, true, false]); + assert_capture_toggled_once(&transitions); } #[tokio::test] @@ -2809,7 +2830,7 @@ async fn render_thread_gates_screen_capture_to_screen_reactive_scene_groups() { .lock() .expect("transition log should lock") .clone(); - assert_eq!(transitions, vec![false, true, false]); + assert_capture_toggled_once(&transitions); } #[tokio::test] From 0692cd6ffe9aaeda182e8bb1d1dc787f37488719 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Tue, 15 Sep 2026 15:48:36 -0700 Subject: [PATCH 2/2] fix(render): keep release sleep out of the late static sleep shortcut When the session goes to sleep after a frame's power snapshot was taken, the frame executor switches to a static sleep frame late: it writes one off-color frame to every device, publishes populated zones, and latches the sleep frame as pushed. That shortcut only satisfies the static off behavior. Under the release behavior the throttle path is what clears the published zones and stops driving devices, and it never ran because the latch told it the sleep frame was already out. Devices stayed held at the off color instead of being released, and the output-sleep test waited the full deadline for zones that never emptied, which is how the Windows lane surfaced it. The late switch now applies only to static sleep. A release sleep lets the admitted frame finish and the next frame takes the release path, one frame later than the static case. The output-sleep test also adopts the startup-order helper for its transition assertions. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013dXbUNEe4yxrGhQLXbzRou --- .../src/render_thread/frame_executor.rs | 30 ++++++++++++++++++- .../tests/render_thread_tests.rs | 28 +++++++++-------- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/crates/hypercolor-daemon/src/render_thread/frame_executor.rs b/crates/hypercolor-daemon/src/render_thread/frame_executor.rs index f0be6b7c2..387d17965 100644 --- a/crates/hypercolor-daemon/src/render_thread/frame_executor.rs +++ b/crates/hypercolor-daemon/src/render_thread/frame_executor.rs @@ -951,11 +951,21 @@ async fn force_static_sleep_snapshot( .note_canvas_frame(frame_number, elapsed_ms); } +/// Whether a frame admitted while awake should end as a static sleep frame. +/// +/// Only the static off behavior can be satisfied by writing one off-color +/// frame here. A release sleep has to clear the published zones and stop +/// driving devices, which the sleep throttle path owns; taking the static +/// shortcut for it would latch the sleep frame as pushed with populated +/// zones still on the bus, so release lets this frame finish and the next +/// frame runs the release path. fn should_switch_to_late_sleep_frame( frame_output_power: crate::output_power::OutputPowerState, latest_output_power: crate::output_power::OutputPowerState, ) -> bool { - !frame_output_power.sleeping() && latest_output_power.sleeping() + !frame_output_power.sleeping() + && latest_output_power.sleeping() + && latest_output_power.effective_off_output_behavior() == OffOutputBehavior::Static } const fn output_frame_source_kind(source: OutputFrameSource) -> OutputFrameSourceKind { @@ -1082,6 +1092,24 @@ mod tests { assert!(!super::should_switch_to_late_sleep_frame(running, running)); } + #[test] + fn late_sleep_frame_leaves_release_sleep_to_the_throttle_path() { + let running = OutputPowerState::default(); + let releasing = OutputPowerState { + session_sleeping: true, + session_brightness: 0.0, + off_output_behavior: OffOutputBehavior::Release, + ..OutputPowerState::default() + }; + + assert!(!super::should_switch_to_late_sleep_frame( + running, releasing + )); + assert!(!super::should_switch_to_late_sleep_frame( + releasing, releasing + )); + } + fn sample_layout(zone_ids: &[&str]) -> SpatialLayout { SpatialLayout { id: "layout".to_owned(), diff --git a/crates/hypercolor-daemon/tests/render_thread_tests.rs b/crates/hypercolor-daemon/tests/render_thread_tests.rs index 3bd9cdf3f..e6223bf0d 100644 --- a/crates/hypercolor-daemon/tests/render_thread_tests.rs +++ b/crates/hypercolor-daemon/tests/render_thread_tests.rs @@ -1369,15 +1369,20 @@ impl InteractionSource for EventOnlySource {} /// `true`. Both orderings are correct, so the contract is the shape of the /// real transitions, not the presence of the startup no-op. fn assert_capture_toggled_once(transitions: &[bool]) { + assert_capture_transitions( + transitions, + &[true, false], + "capture should activate once for the reactive scene and deactivate after it", + ); +} + +/// Assert the demand transitions after the optional startup no-op. +fn assert_capture_transitions(transitions: &[bool], expected: &[bool], context: &str) { let real = match transitions { [false, rest @ ..] => rest, rest => rest, }; - assert_eq!( - real, - [true, false], - "capture should activate once for the reactive scene and deactivate after it; saw {transitions:?}" - ); + assert_eq!(real, expected, "{context}; saw {transitions:?}"); } async fn wait_for_audio_capture_transition(transitions: &Arc>>, expected: bool) { @@ -2136,10 +2141,10 @@ async fn output_sleep_keeps_reactive_input_capture_live() { frame_rx.borrow().zones.is_empty() }) .await; - assert_eq!( - *transitions.lock().expect("transition log should lock"), - [false, true], - "output policy must not disable a live input consumer" + assert_capture_transitions( + &transitions.lock().expect("transition log should lock"), + &[true], + "output policy must not disable a live input consumer", ); { @@ -2148,10 +2153,7 @@ async fn output_sleep_keeps_reactive_input_capture_live() { } render_thread.shutdown().await.expect("shutdown"); - assert_eq!( - *transitions.lock().expect("transition log should lock"), - [false, true, false] - ); + assert_capture_toggled_once(&transitions.lock().expect("transition log should lock")); } // ── Frame Pipeline Tests ────────────────────────────────────────────────────