Compiler-checked hot-path discipline and tier-zero warnings

.clang-tidy

@@ -94,7 +94,11 @@
 #   readability-function-cognitive-complexity / *-function-size — complexity is lizard's job;
 #     it produces the per-commit number repo-health.json trends.
 #   bugprone-branch-clone — fires on deliberate parallel switch arms.
-#   clang-analyzer-security.ArrayBound — analyses the macOS SDK's headers, not our code.
+#   clang-analyzer-security.ArrayBound — analyzes the macOS SDK's headers, not our code.
+#   clang-diagnostic-function-effects — the hot-path check, owned by check_nonblocking.py
+#     and the clang-hotpath card, which splits findings by tick tier and marks what is NEW
+#     against docs/metrics/hotpath-baseline.txt. Leaving it on here double-reports the same
+#     165 findings with none of that context, and buries clang-tidy's own 47 under them.
 #
 # Everything else disabled below is a style opinion we do not hold; enabling any of them
 # would gate a rewrite rather than a fix.
@@ -197,7 +201,8 @@ Checks: >-
   -google-readability-function-size,
   -google-build-using-namespace,
   -google-runtime-int,
-  -clang-analyzer-security.ArrayBound
+  -clang-analyzer-security.ArrayBound,
+  -clang-diagnostic-function-effects
 WarningsAsErrors: ''
 HeaderFilterRegex: 'src/(core|light)/'
 FormatStyle: none

.github/workflows/test.yml

@@ -80,7 +80,11 @@ jobs:
     strategy:
       fail-fast: false          # a TSan race and an ASan UAF are independent signals — report both
       matrix:
-        kind: [address, thread]
+        # `realtime` is the runtime half of the hot-path check: MM_NONBLOCKING marks the tick
+        # methods, -Wfunction-effects proves what it can at compile time, and RTSan catches what
+        # it cannot — allocation or blocking reached through virtual dispatch or a function
+        # pointer. Same attribute drives both, so this lane costs one matrix entry.
+        kind: [address, thread, realtime]
     steps:
       - uses: actions/checkout@v4
         with:
@@ -89,11 +93,21 @@ jobs:
       # (CLAUDE.md § Use uv for every Python invocation), so a build without it fails at configure.
       - uses: astral-sh/setup-uv@v3
       - name: build + run unit tests under ${{ matrix.kind }}sanitizer
+        env:
+          # RTSan is clang-only — GCC rejects `-fsanitize=realtime` outright — and the attribute
+          # it keys off is [[clang::nonblocking]], which GCC ignores anyway. ASan/TSan stay on the
+          # runner default so this lane is the only one that changes compiler.
+          CXX: ${{ matrix.kind == 'realtime' && 'clang++' || '' }}
         run: |
           cmake -S . -B build/san -DCMAKE_BUILD_TYPE=Debug \
             -DCMAKE_CXX_FLAGS="-fsanitize=${{ matrix.kind }} -fno-omit-frame-pointer -g" \
             -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=${{ matrix.kind }}"
           cmake --build build/san --target mm_tests -j"$(nproc)"
           # Leak detection is a different concern from the races/UAF this gate hunts, and it fires on
           # third-party static init; keep the signal on what we're actually looking for.
-          ASAN_OPTIONS=detect_leaks=0 TSAN_OPTIONS=halt_on_error=1 ./build/san/test/mm_tests
+          #
+          # RTSan does NOT halt: the render path has ~50 known blocking calls (frozen in
+          # docs/metrics/hotpath-baseline.txt, backlogged as architecture work), so halting would
+          # fail this lane on every run. It reports; the log is the signal.
+          ASAN_OPTIONS=detect_leaks=0 TSAN_OPTIONS=halt_on_error=1 \
+            RTSAN_OPTIONS=halt_on_error=0 ./build/san/test/mm_tests

CMakeLists.txt

@@ -42,7 +42,27 @@ if(MSVC)
     # Static MSVC runtime so the Windows binary doesn't need vcredist.
     set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
 else()
-    add_compile_options(-Wall -Wextra -Werror)
+    # Tier zero on top of -Wall -Wextra: five warnings that catch real defects the base set
+    # misses. -Wdouble-promotion is the one that earns its place here specifically — the Xtensa
+    # has no FPU, so an accidental float->double promotion is a silent softfloat call in the
+    # render path, and it enforces the integer-math rule the coding standards ask for.
+    # (GCC/Clang only; the MSVC branch above uses /W4 /WX, which has no direct equivalents.)
+    add_compile_options(-Wall -Wextra -Werror
+                        -Wshadow -Wnon-virtual-dtor -Wdouble-promotion
+                        -Wimplicit-fallthrough -Wnull-dereference)
+
+    # Hot-path discipline, enforced by the compiler. MM_NONBLOCKING marks tick/tick20ms/tick1s
+    # (platform.h); -Wfunction-effects then checks TRANSITIVELY that nothing they reach
+    # allocates or blocks — which check_hotpath.py's regex cannot do, since it only sees the
+    # text of the tick body and not its callees. Clang 20+ only; the flag does not exist on
+    # GCC, so the ESP32 build keeps check_hotpath.py for src/platform/esp32/.
+    if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 20)
+        # -Wno-error while the findings are triaged: they are real (status formatting and
+        # the audio sync path on tick), but each needs a judgement — fix, accept with a
+        # scoped reason, or annotate the callee — and a red build blocks every other
+        # gate meanwhile. Drop the -Wno-error once check_hotpath reports zero.
+        add_compile_options(-Wfunction-effects -Wno-error=function-effects)
+    endif()
 endif()
 
 # `uv` is the project's Python launcher (see CLAUDE.md / moondeck/MoonDeck.md).

docs/backlog/backlog-core.md

@@ -341,6 +341,73 @@ Fix options: (a) make every live mutate scenario clear+rebuild its own canvas (c
 
 ## Housekeeping
 
+### Hot path: move blocking work off the render callbacks (architecture)
+
+`-Wfunction-effects` proves the render path really does block — these are not annotation gaps,
+they are synchronous I/O, allocation and lifecycle work reachable from `tick()`. Each needs the
+work moved to a worker or made resumable, so each is a design change rather than a lint fix.
+Confirmed by external review (CodeRabbit, PR #56).
+
+**`tick()` — every frame, the sharp ones:**
+- `PreviewDriver::tick` — `sendFrame()` writes a socket synchronously; `buildAndSendCoordTable()`
+  resizes `keptIdx_`. Currently suppressed at the site with the reason.
+- `ParallelLedDriver::tick` — `tickSync()`/`tickRing()` reach `busWaitIfBusy()`, which spins for
+  the DMA peripheral. Deliberate (the driver owns the bus for the frame) but blocking. Suppressed.
+- `Drivers::tick` — joins/stops the render-split worker synchronously on the timed-out recovery
+  path. Wants asynchronous handling that still preserves buffer ownership.
+- `HueDriver::tick` / `tick1s` — synchronous HTTP (`pushOneChangedLight`, `pollPairing`,
+  `fetchLights`, `fetchGroups`) plus allocation. Wants a queue to a worker.
+- `Layer::tick` — calls `applyState()` on a modifier rebuild, which runs prepare/release and
+  resizes ScratchBuffers. Wants the rebuild deferred to the scheduler's non-render prepare path,
+  keeping today's coalescing of `consumeNeedsRebuild()`.
+- `DemoReelEffect::tick` — `advance()`/`swapTo()` create, delete and `applyState()` child effects
+  from the render callback. Wants a pending-switch flag consumed off-tick.
+- `NetworkSendDriver::tick` — sends inline; wants to enqueue.
+- `RmtLedDriver::tick` — waits on hardware completion and reset; wants polling or offload.
+- `AudioService::tick` — UDP `sendTo`/`recvFrom` every frame (`syncSend`, `syncReceive`,
+  `syncEnsureSocket`).
+
+**`tick20ms()` / `tick1s()` — milder, same shape:**
+- `HttpServerModule::tick20ms` — inline `handleConnection` transport work, so the 100 ms budget
+  cannot actually bound it. Wants resumable connection handling. `tick1s` writes WLED state
+  frames inline; wants the existing resumable sender.
+- `MqttModule::tick1s` — synchronous DNS + discovery-buffer allocation.
+- `NetworkModule::tick1s` — WiFi/AP lifecycle transitions and tree rebuilds.
+- `FilesystemModule::tick1s` — the debounced `flush()` does filesystem work; the poll itself is
+  fine, the save wants a worker.
+- `FileManagerModule::tick1s` — `platform::filesystemUsed()`; wants a cached value.
+- `SystemModule::tick1s` — the P4 `coprocessorWifi()` path calls
+  `esp_hosted_get_coprocessor_fwversion()` synchronously; wants a cached snapshot.
+- `ImprovProvisioningModule::tick` — runs the queued APPLY_OP inline; wants a cold task to
+  execute and publish only the result.
+- `Scheduler::tick` — dispatches all of the above, so its own contract is only as good as theirs.
+
+**Explicitly NOT in scope:** the float-math findings (`BouncingBallsEffect`, `RipplesEffect`,
+`SphereMoveEffect`). Review suggested fixed-point, but the float trajectory IS the ported
+MoonLight behaviour and fidelity is deliberate. If the FPU-less Xtensa cost is real, that is a
+profiling question first, not a lint fix.
+
+`-Wfunction-effects` reports these but never fails a build, and
+`docs/metrics/hotpath-baseline.txt` freezes the known set so a NEW blocking call stands out
+in the report. The pre-commit gate runs the same check incrementally.
+
+### ESP32 clang/LLVM toolchain — extend the clang checks to src/platform/esp32/
+
+Espressif ships an xtensa LLVM (their fork), but the installed `esp-clangd` package contains
+**only `clangd`** — no `clang++` driver — so a clang analysis pass over ESP32 sources needs their
+full LLVM installed separately.
+
+What it would buy: `-Wfunction-effects` (and clang-tidy, clang-query) over `src/platform/esp32/`
+— 20 translation units, the only code the desktop build excludes. Everything else, including the
+LED drivers, already compiles on desktop and is already checked.
+
+What it costs: a second ~1 GB toolchain, maintained purely for analysis — the firmware would
+still be built by GCC, so the analysing compiler is not the shipping compiler. That is a real
+"one rule, one owner" tension, and the reason this is a decision rather than an obvious yes.
+
+Worth revisiting when either the coverage gap bites (a hot-path bug traced to the platform layer
+that the desktop check could not see) or Espressif's LLVM becomes the default toolchain.
+
 ### clang-tidy: triage the 47 clang-analyzer findings, then gate
 
 `.clang-tidy` runs `*` minus a documented disable list and reaches zero on everything except the

docs/coding-standards.md

@@ -138,7 +138,7 @@ Two things worth knowing:
 ## Static checks
 
 - **Platform boundary** (`moondeck/check/check_platform_boundary.py`) — scans all files outside `src/platform/` for `#ifdef` / `#if defined` with platform macros and `#include` of platform-specific headers (`esp_*`, `freertos/*`, `driver/*`, `SDL.h`, `wiringPi.h`, …). Fails if any are found. The platform boundary rule itself: [architecture.md § Platform abstraction](architecture.md#platform-abstraction).
-- **Hot path lint** (`moondeck/check/check_hotpath.py`) — reads the body of every `tick()` / `tick20ms()` / `tick1s()` under `src/` and flags the allocation (`new`, `malloc`, `push_back`, `std::string`, `make_unique`, `make_shared`) and blocking (`delay`, `sleep`, `mutex.lock()`) it can see. A lint, not a proof: it cannot see what a callee allocates, so a clean run means "nothing visible in the render path's own source". A justified exception is marked `// hot-path-ok: <reason>` at the line, so the reason lives at the site. The hot path rule itself: [architecture.md § Hot path discipline](architecture.md#hot-path-discipline). A "no blocking in the hot path" audit must sweep *every* syscall the path can reach (connect, DNS, read, *and* write), not just the loudest one: a single-threaded loop that services I/O must make no blocking call at all (a socket timeout is not a fix, it is the size of the freeze; non-blocking + poll is the only safe shape). Fixing one blocker while an equally-blocking sibling survives is a partial fix that reads as complete.
+- **Hot path check** (`moondeck/check/check_nonblocking.py`) — `MoonModule::tick/tick20ms/tick1s` carry `MM_NONBLOCKING`, and Clang 20+ verifies under `-Wfunction-effects` that nothing they reach allocates or blocks — **transitively**, through the whole call graph. It reports; it does not fail a build: a new blocking call may be legitimate (a driver that must wait for hardware), so the finding is stated and the product owner judges it. `docs/metrics/hotpath-baseline.txt` freezes the known set so a new one stands out. The hot path rule itself: [architecture.md § Hot path discipline](architecture.md#hot-path-discipline). A "no blocking in the hot path" audit must sweep *every* syscall the path can reach (connect, DNS, read, *and* write), not just the loudest one: a single-threaded loop that services I/O must make no blocking call at all (a socket timeout is not a fix, it is the size of the freeze; non-blocking + poll is the only safe shape). Fixing one blocker while an equally-blocking sibling survives is a partial fix that reads as complete.
 - **Code formatting** — `clang-format` with a project `.clang-format` file. Applied in CI; code that doesn't match fails the check. Run locally via editor integration or `clang-format -i`.
 
 ## When checks run