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

[ewowi]

What this lands

Hot-path discipline, checked by the compiler. MoonModule::tick/tick20ms/tick1s carry
MM_NONBLOCKING, and Clang verifies under -Wfunction-effects that nothing the render path
reaches can block or allocate — transitively, through the whole call graph. That is the half
check_hotpath.py's regex cannot see: it reads the text of a tick body and is blind to what its
callees do.

The attribute is inherited by overrides, so three annotations cover ~90 modules. It also sits in
tickChildren's member-pointer type — without that, the indirect dispatch is a hole every
module's tick escapes through. Passing an unannotated method is now a compile error.

MM_NONBLOCKING is empty on GCC: the ESP32 toolchain has neither the attribute nor the warning
and builds with -Werror, so a bare attribute there is a build break. Same compiler-gated shape
as MM_PRINTF_FORMAT.

A MoonDeck card, "clang-hotpath", reports the 181 findings this surfaced, split by tier —
70 on tick() (every frame), 6 on tick20ms(), 95 on tick1s(), 10 unresolved — because
the same blocking call costs orders of magnitude more in one than another. The sharp ones are UDP
sendTo/recvFrom inside AudioService::tick: socket I/O every frame.

Five tier-zero warnings join -Wall -Wextra -Werror: -Wshadow -Wnon-virtual-dtor -Wdouble-promotion -Wimplicit-fallthrough -Wnull-dereference. The ESP32 build is clean under
them too. Three real defects fixed:

• Rings241Layout: 1.1 was a double literal promoting a float — a silent softfloat call on a
  chip with no FPU.
• RubiksCubeEffect: a parameter shadowed the effect's cubeSize control field.
• unit_PreviewDriver: a test double with virtual functions and a public non-virtual destructor.

Correction recorded

Earlier text in this repo claimed check_hotpath.py covers src/platform/esp32/. It does
not. Measured: it scans 67 tick methods, all in src/core/ and src/light/, zero in the
platform layer — which has no tick methods, being free functions the tick path calls into. All 60
of its files compile on desktop, so the compiler check covers the identical set. It reports 0
findings where the compiler reports 181.

Not done here

The 181 findings are backlogged, along with deleting check_hotpath.py (170 lines). The blocker
is gate order, not coverage: the script fails pre-commit today while -Wfunction-effects
carries -Wno-error, so deleting it first would leave hot-path discipline with nothing
enforcing. Sequence: triage → drop -Wno-error → delete and swap the gate.

Performance

tick:4231us(FPS:236) on esp32s3-n16r8, measured on MM-testbench-S3. Desktop tick:129us.
The annotation is compile-time only: ESP32 flash is byte-identical at 1,667,104 B.

Summary by CodeRabbit

• New Features

  • Added a “Last-run log” button to script cards, allowing the most recent script output to be replayed without rerunning the script.
  • Script output is now streamed live and saved for later viewing, with automatic log-size limits.

• Bug Fixes

  • Improved desktop and ESP32 connection-state handling for more reliable network status reporting.
  • Corrected several rendering edge cases involving empty or degenerate grids.

• Documentation

  • Updated static-analysis, hot-path, and repository health documentation and metrics.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bca7209f-95a6-4d35-a727-0929d0724123

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• ✅ Review completed - (🔄 Check again to review again)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch next

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (11)

src/platform/desktop/platform_desktop.cpp (1)

108-129: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Guard the Clang-only diagnostic pragmas.

platform_desktop.cpp is built on desktop GCC/Clang with -Wall -Wextra -Werror; #pragma clang diagnostic only belongs inside a Clang check so GCC doesn’t emit a warning in a warning-as-error build.

Wrap these pragmas in #if defined(__clang__) / #endif.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/desktop/platform_desktop.cpp` around lines 108 - 129, Guard the
clang diagnostic push, ignored -Wfunction-effects, and pop directives
surrounding millis() and micros() with `#if` defined(__clang__) and a matching
`#endif`, leaving the clock implementations unchanged for GCC and Clang builds.

src/light/layers/Layer.h (1)

148-192: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Defer applyState() out of the annotated render tick.

Line 192 can recursively call prepare()/release(), resize ScratchBuffers, and touch module peripherals. A beat-driven modifier can therefore trigger allocation or blocking work directly inside MM_NONBLOCKING tick(). Queue the rebuild request and execute it through the scheduler’s non-render prepare path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/layers/Layer.h` around lines 148 - 192, Defer modifier-triggered
rebuilds out of Layer::tick() instead of calling applyState() from the
MM_NONBLOCKING render path. Preserve the existing coalescing of
consumeNeedsRebuild() requests, but queue the rebuild for the scheduler’s
non-render prepare path so applyState(), including prepare/release,
ScratchBuffer resizing, and peripheral access, runs outside tick().

Source: Path instructions

src/core/HttpServerModule.cpp (1)

67-107: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Keep synchronous transport work out of both annotated callbacks.

• src/core/HttpServerModule.cpp#L67-L107: replace synchronous handleConnection() processing with resumable/nonblocking work; the outer 100 ms budget cannot bound a single call.
• src/core/HttpServerModule.cpp#L111-L112: route WLED state frames through the resumable sender or a transport task instead of inline TcpConnection::write calls.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/HttpServerModule.cpp` around lines 67 - 107, Keep synchronous
transport work out of both annotated callbacks: in src/core/HttpServerModule.cpp
lines 67-107, replace inline handleConnection processing in
HttpServerModule::tick20ms with resumable/nonblocking connection handling so the
outer 100 ms budget can actually bound work; in src/core/HttpServerModule.cpp
lines 111-112, route WLED state frames through the existing resumable sender or
a transport task instead of performing inline TcpConnection::write calls.

Source: Path instructions

src/light/effects/RipplesEffect.h (1)

37-70: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Remove per-light floating-point math from this tick.

sqrt, sin, and floor execute inside the x/z render loop on every frame, so cost scales with the grid size and violates the hot-path rule. Use fixed-point/integer lookup tables or precompute bounded wave data outside tick(); keep the inner loop integer-only.

As per path instructions, render-loop callees must avoid per-light floating-point math.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/effects/RipplesEffect.h` around lines 37 - 70, Replace the
per-light sqrt, sin, and floor calculations in RipplesEffect::tick with
fixed-point or integer lookup data prepared outside the x/z render loop, while
preserving the current ripple geometry, timing, and y-coordinate output. Ensure
the inner loop uses only integer arithmetic and existing bounded dimensions,
with any required wave-table or distance precomputation performed once during
setup or otherwise outside tick().

Source: Path instructions

src/core/ImprovProvisioningModule.h (1)

177-197: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Keep APPLY_OP execution out of the nonblocking tick.

Line 186 invokes HttpServerModule::applyOp(), which can add, replace, or clear modules and trigger allocation/release and prepareTree(). The failure path also calls std::printf at Line 189. Execute the queued operation in a cold/control task and publish only its result/status from tick().

As per path instructions, render-loop callees must avoid blocking operations and heap work.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/ImprovProvisioningModule.h` around lines 177 - 197, Move the queued
APPLY_OP execution out of ImprovProvisioningModule::tick(): have a cold/control
task call HttpServerModule::applyOp(), including its logging and status updates,
then publish only the operation result/status and completion state for tick() to
consume. Keep tick() limited to nonblocking state handling and preserve the
existing Ok/AlreadyExists success behavior and failure reporting.

Source: Path instructions

src/light/drivers/HueDriver.h (1)

108-126: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Move Hue network work out of the annotated tick callbacks.

tick() is marked MM_NONBLOCKING, but pushOneChangedLight() reaches synchronous platform::httpRequest() at Line 655, with a timeout of up to 200 ms. tick1s() similarly performs HTTP requests and heap allocation through pollPairing(), fetchLights(), fetchGroups(), and reportBridge(). These callbacks can stall the render loop for 200–400 ms despite the annotation.

Queue this work on a worker or make the platform API genuinely nonblocking; keep the callbacks limited to bounded state updates.

As per coding guidelines, render-loop callbacks and their callees must avoid blocking operations and heap allocations.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/drivers/HueDriver.h` around lines 108 - 126, Move all Hue network
and allocation work out of the MM_NONBLOCKING callbacks tick() and tick1s().
Replace direct calls to pushOneChangedLight(), pollPairing(), fetchLights(),
fetchGroups(), and reportBridge() with bounded state updates that queue or
signal equivalent work to a worker context, preserving the existing timing and
ordering behavior; ensure the worker performs the synchronous HTTP requests and
heap allocations.

Source: Coding guidelines

test/unit/light/unit_Drivers_rendersplit.cpp (1)

53-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a rationale for the deliberate MM_NONBLOCKING violation in SlowDriver::tick.

test_Drivers_rendersplit.cpp:53 is annotated nonblocking despite using a mutex and condition_variable::wait_for(5s). It also generates a fixed check_nonblocking.py Tick tick() finding, so add an in-file reason explaining the test harness intent and keep the diagnostic scoped if the toolchain accepts a pragma.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/unit/light/unit_Drivers_rendersplit.cpp` around lines 53 - 65, Add an
in-file comment at SlowDriver::tick explaining that its mutex and timed wait are
intentional to hold the test’s use-after-free window open, despite the
MM_NONBLOCKING annotation. If supported by the toolchain, narrowly suppress the
check_nonblocking.py Tick tick() diagnostic around this method only, without
changing the test synchronization behavior.

src/core/FilesystemModule.cpp (1)

50-55: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Move persistence flushing off tick1s().

FilesystemModule::tick1s() is marked MM_NONBLOCKING, but after the debounce it calls flush() → g rows a heap JsonSink and performs LittleFS write/fsync/rename via platform::fsWriteAtomic(). Keep this callback to status polling and route the actual save through a worker / non-hot persistence mechanism.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/FilesystemModule.cpp` around lines 50 - 55, Remove the persistence
call from the nonblocking FilesystemModule::tick1s() callback, retaining only
mount, scheduler, timestamp, dirty-state, and debounce checks for status
polling. Route the debounced save operation through an existing worker or other
non-hot persistence mechanism, ensuring flush() and its filesystem work execute
outside tick1s().

Source: Coding guidelines

src/light/effects/DemoReelEffect.h (1)

56-64: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Defer effect replacement outside the render callback.

advance() calls swapTo() from within tick(), and swapTo() creates/deletes effect children and runs setup/applyState(). That puts allocation and release work on the nonblocking render path; queue the switch and run the replacement in a deferred lifecycle path while making tick() render only the current child.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/effects/DemoReelEffect.h` around lines 56 - 64, Update
DemoReelEffect::tick so it only renders the current child and records a pending
switch when the interval elapses; do not call advance() or swapTo() from the
render callback. Add or use a deferred lifecycle/update path to process the
pending switch outside tick, where effect creation, deletion, setup, and
applyState() can safely run.

Source: Coding guidelines

src/light/drivers/ParallelLedDriver.h (1)

552-578: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Keep ParallelLedDriver::tick() out of MM_NONBLOCKING while it waits on bus transfers.

tickSync() and tickRing() both call busWaitIfBusy(), which forwards a void busWait(uint8_t, uint32_t) wait to the peripheral. On a wedged transfer this callback can block the render thread for the wait budget before tick() returns, so the MM_NONBLOCKING override should not be advertised until those waits are moved outside this hot path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/drivers/ParallelLedDriver.h` around lines 552 - 578, Remove the
MM_NONBLOCKING annotation from ParallelLedDriver::tick() because its tickSync()
and tickRing() paths can block through busWaitIfBusy(); leave the dispatch and
transfer behavior unchanged until those waits are moved out of the render path.

Source: Coding guidelines

src/light/drivers/Drivers.h (1)

385-396: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

The new nonblocking contract is violated by synchronous work in three callbacks.

• src/light/drivers/Drivers.h#L385-L396: remove worker joining/stopping from tick(); use asynchronous recovery.
• src/light/drivers/NetworkSendDriver.h#L208-L208: enqueue network transmission instead of sending the frame inline.
• src/light/drivers/RmtLedDriver.h#L234-L295: poll or offload hardware completion and reset timing instead of waiting in tick().

As per coding guidelines, render-loop callbacks and their callees must avoid blocking operations.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/drivers/Drivers.h` around lines 385 - 396, Remove synchronous
worker joining/stopping from Drivers::tick and replace the timed-out recovery
with asynchronous handling; in src/light/drivers/Drivers.h:385-396, preserve
safe buffer ownership without blocking the render callback. In
src/light/drivers/NetworkSendDriver.h:208, update NetworkSendDriver transmission
to enqueue frames rather than send inline. In
src/light/drivers/RmtLedDriver.h:234-295, update RmtLedDriver hardware
completion and reset handling to poll or offload the work instead of waiting in
tick.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/history/plans/Plan-20260727` - Static analysis and repo health
tooling.md:
- Around line 78-80: Update the collective-noun sentence in the plan text to
read “The bulk of the findings were unannotated helpers,” preserving the
surrounding analysis and meaning.

In `@moondeck/check/check_nonblocking.py`:
- Around line 77-82: Update build_output to check the subprocess result from
cmake; when the build exits with a non-zero return code, surface an error
instead of returning output that is interpreted as no findings. Preserve the
existing combined stdout and stderr behavior for successful builds.
- Around line 127-141: Update the imports and build-directory lookup in the
nonblocking check to use a direct check_clang_tidy import, or an existing public
API, instead of accessing check_clang_query.check_clang_tidy._host_build_dir().
Preserve the current build validation and subsequent filtering behavior.
- Around line 168-172: Update the row-printing loop around subset and clip so
the em-dash default is assigned to a variable before the f-string, then pass
that variable to clip instead of using the Unicode escape inside the f-string
expression. Preserve the existing fallback output and formatting while keeping
the script compatible with Python versions before 3.12.

In `@moondeck/MoonDeck.md`:
- Around line 346-347: Update the tier-frequency statement near “tick()”,
“tick20ms()”, and “tick1s()” to accurately describe the difference between 50
calls per second and 1 call per second; replace “three orders of magnitude” with
“roughly two orders of magnitude” or a similarly non-exaggerated phrase.

In `@src/core/FileManagerModule.cpp`:
- Around line 44-46: Update FileManagerModule::tick1s() so it does not call
platform::filesystemUsed() from the MM_NONBLOCKING path. Cache filesystem usage
from a non-hot-path task and have tick1s() read the cached value, or remove
MM_NONBLOCKING until the usage call is guaranteed nonblocking.

In `@src/core/HttpServerModule.h`:
- Around line 176-177: The nonblocking annotations cover callbacks that still
perform blocking or allocating work. In src/core/HttpServerModule.h lines
176-177, move blocking HTTP response handling out of tick20ms() or use
exclusively nonblocking writes; in src/core/MqttModule.cpp line 414, remove
synchronous DNS and discovery-buffer allocation from tick1s(); and in
src/core/NetworkModule.h line 380, move WiFi/AP lifecycle transitions and tree
rebuilds off the annotated callback. Keep -Wno-error=function-effects explicitly
temporary until all affected paths are asynchronous, bounded, or
allocation-free.

In `@src/core/Scheduler.cpp`:
- Line 66: Remove the MM_NONBLOCKING contract from Scheduler::tick() unless its
dispatch path is made fully nonblocking. Ensure tick20ms(), tick1s(), and their
callees do not perform synchronous transport work such as
HttpServerModule::handleConnection(); otherwise move transport handling outside
the annotated render callback.

In `@src/core/SystemModule.h`:
- Line 232: Update the P4 implementation of tick1s() so its coprocessorWifi()
path does not directly invoke the synchronous
esp_hosted_get_coprocessor_fwversion() query; instead, maintain the firmware
version via a non-render task or nonblocking cached snapshot and have tick1s()
read that cached value while preserving its MM_NONBLOCKING contract.

In `@src/light/drivers/PreviewDriver.h`:
- Line 132: Remove the MM_NONBLOCKING contract from PreviewDriver::tick() until
its reachable work is safe, or refactor sendFrame()’s synchronous socket path
and buildAndSendCoordTable()’s keptIdx_ allocation off the render path; ensure
every operation reachable from tick() avoids blocking I/O and heap allocation
before restoring the annotation.

In `@src/light/effects/BouncingBallsEffect.h`:
- Line 49: The nonblocking render callbacks perform per-element floating-point
work; update BouncingBallsEffect::tick in
src/light/effects/BouncingBallsEffect.h:49-49 to use fixed-point/integer math or
precompute trajectory, damping, square-root, and rounding values off the render
path, and update SphereMoveEffect::tick in
src/light/effects/SphereMoveEffect.h:36-36 to replace per-voxel trigonometric
and distance calculations with integer math or precomputed values.

In `@src/platform/esp32/platform_esp32.cpp`:
- Around line 888-894: Make the cross-task status fields ethLinkUp_,
ethConnected_, and wifiStaConnected_ atomic booleans, and update their
event-handler writes and getter reads to use relaxed stores and loads. Preserve
the existing ethLinkUp() and ethConnected() return behavior while ensuring all
accesses to these flags use the atomic operations.

In `@src/platform/platform.h`:
- Around line 13-25: Update the fallback comment above MM_NONBLOCKING to state
that GCC omits the clang attribute but retains noexcept, accurately describing
the macro’s exception behavior.

---

Outside diff comments:
In `@src/core/FilesystemModule.cpp`:
- Around line 50-55: Remove the persistence call from the nonblocking
FilesystemModule::tick1s() callback, retaining only mount, scheduler, timestamp,
dirty-state, and debounce checks for status polling. Route the debounced save
operation through an existing worker or other non-hot persistence mechanism,
ensuring flush() and its filesystem work execute outside tick1s().

In `@src/core/HttpServerModule.cpp`:
- Around line 67-107: Keep synchronous transport work out of both annotated
callbacks: in src/core/HttpServerModule.cpp lines 67-107, replace inline
handleConnection processing in HttpServerModule::tick20ms with
resumable/nonblocking connection handling so the outer 100 ms budget can
actually bound work; in src/core/HttpServerModule.cpp lines 111-112, route WLED
state frames through the existing resumable sender or a transport task instead
of performing inline TcpConnection::write calls.

In `@src/core/ImprovProvisioningModule.h`:
- Around line 177-197: Move the queued APPLY_OP execution out of
ImprovProvisioningModule::tick(): have a cold/control task call
HttpServerModule::applyOp(), including its logging and status updates, then
publish only the operation result/status and completion state for tick() to
consume. Keep tick() limited to nonblocking state handling and preserve the
existing Ok/AlreadyExists success behavior and failure reporting.

In `@src/light/drivers/Drivers.h`:
- Around line 385-396: Remove synchronous worker joining/stopping from
Drivers::tick and replace the timed-out recovery with asynchronous handling; in
src/light/drivers/Drivers.h:385-396, preserve safe buffer ownership without
blocking the render callback. In src/light/drivers/NetworkSendDriver.h:208,
update NetworkSendDriver transmission to enqueue frames rather than send inline.
In src/light/drivers/RmtLedDriver.h:234-295, update RmtLedDriver hardware
completion and reset handling to poll or offload the work instead of waiting in
tick.

In `@src/light/drivers/HueDriver.h`:
- Around line 108-126: Move all Hue network and allocation work out of the
MM_NONBLOCKING callbacks tick() and tick1s(). Replace direct calls to
pushOneChangedLight(), pollPairing(), fetchLights(), fetchGroups(), and
reportBridge() with bounded state updates that queue or signal equivalent work
to a worker context, preserving the existing timing and ordering behavior;
ensure the worker performs the synchronous HTTP requests and heap allocations.

In `@src/light/drivers/ParallelLedDriver.h`:
- Around line 552-578: Remove the MM_NONBLOCKING annotation from
ParallelLedDriver::tick() because its tickSync() and tickRing() paths can block
through busWaitIfBusy(); leave the dispatch and transfer behavior unchanged
until those waits are moved out of the render path.

In `@src/light/effects/DemoReelEffect.h`:
- Around line 56-64: Update DemoReelEffect::tick so it only renders the current
child and records a pending switch when the interval elapses; do not call
advance() or swapTo() from the render callback. Add or use a deferred
lifecycle/update path to process the pending switch outside tick, where effect
creation, deletion, setup, and applyState() can safely run.

In `@src/light/effects/RipplesEffect.h`:
- Around line 37-70: Replace the per-light sqrt, sin, and floor calculations in
RipplesEffect::tick with fixed-point or integer lookup data prepared outside the
x/z render loop, while preserving the current ripple geometry, timing, and
y-coordinate output. Ensure the inner loop uses only integer arithmetic and
existing bounded dimensions, with any required wave-table or distance
precomputation performed once during setup or otherwise outside tick().

In `@src/light/layers/Layer.h`:
- Around line 148-192: Defer modifier-triggered rebuilds out of Layer::tick()
instead of calling applyState() from the MM_NONBLOCKING render path. Preserve
the existing coalescing of consumeNeedsRebuild() requests, but queue the rebuild
for the scheduler’s non-render prepare path so applyState(), including
prepare/release, ScratchBuffer resizing, and peripheral access, runs outside
tick().

In `@src/platform/desktop/platform_desktop.cpp`:
- Around line 108-129: Guard the clang diagnostic push, ignored
-Wfunction-effects, and pop directives surrounding millis() and micros() with
`#if` defined(__clang__) and a matching `#endif`, leaving the clock implementations
unchanged for GCC and Clang builds.

In `@test/unit/light/unit_Drivers_rendersplit.cpp`:
- Around line 53-65: Add an in-file comment at SlowDriver::tick explaining that
its mutex and timed wait are intentional to hold the test’s use-after-free
window open, despite the MM_NONBLOCKING annotation. If supported by the
toolchain, narrowly suppress the check_nonblocking.py Tick tick() diagnostic
around this method only, without changing the test synchronization behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1ecd018d-7914-481c-8cfe-0d03bf8e5533

📥 Commits

Reviewing files that changed from the base of the PR and between da23a92 and bc02fe4.

📒 Files selected for processing (98)

• CMakeLists.txt
• docs/backlog/backlog-core.md
• docs/history/plans/Plan-20260727 - Static analysis and repo health tooling.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• moondeck/MoonDeck.md
• moondeck/check/check_nonblocking.py
• moondeck/moondeck_config.json
• src/core/AudioService.h
• src/core/DevicesModule.h
• src/core/FileManagerModule.cpp
• src/core/FileManagerModule.h
• src/core/FilesystemModule.cpp
• src/core/FilesystemModule.h
• src/core/FirmwareUpdateModule.h
• src/core/HttpServerModule.cpp
• src/core/HttpServerModule.h
• src/core/ImprovProvisioningModule.h
• src/core/IrService.h
• src/core/MoonModule.h
• src/core/MqttModule.cpp
• src/core/MqttModule.h
• src/core/NetworkModule.h
• src/core/PinsModule.h
• src/core/Scheduler.cpp
• src/core/Scheduler.h
• src/core/SystemModule.h
• src/core/TasksModule.h
• src/light/drivers/DriverBase.h
• src/light/drivers/Drivers.h
• src/light/drivers/HueDriver.h
• src/light/drivers/LightPresetsModule.h
• src/light/drivers/NetworkSendDriver.h
• src/light/drivers/ParallelLedDriver.h
• src/light/drivers/PreviewDriver.h
• src/light/drivers/RmtLedDriver.h
• src/light/effects/AudioSpectrumEffect.h
• src/light/effects/AudioVolumeEffect.h
• src/light/effects/BlurzEffect.h
• src/light/effects/BouncingBallsEffect.h
• src/light/effects/DemoReelEffect.h
• src/light/effects/DistortionWavesEffect.h
• src/light/effects/EffectBase.h
• src/light/effects/FireEffect.h
• src/light/effects/FixedRectangleEffect.h
• src/light/effects/FreqMatrixEffect.h
• src/light/effects/FreqSawsEffect.h
• src/light/effects/GEQ3DEffect.h
• src/light/effects/GEQEffect.h
• src/light/effects/GameOfLifeEffect.h
• src/light/effects/LavaLampEffect.h
• src/light/effects/LinesEffect.h
• src/light/effects/LissajousEffect.h
• src/light/effects/MetaballsEffect.h
• src/light/effects/NetworkReceiveEffect.h
• src/light/effects/Noise2DEffect.h
• src/light/effects/NoiseEffect.h
• src/light/effects/NoiseMeterEffect.h
• src/light/effects/PaintBrushEffect.h
• src/light/effects/ParticlesEffect.h
• src/light/effects/PlasmaEffect.h
• src/light/effects/PraxisEffect.h
• src/light/effects/RainbowEffect.h
• src/light/effects/RandomEffect.h
• src/light/effects/RingsEffect.h
• src/light/effects/RipplesEffect.h
• src/light/effects/RubiksCubeEffect.h
• src/light/effects/SineEffect.h
• src/light/effects/SolidEffect.h
• src/light/effects/SphereMoveEffect.h
• src/light/effects/SpiralEffect.h
• src/light/effects/StarFieldEffect.h
• src/light/effects/StarSkyEffect.h
• src/light/effects/TetrixEffect.h
• src/light/effects/TextEffect.h
• src/light/effects/WaveEffect.h
• src/light/layers/Layer.h
• src/light/layers/Layers.h
• src/light/layouts/LayoutBase.h
• src/light/layouts/Rings241Layout.h
• src/light/modifiers/ModifierBase.h
• src/light/modifiers/RandomMapModifier.h
• src/light/modifiers/RotateModifier.h
• src/light/moonlive/MoonLiveEffect.h
• src/platform/desktop/platform_desktop.cpp
• src/platform/esp32/platform_esp32.cpp
• src/platform/platform.h
• test/unit/core/unit_ModuleFactory.cpp
• test/unit/core/unit_MoonModule_lifecycle.cpp
• test/unit/core/unit_Services.cpp
• test/unit/core/unit_SystemModule.cpp
• test/unit/light/unit_Drivers_container.cpp
• test/unit/light/unit_Drivers_rendersplit.cpp
• test/unit/light/unit_Layer_extrude.cpp
• test/unit/light/unit_Layer_live_modifier.cpp
• test/unit/light/unit_Layer_persistence.cpp
• test/unit/light/unit_Layouts_toggle_cycle.cpp
• test/unit/light/unit_PreviewDriver.cpp

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C3 'coprocessorWifi|esp_hosted_get_coprocessor_fwversion' src/core src/platform

Repository: MoonModules/projectMM

Length of output: 6815

---

🌐 Web query:

ESP-IDF esp_hosted_get_coprocessor_fwversion source esp_hosted.c

💡 Result:

The function esp_hosted_get_coprocessor_fwversion is a part of the ESP-Hosted component in the ESP-IDF ecosystem, used to retrieve the current firmware version of the connected slave coprocessor [1][2]. API Signature The function is defined as: esp_err_t esp_hosted_get_coprocessor_fwversion(esp_hosted_coprocessor_fwver_t *version); [1][2] It populates a structure containing the major, minor, and patch version numbers [1][2]: typedef struct { uint32_t major1; // Major version uint32_t minor1; // Minor version uint32_t patch1; // Patch version } esp_hosted_coprocessor_fwver_t; [1][2] Implementation Details The function typically communicates with the slave device using Remote Procedure Calls (RPC) to fetch the version information synchronously [3]. It is commonly used in OTA (Over-The-Air) update workflows, allowing the host to verify the current slave version before initiating an update [1][2]. Historically, this function was found in files such as esp_hosted_api.c [3], though as the ESP-Hosted codebase has evolved (including significant refactors and code reorganizations), its location may vary depending on the specific version of the ESP-Hosted component being used [3][4]. Users should ensure they are using the latest version of the esp_hosted component for their specific ESP-IDF project [4][5].

Citations:

• 1: https://github.com/espressif/esp-hosted-mcu/tree/main/examples/host_performs_slave_ota
• 2: https://components.espressif.com/components/espressif/esp_hosted/versions/2.7.4/examples/host_performs_slave_ota?language=en
• 3: espressif/esp-hosted-mcu@6b56550
• 4: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.5/readme?language=en
• 5: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.3/dependencies?language=en

---

🌐 Web query:

ESP-IDF esp_wifi_remote SDIO esp_hosted_get_coprocessor_fwversion nonblocking

💡 Result:

The esp_hosted_get_coprocessor_fwversion function, provided by the esp_hosted component, is used to retrieve the firmware version of a connected ESP co-processor [1][2]. It is typically utilized within the context of OTA (Over-the-Air) update workflows to check if an update is necessary by comparing the slave's current version against an available firmware image [1][2]. Based on official documentation and common usage patterns, here are key points regarding your query: Functionality and Blocking Behavior The function esp_hosted_get_coprocessor_fwversion is a synchronous, blocking call [1][2]. It initiates a Remote Procedure Call (RPC) request across the transport bus (such as SDIO) to the co-processor and waits for the response [3][4]. Because it must perform a round-trip communication over the transport interface, it will block the calling task until the version information is returned or a timeout occurs [3][1]. Implementation Context The esp_wifi_remote component relies on the esp_hosted component as its primary backend for transport (e.g., via SDIO or SPI) [3][4][5]. While esp_wifi_remote provides a transparent API for Wi-Fi, the specific function for querying co-processor firmware version belongs to the underlying esp_hosted transport layer [1][2]. Considerations for Non-blocking Needs If you require non-blocking behavior to prevent the calling task from stalling, you should handle the version query in a dedicated task or thread [6][7]. Since esp_hosted is designed for host-slave communication, perform this query after the hosted transport has successfully initialized (esp_hosted_init) [7][2]. Calling it immediately upon boot before the transport is ready may result in failures or exceptions [7]. Summary of Key Dependencies - Transport: The esp_hosted component handles the communication (SDIO/SPI) required for the RPC [8][9]. - API Layer: The esp_wifi_remote component utilizes these underlying transport mechanisms to translate Wi-Fi API calls [3][4]. - Versioning: Ensure the slave firmware uses modern semantic versioning (X.Y.Z), as older versions based on git hashes can cause issues in comparison logic during OTA updates [2][10].

Citations:

• 1: https://github.com/espressif/esp-hosted-mcu/tree/main/examples/host_performs_slave_ota
• 2: https://components.espressif.com/components/espressif/esp_hosted/versions/2.11.5/examples/host_performs_slave_ota?language=
• 3: https://components.espressif.com/components/espressif/esp_wifi_remote/versions/1.6.2/readme
• 4: https://developer.espressif.com/blog/2025/09/esp-wifi-remote/
• 5: https://github.com/espressif/esp-wifi-remote
• 6: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.5/changelog?language=en
• 7: Issues updating the slave from HTTPS and query it's current firmware version (EHM-74) espressif/esp-hosted-mcu#71
• 8: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.1/readme?language=en
• 9: https://components.espressif.com/components/espressif/esp_hosted/versions/0.0.9/readme?language=en
• 10: https://components.espressif.com/components/espressif/esp_hosted/versions/2.7.4/examples/host_performs_slave_ota?language=en

---

🌐 Web query:

ESP-IDF esp_hosted_get_coprocessor_fwversion implementation source

💡 Result:

The function esp_hosted_get_coprocessor_fwversion is part of the ESP-Hosted component in the ESP-IDF ecosystem [1][2]. It is designed to query the firmware version of the connected co-processor [3][4]. Implementation Details: The function is declared to return an esp_err_t and accepts a pointer to an esp_hosted_coprocessor_fwver_t structure, which stores the major, minor, and patch version numbers [3][4]. In the source code, this function typically acts as a wrapper that calls an underlying RPC (Remote Procedure Call) mechanism to communicate with the co-processor [5]. For example, in many versions of the ESP-Hosted codebase, the implementation in the host porting layer is: esp_err_t esp_hosted_get_coprocessor_fwversion(esp_hosted_coprocessor_fwver_t *ver_info) { return rpc_get_coprocessor_fwversion(ver_info); } The rpc_get_coprocessor_fwversion function subsequently handles the synchronous communication required to retrieve the version information from the slave device [5]. Usage: To use this function, you populate a local instance of the version structure and pass its address to the function [6][7]: esp_hosted_coprocessor_fwver_t ver_info; if (esp_hosted_get_coprocessor_fwversion(&ver_info) == ESP_OK) { ESP_LOGI("TAG", "Coprocessor version: %u.%u.%u", ver_info.major1, ver_info.minor1, ver_info.patch1); } You can find the exact source files within the espressif/esp-hosted component repository, typically under the host driver or API source directories, such as host/api/src/esp_hosted_api.c [5].

Citations:

• 1: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.2/readme?language=en
• 2: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.5/readme?language=en
• 3: https://github.com/espressif/esp-hosted-mcu/tree/main/examples/host_performs_slave_ota
• 4: https://components.espressif.com/components/espressif/esp_hosted/versions/2.12.1/examples/host_performs_slave_ota?language=
• 5: espressif/esp-hosted-mcu@6b56550
• 6: Issues updating the slave from HTTPS and query it's current firmware version (EHM-74) espressif/esp-hosted-mcu#71
• 7: https://api-docs-beta.esphome.io/esp32__hosted__update_8cpp_source

---

🌐 Web query:

site:github.com/espressif/esp-hosted-mcu "rpc_get_coprocessor_fwversion" "esp_hosted_get_coprocessor_fwversion"

💡 Result:

In the ESP-Hosted framework (specifically esp-hosted-mcu), esp_hosted_get_coprocessor_fwversion and rpc_get_coprocessor_fwversion are related functions used to retrieve firmware version information from the co-processor [1][2]. esp_hosted_get_coprocessor_fwversion serves as a public-facing API function intended for application use [2]. It acts as a wrapper that internally calls rpc_get_coprocessor_fwversion to perform the actual Remote Procedure Call (RPC) to the slave co-processor [1][3]. When called, the function populates an esp_hosted_coprocessor_fwver_t structure with version details such as major, minor, and patch numbers, as well as additional information like revision, build, and chip target details [3]. As of recent updates, the RPC implementation has been extended to include identifiers like chip_id and target name to provide more comprehensive version and hardware identification [3].

Citations:

• 1: espressif/esp-hosted-mcu@6b56550
• 2: espressif/esp-hosted-mcu@08c6580
• 3: espressif/esp-hosted-mcu@1fff587

---

🌐 Web query:

github esp-hosted-mcu platform_port/esp32 platform_port/esp32/rpc.c rpc_get_coprocessor_fwversion function source

💡 Result:

In the esp-hosted-mcu repository, the rpc_get_coprocessor_fwversion function is primarily defined and declared within the RPC wrapper layer, rather than a specific platform_port/esp32/rpc.c file [1]. Based on the repository structure, the function is declared in host/drivers/rpc/wrap/rpc_wrap.h [2][1]: esp_err_t rpc_get_coprocessor_fwversion(esp_hosted_coprocessor_fwver_t *ver_info); The implementation is located in host/drivers/rpc/wrap/rpc_wrap.c [1][3], which handles the synchronous execution of the RPC call [1]. This function facilitates communication with the co-processor to retrieve firmware version information, identified in the protobuf definition as Req_GetCoprocessorFwVersion (ID 350) [4][5]. Historically, the project underwent code reorganization [1], and while there are platform-specific ports (e.g., host/port/), the core RPC logic resides in the host/drivers/rpc/ directory [6]. You can track the implementation details and recent changes in host/drivers/rpc/wrap/rpc_wrap.c [3].

Citations:

• 1: espressif/esp-hosted-mcu@6b56550
• 2: https://github.com/espressif/esp-hosted-mcu/blob/main/host/drivers/rpc/wrap/rpc_wrap.h
• 3: espressif/esp-hosted-mcu@47d724e
• 4: https://github.com/espressif/esp-hosted-mcu/blob/main/common/proto/esp_hosted_rpc.proto
• 5: https://github.com/espressif/esp-hosted-mcu/blob/main/docs/implemented_rpcs.md
• 6: https://github.com/espressif/esp-hosted-mcu/blob/4b5b4700993ce2102b04cea5409aeae5e4077270/CMakeLists.txt

---

🌐 Web query:

raw file host/drivers/rpc/wrap/rpc_wrap.c esp_hosted_get_coprocessor_fwversion rpc_call

💡 Result:

The function esp_hosted_get_coprocessor_fwversion is a component of the ESP-Hosted framework [1][2]. It is used to retrieve the currently running firmware version of the coprocessor (slave) device [2][3]. In the ESP-Hosted architecture, this function typically acts as a wrapper that calls an underlying Remote Procedure Call (RPC) function, such as rpc_get_coprocessor_fwversion, to communicate with the coprocessor [4]. The retrieved version information is stored in an esp_hosted_coprocessor_fwver_t structure, which contains the following fields: typedef struct { uint32_t major1; // Major version uint32_t minor1; // Minor version uint32_t patch1; // Patch version } esp_hosted_coprocessor_fwver_t; This function is commonly used in Over-the-Air (OTA) update workflows to check the current slave firmware version and determine whether an update is necessary [1][3]. If the versions match, the host can skip the OTA process [1][2]. While the file host/drivers/rpc/wrap/rpc_wrap.c is a central file in the ESP-Hosted repository where various RPC wrappers are defined and managed [5][4], the specific function signature and implementation details are provided by the ESP-Hosted component libraries [2][3].

Citations:

• 1: https://github.com/espressif/esp-hosted-mcu/tree/main/examples/host_performs_slave_ota
• 2: https://components.espressif.com/components/espressif/esp_hosted/versions/2.7.3/examples/host_performs_slave_ota?language=en
• 3: https://components.espressif.com/components/espressif/esp_hosted/versions/2.11.5/examples/host_performs_slave_ota?language=
• 4: espressif/esp-hosted-mcu@6b56550
• 5: espressif/esp-hosted-mcu@47d724e

---

🌐 Web query:

"GetCoprocessorFwVersion" "#define RPC_ID" "sync" "esp_hosted"

💡 Result:

In the context of the ESP-Hosted framework, GetCoprocessorFwVersion is a defined remote procedure call (RPC) used by the host to query the firmware version of the slave co-processor [1][2][3]. Key details: - RPC ID: The RPC identifier for Req_GetCoprocessorFwVersion is 350 (0x15e) [1][2]. - Functionality: The request allows the host to retrieve the slave's firmware version, which is useful for tasks such as OTA (Over-the-Air) update validation to avoid redundant updates [3]. - Recent Updates: As of version 3.0.5, the RPC was extended to include additional information, specifically the co-processor name and chip ID [4]. - Implementation: This RPC is defined in the project's protocol buffer files (esp_hosted_rpc.proto) [1] and implemented within the ESP-Hosted C/C++ driver component as esp_hosted_get_coprocessor_fwversion [3]. The framework uses this RPC to ensure the host and slave are synchronized regarding firmware status, enabling efficient system management and update procedures [3][5][4].

Citations:

• 1: https://github.com/espressif/esp-hosted-mcu/blob/main/common/proto/esp_hosted_rpc.proto
• 2: https://github.com/espressif/esp-hosted-mcu/blob/main/docs/implemented_rpcs.md
• 3: https://components.espressif.com/components/espressif/esp_hosted/versions/2.7.3/examples/host_performs_slave_ota?language=en
• 4: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.5/changelog?language=en
• 5: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.4/readme?language=en

---

🌐 Web query:

"function esp_hosted_call_rpc" esp-hosted source esp_hosted_call_rpc

💡 Result:

In the ESP-Hosted framework, there is no public API function explicitly named esp_hosted_call_rpc. The framework relies on a protobuf-based Remote Procedure Call (RPC) mechanism to allow a Host system (Linux or MCU) to offload Wi-Fi and Bluetooth tasks to an ESP co-processor [1][2]. The ESP-Hosted architecture implements RPCs using the following structure: 1. Protocol Definition: RPC messages, request types, and event IDs are defined in common/proto/esp_hosted_rpc.proto [3]. 2. Request Handling: Host-side applications typically use high-level library functions (e.g., wifi_connect_ap, wifi_get_mac) provided by the ESP-Hosted control library rather than calling a raw "call_rpc" function [4][1][5]. 3. Underlying Mechanism: These high-level functions internally handle the serialization of requests into protobuf messages, which are then transmitted over the communication bus (SPI, SDIO, or UART) to the co-processor [1]. 4. Custom RPCs: For user-defined functionality, developers can utilize the Custom RPC feature (specifically Req_CustomRpc with ID 388) [3][6]. On the host side, custom behavior is registered using functions such as esp_hosted_register_custom_callback [7]. If you are encountering a reference to esp_hosted_call_rpc in your source code, it is likely a custom wrapper, a deprecated function, or a misunderstanding of the internal API naming, as the official ESP-Hosted implementation manages RPC communication via its control library abstraction layers [4][1].

Citations:

• 1: https://github.com/espressif/esp-hosted-mcu/blob/main/README.md
• 2: https://github.com/espressif/esp-hosted
• 3: https://github.com/espressif/esp-hosted-mcu/blob/main/common/proto/esp_hosted_rpc.proto
• 4: https://github.com/espressif/esp-hosted/blob/61471c11/esp_hosted_fg/host/control_lib/src/ctrl_core.c
• 5: https://github.com/espressif/esp-hosted/blob/master/esp_hosted_fg/docs/common/ctrl_apis.md
• 6: https://github.com/espressif/esp-hosted-mcu/blob/main/docs/implemented_rpcs.md
• 7: https://github.com/espressif/esp-hosted-mcu/blob/f8cb41c25a6dbcc34e2c6ce29c0916994c7b7059/examples/host_peer_data_transfer/main/peer_data_example.c

---

🌐 Web query:

"glibc.h" "esp_hosted_get_coprocessor_fwversion" "esp_hosted_api.c"

💡 Result:

The function esp_hosted_get_coprocessor_fwversion is an API provided by the ESP-Hosted framework [1][2]. It is used to query the firmware version currently running on the ESP-based co-processor [1][3]. Technical details regarding your query components: 1. esp_hosted_get_coprocessor_fwversion: This function retrieves the semantic version (major, minor, and patch) of the co-processor firmware [1][2]. It is typically used in OTA (Over-the-Air) update workflows to compare the running version with a target version to determine if an update is required [1][2]. It is defined in the ESP-Hosted API headers and implemented within the project's source files [1][4]. 2. esp_hosted_api.c: This is a core source file in the ESP-Hosted framework that implements various host-side API functions, including those for communication, transport configuration, and firmware management [4][5]. It serves as part of the interface between the host application and the ESP co-processor [6][7]. 3. glibc.h: There is no direct functional link between glibc.h and the ESP-Hosted API functions in the context of standard usage. While Espressif maintains a repository for the GNU C Library (glibc) [8], it is independent of the ESP-Hosted framework's control plane APIs. If you encountered a reference to glibc.h in your specific development environment, it is likely an incidental header included by the build system or the host operating system's standard library, rather than a component of the ESP-Hosted solution itself [4][9]. In summary, esp_hosted_get_coprocessor_fwversion is a standard function found in the ESP-Hosted API (often implemented in or linked with esp_hosted_api.c) used for system diagnostics and firmware lifecycle management [1][10][2].

Citations:

• 1: https://github.com/espressif/esp-hosted-mcu/tree/main/examples/host_performs_slave_ota
• 2: https://components.espressif.com/components/espressif/esp_hosted/versions/2.11.5/examples/host_performs_slave_ota?language=
• 3: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.4/readme?language=en
• 4: https://github.com/espressif/esp-hosted-mcu/blob/0c4423aba2331c2b1b9ebeac45636fab9b819161/CMakeLists.txt
• 5: https://github.com/espressif/esp-hosted-mcu/blob/c1b0e6f548598af889115755044f0c0eaecae9b5/CMakeLists.txt
• 6: https://github.com/espressif/esp-hosted
• 7: https://components.espressif.com/components/espressif/esp_hosted/versions/3.0.2/readme?language=en
• 8: https://github.com/espressif/glibc
• 9: https://github.com/espressif/esp-hosted-mcu/blob/514997782548f762a2a679dfb205862758a525fe/CMakeLists.txt
• 10: Issues updating the slave from HTTPS and query it's current firmware version (EHM-74) espressif/esp-hosted-mcu#71

---

Make coprocessorWifi() nonblocking or cache it from another task.

tick1s() is MM_NONBLOCKING, but the P4 implementation calls esp_hosted_get_coprocessor_fwversion() once per second, which is a synchronous RPC/transport query. Cache the version in a non-render task or provide a nonblocking snapshot so this hot-path override does not become a timed RPC wait.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/SystemModule.h` at line 232, Update the P4 implementation of
tick1s() so its coprocessorWifi() path does not directly invoke the synchronous
esp_hosted_get_coprocessor_fwversion() query; instead, maintain the firmware
version via a non-render task or nonblocking cached snapshot and have tick1s()
read that cached value while preserving its MM_NONBLOCKING contract.

[coderabbitai]

🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

Do not declare PreviewDriver::tick() nonblocking while blocking I/O and allocation remain reachable.

tick() can call sendFrame() through the documented synchronous begin/push/end socket path, and buildAndSendCoordTable() can allocate keptIdx_. Move those operations off the render path or make every reachable path nonblocking before applying this contract.

As per coding guidelines, render-loop callees must avoid blocking operations and heap allocation; as per path instructions, light-driver loop methods must not allocate on the hot path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/drivers/PreviewDriver.h` at line 132, Remove the MM_NONBLOCKING
contract from PreviewDriver::tick() until its reachable work is safe, or
refactor sendFrame()’s synchronous socket path and buildAndSendCoordTable()’s
keptIdx_ allocation off the render path; ensure every operation reachable from
tick() avoids blocking I/O and heap allocation before restoring the annotation.

Sources: Coding guidelines, Path instructions

[coderabbitai]

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Remove per-element floating-point work from these nonblocking render callbacks.

• src/light/effects/BouncingBallsEffect.h#L49-L49: replace per-ball trajectory, damping, square-root, and rounding calculations with fixed-point/integer math or move them off the render path.
• src/light/effects/SphereMoveEffect.h#L36-L36: replace per-voxel trigonometric/distance calculations with integer math or move them off the render path.

As per coding guidelines, render-loop callees must avoid per-light floating-point math.

📍 Affects 2 files

• src/light/effects/BouncingBallsEffect.h#L49-L49 (this comment)
• src/light/effects/SphereMoveEffect.h#L36-L36

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/effects/BouncingBallsEffect.h` at line 49, The nonblocking render
callbacks perform per-element floating-point work; update
BouncingBallsEffect::tick in src/light/effects/BouncingBallsEffect.h:49-49 to
use fixed-point/integer math or precompute trajectory, damping, square-root, and
rounding values off the render path, and update SphereMoveEffect::tick in
src/light/effects/SphereMoveEffect.h:36-36 to replace per-voxel trigonometric
and distance calculations with integer math or precomputed values.

Source: Coding guidelines

[coderabbitai]

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)

moondeck/moondeck.py (1)

1794-1812: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Prevent concurrent runs from corrupting the “last run” log.

A fast rerun can leave the previous stream handler finishing after _running[script_id] has been replaced. Both handlers open the same script_id.log with "w" and then write to it, so the stored result can be truncated, interleaved, or overwritten by the older run. Use a per-run generation/file and publish only the current run atomically.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@moondeck/moondeck.py` around lines 1794 - 1812, Update the stream-handling
logic around the per-script log creation and writing loop to use a unique
generation-specific log file for each run instead of opening the shared
script_id.log with "w". Track the active run generation associated with
_running[script_id], and only atomically publish that run’s completed log as the
current script_id.log when it is still current; older handlers must never
overwrite or truncate the current result.

docs/history/plans/Plan-20260727 - Static analysis and repo health tooling.md (2)

68-80: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use American English spelling in the plan.

Replace analyser with analyzer in this changed documentation block.

As per coding guidelines, **/*.md uses American English spelling throughout.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/history/plans/Plan-20260727` - Static analysis and repo health
tooling.md around lines 68 - 80, Update the changed documentation heading and
prose around “lizard” to use the American English spelling “analyzer” instead of
“analyser,” leaving the rest of the plan unchanged.

Source: Coding guidelines

---

136-138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the GCC description of MM_NONBLOCKING.

The macro expands to noexcept on non-Clang builds; only the Clang attribute and function-effects warning are unavailable.

• docs/history/plans/Plan-20260727 - Static analysis and repo health tooling.md#L136-L138: replace “macro (empty on GCC)” with wording that states the noexcept expansion.
• moondeck/MoonDeck.md#L359-L361: replace “MM_NONBLOCKING is empty on GCC” with the same accurate contract.

As per coding guidelines, documentation pages remain synchronized with the current code.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/history/plans/Plan-20260727` - Static analysis and repo health
tooling.md around lines 136 - 138, The documentation incorrectly describes
MM_NONBLOCKING as empty on GCC. Update docs/history/plans/Plan-20260727 - Static
analysis and repo health tooling.md lines 136-138 and moondeck/MoonDeck.md lines
359-361 to state that MM_NONBLOCKING expands to noexcept on non-Clang builds,
while only the Clang attribute and function-effects warning are unavailable;
keep both documentation pages synchronized.

Source: Coding guidelines

docs/backlog/backlog-core.md (1)

393-415: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Keep the hot-path inventory canonical across documentation.

The same 181-finding counts and hotspot narrative are copied into three documents, so future triage will require synchronized edits.

• docs/backlog/backlog-core.md#L393-L415: keep this as the canonical triage inventory.
• docs/history/plans/Plan-20260727 - Static analysis and repo health tooling.md#L127-L161: retain historical rationale but link to the canonical inventory.
• moondeck/MoonDeck.md#L347-L366: document checker semantics and link to the current inventory instead of embedding counts.

As per path instructions, docs/**/*.md should avoid duplicating facts and keep each fact in one canonical location.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/backlog/backlog-core.md` around lines 393 - 415, Keep
docs/backlog/backlog-core.md lines 393-415 as the canonical hot-path triage
inventory. In docs/history/plans/Plan-20260727 - Static analysis and repo health
tooling.md lines 127-161, retain the historical rationale but replace duplicated
counts and narrative with a link to the canonical inventory. In
moondeck/MoonDeck.md lines 347-366, describe checker semantics and link to the
current inventory instead of embedding counts; do not duplicate these facts
across documents.

Source: Path instructions

src/platform/esp32/platform_esp32.cpp (1)

404-426: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Publish the static-IP snapshot atomically, not just the mode flag.

ethStaticIp_, ethStaticGw_, and the other byte arrays are still plain shared storage. The render task writes them while the Ethernet event task reads them during reconnect; ethStatic_.load(acquire) only orders the transition to true, not later edits while the flag remains true. A concurrent live address change can therefore apply a torn or mixed configuration. Use a coherent atomic snapshot, lock, or versioned double-buffer, and apply the same fix to the STA arrays.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/esp32/platform_esp32.cpp` around lines 404 - 426, Make the
Ethernet static-address configuration in ethStaticIp_, ethStaticGw_,
ethStaticMask_, and ethStaticDns_ a coherent cross-thread snapshot instead of
relying only on ethStatic_ atomic ordering. Protect concurrent writes from
netSetStaticIPv4/syncAddressingLive and reads during the Ethernet reconnect
handler with an appropriate lock or versioned double-buffer, and apply the same
synchronization approach to the corresponding STA address arrays.

src/light/drivers/ParallelLedDriver.h (1)

704-710: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Close the nonblocking path on ParallelLedDriver::tick1s().

MM_NONBLOCKING is a compiler-enforced annotation, but the peripheral KPI call chain from ParallelLedDriver::tick1s() is not: LedPeripheral::busLastTransmitUs() and LedPeripheral::refreshBusKpi() still lack MM_NONBLOCKING, and the MoonI80 override of refreshBusKpi() reads platform::moonI80Ws2812RingStats(...) before updating the diagnostic string. Annotate and enforce the full nonblocking chain, or drop the claim from this override.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/drivers/ParallelLedDriver.h` around lines 704 - 710, Make the
entire ParallelLedDriver::tick1s() KPI call chain nonblocking: annotate
LedPeripheral::busLastTransmitUs(), LedPeripheral::refreshBusKpi(), and the
MoonI80 refreshBusKpi() override with MM_NONBLOCKING, and ensure the
platform::moonI80Ws2812RingStats() path satisfies that constraint. If these
calls cannot be made nonblocking, remove MM_NONBLOCKING from
ParallelLedDriver::tick1s() instead.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@moondeck/check/check_nonblocking.py`:
- Around line 90-98: Update the subprocess.run call in the build-check flow to
pass check=False, since the code intentionally handles proc.returncode. Rename
the errs comprehension variable l to error_line and update its use in the
subsequent print loop.
- Around line 82-100: Update build_output() to fail closed after a successful
build unless its diagnostics contain [-Wfunction-effects], or validate through
the existing build configuration that the selected compiler satisfies the Clang
20+ requirement. Return None and report an incomplete/unsupported diagnostics
configuration when the contract is not met; preserve existing failed-build
handling and successful output behavior when the warning is present.

In `@moondeck/moondeck_ui/app.js`:
- Around line 493-512: Update the .log-btn click handler to clear logEl before
initiating fetch, then serialize log loads using a per-request token or
AbortController so stale responses cannot overwrite the currently selected
script’s log. Ensure failed requests append errors to the cleared log rather
than prior content, and preserve the existing no-stored-run and successful log
behavior.
- Line 380: Add aria-label="Show this script's last run" to the log button while
preserving the existing title attribute and button behavior.

In `@moondeck/moondeck.py`:
- Line 2053: Add the missing type annotations to _serve_log: annotate script_id
as str and declare the method return type as None, preserving its existing
behavior.
- Around line 1816-1829: Update the SSE process cleanup around proc.wait(), the
BrokenPipeError/ConnectionResetError handler, and the finally block so a client
disconnect still terminates or otherwise drains the child, waits for it, records
its exit code, and finalizes the log before closing it and removing the process
from _running.
- Around line 1791-1799: Bound persisted script logs and endpoint reads: update
the streaming log handling around LOG_DIR, log_path, and log so the file is
capped or rolled over at a fixed byte budget, and update the serving code at
moondeck/moondeck.py lines 2063-2072 to replace unbounded read_bytes() with a
bounded tail read. Apply the required change at both sites in
moondeck/moondeck.py; the endpoint must return only the configured maximum log
bytes.
- Around line 1794-1799: Make log persistence best-effort in the SSE handler:
wrap LOG_DIR.mkdir, log_path.open/write, and subsequent flush operations in
OSError handling. On any filesystem failure, disable or discard the log and
continue streaming process output without propagating the exception.
- Line 1799: Update the timestamp formatting in the log-writing code around
script_id to use a timezone-aware datetime, such as the local time from
datetime.now().astimezone() serialized with isoformat(), or UTC with its offset
included. Preserve the existing log entry structure while ensuring the timestamp
contains explicit timezone information.

In `@src/light/drivers/ParallelLedDriver.h`:
- Around line 552-561: Remove the MM_NONBLOCKING annotation from
ParallelLedDriver::tick(), along with the surrounding clang diagnostic
suppression for -Wfunction-effects. Preserve the existing tickSync()/tickRing()
behavior and deliberate busWaitIfBusy() handling; do not claim nonblocking
behavior unless the wait is moved off the render path or replaced with an
immediate busy return.

In `@src/platform/esp32/platform_esp32.cpp`:
- Line 1370: Make wifiStaAssociated_ an atomic flag and update its accesses,
including the read in the WiFi event handler near wifiStaConnected_. Use relaxed
memory-order loads and stores consistently so the event handler and
netSetStaticIPv4() can safely exchange the association state.

In `@test/unit/light/unit_Drivers_rendersplit.cpp`:
- Around line 53-58: Scope the intentional blocking SlowDriver::tick() fixture
out of the nonblocking error gate by adding a narrow Clang function-effects
diagnostic suppression around its override, preserving the existing
justification comments and keeping the suppression limited to this test double.

---

Outside diff comments:
In `@docs/backlog/backlog-core.md`:
- Around line 393-415: Keep docs/backlog/backlog-core.md lines 393-415 as the
canonical hot-path triage inventory. In docs/history/plans/Plan-20260727 -
Static analysis and repo health tooling.md lines 127-161, retain the historical
rationale but replace duplicated counts and narrative with a link to the
canonical inventory. In moondeck/MoonDeck.md lines 347-366, describe checker
semantics and link to the current inventory instead of embedding counts; do not
duplicate these facts across documents.

In `@docs/history/plans/Plan-20260727` - Static analysis and repo health
tooling.md:
- Around line 68-80: Update the changed documentation heading and prose around
“lizard” to use the American English spelling “analyzer” instead of “analyser,”
leaving the rest of the plan unchanged.
- Around line 136-138: The documentation incorrectly describes MM_NONBLOCKING as
empty on GCC. Update docs/history/plans/Plan-20260727 - Static analysis and repo
health tooling.md lines 136-138 and moondeck/MoonDeck.md lines 359-361 to state
that MM_NONBLOCKING expands to noexcept on non-Clang builds, while only the
Clang attribute and function-effects warning are unavailable; keep both
documentation pages synchronized.

In `@moondeck/moondeck.py`:
- Around line 1794-1812: Update the stream-handling logic around the per-script
log creation and writing loop to use a unique generation-specific log file for
each run instead of opening the shared script_id.log with "w". Track the active
run generation associated with _running[script_id], and only atomically publish
that run’s completed log as the current script_id.log when it is still current;
older handlers must never overwrite or truncate the current result.

In `@src/light/drivers/ParallelLedDriver.h`:
- Around line 704-710: Make the entire ParallelLedDriver::tick1s() KPI call
chain nonblocking: annotate LedPeripheral::busLastTransmitUs(),
LedPeripheral::refreshBusKpi(), and the MoonI80 refreshBusKpi() override with
MM_NONBLOCKING, and ensure the platform::moonI80Ws2812RingStats() path satisfies
that constraint. If these calls cannot be made nonblocking, remove
MM_NONBLOCKING from ParallelLedDriver::tick1s() instead.

In `@src/platform/esp32/platform_esp32.cpp`:
- Around line 404-426: Make the Ethernet static-address configuration in
ethStaticIp_, ethStaticGw_, ethStaticMask_, and ethStaticDns_ a coherent
cross-thread snapshot instead of relying only on ethStatic_ atomic ordering.
Protect concurrent writes from netSetStaticIPv4/syncAddressingLive and reads
during the Ethernet reconnect handler with an appropriate lock or versioned
double-buffer, and apply the same synchronization approach to the corresponding
STA address arrays.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9dc4efb0-2e05-45bd-bdd6-b1d38590c446

📥 Commits

Reviewing files that changed from the base of the PR and between bc02fe4 and 5f2116d.

📒 Files selected for processing (22)

• docs/backlog/backlog-core.md
• docs/history/plans/Plan-20260727 - Static analysis and repo health tooling.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• moondeck/MoonDeck.md
• moondeck/check/check_nonblocking.py
• moondeck/moondeck.py
• moondeck/moondeck_ui/app.js
• moondeck/moondeck_ui/style.css
• src/light/drivers/LedPeripheral.h
• src/light/drivers/MoonLedDriver.h
• src/light/drivers/MultiPinLedDriver.h
• src/light/drivers/ParallelLedDriver.h
• src/light/drivers/ParlioLedDriver.h
• src/light/drivers/PreviewDriver.h
• src/platform/desktop/platform_desktop.cpp
• src/platform/esp32/platform_esp32.cpp
• test/unit/light/unit_Drivers_rendersplit.cpp
• test/unit/light/unit_ParallelLedDriver_doublebuffer.cpp
• test/unit/light/unit_ParallelLedDriver_pinexpander.cpp
• test/unit/light/unit_ParallelLedDriver_ring.cpp
• test/unit/light/unit_ParallelLedDriver_swap.cpp

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Reap the child and finalize the log after client disconnects.

When the SSE client disconnects, the exception path skips proc.wait() and the exit-code record, then removes the process from _running. The child can continue without a Stop handle, and the log remains incomplete. Continue draining/logging after disconnect, or terminate and reap the child before closing the log and unregistering it.

🧰 Tools

🪛 ast-grep (0.44.1)

[info] 1819-1819: use jsonify instead of json.dumps for JSON output
Context: json.dumps(exit_msg)
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

---

[info] 1820-1820: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"exitCode": proc.returncode})
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@moondeck/moondeck.py` around lines 1816 - 1829, Update the SSE process
cleanup around proc.wait(), the BrokenPipeError/ConnectionResetError handler,
and the finally block so a client disconnect still terminates or otherwise
drains the child, waits for it, records its exit code, and finalizes the log
before closing it and removing the process from _running.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | rg '(^|/)ParallelLedDriver\.h$|ParallelLedDriver\.(h|hpp|cpp)$|platform_config\.h|function-effects|function-effects|NONBLOCKING' || true

echo "== target lines =="
cat -n src/light/drivers/ParallelLedDriver.h | sed -n '520,575p'

echo "== nearby blocking/wait definitions/usages =="
rg -n "busWaitIfBusy|MM_NONBLOCKING|function-effects|tickSync|tickRing|tick1s" src/light src/platform || true

echo "== helper definitions outline =="
rg -n "class .*Driver|struct .*Driver|ParallelLedDriver|busWaitIfBusy|tickSync|tickRing" src/light/drivers -g '*.{h,hpp,cpp,cc,h}' || true

Repository: MoonModules/projectMM

Length of output: 28253

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== target lines =="
cat -n src/light/drivers/ParallelLedDriver.h | sed -n '520,575p'

echo "== nearby blocking/wait definitions/usages =="
rg -n "busWaitIfBusy|MM_NONBLOCKING|function-effects|tickSync|tickRing|tick1s" src/light src/platform || true

Repository: MoonModules/projectMM

Length of output: 19885

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== ParallelLedDriver tick/render wait sections =="
cat -n src/light/drivers/ParallelLedDriver.h | sed -n '586,755p'
cat -n src/light/drivers/ParallelLedDriver.h | sed -n '755,850p'

echo "== all MM_NONBLOCKING callers/annotations around wait helpers =="
python3 - <<'PY'
import re, pathlib
p = pathlib.Path('src/light/drivers/ParallelLedDriver.h')
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
    if re.search(r'busWaitIfBusy|tickSync|tickRing|MM_NONBLOCKING|pragma clang diagnostic ignored "-Wfunction-effects"', line):
        print(f"{i}: {line}")
PY

Repository: MoonModules/projectMM

Length of output: 23278

---

Remove MM_NONBLOCKING from ParallelLedDriver::tick() or make the wait nonblocking.

tick() is annotated MM_NONBLOCKING, but both per-frame paths reach busWaitIfBusy(), which waits on the render thread for up to a 20–100 ms bus timeout (notably in tickSync()). This violates the hot-path contract and the clang nonblocking static checking; move the wait off the render loop or return when the DMA is busy instead of suppressing -Wfunction-effects.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/drivers/ParallelLedDriver.h` around lines 552 - 561, Remove the
MM_NONBLOCKING annotation from ParallelLedDriver::tick(), along with the
surrounding clang diagnostic suppression for -Wfunction-effects. Preserve the
existing tickSync()/tickRing() behavior and deliberate busWaitIfBusy() handling;
do not claim nonblocking behavior unless the wait is moved off the render path
or replaced with an immediate busy return.

