From 2fa52312151476831707035b2b9d623b7bd2282a Mon Sep 17 00:00:00 2001 From: Bernardo Magri Date: Sat, 11 Jul 2026 08:19:05 +0100 Subject: [PATCH] =?UTF-8?q?fix(theme):=20#79=20review=20fixes=20=E2=80=94?= =?UTF-8?q?=20wrap-around=20schedule=20+=20HH:MM=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two low-severity edge bugs from a fresh-eyes review of the session batch: - cmd_auto: an inverted schedule (sunrise > sunset, i.e. a day window that crosses midnight) stayed perpetually on the day theme — the old `not between` shortcut is an empty interval when sunrise > sunset. Replaced with a real wrap-around: daytime = mins >= sunrise or mins < sunset, so both directions switch. Normal sunrise < sunset path is unchanged (checks.auto-theme still holds). - rofi autotheme HH:MM validator: `[0-2][0-9]` accepted 24:00–29:59. Tightened the case pattern to `[01][0-9]:[0-5][0-9]|2[0-3]:[0-5][0-9]` so out-of-range hours are rejected (they'd otherwise skew the boundary). (Two other review findings dismissed: papirus DOES ship breeze/breeze-dark so that iconPacks prefix is correct; the doctor RAM-unknown row is cosmetic since /proc/meminfo is always readable.) Verification: py_compile; boundary truth-table (normal + wrap-around); HH:MM case table (23:00/19:59/20:00 accept, 24/25/30:00 reject); auto --which normal-case unchanged; menu bash -n; flake check green. Co-Authored-By: Claude Opus 4.8 --- agent/JOURNAL.md | 18 ++++++++++++++++++ modules/home/rofi.nix | 2 +- .../nomarchy-theme-sync/nomarchy-theme-sync.py | 11 +++++++---- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/agent/JOURNAL.md b/agent/JOURNAL.md index 18035e0..4e6d0e8 100644 --- a/agent/JOURNAL.md +++ b/agent/JOURNAL.md @@ -17,6 +17,24 @@ Template: --- +## 2026-07-11 — Code-review fixes (parallel review of the session batch) +- **Task:** Bernardo asked for parallel work; spawned a fresh-eyes opus + review of `2055842..HEAD` (static). It found the batch sound — 2 real LOW + edge bugs, 1 false positive, 1 cosmetic. +- **Did:** (1) `cmd_auto` — inverted sunrise/sunset (sunrise > sunset) went + perpetually day; replaced the `not between` shortcut with a real + wrap-around `mins >= sunrise or mins < sunset`, so a cross-midnight day + window switches both ways. (2) `rofi.nix` autotheme HH:MM guard tightened + `[0-2][0-9]` → `[01][0-9]|2[0-3]` so 24–29 are rejected. Dismissed the + `breeze` finding (papirus DOES ship breeze/breeze-dark — verified) and the + cosmetic doctor-RAM one. +- **Verified:** py_compile; boundary truth-table (normal + wrap-around); + HH:MM case table (23:00/19:59/20:00 accept, 24/25/30:00 reject); `auto + --which` normal-case unchanged; menu bash -n; flake check green. Normal + case unchanged so `checks.auto-theme` still holds. +- **Pending:** hibernate-swapfile VM test still running (uncommitted flake.nix). +- **Next suggestion:** land hibernate-swapfile once its VM test returns. + ## 2026-07-10 — #79 V2: checks.auto-theme (simulated sunset) — was mis-framed V3 - **Task:** Bernardo challenged the "V3 real-session" framing — a simulated sunset IS VM-testable. He's right; I'd conflated "needs a real desktop" diff --git a/modules/home/rofi.nix b/modules/home/rofi.nix index 1b05812..4470424 100644 --- a/modules/home/rofi.nix +++ b/modules/home/rofi.nix @@ -752,7 +752,7 @@ ${themeRows} case "$choice" in "Sunset"*) key=sunset; label=Sunset; cur="''${sunset:-20:00}" ;; esac t=$(rofi_menu -p "$label HH:MM (now $cur)" < /dev/null) || exec "$0" autotheme case "$t" in - [0-2][0-9]:[0-5][0-9]) + [01][0-9]:[0-5][0-9]|2[0-3]:[0-5][0-9]) nomarchy-theme-sync --quiet set "settings.autoTheme.$key" "$t" --no-switch ;; "") : ;; *) notify-send "Auto theme" "Ignored '$t' — use HH:MM (24-hour)." ;; diff --git a/pkgs/nomarchy-theme-sync/nomarchy-theme-sync.py b/pkgs/nomarchy-theme-sync/nomarchy-theme-sync.py index 9c6b490..7c46d4a 100644 --- a/pkgs/nomarchy-theme-sync/nomarchy-theme-sync.py +++ b/pkgs/nomarchy-theme-sync/nomarchy-theme-sync.py @@ -528,10 +528,13 @@ def cmd_auto(args) -> None: sunset = _hhmm_to_min(at.get("sunset", "20:00"), "20:00") now = time.localtime() mins = now.tm_hour * 60 + now.tm_min - # Normal case sunrise < sunset: daytime is the span between them. If a - # user inverts them, treat the between-span as night (still coherent). - between = sunrise <= mins < sunset - daytime = between if sunrise <= sunset else not between + # Daytime is the span between sunrise and sunset. Also handle a + # wrap-around pair (sunrise after sunset → the day window crosses + # midnight), so an inverted schedule still switches both ways. + if sunrise <= sunset: + daytime = sunrise <= mins < sunset + else: + daytime = mins >= sunrise or mins < sunset target = day if daytime else night if args.which: