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

.clang-tidy

@@ -94,15 +94,20 @@
 #   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.
 #
-# NOT GATED YET. `WarningsAsErrors` stays empty until the remaining 47 clang-analyzer findings
-# are triaged (backlog: "clang-tidy: triage the 47 clang-analyzer findings"). Switching it to
-# `'*'` is the ratchet that stops a zero decaying, and it is one line — but it fails the gate
-# today, so it lands with that triage rather than before it.
+# NOT A GATE, BY DESIGN. `WarningsAsErrors` stays empty: this is a report, and other tools or
+# scripts decide what to do with what it finds. Turning findings into build errors would fail
+# every build on the ones that remain, and a gate nobody can satisfy gets disabled rather than
+# obeyed — it would also push people to NOLINT a finding under time pressure, which is the
+# opposite of what a report is for.
 ---
 Checks: >-
   *,
@@ -197,7 +202,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,20 +80,46 @@ 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:
           persist-credentials: false
       # The CMake configure needs uv on PATH — it resolves UV_EXECUTABLE for the UI-embed step
       # (CLAUDE.md § Use uv for every Python invocation), so a build without it fails at configure.
       - uses: astral-sh/setup-uv@v3
+      # RTSan needs Clang 20+; ubuntu-latest ships Clang 18, which rejects
+      # `-fsanitize=realtime` at the compiler-probe stage. Install a new enough one for that
+      # lane only — ASan/TSan run on the runner's default compiler.
+      - name: install clang for RealtimeSanitizer
+        if: matrix.kind == 'realtime'
+        run: |
+          wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | sudo tee /etc/apt/trusted.gpg.d/llvm.asc
+          sudo add-apt-repository -y "deb http://apt.llvm.org/$(lsb_release -cs)/ llvm-toolchain-$(lsb_release -cs)-20 main"
+          sudo apt-get install -y clang-20
       - name: build + run unit tests under ${{ matrix.kind }}sanitizer
         run: |
-          cmake -S . -B build/san -DCMAKE_BUILD_TYPE=Debug \
+          # RTSan is clang-only — GCC rejects `-fsanitize=realtime` outright — and the attribute
+          # it keys off is [[clang::nonblocking]], which GCC ignores anyway. Passed as a CMake
+          # variable on this lane only, rather than exporting CXX: an empty CXX="" on the other
+          # lanes is not the same as unset, and some probes read it as a broken compiler path.
+          compiler=""
+          if [ "${{ matrix.kind }}" = "realtime" ]; then
+            compiler="-DCMAKE_CXX_COMPILER=clang++-20"
+          fi
+          cmake -S . -B build/san -DCMAKE_BUILD_TYPE=Debug $compiler \
             -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,23 +341,116 @@ Fix options: (a) make every live mutate scenario clear+rebuild its own canvas (c
 
 ## Housekeeping
 
-### clang-tidy: triage the 47 clang-analyzer findings, then gate
+### 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.
+
+### lizard: 94 functions are measured under a mis-parsed name (baseline can't pin them)
+
+lizard's C++ parser loses the function name on certain bodies and falls back to the first keyword
+or cast it meets inside, so `mm::SolidEffect::tick` is reported as `mm::SolidEffect::static_cast<lengthType>`
+and `mm::NetworkModule::tick1s` as `mm::NetworkModule::switch`. Measured across `src/`: **94 of
+2404** functions carry such a name (`if` 47, `for` 41, `static_cast` 5, `switch` 1), of which **10**
+are over threshold and therefore reach the report.
+
+The consequence is the part that matters. `whitelizard.txt` matches by NAME, so those entries pin
+nothing: **35 of 162** baseline lines name a function lizard no longer produces. Each is a function
+whose complexity is now unmeasured against the baseline — real growth in it would either surface as
+a spurious "NEW violation" under the fallback name, or not surface at all. The check currently
+reports `FAIL — 9 NEW` on an unmodified tree for exactly this reason, which trains the reader to
+ignore the number.
+
+It is not a threshold problem and not fixable by re-baselining: re-running `--baseline` just freezes
+today's fallback names, which shift again the moment a line moves inside the body. Real options, in
+order of preference: key the baseline on `file:startline`-anchored identity or lizard's `long_name`
+instead of `name`; pre-process so the parser keeps the name; or replace lizard's C++ front end with
+a clang-AST-based complexity pass (`check_clang_query.py` already has the AST machinery, so the
+metric could move there and drop the dependency entirely). Sizeable enough for its own `/plan`.
+
+### clang-tidy: triage the remaining 30 findings
 
 `.clang-tidy` runs `*` minus a documented disable list and reaches zero on everything except the
-path-sensitive `clang-analyzer-*` family: **47 findings**, led by `core.UndefinedBinaryOperatorResult`
-(11), `bugprone-unchecked-string-to-number-conversion` (9), `security.insecureAPI.strcpy` (5),
-`cplusplus.NewDeleteLeaks` (4) and `core.NonNullParamChecker` (4). `NewDeleteLeaks` and
-`cplusplus.Move` are the two worth reading first — the analyzer traces real paths, so a finding is
-a claim about an execution, not a style opinion.
+path-sensitive `clang-analyzer-*` family: **30 findings**, led by
+`bugprone-unchecked-string-to-number-conversion` (9), `security.insecureAPI.strcpy` (5),
+`core.NonNullParamChecker` (4), `deadcode.DeadStores` (3) and `unix.cstring.NullArg` (3). Roughly
+half sit in test code (`strcpy` into fixed local buffers, a deliberate divide-by-zero probe).
+
+The nine `unchecked-string-to-number-conversion` sites were read individually and are all safe:
+HueDriver guards on `if (id > 0)`, JsonUtil documents 0-means-absent, MqttModule uses a `v = -1`
+sentinel and clamps. They stay reported rather than suppressed — a report shows the real situation,
+and a `NOLINT` would hide a genuine class of bug for the next person who writes one of these.
 
 These surfaced late for an instructive reason: the report parser's check-name pattern rejected the
 `,-warnings-as-errors` suffix clang-tidy appends when `WarningsAsErrors` is set, so **every finding
 was silently dropped** the moment the ratchet was switched on. Fixed; recorded here because it is
 the sixth silent-zero this tooling has produced, and each looked like a clean tree.
 
-Once triaged (fix / `NOLINT` with a reason / disable with a measured one), set
-`WarningsAsErrors: '*'`. That one line is the ratchet that stops a zero decaying back into noise;
-it is deliberately not set today because it would fail the gate on the 47.
+`WarningsAsErrors` stays empty — clang-tidy is a report, not a gate (the same rule the hot-path
+check follows). A finding is fixed where it is real, or stays visible with its reason; it does not
+become a build error, which would only invite a `NOLINT` under time pressure.
 
 
 ### Heap-allocate the `registerType<T>` boot probe (lift a per-module lesson into core)

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