Sources: Coding guidelines, Path instructions

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Scope the intentional blocking fixture out of the nonblocking error gate.

The comment explains why SlowDriver::tick() blocks, but it does not suppress -Wfunction-effects. When the planned -Wno-error=function-effects exemption is removed, this deliberate test double will become a hard build failure. Add a narrow Clang diagnostic suppression with the same justification, or explicitly exclude this fixture from the hot-path gate.

🧰 Tools

🪛 Cppcheck (2.21.0)

[style] 53-53: The function 'atan2_8' is never used.

(unusedFunction)

---

[style] 56-56: The function 'userEditable' is never used.

(unusedFunction)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/unit/light/unit_Drivers_rendersplit.cpp` around lines 53 - 58, Scope the
intentional blocking SlowDriver::tick() fixture out of the nonblocking error
gate by adding a narrow Clang function-effects diagnostic suppression around its
override, preserving the existing justification comments and keeping the
suppression limited to this test double.

[coderabbitai]

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

src/platform/esp32/platform_esp32.cpp (1)

1115-1135: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reset wifiStaAssociated_ during wifiStaStop().

Line 1133 clears only wifiStaConnected_. If the queued STA_DISCONNECTED event is processed after handlers are unregistered, wifiStaAssociated_ can remain true. A subsequent static STA setup then passes the Line 1372 association check and publishes connected state before L2 association. Store false during teardown, alongside the connected flag.

As per path instructions, platform state shared between event and scheduler tasks must remain synchronized across lifecycle transitions.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/esp32/platform_esp32.cpp` around lines 1115 - 1135, Update
wifiStaStop() to also atomically store false in wifiStaAssociated_ during
teardown, alongside wifiStaConnected_. Ensure both association and connection
state are reset before a later wifiStaInit() or static STA setup.

Source: Path instructions

src/platform/platform.h (1)

341-351: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Make the IP getters genuinely nonblocking across platforms.

The new contract is inconsistent: the WiFi fallback remains outside MM_NONBLOCKING, while the desktop Ethernet implementation performs synchronous route/socket work. A tick caller can therefore either escape compiler checking or block inside a function explicitly promised nonblocking.

• src/platform/platform.h#L341-L351: apply one consistent contract to both IPv4 getters, backed by cached/lock-free state.
• src/platform/desktop/platform_desktop.cpp#L658-L677: remove hostIp() route resolution from the getter and return cached octets.

As per path instructions, platform implementations must match the nonblocking API contract declared in platform.h.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/platform.h` around lines 341 - 351, Make both IPv4 getters
genuinely nonblocking: in src/platform/platform.h lines 341-351, declare
wifiStaGetIPv4 with the same MM_NONBLOCKING contract as ethGetIPv4; in
src/platform/desktop/platform_desktop.cpp lines 658-677, update the getter
implementation to return cached octets and remove synchronous hostIp() route
resolution, ensuring the cache is maintained through the existing platform
connection/update flow.

Source: Path instructions

moondeck/MoonDeck.md (1)

356-361: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the ESP32 hot-path coverage claim.

src/platform/esp32/ builds are excluded from the desktop Clang build, so -Wfunction-effects only sees tick call edges and MM_NONBLOCKING declarations, not the ESP32 function bodies. Document this coverage gap and link the backlog item, or require separate ESP32 Clang/LLVM analysis before claiming full coverage.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@moondeck/MoonDeck.md` around lines 356 - 361, Update the ESP32 hot-path
coverage statement in the documentation to acknowledge that desktop Clang
analysis does not inspect src/platform/esp32 function bodies. Either link the
relevant backlog item documenting this gap or state that separate ESP32
Clang/LLVM analysis is required before claiming full coverage.

Source: Path instructions

src/light/effects/MetaballsEffect.h (1)

37-42: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Guard degenerate grids before pointer arithmetic.

MetaballsEffect::tick() enters the outer loop when w == 0 && h > 0 and computes buf + y * w * cpl, which still performs pointer arithmetic for each row. Add the same if (w <= 0 || h <= 0) return; guard as LavaLampEffect before entering the loop.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/effects/MetaballsEffect.h` around lines 37 - 42, Update
MetaballsEffect::tick() to return immediately when width or height is
non-positive, placing the if (w <= 0 || h <= 0) guard before any loop or buffer
pointer arithmetic. Keep the existing rendering logic unchanged for valid grids.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/test.yml:
- Around line 96-100: Update the workflow environment assignment for CXX so
non-realtime matrix kinds leave it unset rather than exporting an empty string.
Preserve clang++ for the realtime kind, and use an initialization-time
CMAKE_CXX_COMPILER override only if needed to select the compiler without
setting CXX globally.

In `@docs/history/plans/Plan-20260727` - Static analysis and repo health
tooling.md:
- Around line 127-132: Update the `[[clang::nonblocking]]` plan entry to match
the checked-in `docs/metrics/hotpath-baseline.txt` count of 108 entries, or
regenerate the documentation and baseline together so all references report the
same total.

In `@moondeck/check/check_nonblocking.py`:
- Around line 227-230: Update the baseline handling around read_baseline() to
track BASELINE.exists() separately from the returned set, distinguishing a
missing baseline from an existing but empty baseline. Use file existence when
determining whether each finding is new, while retaining the known-set
membership check, and ensure the report’s “no baseline exists” status only
applies when the baseline file is absent.
- Around line 147-158: The clean-build validation in build_output must
distinguish whether -Wfunction-effects was enabled from whether diagnostics were
emitted. Add or reuse a build-capability flag/check for the warning, accept zero
diagnostics when capability is present, and only fail closed when the warning is
unavailable; ensure the incremental bypass is allowed only after the
full-capability prerequisite succeeds.

In `@moondeck/moondeck.py`:
- Around line 1806-1813: Update the log setup in the run/stream handling flow
around LOG_DIR and _running[script_id] so concurrent runs never open the shared
final log with "w". Create a PID-specific temporary log file, write the run
output there, and atomically promote it to <script_id>.log only when that
process is still the active _running[script_id]; ensure an older or killed
process cannot replace the newer run’s log.

In `@src/core/AudioService.h`:
- Around line 304-309: Move kSilence out of latestFrame() and define it at class
or namespace scope as a static constexpr AudioFrame, then update latestFrame()
to reference that shared constant while preserving the existing silence fallback
behavior.

In `@src/light/drivers/ParallelLedDriver.h`:
- Line 559: The MM_NONBLOCKING annotations are premature because these
render-loop paths still block or allocate. In
src/light/drivers/ParallelLedDriver.h#L559-L559, remove the annotation or
replace busWaitIfBusy() with nonblocking polling; in
src/light/drivers/PreviewDriver.h#L135-L135, move synchronous sendFrame() and
keptIdx_ allocation off the render path; and in
src/light/drivers/RmtLedDriver.h#L234-L234, move rmtWs2812Wait() off tick or
implement polling/state-machine behavior. Ensure each tick path avoids blocking
operations and heap allocation before retaining MM_NONBLOCKING.

---

Outside diff comments:
In `@moondeck/MoonDeck.md`:
- Around line 356-361: Update the ESP32 hot-path coverage statement in the
documentation to acknowledge that desktop Clang analysis does not inspect
src/platform/esp32 function bodies. Either link the relevant backlog item
documenting this gap or state that separate ESP32 Clang/LLVM analysis is
required before claiming full coverage.

In `@src/light/effects/MetaballsEffect.h`:
- Around line 37-42: Update MetaballsEffect::tick() to return immediately when
width or height is non-positive, placing the if (w <= 0 || h <= 0) guard before
any loop or buffer pointer arithmetic. Keep the existing rendering logic
unchanged for valid grids.

In `@src/platform/esp32/platform_esp32.cpp`:
- Around line 1115-1135: Update wifiStaStop() to also atomically store false in
wifiStaAssociated_ during teardown, alongside wifiStaConnected_. Ensure both
association and connection state are reset before a later wifiStaInit() or
static STA setup.

In `@src/platform/platform.h`:
- Around line 341-351: Make both IPv4 getters genuinely nonblocking: in
src/platform/platform.h lines 341-351, declare wifiStaGetIPv4 with the same
MM_NONBLOCKING contract as ethGetIPv4; in
src/platform/desktop/platform_desktop.cpp lines 658-677, update the getter
implementation to return cached octets and remove synchronous hostIp() route
resolution, ensuring the cache is maintained through the existing platform
connection/update flow.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1bfeaa66-fb23-446a-b183-245b9cb3e32d

📥 Commits

Reviewing files that changed from the base of the PR and between 5f2116d and 1d0184f.

📒 Files selected for processing (27)

• .clang-tidy
• .github/workflows/test.yml
• docs/backlog/backlog-core.md
• docs/coding-standards.md
• docs/history/plans/Plan-20260727 - Static analysis and repo health tooling.md
• docs/metrics/hotpath-baseline.txt
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• moondeck/MoonDeck.md
• moondeck/check/check_hotpath.py
• moondeck/check/check_nonblocking.py
• moondeck/event/_gates.py
• moondeck/moondeck.py
• moondeck/moondeck_config.json
• moondeck/moondeck_ui/app.js
• src/core/AudioService.h
• src/light/drivers/ParallelLedDriver.h
• src/light/drivers/PreviewDriver.h
• src/light/drivers/RmtLedDriver.h
• src/light/effects/LavaLampEffect.h
• src/light/effects/MetaballsEffect.h
• src/platform/desktop/platform_desktop.cpp
• src/platform/esp32/platform_esp32.cpp
• src/platform/esp32/platform_esp32_rmt.cpp
• src/platform/platform.h
• test/scenarios/light/scenario_peripheral_grid_sweep.json
• test/scenarios/light/scenario_peripheral_switch.json

💤 Files with no reviewable changes (2)

• moondeck/check/check_hotpath.py
• moondeck/moondeck_config.json

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Synchronize the documented baseline count.

This section says hotpath-baseline.txt freezes 107 pairs, but the checked-in baseline contains 108 entries on Lines 12–119. Update the count or regenerate the documentation and baseline together so the plan and tooling reports do not disagree.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/history/plans/Plan-20260727` - Static analysis and repo health
tooling.md around lines 127 - 132, Update the `[[clang::nonblocking]]` plan
entry to match the checked-in `docs/metrics/hotpath-baseline.txt` count of 108
entries, or regenerate the documentation and baseline together so all references
report the same total.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

found=0
while IFS= read -r file; do
  echo "== $file =="
  rg -n -- "CMAKE_CXX_COMPILER(_ID|_VERSION)?|-Wfunction-effects" "$file" || true
  found=1
done < <(fd -t f '^compile_commands\.json$|^CMakeCache\.txt$' .)

if [ "$found" -eq 0 ]; then
  echo "No compiler configuration files found" >&2
  exit 1
fi

Repository: MoonModules/projectMM

Length of output: 211

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "repo root:"
pwd
echo "tracked candidate files:"
git ls-files | rg '(^|/)(CMakeLists\.txt|moondeck/check/check_nonblocking\.py|moondeck/event/_gates\.py|compile_commands\.json|CMakeCache\.txt)$' || true

echo
echo "search Wfunction-effects and gate references:"
rg -n -- "-Wfunction-effects|function-effects|check_nonblocking|detect compiler|compiler version" . || true

Repository: MoonModules/projectMM

Length of output: 6230

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== moondeck/check/check_nonblocking.py outline =="
ast-grep outline moondeck/check/check_nonblocking.py --view compact || true

echo
echo "== moondeck/check/check_nonblocking.py relevant sections =="
sed -n '1,220p' moondeck/check/check_nonblocking.py | cat -n

echo
echo "== moondeck/event/_gates.py relevant sections =="
sed -n '1,190p' moondeck/event/_gates.py | cat -n

echo
echo "== root CMakeLists relevant sections =="
sed -n '45,75p' CMakeLists.txt | cat -n

Repository: MoonModules/projectMM

Length of output: 25135

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== check_clang_tidy module =="
sed -n '1,220p' moondeck/check/check_clang_tidy.py | cat -n

echo
echo "== check_clang_query module =="
sed -n '1,240p' moondeck/check/check_clang_query.py | cat -n

echo
echo "== static verifier for probe behavior and config condition =="
python3 - <<'PY'
from pathlib import Path
import re
src = Path("moondeck/check/check_nonblocking.py").read_text()
cmake = Path("CMakeLists.txt").read_text()
print("probe_looks_at_build_output_diagnostic = ", re.search(r'if clean and "(\[-Wfunction-effects\])" not in out', src) is not None)
print("clean_returns_none_on_missing_diagnostic = ", re.search(r'if clean and .* in out:\s+print\(".*\).?\s+return None', src, re.S) is not None)
print("cmake_conditional_add_compile_options = ", re.search(r'add_compile_options\(\s*-Wfunction-effects\s*-Wno-error=function-effects\s*\)', cmake, re.S) is not None)
print("cmake_requires_clang_and_version_ge_20 = ", re.search(r'if\(CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 20\)', cmake) is not None)
PY

Repository: MoonModules/projectMM

Length of output: 26881

---

Reject full rebuilds with zero diagnostics only when -Wfunction-effects was not enabled.

CMakeLists.txt adds the warning only for Clang >= 20, so a clean Clang-build has no [-Wfunction-effects] diagnostic to probe. Use a build-capability check/flag instead of returned-warning counts for build_output(), and keep the incremental bypass restricted to when the full-capability prerequisite is satisfied.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@moondeck/check/check_nonblocking.py` around lines 147 - 158, The clean-build
validation in build_output must distinguish whether -Wfunction-effects was
enabled from whether diagnostics were emitted. Add or reuse a build-capability
flag/check for the warning, accept zero diagnostics when capability is present,
and only fail closed when the warning is unavailable; ensure the incremental
bypass is allowed only after the full-capability prerequisite succeeds.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Distinguish a missing baseline from an empty baseline.

--baseline legitimately writes an empty file when there are no findings. Because read_baseline() returns an empty set for both missing and empty files, bool(known) is false and later findings are marked new=False; the report also says that no baseline exists. Track BASELINE.exists() separately.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@moondeck/check/check_nonblocking.py` around lines 227 - 230, Update the
baseline handling around read_baseline() to track BASELINE.exists() separately
from the returned set, distinguishing a missing baseline from an existing but
empty baseline. Use file existence when determining whether each finding is new,
while retaining the known-set membership check, and ensure the report’s “no
baseline exists” status only applies when the baseline file is absent.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Prevent overlapping runs from corrupting the “last run” log.

A fast Run→Run creates two stream handlers that both open <script_id>.log with "w" and can interleave output. The older, killed process can therefore overwrite or append to the newer run’s log. Write each process to a PID-specific temporary file and atomically promote it only if that process is still _running[script_id].

Proposed direction

- log = (LOG_DIR / f"{script_id}.log").open("w", encoding="utf-8")
+ log_path = LOG_DIR / f".{script_id}.{proc.pid}.log"
+ published_log_path = LOG_DIR / f"{script_id}.log"
+ log = log_path.open("w", encoding="utf-8")
...
+ # After closing the per-process log:
+ with _lock:
+     is_current = _running.get(script_id) is proc
+ if is_current:
+     os.replace(log_path, published_log_path)
+ else:
+     log_path.unlink(missing_ok=True)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@moondeck/moondeck.py` around lines 1806 - 1813, Update the log setup in the
run/stream handling flow around LOG_DIR and _running[script_id] so concurrent
runs never open the shared final log with "w". Create a PID-specific temporary
log file, write the run output there, and atomically promote it to
<script_id>.log only when that process is still the active _running[script_id];
ensure an older or killed process cannot replace the newer run’s log.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | rg '(^|/)AudioService\.(h|hpp|cpp)$|CMakeLists|.*\.cmake|.*clang|.*compiler|.*Warnings|.*flags' | sed -n '1,120p'

echo "== location =="
fd -a 'AudioService\.h|AudioService\.cpp' . | sed 's#^\./##'

echo "== AudioService functions around latestFrame =="
ast-grep outline src/core/AudioService.h --match latestFrame --view expanded || true
wc -l src/core/AudioService.h
sed -n '260,330p' src/core/AudioService.cpp || true
sed -n '280,320p' src/core/AudioService.h || true

echo "== mm_nonblocking definitions/usages =="
rg -n "MM_NONBLOCKING|function-effects|non-blocking|nonblocking|NONBLOCKING" . --glob '!build/**' --glob '!dist/**' --glob '!node_modules/**' | sed -n '1,160p'

echo "== compiler/config references =="
rg -n "\-Wfunction-effects|function-effects|Wno-function-effects|Werror|clang|CXXFLAGS|CMAKE_CXX_FLAGS|ESP" . --glob '!build/**' --glob '!dist/**' --glob '!node_modules/**' | sed -n '1,200p'

echo "== all local static constexpr in src/core? =="
rg -n "static constexpr .*\\{}|static constexpr .*;" src/core/AudioService.h src/core -g '*.{h,hpp,cpp,cxx,cc}' | sed -n '1,200p'

Repository: MoonModules/projectMM

Length of output: 50377

---

🌐 Web query:

Clang -Wfunction-effects constexpr local variable static constexpr does not make variable static storage duration

💡 Result:

In C++, declaring a local variable with constexpr does not imply static storage duration [1]. Storage duration and the constexpr specifier are independent concepts [2][1]. A local variable declared as constexpr has automatic storage duration by default, just like any other non-static local variable [1]. If you require the variable to have static storage duration, you must explicitly use the static keyword (e.g., static constexpr int x = 42;) [3][4]. Regarding your observation about Clang: 1. Separate Concerns: The constexpr specifier governs constant initialization and implies that the variable is a constant [5][2]. It does not modify the storage duration of the object [1]. 2. Function-Scope Static constexpr: Historically, C++ had restrictive rules regarding static variables within constexpr functions, but these were significantly relaxed in C++23 [3][4]. The fact that a variable is constexpr does not automatically promote its storage duration to static; rather, static and constexpr are orthogonal attributes that can be combined when specific lifetime requirements (like persisting across function calls) are needed [3][4]. 3. Implementation Behavior: If you observe behavior where a local constexpr variable behaves as if it has automatic storage duration (e.g., its lifetime is tied to the block scope), this is the expected and standard-compliant behavior [1]. If you are experiencing issues where a static constexpr variable is being treated as automatic, it would be a deviation from the standard, though it is more common to encounter confusion between the constant nature of the variable (which allows it to be used in constant expressions) and its storage duration (which determines its lifetime) [3][4][6]. In summary, constexpr is not a storage class specifier [2]. To ensure a local variable has static storage duration, you must explicitly declare it with static [3][4].

Citations:

• 1: https://open-std.org/jtc1/sc22/wg14/www/docs/n2977.htm
• 2: https://eel.is/c++draft/dcl.constexpr
• 3: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2647r1.html
• 4: https://stackoverflow.com/questions/76547561/can-you-declare-static-local-variables-in-a-constexpr-function
• 5: https://cppreference.com/cpp/language/constexpr
• 6: https://stackoverflow.com/questions/73453234/are-function-scope-static-constexpr-variable-static-storage-duration

---

🌐 Web query:

clang nonblocking attribute constexpr function local static constexpr variable static constexpr

💡 Result:

The Clang [[clang::nonblocking]] attribute is a language extension used to verify that a function never performs operations that could block execution, such as locking, heap allocation, or throwing exceptions [1][2]. This attribute applies to function types (including lambdas, blocks, and member functions) and allows the compiler to perform static analysis to ensure the function body—and any functions it calls—adhere to these performance constraints [1][3]. Regarding the use of static constexpr variables within constexpr functions, this is permitted as of C++23 [4][5]. Previously, such declarations were prohibited, even if the control flow did not reach them [4]. The restriction was lifted by the C++23 standard (via paper P2647R1) to allow static constexpr variables, as they are constant-initialized and do not introduce runtime state or side effects, thus maintaining the purity requirements of constexpr functions [6][4][7]. Key technical points: - The [[clang::nonblocking]] attribute acts as a performance contract. When applied to a function body, the compiler verifies that the function does not contain prohibited constructs. If the compiler cannot analyze a called function (e.g., an external function), it must assume that function is potentially blocking unless it is appropriately annotated [1][3]. - As of C++23, static constexpr variables are valid inside constexpr functions [4][5]. These variables are initialized at compile-time. Because their value is known constant data, they do not require thread-safety synchronization mechanisms (like those needed for runtime-initialized static local variables) at runtime [8]. - While static constexpr is allowed, general static variables (without constexpr) remain prohibited within constexpr functions because their initialization could involve dynamic code execution, violating the stateless nature required for constant expression evaluation [6][9][7].

Citations:

• 1: https://clang.llvm.org/docs/FunctionEffectAnalysis.html
• 2: https://releases.llvm.org/22.1.0/tools/clang/docs/FunctionEffectAnalysis.html
• 3: https://discourse.llvm.org/t/libc-rfc-supporting-the-clang-nonblocking-attribute/89582
• 4: https://stackoverflow.com/questions/76547561/can-you-declare-static-local-variables-in-a-constexpr-function
• 5: https://exchangetuts.com/can-you-declare-static-local-variables-in-a-constexpr-function-1766910903062939
• 6: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2647r1.html
• 7: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2647r0.html
• 8: https://stackoverflow.com/questions/76445905/function-local-static-constexpr-variables
• 9: https://stackoverflow.com/questions/62458079/static-constexpr-variables-in-a-constexpr-function

---

🌐 Web query:

"[[clang::nonblocking]]" "function-effects" local static

💡 Result:

In Clang, functions marked with the [[clang::nonblocking]] attribute are strictly prohibited from declaring static local variables [1][2][3]. The Clang compiler enforces this rule during Function Effect Analysis, and attempting to use a static local variable within such a function will trigger a compiler warning [1][4]. This restriction exists primarily for two reasons: 1. Thread-safe Initialization: Compilers typically generate a lock to ensure thread-safe initialization for static local variables (often used for Meyers singletons) [1][2]. This lock violates the performance constraints of the nonblocking attribute [5][6]. 2. Heap Allocation: The process of initializing a static local variable can inadvertently trigger heap allocation, which is also prohibited within nonblocking and nonallocating contexts [2][6]. If you need to use a lazily-constructed global that must be accessed from a nonblocking function, official Clang documentation recommends replacing the static local variable with a global variable and ensuring its initialization occurs before the nonblocking context is invoked [1][3].

Citations:

• 1: https://clang.llvm.org/docs/FunctionEffectAnalysis.html
• 2: https://llvm.googlesource.com/llvm-project/+/9a553d3766aacb69e884823da92dedff264e3f0f/clang/docs/FunctionEffectAnalysis.rst
• 3: https://rocm.docs.amd.com/projects/llvm-project/en/latest/LLVM/clang/html/FunctionEffectAnalysis.html
• 4: https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20240923/625021.html
• 5: https://releases.llvm.org/22.1.0/tools/clang/docs/FunctionEffectAnalysis.html
• 6: https://llvm.googlesource.com/llvm-project/+/fc5fcc0c95c97e369be8019ebf1f7556625b9a0a/clang/docs/FunctionEffectAnalysis.rst

---

🌐 Web query:

site:clang.llvm.org/docs/FunctionEffectAnalysis.html "non-static" "static local" constexpr

💡 Result:

In C++, local variables inside a constexpr function follow standard C++ rules regarding storage duration. They are not implicitly static; by default, they have automatic storage duration unless explicitly declared as static [1]. The use of static local variables is permitted within functions, including those marked as constexpr. However, static local variables are prohibited in functions marked with certain attributes that impose strict constraints, such as the clang nonblocking attribute, because they involve initialization patterns that may require locks or heap allocation [1]. In the context of constexpr functions, note the following: 1. Automatic local variables: These are created and destroyed during the execution of the function. They are fully compatible with constant expression evaluation. 2. Static local variables: While allowed in standard C++ functions, they are generally not compatible with compile-time constant evaluation (constexpr). If a constexpr function attempts to evaluate a code path that requires the initialization or use of a static local variable at compile-time, it will typically fail to be a constant expression. Example of static usage prohibited in specific restricted contexts: void example [[clang::nonblocking]] { static Logger* logger = createLogger; // Trigger warning: nonblocking function must not have static local variables [1] }

Citations:

• 1: https://clang.llvm.org/docs/FunctionEffectAnalysis.html

---

🌐 Web query:

site:clang.llvm.org/docs/FunctionEffectAnalysis.html "constexpr local static"

💡 Result:

It is syntactically valid in C++ to declare a variable as both static and constexpr within a function scope. While the provided documentation focuses on the performance implications of non-constexpr static local variables—specifically noting that compilers typically inject locks to ensure thread-safe initialization for such variables [1]—the constexpr specifier fundamentally alters this behavior. A variable declared as static constexpr is evaluated at compile time. Because its value is determined during compilation, the runtime initialization overhead and thread-safety concerns associated with standard static local variables do not apply [1]. In practice, using static with constexpr in a local scope is often redundant. A constexpr variable in a local scope already possesses a constant value known at compile time, and adding static does not change its constant nature or its availability for constant expressions. The static specifier merely forces the variable to have static storage duration rather than automatic storage duration.

Citations:

• 1: https://clang.llvm.org/docs/FunctionEffectAnalysis.html

---

🌐 Web query:

clang 18 19 nonblocking static constexpr warning

💡 Result:

In Clang, the [[clang::nonblocking]] attribute is a performance constraint that allows the compiler to verify at compile-time that a function adheres to real-time safety requirements [1][2]. Functions marked [[clang::nonblocking]] are prohibited from containing language constructs or calling functions that might result in non-deterministic execution time [1][3]. Warning regarding static variables: Functions marked with [[clang::nonblocking]] are explicitly prohibited from declaring static local variables [1][4]. The compiler issues a warning for this because the initialization of static local variables (such as Meyers singletons) typically involves the generation of a lock by the compiler to ensure thread-safe initialization, which violates the non-blocking constraint [1][4]. Diagnostics and Control: - Violations of the nonblocking rules, including the use of static local variables, are diagnosed under the -Wfunction-effects warning category [1][4]. - Clang also issues a warning if a function is declared [[clang::nonblocking]] without also being noexcept, as nonblocking implies noexcept semantics; this diagnostic is controlled by the -Wperf-constraint-implies-noexcept flag [1][4]. - Note that -Wfunction-effects may be disabled by default in some versions, requiring explicit enablement if you wish to see these diagnostics [5]. Key constraints for [[clang::nonblocking]] functions include: - Must not allocate or deallocate memory [1][6]. - Must not throw or catch exceptions [1][6]. - Must not use static local variables or thread-local variables [1][7][8]. - Must not call functions that do not also have the nonblocking attribute [1][6][5]. These features were introduced and refined in recent Clang releases (starting around Clang 19) [6][8][5]. For comprehensive real-time safety, it is recommended to use these compile-time checks in conjunction with the RealtimeSanitizer (RTSan), which can detect violations at runtime [3].

Citations:

• 1: https://clang.llvm.org/docs/FunctionEffectAnalysis.html
• 2: https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20241028/637110.html
• 3: https://clang.llvm.org/docs/RealtimeSanitizer.html
• 4: https://releases.llvm.org/22.1.0/tools/clang/docs/FunctionEffectAnalysis.html
• 5: llvm/llvm-project@7fe43ad
• 6: https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20240819/611229.html
• 7: https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20240520/581255.html
• 8: https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20240812/609310.html

---

🌐 Web query:

clang 20 nonblocking function static constexpr warning

💡 Result:

In Clang 20 and later, functions marked with the [[clang::nonblocking]] attribute are subject to "Function Effect Analysis," which verifies that the function does not contain language constructs or calls that could cause blocking [1][2]. One of the specific restrictions enforced by this analysis is that a nonblocking function must not declare static local variables [2][3]. If you define a static variable—including a static constexpr variable—inside a function annotated with [[clang::nonblocking]], the compiler will emit a warning [2][3]. The reason for this restriction is that the compiler must generate a lock to ensure thread-safe initialization of static local variables (often used for Meyers singletons), which inherently violates the nonblocking guarantee [2]. Furthermore, the initialization of such variables can inadvertently trigger heap allocations, which is also prohibited in nonblocking contexts [2]. If you encounter this warning, you must remove the static local variable from the function or move it out of the nonblocking context [2]. Violations of these rules are diagnosed under the -Wfunction-effects diagnostic category [2].

Citations:

• 1: https://releases.llvm.org/20.1.0/tools/clang/docs/ReleaseNotes.html
• 2: https://releases.llvm.org/20.1.0/tools/clang/docs/FunctionEffectAnalysis.html
• 3: https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20240812/609310.html

---

Move the silence fallback out of the function.

latestFrame() is MM_NONBLOCKING, and static constexpr AudioFrame kSilence{} is still a function-local static. Define it as a class- or namespace-scope static constexpr; otherwise Clang’s -Wfunction-effects can mark this nonblocking path as noncompliant.

Proposed fix

 class AudioService : public MoonModule {
 public:
+    static constexpr AudioFrame kSilence{};
+
     static const AudioFrame* latestFrame() MM_NONBLOCKING {
-        static constexpr AudioFrame kSilence{};
         AudioService* a = ActiveInstance<AudioService>::active();
         return a ? &a->frame_ : &kSilence;
     }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	static const AudioFrame* latestFrame() MM_NONBLOCKING {
	// constexpr, not a function-local static: a static needs a guard variable and a
	// one-time lock on first use, which is a blocking operation on the render path — and
	// this is called from EIGHT audio-reactive effects' tick(). constexpr is initialized at
	// compile time, so there is no guard and no lock.
	static constexpr AudioFrame kSilence{};
	static constexpr AudioFrame kSilence{};
	static const AudioFrame* latestFrame() MM_NONBLOCKING {
	// constexpr, not a function-local static: a static needs a guard variable and a
	// one-time lock on first use, which is a blocking operation on the render path — and
	// this is called from EIGHT audio-reactive effects' tick(). constexpr is initialized at
	// compile time, so there is no guard and no lock.
	AudioService* a = ActiveInstance<AudioService>::active();
	return a ? &a->frame_ : &kSilence;
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/AudioService.h` around lines 304 - 309, Move kSilence out of
latestFrame() and define it at class or namespace scope as a static constexpr
AudioFrame, then update latestFrame() to reference that shared constant while
preserving the existing silence fallback behavior.

[coderabbitai]

🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

Apply MM_NONBLOCKING only after the complete render path satisfies the contract.

These three ticks still reach blocking waits, synchronous socket I/O, or allocation. Remove the annotations until each path is redesigned as nonblocking, otherwise Clang rejects the call graph or the runtime contract is false.

• src/light/drivers/ParallelLedDriver.h#L559-L559: remove the annotation or replace busWaitIfBusy() with nonblocking polling.
• src/light/drivers/PreviewDriver.h#L135-L135: move synchronous sendFrame() work and keptIdx_ allocation off the render path.
• src/light/drivers/RmtLedDriver.h#L234-L234: move rmtWs2812Wait() off tick or replace it with a poll/state machine.

As per coding guidelines, render-loop callees must avoid blocking operations and heap allocation; as per path instructions, light-driver loop methods must not block or allocate.

📍 Affects 3 files

• src/light/drivers/ParallelLedDriver.h#L559-L559 (this comment)
• src/light/drivers/PreviewDriver.h#L135-L135
• src/light/drivers/RmtLedDriver.h#L234-L234

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/drivers/ParallelLedDriver.h` at line 559, The MM_NONBLOCKING
annotations are premature because these render-loop paths still block or
allocate. In src/light/drivers/ParallelLedDriver.h#L559-L559, remove the
annotation or replace busWaitIfBusy() with nonblocking polling; in
src/light/drivers/PreviewDriver.h#L135-L135, move synchronous sendFrame() and
keptIdx_ allocation off the render path; and in
src/light/drivers/RmtLedDriver.h#L234-L234, move rmtWs2812Wait() off tick or
implement polling/state-machine behavior. Ensure each tick path avoids blocking
operations and heap allocation before retaining MM_NONBLOCKING.

Sources: Coding guidelines, Path instructions

[ewowi]

Superseded by #57. Closed automatically when the head branch next was renamed to next-iteration (which is the branch name the CodeQL workflow actually triggers on). Same commits, no content lost.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

moondeck/check/check_nonblocking.py (1)

68-77: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Align baseline-state documentation with the new flag.

read_baseline() says a missing file means “everything is new,” but main() deliberately sets new=False until a baseline exists. Update the docstring/comments to describe “no comparison yet”; otherwise future changes may reintroduce the missing-versus-empty baseline bug.

Also applies to: 227-234

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@moondeck/check/check_nonblocking.py` around lines 68 - 77, Update the
documentation in read_baseline() and the corresponding comments in main() to
describe a missing baseline as “no comparison yet,” matching main()’s new=False
behavior until a baseline exists. Do not alter the baseline parsing or new-flag
logic.

src/platform/esp32/platform_esp32.cpp (1)

404-413: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Publish the static-address snapshot atomically, not just its state flag.

ethStaticIp_/ethStaticGw_/etc. and the WiFi equivalents remain plain arrays. netSetStaticIPv4() writes them on the caller/render task while the link/association handlers read them during re-pin. The release/acquire flag only synchronizes the first publication; later edits while the flag is already true can race and produce a torn IPv4 configuration. Serialize the update or publish a versioned immutable snapshot.

Also applies to: 483-492

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/esp32/platform_esp32.cpp` around lines 404 - 413, The static
IPv4 configuration arrays updated by netSetStaticIPv4() must not be read
concurrently by link or association event handlers. Serialize reads and writes
of ethStaticIp_/ethStaticGw_ and the corresponding WiFi arrays with a shared
synchronization mechanism, or replace them with a versioned immutable snapshot;
ensure re-pin handlers obtain one consistent snapshot while preserving the
existing state-flag behavior.

docs/backlog/backlog-core.md (1)

390-392: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Keep report semantics in one canonical document.

The report-only, baseline, and pre-commit behavior here duplicates moondeck/MoonDeck.md and the implementation plan. Keep this backlog entry focused on architectural remediation and link to the canonical tooling documentation so counts and gate behavior do not drift.

As per path instructions, documentation keeps each fact in one canonical location and links elsewhere.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/backlog/backlog-core.md` around lines 390 - 392, Refocus this backlog
entry on architectural remediation by removing duplicated descriptions of
-Wfunction-effects reporting, the hotpath baseline, and pre-commit gating.
Replace them with links to the canonical tooling documentation and
implementation plan, preserving only the remediation context so these facts
remain maintained in one location.

Source: Path instructions

♻️ Duplicate comments (1)

moondeck/check/check_nonblocking.py (1)

128-160: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not use emitted warnings as the capability probe.

A clean Clang 20+ build with zero findings has no [-Wfunction-effects] diagnostic, so the current clean-build check reports failure exactly when the tree becomes clean. Conversely, --incremental skips that check and can silently pass under GCC or older Clang. Validate compiler/version/flag capability from the build configuration for both modes, then accept zero diagnostics as clean.

Also applies to: 204-218

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@moondeck/check/check_nonblocking.py` around lines 128 - 160, Replace the
diagnostic-output check in build_output with a capability probe based on the
configured compiler/version and whether -Wfunction-effects is enabled, rather
than searching for emitted warnings. Apply this validation for both clean and
incremental builds, while still returning the build output and treating zero
diagnostics as a valid clean result when capability is confirmed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/metrics/whitelizard.txt`:
- Line 43: Update the whitelist entry for FreqSawsEffect to target the stable
mm::FreqSawsEffect::tick override/baseline instead of the ambiguous
mm::FreqSawsEffect::for token, preserving the existing complexity baseline and
violation tracking for tick.

In `@moondeck/MoonDeck.md`:
- Around line 362-366: Update the spelling in MoonDeck.md to use American
English consistently, replacing any occurrences of “analyse,” “analysed,” or
“analyser” with “analyze,” “analyzed,” or “analyzer,” including the changed
section and elsewhere in the document.

In `@src/light/layers/Layer.h`:
- Around line 165-174: Align the zero-grid contract across the three referenced
sites: in src/light/layers/Layer.h lines 165-174, revise the orchestration
comments to clarify that Layer gates effect dispatch without making non-zero
dimensions an effect precondition; in test/unit/light/unit_Effects_gridsweep.cpp
lines 11-16, document that direct invocation verifies effect safety at zero
dimensions; and in lines 74-82, retain the direct call while removing the claim
that zero-grid safety is outside the effect contract. Ensure effects remain safe
for all grid sizes and tick rates, including 0×0×0, without crashes or
zero-truncated animation math.

In `@test/unit/core/unit_Buffer.cpp`:
- Around line 42-43: Extend the lint suppression covering the moved-from
assertions in the buffer test so both CHECK statements, including a.count() == 0
and the nearby a.data() assertion, are exempted from the specified move
warnings. Use a second directive or a scoped NOLINTBEGIN/NOLINTEND without
changing the assertions.

In `@test/unit/core/unit_NetworkModule.cpp`:
- Around line 101-105: Update the comment beside the zero-initialized out buffer
in the parseDottedQuad test so it only describes behavior covered by the
assertions; remove the claim about failed parses comparing against zero unless
the CHECK_FALSE cases are extended with explicit unchanged-output assertions.

---

Outside diff comments:
In `@docs/backlog/backlog-core.md`:
- Around line 390-392: Refocus this backlog entry on architectural remediation
by removing duplicated descriptions of -Wfunction-effects reporting, the hotpath
baseline, and pre-commit gating. Replace them with links to the canonical
tooling documentation and implementation plan, preserving only the remediation
context so these facts remain maintained in one location.

In `@moondeck/check/check_nonblocking.py`:
- Around line 68-77: Update the documentation in read_baseline() and the
corresponding comments in main() to describe a missing baseline as “no
comparison yet,” matching main()’s new=False behavior until a baseline exists.
Do not alter the baseline parsing or new-flag logic.

In `@src/platform/esp32/platform_esp32.cpp`:
- Around line 404-413: The static IPv4 configuration arrays updated by
netSetStaticIPv4() must not be read concurrently by link or association event
handlers. Serialize reads and writes of ethStaticIp_/ethStaticGw_ and the
corresponding WiFi arrays with a shared synchronization mechanism, or replace
them with a versioned immutable snapshot; ensure re-pin handlers obtain one
consistent snapshot while preserving the existing state-flag behavior.

---

Duplicate comments:
In `@moondeck/check/check_nonblocking.py`:
- Around line 128-160: Replace the diagnostic-output check in build_output with
a capability probe based on the configured compiler/version and whether
-Wfunction-effects is enabled, rather than searching for emitted warnings. Apply
this validation for both clean and incremental builds, while still returning the
build output and treating zero diagnostics as a valid clean result when
capability is confirmed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4294f205-1725-403e-8e07-b7c40f2d678c

📥 Commits

Reviewing files that changed from the base of the PR and between 1d0184f and 4a2effa.

📒 Files selected for processing (39)

• .clang-tidy
• .github/workflows/test.yml
• docs/backlog/backlog-core.md
• docs/history/plans/Plan-20260727 - Static analysis and repo health tooling.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/metrics/whitelizard.txt
• moondeck/MoonDeck.md
• moondeck/check/check_clang_tidy.py
• moondeck/check/check_nonblocking.py
• src/light/effects/BlurzEffect.h
• src/light/effects/BouncingBallsEffect.h
• src/light/effects/FixedRectangleEffect.h
• src/light/effects/FreqMatrixEffect.h
• src/light/effects/FreqSawsEffect.h
• src/light/effects/GEQ3DEffect.h
• src/light/effects/GEQEffect.h
• src/light/effects/LavaLampEffect.h
• src/light/effects/LissajousEffect.h
• src/light/effects/MetaballsEffect.h
• src/light/effects/Noise2DEffect.h
• src/light/effects/NoiseMeterEffect.h
• src/light/effects/ParticlesEffect.h
• src/light/effects/PraxisEffect.h
• src/light/effects/RingsEffect.h
• src/light/effects/RipplesEffect.h
• src/light/effects/RubiksCubeEffect.h
• src/light/effects/SolidEffect.h
• src/light/effects/SphereMoveEffect.h
• src/light/effects/StarFieldEffect.h
• src/light/effects/StarSkyEffect.h
• src/light/effects/TetrixEffect.h
• src/light/effects/TextEffect.h
• src/light/layers/Layer.h
• src/platform/esp32/platform_esp32.cpp
• test/scenarios/light/scenario_peripheral_grid_sweep.json
• test/unit/core/unit_Buffer.cpp
• test/unit/core/unit_NetworkModule.cpp
• test/unit/light/unit_Effects_gridsweep.cpp

💤 Files with no reviewable changes (23)

• src/light/effects/BlurzEffect.h
• src/light/effects/TextEffect.h
• src/light/effects/LissajousEffect.h
• src/light/effects/PraxisEffect.h
• src/light/effects/GEQ3DEffect.h
• src/light/effects/SphereMoveEffect.h
• src/light/effects/ParticlesEffect.h
• src/light/effects/RipplesEffect.h
• src/light/effects/FreqMatrixEffect.h
• src/light/effects/NoiseMeterEffect.h
• src/light/effects/BouncingBallsEffect.h
• src/light/effects/LavaLampEffect.h
• src/light/effects/StarFieldEffect.h
• src/light/effects/RingsEffect.h
• src/light/effects/MetaballsEffect.h
• src/light/effects/Noise2DEffect.h
• src/light/effects/GEQEffect.h
• src/light/effects/RubiksCubeEffect.h
• src/light/effects/FixedRectangleEffect.h
• src/light/effects/StarSkyEffect.h
• src/light/effects/TetrixEffect.h
• src/light/effects/SolidEffect.h
• src/light/effects/FreqSawsEffect.h

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C4 'whitelizard|baseline|FreqSawsEffect|function|NAME' \
  moondeck/check/check_lizard.py docs/metrics/whitelizard.txt

Repository: MoonModules/projectMM

Length of output: 15849

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== referenced file outline =="
ast-grep outline src/light/effects/FreqSawsEffect.h --view expanded || true

echo "== relevant source snippets =="
cat -n src/light/effects/FreqSawsEffect.h | sed -n '1,220p'

echo "== diff summary/name changes around FreqSawsEffect =="
git diff -- docs/metrics/whitelizard.txt | sed -n '1,140p'

echo "== exact baseline entries =="
python3 - <<'PY'
from pathlib import Path
p=Path("docs/metrics/whitelizard.txt")
for i,line in enumerate(p.read_text().splitlines(),1):
    if "FreqSawsEffect" in line:
        print(f"{i}: {line}")
PY

Repository: MoonModules/projectMM

Length of output: 12702

---

🌐 Web query:

lizard linter C++ lambdas keyword token function name behavior

💡 Result:

Lizard, a static code complexity analyzer, treats C++ lambda expressions by aggregating them into the parent function rather than treating them as independent, top-level functions [1][2]. This design choice is intended to simplify complexity analysis, as nested constructs like lambdas are often considered idiomatic or secondary to the primary function's logic [2]. Technical behavior details: - Parsing: Lizard's C++ parser (implemented in clike.py) uses finite state machines to identify functions [3][4][5]. Because lambdas are nested, the tool's logic directs them to aggregate into the parent scope [1]. - Limitations: Due to the complexity of parsing C++ syntax without a full compiler frontend, Lizard may misinterpret certain lambda structures [6][7]. Users have reported issues where lambdas, particularly those with complex capture lists [...] or nested braces, can cause the parser to incorrectly track brackets or misidentify the function state [1]. - Function Names: Since lambdas are not treated as independent functions, they do not appear as separate entries in Lizard's function list output [8]. Consequently, they do not have independent function names or separate cyclomatic complexity metrics in the standard report [1][8]. - Tokenization: Lizard utilizes a token-based pipeline [5]. While it successfully tokenizes much of C++, its inability to perform full macro expansion or semantic analysis means that advanced or highly obfuscated C++ constructs—including some lambda-heavy patterns—can confuse the bracket-stacking logic used to determine function boundaries [6][7]. In summary, if you are analyzing code where lambda complexity is significant, Lizard's default behavior is to hide this complexity within the parent function's total score rather than surfacing it as individual function metrics [2]. If the parser struggles with a specific lambda, it is often due to the limitations of its lightweight, regex-based state machine approach to parsing C++ [6][4].

Citations:

• 1: #432 cognitive complexity terryyin/lizard#440
• 2: https://github.com/iCodeIN/lizard/blob/bb8aa4d200a2935b07e6d2bf1e6072b449bc5ae3/theory.rst
• 3: https://deepwiki.com/terryyin/lizard/3.1-c-and-c++-language-support
• 4: https://github.com/terryyin/lizard/blob/54926628/lizard_languages/clike.py
• 5: https://deepwiki.com/terryyin/lizard/2.2-token-processing-pipeline
• 6: https://pypi.org/project/lizard/1.21.6/
• 7: https://pypi.org/project/lizard/
• 8: https://raw.githubusercontent.com/terryyin/lizard/master/README.rst

---

🌐 Web query:

GitHub terryyin/lizard lark python lizard_languages clike.py __init__ function_name keyword issue

💡 Result:

In the lizard static analysis tool, the clike.py module handles C/C++ parsing using a state-machine-based approach [1][2]. The function name identification process involves a specialized state, often managed by _state_function, which listens for function-like patterns [3][2]. Regarding the specific concern about function names, keywords, and parsing: 1. Function Name Identification: The tool uses a state machine to track tokens. A potential function is only confirmed when the parser encounters an opening brace { [1]. The internal logic includes checks to prevent non-word tokens from being incorrectly identified as function names [4]. 2. Keyword Handling: The parser must distinguish between actual function names and reserved keywords. For example, in the code, specific checks are implemented (such as those handling void or operator tokens) to ensure that keywords are not misidentified as function names during the parameter or definition parsing phases [3]. 3. Common Issues: If you are encountering an issue where a keyword is being misidentified as a function name, or a function name is being missed, it often relates to the interaction between the token generation and the parallel state machines (CLikeStates, CLikeNestingStackStates) [1][2]. The repository includes regression tests specifically for scenarios like function name detection (e.g., ensuring class or other keywords are handled correctly) [4]. If you are debugging a specific parsing error, ensure your lizard version is up to date, as many parsing improvements—particularly regarding raw string literals or complex function declarations—are implemented by refining the tokenizer patterns in clike.py [5]. If a specific keyword is causing issues, you can examine the _state_function or _state_dec methods in clike.py to see how that token is currently being consumed by the state machine [3].

Citations:

• 1: https://deepwiki.com/terryyin/lizard/2.2-token-processing-pipeline
• 2: https://deepwiki.com/terryyin/lizard/3.1-c-and-c++-language-support
• 3: https://github.com/terryyin/lizard/blob/54926628/lizard_languages/clike.py
• 4: https://github.com/terryyin/lizard/blob/54926628/test/test_languages/testCAndCPP.py
• 5: terryyin/lizard@414f806

---

Preserve a stable baseline identity for FreqSawsEffect::tick.

The whitelist entry now uses mm::FreqSawsEffect::for, which can match a keyword/token inside the function instead of the callable method and changes the recorded complexity from 24/62. Use a stable .tick override/baseline entry so the gate continues to track violations for mm::FreqSawsEffect::tick.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/metrics/whitelizard.txt` at line 43, Update the whitelist entry for
FreqSawsEffect to target the stable mm::FreqSawsEffect::tick override/baseline
instead of the ambiguous mm::FreqSawsEffect::for token, preserving the existing
complexity baseline and violation tracking for tick.

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Normalize spelling to American English.

The changed section uses analyzed, while LanguageTool reports mixed analyze/analyse variants elsewhere in this document. Normalize all occurrences to analyze, analyzed, and analyzer.

As per coding guidelines, **/*.md uses American English spelling throughout.

🧰 Tools

🪛 LanguageTool

[uncategorized] ~364-~364: Do not mix variants of the same word (‘analyze’ and ‘analyse’) within a single text.
Context: ... site, the function's own body is never analyzed. A platform function that blocks intern...

(EN_WORD_COHERENCY)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@moondeck/MoonDeck.md` around lines 362 - 366, Update the spelling in
MoonDeck.md to use American English consistently, replacing any occurrences of
“analyse,” “analysed,” or “analyser” with “analyze,” “analyzed,” or “analyzer,”
including the changed section and elsewhere in the document.

Sources: Coding guidelines, Linters/SAST tools

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Align the zero-grid contract across production and tests.

The Layer guard correctly skips normal effect dispatch for degenerate grids, but the comments in both files incorrectly make non-zero dimensions an effect precondition.

• src/light/layers/Layer.h#L165-L174: document this as Layer-level orchestration gating, not permission for effects to assume non-zero dimensions.
• test/unit/light/unit_Effects_gridsweep.cpp#L11-L16: state that direct invocation verifies effect safety at zero dimensions.
• test/unit/light/unit_Effects_gridsweep.cpp#L74-L82: retain the direct call and remove the claim that zero-grid safety is outside the effect contract.

As per path instructions, effects must work at every grid size and tick rate, including 0×0×0, without crashes or animation math truncating to zero.

📍 Affects 2 files

• src/light/layers/Layer.h#L165-L174 (this comment)
• test/unit/light/unit_Effects_gridsweep.cpp#L11-L16
• test/unit/light/unit_Effects_gridsweep.cpp#L74-L82

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/light/layers/Layer.h` around lines 165 - 174, Align the zero-grid
contract across the three referenced sites: in src/light/layers/Layer.h lines
165-174, revise the orchestration comments to clarify that Layer gates effect
dispatch without making non-zero dimensions an effect precondition; in
test/unit/light/unit_Effects_gridsweep.cpp lines 11-16, document that direct
invocation verifies effect safety at zero dimensions; and in lines 74-82, retain
the direct call while removing the claim that zero-grid safety is outside the
effect contract. Ensure effects remain safe for all grid sizes and tick rates,
including 0×0×0, without crashes or zero-truncated animation math.

Source: Path instructions

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C2 'NOLINTNEXTLINE|CHECK\(a\.(data|count)\(' \
  test/unit/core/unit_Buffer.cpp

Repository: MoonModules/projectMM

Length of output: 722

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- relevant file context ---\n'
sed -n '1,90p' test/unit/core/unit_Buffer.cpp

printf '\n--- clang-tidy/lint config candidates ---\n'
git ls-files | rg -n '(^|/)(\.clang-tidy|clang-tidy|CMakeLists\.txt|Makefile|compile_commands\.json)$' || true

printf '\n--- references to moved-from assertions ---\n'
rg -n -C1 'moved-from state|a\.count|\(\(&&|&&\)\)' test/unit/core/unit_Buffer.cpp

Repository: MoonModules/projectMM

Length of output: 250

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- relevant file context ---'
sed -n '1,90p' test/unit/core/unit_Buffer.cpp

printf '%s\n' ''
printf '%s\n' '--- clang-tidy/lint config candidates ---'
git ls-files | rg -n '(^|/)(\.clang-tidy|clang-tidy|CMakeLists\.txt|Makefile|compile_commands\.json)$' || true

printf '%s\n' ''
printf '%s\n' '--- references to moved-from assertions ---'
rg -n -C1 'moved-from state|a\.count|\(\(&&|&&\)\)' test/unit/core/unit_Buffer.cpp

Repository: MoonModules/projectMM

Length of output: 3528

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- .clang-tidy ---'
sed -n '1,220p' .clang-tidy

printf '%s\n' ''
printf '%s\n' '--- run-clang-tidy references/scripts ---'
rg -n -i 'run-clang-tidy|clang-tidy|NOLINT|bugprone-use-after-move|cppcoreguidelines-use-after-move' . --glob '!build/**' --glob '!dist/**' || true

printf '%s\n' ''
printf '%s\n' '--- relevant doctest/matchers if present ---'
fd -a . . | rg 'doctest' | head -20

Repository: MoonModules/projectMM

Length of output: 27066

---

Cover both moved-from assertions with the suppression.

NOLINTNEXTLINE only suppresses the single next line, so CHECK(a.count() == 0); is still unprotected under the repo’s clang-tidy config. Add a second directive or use NOLINTBEGIN/NOLINTEND.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/unit/core/unit_Buffer.cpp` around lines 42 - 43, Extend the lint
suppression covering the moved-from assertions in the buffer test so both CHECK
statements, including a.count() == 0 and the nearby a.data() assertion, are
exempted from the specified move warnings. Use a second directive or a scoped
NOLINTBEGIN/NOLINTEND without changing the assertions.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the test comment aligned with the assertions.

Line 104 says a failed parse compares against known zero, but the test never inspects out after a CHECK_FALSE. Remove that claim or add explicit unchanged-output assertions for rejected inputs.

🧰 Tools

🪛 Cppcheck (2.21.0)

[style] 103-103: The function 'appearsInUi' is never used.

(unusedFunction)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/unit/core/unit_NetworkModule.cpp` around lines 101 - 105, Update the
comment beside the zero-initialized out buffer in the parseDottedQuad test so it
only describes behavior covered by the assertions; remove the claim about failed
parses comparing against zero unless the CHECK_FALSE cases are extended with
explicit unchanged-output assertions.