MoonLive: scripted effects, modifiers and layouts, compiled on the device

[ewowi]

Scripted effects, modifiers and layouts you can write on a running device, and the container rename that landed alongside them.

MoonLive: write a script, see it on the wall

A source control holds text; the engine compiles it to native machine code and calls it each tick through a function pointer. Not an interpreter — a compiler with three backends (Xtensa, RISC-V, arm64) behind one IR, so a script runs at hand-written speed on the render path.

Three bindings ship: a scripted effect (colors), modifier (where one light lands), and layout (where the lights physically are). Eleven example scripts live in moonlive/, each compiled by the test suite so a language change that breaks one is caught.

// moonlive/layouts/ring.mlv — 24 lights in an 11x11 box
uint8_t count = 24;   // @control 3..255
uint8_t radius = 5;   // @control 1..127

for (i = 0; i < count; i = i + 1) {
  addLight(scale(cos(i * turn(count)), radius * 2 + 1),
           scale(sin(i * turn(count)), radius * 2 + 1), 0);
}

A uint8_t x = 4; // @control 0..64 line becomes a real slider, bound to a live arena byte the running code reads — moving it does not recompile.

System variables, not hidden declarations

The values a script is given — t, the layer's width/height/depth, and the x/y/z a modifier folds — are a first-class construct: the host registers them, the compiler resolves them before locals and controls, and a script cannot declare a name that shadows one.

They used to be faked. Each binding pasted uint8_t width = 0; declarations on top of the user's script and hid them from the UI by index, which meant an effect could declare its own width and silently disagree with the layer it was drawing into — set 16 on an 8×8 panel and it drew off the edge.

Each binding supplies only what it writes. That is what keeps x and y usable as ordinary loop counters in a layout while they mean "the light being folded" in a modifier, and what turns a layout reading width into a named error rather than a silent zero that places no lights.

Layers → Effects

The three top-level containers are now Layouts, Effects, Drivers — L.E.D. The old name sat one character from its own child (Layers holding Layers) and read as a near-twin of Layouts. The tree keeps its shape; the child Layer is untouched.

Breaking

Two, both in MIGRATING.md with the action to take:

• A saved layout script declaring width/height no longer compiles and its fixture goes dark until edited (the shipped grid.mlv uses cols/rows).
• The Layers container rename costs the saved light tree and preset role tints; both are one file edit on the device, or a rebuild in the UI.

Bugs this branch fixed

• Two heap overruns that crashed a layout resize — one in the blend map, one in the folded-LUT scatter. 320 resizes clean afterward.
• A for typo compiled clean. for (i = 0; j < 3; i = i + 1) emitted code testing i whatever the clauses named, and trailing junk after the step was dropped. A wrong fixture with no diagnostic — the hardest kind to trace.
• A nested loop reusing its variable name emitted a program that never returned: a watchdog reset from a script anyone could type.
• Three register miscompiles: RISC-V scratch aliasing a vreg, -1 materialised as 65535 on two backends, and nested loops refused on Xtensa for scratch that a program without inline ops never needed.
• controlSlot() was bounded by the control count, so every system-variable write hit a null pointer and vanished.
• The rename missed MoonDeck: run_live_scenario.py could not restore the Effects subtree, so a live scenario would leave a bench board with a destroyed config.

Verification

• 1226 unit tests, 20 scenarios inside their existing contracts — no budget renegotiated.
• Desktop tick 150 → 122 µs across the branch (the heap-overrun and register-reuse work, not MoonLive).
• ASan and TSan clean; the sanitizer jobs also caught a missing #include <string> that every local clang build supplied transitively, which is why a GCC gate (CI's toolchain) now runs on every commit.
• A Reviewer pass over the full diff plus four CodeRabbit rounds; findings fixed or skipped with a reason in the commit bodies.
• Bench-verified on S3 and S31 (8×8 panel), desktop preview throughout.

Known limits

• lines.mlv does not compile on Xtensa — fill plus two loops exceeds the S3 register budget. It runs on S31 and desktop; every other shipped script compiles on all three. The fix is spilling to the stack, which is its own branch.
• A scripted modifier cannot reshape the grid yet (system variables are read-only), and cannot drop a light out of bounds (a coordinate is a byte). Both backlogged by name.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The pull request renames the light-pipeline container from Layers to Effects. It adds MoonLive scripting for layouts, modifiers, loops, arithmetic, built-ins, backend code generation, diagnostics, and validation. It also adds light-buffer bounds checks and ESP32 panic decoding.

Changes

Effects rename and runtime integration

Layer / File(s)	Summary
Effects container and preset contract src/light/layers/Effects.h, src/core/ControlModule.h, src/light/drivers/Drivers.h	Renames the public container to Effects, changes the preset role to effects, and updates driver binding to setEffects().
Application, UI, scenarios, and tests src/main.cpp, src/ui/*, test/scenarios/*, test/unit/*	Registers and wires Effects, updates fixture hierarchies, and adapts preset, driver, lifecycle, and composition tests.
Documentation and migration notes docs/*, README.md	Documents the renamed container, persisted Effects.json data, updated preset keys, and unchanged child Layer names.

MoonLive compiler and execution

Layer / File(s)	Summary
Compiler and IR src/core/moonlive/*	Adds arithmetic expressions, scoped for loops, three-argument calls, labels, branches, system variables, and code-generation diagnostics.
Platform backends src/platform/desktop/moonlive_*, src/platform/esp32/moonlive_*	Updates register allocation, call conventions, branching, immediate encoding, label lowering, and scratch-register handling.
Script built-ins src/light/moonlive/MoonLiveBuiltins_light.h, src/light/moonlive/MoonLiveEffect.h	Adds rate-limited print, addLight callback dispatch, source replacement, print-budget resets, and heap reporting.

MoonLive modules and validation

Layer / File(s)	Summary
Scripted layouts and modifiers src/light/moonlive/MoonLiveLayout.h, src/light/moonlive/MoonLiveModifier.h	Adds script-backed coordinate generation and coordinate transformation with controls, recompilation, error fallback, and rebuild signaling.
Example scripts and documentation moonlive/*, moonlive/README.md, docs/moonmodules/light/*	Adds layout, effect, and modifier scripts with authoring and runtime documentation.
Tests and scenarios test/unit/light/unit_MoonLive*.cpp, test/scenarios/light/scenario_MoonLive_pipeline.json	Adds parser, compiler, concurrency, memory, layout, modifier, and end-to-end pipeline coverage.

Light safety and diagnostics

Layer / File(s)	Summary
Mapped-buffer bounds protection src/light/layers/BlendMap.h, src/light/layers/Layer.h, test/unit/light/unit_BlendMap.cpp	Bounds LUT reads and writes during resize transitions and drops excess destinations instead of writing past allocated buffers.
MoonDeck diagnostics moondeck/run/monitor_esp32.py, moondeck/moonlive/disasm.py, moondeck/MoonDeck.md	Adds optional firmware ELF panic decoding and a MoonLive Xtensa emission/disassembly command.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

• MoonModules/projectMM#30: Both PRs modify MoonLive compiler, runtime, IR, and backend code.
• MoonModules/projectMM#13: Both PRs update light-pipeline container binding and Drivers wiring.
• MoonModules/projectMM#62: Both PRs modify ControlModule preset capture roles and subtree names.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.37% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the MoonLive scripted effects, modifiers, and layouts added by the pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

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

---

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: 14

🤖 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/architecture.md`:
- Line 33: Update both table-of-contents links for the “Effects and Layer”
section to use the correct Markdown anchor `#effects-and-layer` instead of
`#layers-and-layer`, including the reference near line 475.

In `@docs/backlog/system-modules.md`:
- Line 65: Rewrite the sentence around the “Services is to System” comparison so
it describes Services as the existing top-level container for user-added Audio
and IR modules, analogous to Effects containing user-added Layer children.
Remove the implication that Effects contains effect children directly or that
MQTT is a user-added Service; preserve MQTT as code-wired under Network.

In `@docs/gettingstarted.md`:
- Around line 272-279: Propagate the Layers-to-Effects terminology across all
specified documentation: in docs/gettingstarted.md lines 257-261 and 290-299 and
README.md line 23, use the pipeline name “Layouts → Effects → Drivers”; in
docs/moonmodules/core/control.md lines 51 and 53, change preset-role references
from “layer” to “Effects” or “effects”; in docs/moonmodules/light/supporting.md
lines 21-27, add and use the effects anchor, update the test link accordingly,
and retain layers only as an intentional compatibility alias.

In `@docs/moonmodules/core/services.md`:
- Line 7: Update the Services heading in the core services documentation from a
third-level heading to ## Services so it is correctly nested beneath # Core
services and provides the proper parent level for Audio and IR.

In `@docs/usecases/build-your-own-moonmodules.md`:
- Line 322: Update docs/usecases/build-your-own-moonmodules.md at line 322 to
describe the hierarchy as Layouts → Effects → Layer → Effect/Modifier → Drivers,
with Layouts, Effects, and Drivers as top-level sibling containers and Layer
nested under Effects. Update docs/usecases/home-automation.md at line 111 to
direct users to add the Hue driver in the top-level Drivers container, not under
a Layer.
- Line 322: Update the architecture document link description in the “The
architecture doc” bullet to reflect the hierarchy `Layouts → Effects → Layer →
Effect/Modifier → Drivers`, distinguishing the top-level Effects container from
its child Effect/Modifier stage. Keep the existing hot-path rules reference
unchanged.

In `@src/core/Scheduler.cpp`:
- Line 157: Update the nearby explanatory text to say “two Layer instances”
rather than “two Effects,” matching ModuleFactory::create("Layer") and the
duplicate Layer children under the Effects container; leave the uniqueness logic
unchanged.

In `@src/light/effects/EffectBase.h`:
- Line 69: Restore the MoonLight prior-art URL path in the comments by replacing
Effects/VirtualLayer.h with Layers/VirtualLayer.h in
src/light/effects/EffectBase.h lines 69-69, src/light/layers/Buffer.h lines
28-28, and src/light/layers/Layer.h lines 33-33; no other changes are needed.

In `@src/light/layers/Effects.h`:
- Around line 45-48: Update the comments around the role-filtered loop
propagation and the corresponding comment near line 103 to describe child
modules as Layer instances or having ModuleRole::Layer, not Effects. Keep
Effects reserved for the container terminology and align both comments with the
implementation.
- Line 16: Replace the broken MoonLight prior-art URL in the comment near the
Effects container in src/light/layers/Effects.h:16-16 with one verified
canonical reference, or remove the link. Apply the same consistent update to the
Drivers prior-art comment in src/light/drivers/Drivers.h:71-71.

In `@src/main.cpp`:
- Line 158: Add a one-time migration alongside
mm::ModuleFactory::registerType<mm::Effects> that rewrites persisted “Layers.*”
keys to “Effects.*” and “captures”:“Layers” values to “Effects” across settings
and preset payloads before loading them. Ensure existing data remains
applicable, and add coverage or documentation for the complete migration.
- Line 158: Update the documentation link in
mm::ModuleFactory::registerType<mm::Effects> from the stale `#layers` anchor to
light/supporting.md#effects, unless an explicit compatibility anchor is
intentionally retained at the Effects heading.

In `@test/scenario_runner.cpp`:
- Around line 316-323: Keep Effects/Layer terminology consistent across the
affected tests: in test/scenario_runner.cpp lines 316-323, describe child Layer
modules; update the specified descriptions in
test/unit/light/unit_Effects_container.cpp lines 29-32, 90-91, 117-119, and
141-145 to refer to child Layers, enabled Layer buffers, and Drivers compositing
Layers. In test/unit/light/unit_Effects_container.cpp lines 258-265, 280-287,
302-308, 322-329, 349-358, 404-408, 424-433, and 451, rename the local
mm::Effects variable from layers to effects without changing behavior.

In `@test/unit/light/unit_Drivers_container.cpp`:
- Line 11: Update the comment describing Layer::tick() and Effects::tick() to
replace “child Effects” with “child Layer modules,” accurately identifying
Effects as the top-level container and Layer as its child type.

🪄 Autofix

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: ba437a56-c961-4f03-931a-3d9a52d331d3

📥 Commits

Reviewing files that changed from the base of the PR and between 611ee89 and 9afeec1.

⛔ Files ignored due to path filters (1)

• docs/assets/light/Effects.png is excluded by !**/*.png

📒 Files selected for processing (63)

• docs/MIGRATING.md
• docs/architecture.md
• docs/backlog/power-functions-analysis-top-down.md
• docs/backlog/system-modules.md
• docs/coding-standards.md
• docs/gettingstarted.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/moonmodules/core/control.md
• docs/moonmodules/core/services.md
• docs/moonmodules/light/supporting.md
• docs/usecases/build-your-own-moonmodules.md
• docs/usecases/home-automation.md
• src/core/ControlModule.h
• src/core/FilesystemModule.h
• src/core/HttpServerModule.cpp
• src/core/ModuleFactory.h
• src/core/MoonModule.h
• src/core/Scheduler.cpp
• src/core/Services.h
• src/light/drivers/Drivers.h
• src/light/effects/EffectBase.h
• src/light/layers/Buffer.h
• src/light/layers/Effects.h
• src/light/layers/Layer.h
• src/light/layers/MappingLUT.h
• src/light/layouts/Layouts.h
• src/main.cpp
• src/ui/app.js
• src/ui/style.css
• test/CMakeLists.txt
• test/scenario_runner.cpp
• test/scenarios/core/scenario_MoonModule_control_change.json
• test/scenarios/light/scenario_Audio_mutation.json
• test/scenarios/light/scenario_Effects_composition.json
• test/scenarios/light/scenario_GridBlacks_blackpixel.json
• test/scenarios/light/scenario_GridLayout_resize.json
• test/scenarios/light/scenario_MoonLiveEffect_controls.json
• test/scenarios/light/scenario_MoonLiveEffect_livescript.json
• test/scenarios/light/scenario_modifier_chain.json
• test/scenarios/light/scenario_modifier_swap.json
• test/scenarios/light/scenario_perf_full.json
• test/scenarios/light/scenario_perf_light.json
• test/scenarios/light/scenario_peripheral_grid_sweep.json
• test/scenarios/light/scenario_peripheral_switch.json
• test/unit/core/unit_ControlModule.cpp
• test/unit/core/unit_FilesystemModule_subtree.cpp
• test/unit/core/unit_MoonModule_lifecycle.cpp
• test/unit/core/unit_MqttModule.cpp
• test/unit/core/unit_Scheduler_unique_names.cpp
• test/unit/core/unit_Services.cpp
• test/unit/light/golden_frame.h
• test/unit/light/unit_Canvas.cpp
• test/unit/light/unit_Drivers_container.cpp
• test/unit/light/unit_Drivers_rendersplit.cpp
• test/unit/light/unit_Effects_container.cpp
• test/unit/light/unit_Layer_live_modifier.cpp
• test/unit/light/unit_Layer_persistence.cpp
• test/unit/light/unit_Layer_phase_animation.cpp
• test/unit/light/unit_Layouts_container.cpp
• test/unit/light/unit_Layouts_toggle_cycle.cpp
• test/unit/light/unit_PreviewDriver.cpp
• test/unit/light/unit_RandomMapModifier.cpp

[coderabbitai]

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

Propagate the Layers → Effects rename through public documentation.

The visible rename is applied, but old container names remain in pipeline prose, preset-role prose, and anchors. Use Effects for the top-level container and Layer only for child modules.

• docs/gettingstarted.md#L272-L279: update the pipeline references at Lines 257-261 and 290-299, and README.md Line 23, to use Layouts → Effects → Drivers.
• docs/moonmodules/core/control.md#L38-L45: change the preset-role prose at Lines 51 and 53 from layer to Effects or effects.
• docs/moonmodules/light/supporting.md#L21-L27: add and use the effects anchor, update the test link, and retain layers only as an intentional compatibility alias.

📍 Affects 3 files

• docs/gettingstarted.md#L272-L279 (this comment)
• docs/moonmodules/core/control.md#L38-L45
• docs/moonmodules/light/supporting.md#L21-L27

🤖 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/gettingstarted.md` around lines 272 - 279, Propagate the
Layers-to-Effects terminology across all specified documentation: in
docs/gettingstarted.md lines 257-261 and 290-299 and README.md line 23, use the
pipeline name “Layouts → Effects → Drivers”; in docs/moonmodules/core/control.md
lines 51 and 53, change preset-role references from “layer” to “Effects” or
“effects”; in docs/moonmodules/light/supporting.md lines 21-27, add and use the
effects anchor, update the test link accordingly, and retain layers only as an
intentional compatibility alias.

[coderabbitai]

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

Replace the duplicated broken prior-art links.

The same prior-art URL appears in both changed comments and returns 404. Replace both links with one verified canonical MoonLight reference, or remove both links. ()

• src/light/layers/Effects.h#L16-L16: update the Effects container prior-art link.
• src/light/drivers/Drivers.h#L71-L71: update the Drivers prior-art link.

📍 Affects 2 files

• src/light/layers/Effects.h#L16-L16 (this comment)
• src/light/drivers/Drivers.h#L71-L71

🤖 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/Effects.h` at line 16, Replace the broken MoonLight
prior-art URL in the comment near the Effects container in
src/light/layers/Effects.h:16-16 with one verified canonical reference, or
remove the link. Apply the same consistent update to the Drivers prior-art
comment in src/light/drivers/Drivers.h:71-71.

Source: MCP tools

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Migrate persisted keys as well as the filename.

Changing the registered type to "Effects" changes more than /.config/Layers.json to Effects.json. Existing persistence and preset payloads also contain old "Layers.*" keys and "captures":"Layers" values, while the updated fixtures require "Effects.*" and "captures":"Effects". Renaming only the file leaves settings unapplied and makes old presets non-applicable.

Add a one-time migration, or document and test a complete rewrite of the old keys and capture values before shipping.

🤖 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/main.cpp` at line 158, Add a one-time migration alongside
mm::ModuleFactory::registerType<mm::Effects> that rewrites persisted “Layers.*”
keys to “Effects.*” and “captures”:“Layers” values to “Effects” across settings
and preset payloads before loading them. Ensure existing data remains
applicable, and add coverage or documentation for the complete migration.

---

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n '^#+ .*(Effects|Layers)' docs/moonmodules/light/supporting.md

Repository: MoonModules/projectMM

Length of output: 174

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "### File context"
sed -n '1,50p' docs/moonmodules/light/supporting.md

echo
echo "### Anchor usage"
rg -n 'supporting\.md#[A-Za-z0-9-]+' .

Repository: MoonModules/projectMM

Length of output: 3263

---

Fix the Effects documentation anchor.

light/supporting.md has an old #layers anchor before the renamed Effects section and no matching #layers heading. registerType<mm::Effects>("Effects", "light/supporting.md#layers") points to the wrong section. Update it to light/supporting.md#effects, or keep an explicit compatibility anchor at the Effects heading.

🤖 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/main.cpp` at line 158, Update the documentation link in
mm::ModuleFactory::registerType<mm::Effects> from the stale `#layers` anchor to
light/supporting.md#effects, unless an explicit compatibility anchor is
intentionally retained at the Effects heading.

[coderabbitai]

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep the Effects/Layer boundary consistent in test comments and names.

The runtime contract uses Effects as the grouping container and Layer as its child role. The changed test text and local names use Effects for child Layers or layers for an mm::Effects object.

• test/scenario_runner.cpp#L316-L323: change “child Effects” to “child Layer modules”.
• test/unit/light/unit_Effects_container.cpp#L29-L32: describe composition across child Layers.
• test/unit/light/unit_Effects_container.cpp#L90-L91: rename “two child Effects” to “two child Layers”.
• test/unit/light/unit_Effects_container.cpp#L117-L119: refer to enabled Layer buffers.
• test/unit/light/unit_Effects_container.cpp#L141-L145: describe Drivers compositing Layers from the Effects container.
• test/unit/light/unit_Effects_container.cpp#L258-L265: rename the local layers variable to effects.
• test/unit/light/unit_Effects_container.cpp#L280-L287: rename the local layers variable to effects.
• test/unit/light/unit_Effects_container.cpp#L302-L308: rename the local layers variable to effects.
• test/unit/light/unit_Effects_container.cpp#L322-L329: rename the local layers variable to effects.
• test/unit/light/unit_Effects_container.cpp#L349-L358: rename the local layers variable to effects.
• test/unit/light/unit_Effects_container.cpp#L404-L408: rename the local layers variable to effects.
• test/unit/light/unit_Effects_container.cpp#L424-L433: rename the local layers variable to effects.
• test/unit/light/unit_Effects_container.cpp#L451-L451: rename the local layers variable to effects.

As per coding guidelines, use standard, recognizable names and make test descriptions state user-understandable behavior.

📍 Affects 2 files

• test/scenario_runner.cpp#L316-L323 (this comment)
• test/unit/light/unit_Effects_container.cpp#L29-L32
• test/unit/light/unit_Effects_container.cpp#L90-L91
• test/unit/light/unit_Effects_container.cpp#L117-L119
• test/unit/light/unit_Effects_container.cpp#L141-L145
• test/unit/light/unit_Effects_container.cpp#L258-L265
• test/unit/light/unit_Effects_container.cpp#L280-L287
• test/unit/light/unit_Effects_container.cpp#L302-L308
• test/unit/light/unit_Effects_container.cpp#L322-L329
• test/unit/light/unit_Effects_container.cpp#L349-L358
• test/unit/light/unit_Effects_container.cpp#L404-L408
• test/unit/light/unit_Effects_container.cpp#L424-L433
• test/unit/light/unit_Effects_container.cpp#L451-L451

🤖 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/scenario_runner.cpp` around lines 316 - 323, Keep Effects/Layer
terminology consistent across the affected tests: in test/scenario_runner.cpp
lines 316-323, describe child Layer modules; update the specified descriptions
in test/unit/light/unit_Effects_container.cpp lines 29-32, 90-91, 117-119, and
141-145 to refer to child Layers, enabled Layer buffers, and Drivers compositing
Layers. In test/unit/light/unit_Effects_container.cpp lines 258-265, 280-287,
302-308, 322-329, 349-358, 404-408, 424-433, and 451, rename the local
mm::Effects variable from layers to effects without changing behavior.

Source: Coding guidelines

[coderabbitai]

Actionable comments posted: 9

🤖 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/MIGRATING.md`:
- Around line 33-38: Update the migration documentation in docs/MIGRATING.md
(lines 33-38) to represent preset captures as the single string value the loader
reads, using "Layers" only for the file-edit migration path; update the
corresponding guidance in docs/moonmodules/core/control.md (lines 35-43)
consistently. Do not document captures as an array.

In `@docs/moonmodules/light/MoonLiveModifier.md`:
- Around line 41-47: Update the Limits section of MoonLiveModifier documentation
to state that values written by setXYZ are stored as bytes and therefore
truncate or wrap to the 0–255 range, including transformed outputs beyond that
range. Place this behavior alongside the existing input-coordinate limit
description and keep the documented input behavior unchanged.

In `@README.md`:
- Line 23: Update the pipeline description near the `layouts → layers with
effects + modifiers → drivers` text to name the top-level stage `Effects` and
describe its child layers separately, matching the terminology used in the
pluggable pipeline description while preserving the existing pipeline order.

In `@src/light/moonlive/MoonLiveBuiltins_light.h`:
- Around line 34-47: The print burst cap is process-lifetime rather than per-run
because mm_light_print uses a never-reset function-local static counter. In
src/light/moonlive/MoonLiveBuiltins_light.h:34-47, either reset the counter at
the start of each script run sequence or revise the surrounding comment to
accurately describe per-boot behavior; in
docs/moonmodules/light/MoonLiveModifier.md:39, update the documentation to match
the shipped behavior, explicitly replacing the per-run burst claim.

In `@src/light/moonlive/MoonLiveModifier.h`:
- Around line 138-151: Remove the unused public test accessors sourceForTest()
and boxForTest() from MoonLiveModifier; do not alter the release() or
setSource() behavior.
- Around line 165-175: Replace the redundant compiled_ buffer in the MoonLive
modifier state with a compact checksum of full_. Update the rebuild-detection
logic to compute and compare the checksum, refreshing it whenever the mapping is
rebuilt, while preserving the existing source-change behavior and using the
repository’s existing core/crc helper.

In `@test/unit/light/unit_MoonLiveModifier.cpp`:
- Around line 167-209: Remove the first duplicate rebuild-signal test case and
its preceding explanatory comment. Keep the second test case, “editing a script
asks the layer to rebuild its mapping,” including its rebuild-consumption checks
and final modifyLogical assertion.
- Around line 240-265: Update the test case “a script that computes a position
outside the grid does not black out the fixture” so it observes the modifier’s
mapped output rather than the prefilled layer buffer. Remove the direct buffer
fill/count assertion and inspect the mapping fold’s counts or destinations after
applying the layer, or render through a driver and count emitted lights; ensure
the assertion fails when no light is mapped.
- Around line 147-165: Update the test case “transforming a wall's worth of
lights stays within a rebuild's budget” to remove the timing-dependent CHECK on
elapsed microseconds and the associated wall-clock assertion. Preserve the loop
and MESSAGE measurement for reporting, and replace the timing-based validation
with a deterministic assertion that directly verifies the script is not compiled
once per modifyLogical call, using the existing observable API or state.

🪄 Autofix

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: 6f4521ac-651a-4bef-9cee-475ff9d31d5e

📥 Commits

Reviewing files that changed from the base of the PR and between 9afeec1 and f99ee5e.

📒 Files selected for processing (25)

• README.md
• docs/MIGRATING.md
• docs/architecture.md
• docs/backlog/system-modules.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/moonmodules/core/control.md
• docs/moonmodules/core/services.md
• docs/moonmodules/light/MoonLiveModifier.md
• docs/usecases/build-your-own-moonmodules.md
• docs/usecases/home-automation.md
• src/core/ControlModule.h
• src/core/moonlive/MoonLiveCompiler.cpp
• src/light/drivers/Drivers.h
• src/light/layers/Effects.h
• src/light/layers/Layer.h
• src/light/moonlive/MoonLiveBuiltins_light.h
• src/light/moonlive/MoonLiveModifier.h
• src/main.cpp
• src/ui/app.js
• test/CMakeLists.txt
• test/unit/core/unit_ControlModule.cpp
• test/unit/light/unit_Drivers_container.cpp
• test/unit/light/unit_Effects_container.cpp
• test/unit/light/unit_MoonLiveModifier.cpp

[coderabbitai]

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Expect one persisted captures shape across implementation and documentation.
rg -n -C 8 \
  'saveSubtreeTo|applySubtree|captures' \
  --glob '*.{cpp,h,hpp,md,json}'

Repository: MoonModules/projectMM

Length of output: 159

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files matching target names:"
git ls-files | rg '(^docs/MIGRATING\.md$|^docs/moonmodules/core/control\.md$|FilesystemModule\.md$|\.cpp$|\.h$|\.hpp$|\.json$)' | sed -n '1,200p'

echo
echo "Search presets/captures/persistence names without filtering by extension first:"
rg -n -C 5 'captures|saveSubtreeTo|applySubtree|Layers|Effects' . --glob '!**/.git/**' | sed -n '1,300p'

Repository: MoonModules/projectMM

Length of output: 36383

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Target docs lines:"
sed -n '1,80p' docs/MIGRATING.md
echo
sed -n '1,70p' docs/moonmodules/core/control.md

echo
echo "Source files containing LightPresetsModule/preset/captures/saveSubtreeTo/applySubtree:"
rg -n -C 6 'LightPresetsModule|LightPresets|preset|presets|captures|saveSubtreeTo|applySubtree|Layers|Effects' \
  src moondeck library.json espidf component.mk CMakeLists.txt -g '!**/build/**' -g '!**/.git/**' | sed -n '1,400p'

echo
echo "List LightPresetsModule candidate files:"
git ls-files | rg '(^src/light/drivers/LightPresetsModule\.(h|cpp)$|.*ControlModule.*|.*FilesystemModule.*)'

Repository: MoonModules/projectMM

Length of output: 49643

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 12 'class ControlModule|struct P|presets|captures|preset|apply\(|save\(' \
  src/core/Control*.h src/core/Control*.cpp src/core/ControlModule.cpp || true

python3 - <<'PY'
import subprocess, re
files = subprocess.check_output(['git','ls-files'], text=True).splitlines()
for f in files:
    if f.endswith(('.h','.hpp','.cpp','.md')):
        text = open(f, encoding='utf-8', errors='surrogateescape').read()
        for name in ['ControlModule','presetsRevision','presetCount','presetName','activePreset','applyPreset','capture','captures']:
            if name in text:
                lines = text.splitlines()
                for i,l in enumerate(lines,1):
                    if name in l:
                        print(f"{f}:{i}: {l.strip()}")
PY

Repository: MoonModules/projectMM

Length of output: 50378

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "ControlModule implementation excerpt:"
sed -n '1,240p' src/core/ControlModule.cpp
echo
sed -n '240,420p' src/core/ControlModule.cpp

echo
echo "Read-only semantic probe: parseString ignores array value."
python3 - <<'PY'
# Mirror the minimal parseString path from JsonUtil/ControlModule behavior without running repo code:
# parseString(head, "captures", p.captures) expects "captures":"Value"; when captures is ["Layers"]
# the value token is [, so the string parse does not produce "Layers".
import json
for value in ["Effects", '["Layers"]']:
    data = {"captures": value}
    print(value, "== parseString target ==")
    try:
        print(json.loads(data))
    except Exception as e:
        print("exception", *type(e).__name__, str(e))
PY

Repository: MoonModules/projectMM

Length of output: 267

---

Keep the migration capture key as a string.

ControlModule persists preset captures as a single subtree string (for example "Effects"); "captures": ["Layers"] does not match the single-value capture the loader reads. The migration instruction should use "Layers" in the migrate-to example only if a file edit is the chosen path.

📍 Affects 2 files

• docs/MIGRATING.md#L33-L38 (this comment)
• docs/moonmodules/core/control.md#L35-L43

🤖 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/MIGRATING.md` around lines 33 - 38, Update the migration documentation
in docs/MIGRATING.md (lines 33-38) to represent preset captures as the single
string value the loader reads, using "Layers" only for the file-edit migration
path; update the corresponding guidance in docs/moonmodules/core/control.md
(lines 35-43) consistently. Do not document captures as an array.

Source: Coding guidelines

[coderabbitai]

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

Keep the README pipeline terminology consistent.

Line 23 now uses Effects, but Line 106 still describes layouts → layers with effects + modifiers → drivers. The README presents two names for the same top-level stage. Update Line 106 to use Effects and distinguish its Layer children.

🤖 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 `@README.md` at line 23, Update the pipeline description near the `layouts →
layers with effects + modifiers → drivers` text to name the top-level stage
`Effects` and describe its child layers separately, matching the terminology
used in the pluggable pipeline description while preserving the existing
pipeline order.

Source: Coding guidelines

[coderabbitai]

Actionable comments posted: 4

Caution

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

⚠️ Outside diff range comments (2)

docs/moonmodules/light/MoonLiveModifier.md (1)

49-55: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document script-declared controls.

The table lists only source, but MoonLiveModifier::defineControls() also exposes each user-declared MoonLive control after compilation. Document how a script declares these controls and state that changing one rebuilds the mapping.

As per coding guidelines: “Documentation must describe the system as it currently exists.”

🤖 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/moonmodules/light/MoonLiveModifier.md` around lines 49 - 55, Update the
Controls section of MoonLiveModifier documentation to include the user-declared
controls exposed by MoonLiveModifier::defineControls(). Describe how scripts
declare these controls and state that changing a declared control rebuilds the
mapping, alongside the existing source control behavior.

Source: Coding guidelines

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

151-174: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Use an observable compile count.

dynamicBytes() is engine_.codeCap(). Recompiling the same source can produce the same executable capacity, so Line 174 still passes if modifyLogical() compiles on every call. Add a compile-count test seam or an executable-allocation counter and assert that the count remains one across the fold.

🤖 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_MoonLiveModifier.cpp` around lines 151 - 174, Replace
the dynamicBytes() comparison in the MoonLiveModifier test with an observable
compilation count, exposed through an appropriate test seam or
executable-allocation counter. Capture the count after m.prepare(), assert it
reflects one compilation, and verify it remains unchanged after all
modifyLogical() calls; do not rely on executable capacity as the recompilation
indicator.

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 `@src/light/moonlive/MoonLiveBuiltins_light.h`:
- Around line 47-52: Update mm_light_print and its printBudget diagnostic flow
so effect ticks perform only bounded, nonallocating queue writes and remain
noexcept, removing both std::printf calls from the tick path. Store the
formatted diagnostic records in a preallocated queue, then drain that queue from
a housekeeping path through the existing platform output seam, preserving the
print budget and burst-spent message behavior.

In `@src/platform/desktop/moonlive_asm_host.cpp`:
- Around line 48-59: Complete signed-immediate lowering in
HostAssembler::mulImm() so negative multipliers, including -1, are materialized
with signed semantics; update src/platform/desktop/moonlive_asm_host.cpp lines
48-59, while HostAssembler::movImm() is only context. In
src/platform/esp32/moonlive_asm_xtensa.cpp lines 58-71, prevent values below
-2048 from falling through to unsigned construction by materializing the full
signed value or rejecting the literal during compilation. Add backend tests
covering subtraction and a literal below -2048.

In `@test/unit/light/unit_MoonLiveModifier.cpp`:
- Around line 224-259: Update the test case name to distinguish the intentional
out-of-grid mapping from the default-script assertion, then add CHECK(inside ==
0) after the loop in “a script that computes a position outside the grid leaves
lights mapped” to assert every transformed coordinate is unmapped while
preserving the existing defInside check.
- Around line 290-308: Update the subtraction regression test around “a
subtraction produces the whole value, not just its low byte” to run against a
multi-light buffer and use the subtraction result as a bounds-checked element
index. Assert that the expected destination element is written, ensuring values
such as 99 and an incorrectly widened result cannot both pass through setXYZ’s
byte truncation.

---

Outside diff comments:
In `@docs/moonmodules/light/MoonLiveModifier.md`:
- Around line 49-55: Update the Controls section of MoonLiveModifier
documentation to include the user-declared controls exposed by
MoonLiveModifier::defineControls(). Describe how scripts declare these controls
and state that changing a declared control rebuilds the mapping, alongside the
existing source control behavior.

In `@test/unit/light/unit_MoonLiveModifier.cpp`:
- Around line 151-174: Replace the dynamicBytes() comparison in the
MoonLiveModifier test with an observable compilation count, exposed through an
appropriate test seam or executable-allocation counter. Capture the count after
m.prepare(), assert it reflects one compilation, and verify it remains unchanged
after all modifyLogical() calls; do not rely on executable capacity as the
recompilation indicator.

🪄 Autofix

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: e555b367-e5cf-45b3-b0b5-f02211e7051e

📥 Commits

Reviewing files that changed from the base of the PR and between f99ee5e and 6b56dce.

📒 Files selected for processing (11)

• docs/MIGRATING.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/moonmodules/light/MoonLiveModifier.md
• src/light/moonlive/MoonLiveBuiltins_light.h
• src/light/moonlive/MoonLiveEffect.h
• src/light/moonlive/MoonLiveModifier.h
• src/platform/desktop/moonlive_asm_host.cpp
• src/platform/esp32/moonlive_asm_xtensa.cpp
• test/unit/light/unit_Effects_container.cpp
• test/unit/light/unit_MoonLiveModifier.cpp

[coderabbitai]

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

Remove blocking output from the effect tick path.

MoonLiveEffect::tick() can invoke mm_light_print() through engine_.run(). Line 50 calls std::printf, which can block the render tick. Store bounded diagnostic records in a preallocated queue and drain them from a housekeeping path through a platform seam.

As per path instructions: “render/tick code is nonblocking and noexcept; avoid allocation, blocking I/O, delays, or network calls there.”

🤖 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/moonlive/MoonLiveBuiltins_light.h` around lines 47 - 52, Update
mm_light_print and its printBudget diagnostic flow so effect ticks perform only
bounded, nonallocating queue writes and remain noexcept, removing both
std::printf calls from the tick path. Store the formatted diagnostic records in
a preallocated queue, then drain that queue from a housekeeping path through the
existing platform output seam, preserving the print budget and burst-spent
message behavior.

Source: Path instructions

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Test subtraction through an untruncated observer.

Each print() result is immediately stored through setXYZ(), which keeps only its low byte. Both 99 and 65635 store as 99, so these checks pass when -1 is materialized as 65535. Execute the script against a multi-light buffer and use the subtraction result as a bounds-checked element index, then assert that the expected destination was written.

🤖 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_MoonLiveModifier.cpp` around lines 290 - 308, Update the
subtraction regression test around “a subtraction produces the whole value, not
just its low byte” to run against a multi-light buffer and use the subtraction
result as a bounds-checked element index. Assert that the expected destination
element is written, ensuring values such as 99 and an incorrectly widened result
cannot both pass through setXYZ’s byte truncation.

[coderabbitai]

Actionable comments posted: 14

🤖 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/moonmodules/light/MoonLiveLayout.md`:
- Around line 52-62: Update the two-pass explanation near the claim that
identical arithmetic prevents drift so the guarantee explicitly applies only to
deterministic scripts. Acknowledge that supported stateful or random built-ins
such as random16 can produce different count and placement results, consistent
with the existing Limits warning.

In `@moondeck/moonlive/disasm.py`:
- Around line 58-69: Resolve Ruff E741 in the disassembly parsing by renaming
the ambiguous l variables to line in both comprehensions, including the hexbytes
construction and the objdump line search, without changing their behavior.
- Around line 42-46: Update the emitter build flow around subprocess.run to use
the project’s MoonDeck build entry point instead of invoking c++ directly.
Ensure the temporary emitter is built through the MoonDeck script or command so
project-configured compiler flags and dependencies are applied, while preserving
the existing emitter output behavior.

In `@moondeck/moonlive/emit_xtensa.cpp`:
- Around line 5-15: Remove the direct Xtensa implementation includes and
__XTENSA__ definition from the translation unit containing lowerToBytes. Expose
the required backend functionality through the platform-layer library/API under
src/platform/**, then have the MoonDeck utility depend on and link against that
API without compiling platform implementation files directly; preserve a single
backend definition.

In `@moonlive/effects/gradient.mlv`:
- Around line 3-5: Update InlineOp::StoreElem in moonlive_lower_riscv.cpp to
copy the index register into scratch before multiplying it for the byte offset,
preserving the original loop variable register for subsequent uses and the
for-step. Keep MoonLiveCompiler.cpp’s loop-variable register behavior and the
gradient example unchanged.

In `@moonlive/layouts/two-rows.mlv`:
- Around line 4-6: Update the light-emission loop in the two-row layout to
output all y=0 lights with ascending x coordinates first, then output all y=1
lights with descending x coordinates. Replace the current paired addLight calls
so the return row forms a reverse traversal.

In `@moonlive/README.md`:
- Around line 26-29: Remove the synchronous print path from render-executed
MoonLive scripts: update mm_light_print and its supporting code so rendering
only enqueues into a bounded, nonallocating noexcept queue drained outside the
render path, or disable the diagnostic in render builds. Ensure no std::printf
call remains reachable from render/tick code, and preserve the existing
burst-limit behavior where applicable.

In `@src/core/moonlive/MoonLiveCompiler.cpp`:
- Around line 417-433: Validate the condition and step identifiers in the
for-loop parsing flow against the active loop variable varName, rejecting
mismatches instead of silently using the loop local. Update the locals lookup
used by this flow to search from localCount - 1 down to zero so nested loop
variables shadow outer locals correctly; alternatively reject duplicate local
names.
- Around line 467-470: Update the loop-generation logic around the BranchGe and
BranchNe emissions to reject or safely bound steps that cannot advance toward
the limit, including a zero step such as i = i. Prefer validating that the
supported step is provably positive before emitting the back edge; otherwise
emit a bounded iteration guard so counter cannot loop indefinitely.

In `@src/platform/desktop/moonlive_asm_host.cpp`:
- Around line 112-120: Update the host-call frame and argument save/restore
logic in the surrounding assembly emission to preserve x3 alongside x0-x2, since
R3 carries elapsed time and may be clobbered by built-ins. Increase the stack
allocation and matching deallocation by a 16-byte-aligned amount, save x3 before
the call, and restore it afterward while keeping existing argument setup
unchanged.

In `@src/platform/esp32/moonlive_asm_riscv.cpp`:
- Around line 16-20: Remove the kScratchFn-to-x16 overlap by reserving a
register that is not included in kRvReg, and update call() to use that register
for the host-result stash. Ensure the selected scratch register does not
conflict with address calculation via kScratchAddr, and keep the result copy to
d outside the registers restored by call().

In `@src/platform/esp32/moonlive_lower_riscv.cpp`:
- Around line 64-69: Update the RISC-V call path used by the IrOp::Call case in
moonlive_lower_riscv.cpp and RiscvAssembler::call so the host-call return value
is saved in an unused stack slot before restoring saved[]/kScratchFn, then
reloaded into the destination afterward. Preserve existing argument passing and
restoration behavior while ensuring random16() and print() receive the actual a0
result.

In `@test/scenarios/light/scenario_MoonLive_pipeline.json`:
- Around line 13-52: Update the fixture to add an Effects module configured with
the existing Layouts module, parent Layer under Effects, and replace Drivers’
direct layer property with the effects reference. Preserve the existing module
IDs and driver setup while routing the scenario through Effects::setLayouts and
Drivers::setEffects.

In `@test/unit/light/unit_MoonLiveScripts.cpp`:
- Around line 70-73: Gate the native-code success assertions in the MoonLive
compile tests around the `engine.compile` checks, including the additional
affected cases, using the project’s supported-host/backend availability
condition. Preserve parser or script-validity coverage on unsupported hosts,
while only requiring `ok` and reporting compile failures when the native backend
is available.

🪄 Autofix

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: eb712dde-aa4e-452c-a474-21717ddbee0c

📥 Commits

Reviewing files that changed from the base of the PR and between 6b56dce and aa2e75f.

📒 Files selected for processing (40)

• docs/backlog/backlog-light.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/moonmodules/light/MoonLiveLayout.md
• docs/moonmodules/light/MoonLiveModifier.md
• moondeck/moonlive/disasm.py
• moondeck/moonlive/emit_xtensa.cpp
• moonlive/README.md
• moonlive/effects/gradient.mlv
• moonlive/effects/random-pixel.mlv
• moonlive/layouts/diagonal.mlv
• moonlive/layouts/grid.mlv
• moonlive/layouts/reversed-row.mlv
• moonlive/layouts/two-rows.mlv
• moonlive/modifiers/mirror.mlv
• moonlive/modifiers/shift.mlv
• moonlive/modifiers/transpose.mlv
• src/core/moonlive/MoonLiveBuiltins.h
• src/core/moonlive/MoonLiveCompiler.cpp
• src/core/moonlive/MoonLiveIr.h
• src/light/moonlive/MoonLiveBuiltins_light.h
• src/light/moonlive/MoonLiveEffect.h
• src/light/moonlive/MoonLiveLayout.h
• src/main.cpp
• src/platform/desktop/moonlive_asm_host.cpp
• src/platform/desktop/moonlive_asm_host.h
• src/platform/desktop/moonlive_lower_host.cpp
• src/platform/esp32/moonlive_asm_riscv.cpp
• src/platform/esp32/moonlive_asm_riscv.h
• src/platform/esp32/moonlive_asm_xtensa.cpp
• src/platform/esp32/moonlive_asm_xtensa.h
• src/platform/esp32/moonlive_lower_riscv.cpp
• src/platform/esp32/moonlive_lower_xtensa.cpp
• test/CMakeLists.txt
• test/scenario_runner.cpp
• test/scenarios/light/scenario_MoonLive_pipeline.json
• test/scenarios/light/scenario_peripheral_grid_sweep.json
• test/unit/light/unit_MoonLiveLayout.cpp
• test/unit/light/unit_MoonLiveModifier.cpp
• test/unit/light/unit_MoonLiveScripts.cpp

[coderabbitai]

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

Use the MoonDeck build entry point.

This direct c++ invocation bypasses the project build configuration. Build the temporary emitter through the MoonDeck build path so its compiler flags and dependencies match the project.

As per coding guidelines, “Use the project's MoonDeck scripts for build, flash, monitor, test, and check tasks rather than invoking alternate tooling directly.”

🧰 Tools

🪛 ast-grep (0.45.0)

[error] 41-46: Command coming from incoming request
Context: subprocess.run(
["c++", "-std=c++20", "-O0", "-I", os.path.join(ROOT, "src"),
"-I", os.path.join(ROOT, "src", "platform", "desktop"),
TOOL_SRC, os.path.join(ROOT, "src", "core", "moonlive", "MoonLiveCompiler.cpp"),
"-o", emitter],
capture_output=True, text=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

🪛 Ruff (0.16.1)

[error] 42-42: subprocess call: check for execution of untrusted input

(S603)

---

[warning] 42-42: subprocess.run without explicit check argument

Add explicit check=False

(PLW1510)

---

[error] 43-46: Starting a process with a partial executable path

(S607)

🤖 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/moonlive/disasm.py` around lines 42 - 46, Update the emitter build
flow around subprocess.run to use the project’s MoonDeck build entry point
instead of invoking c++ directly. Ensure the temporary emitter is built through
the MoonDeck script or command so project-configured compiler flags and
dependencies are applied, while preserving the existing emitter output behavior.

Source: Coding guidelines

[coderabbitai]

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Keep the Xtensa backend behind the platform layer.

moondeck/moonlive/emit_xtensa.cpp defines __XTENSA__ and directly includes platform implementation files. This moves platform-specific implementation ownership into a MoonDeck utility and can create duplicate definitions when normal backend sources are also linked. Expose a platform-layer library or API and link the utility to it.

As per path instructions, src/platform/** is the only place where platform-specific includes and hardware APIs are allowed.

🤖 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/moonlive/emit_xtensa.cpp` around lines 5 - 15, Remove the direct
Xtensa implementation includes and __XTENSA__ definition from the translation
unit containing lowerToBytes. Expose the required backend functionality through
the platform-layer library/API under src/platform/**, then have the MoonDeck
utility depend on and link against that API without compiling platform
implementation files directly; preserve a single backend definition.

Source: Path instructions

[coderabbitai]

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

Exercise the Effects container in this end-to-end scenario.

The fixture creates a standalone Layer, and Drivers uses its direct layer fallback. This bypasses Effects::setLayouts and Drivers::setEffects, which are part of this rename.

Add an Effects fixture with Layouts configured. Parent Layer under Effects. Configure Drivers with 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 `@test/scenarios/light/scenario_MoonLive_pipeline.json` around lines 13 - 52,
Update the fixture to add an Effects module configured with the existing Layouts
module, parent Layer under Effects, and replace Drivers’ direct layer property
with the effects reference. Preserve the existing module IDs and driver setup
while routing the scenario through Effects::setLayouts and Drivers::setEffects.

[coderabbitai]

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (3)

src/platform/esp32/moonlive_asm_xtensa.cpp (2)

174-180: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Stage argument A before overwriting a11.

a11 is a valid source register in kXtReg. If a maps to a11, Line 179 overwrites it before Line 180 reads it. A three-argument built-in then receives argument B as argument A.

Use a non-vreg scratch such as a13 to stage argument A before assigning a11.

Proposed fix

-    emit2(uint16_t((uint32_t(ar(c)) << 8) | (12 << 4) | 0xd));   // mov a12, argC
-    emit2(uint16_t((uint32_t(ar(b)) << 8) | (11 << 4) | 0xd));   // mov a11, argB
-    emit2(uint16_t((uint32_t(ar(a)) << 8) | (10 << 4) | 0xd));   // mov a10, argA
+    emit2(uint16_t((uint32_t(ar(a)) << 8) | (13 << 4) | 0xd));   // mov a13, argA
+    emit2(uint16_t((uint32_t(ar(c)) << 8) | (12 << 4) | 0xd));   // mov a12, argC
+    emit2(uint16_t((uint32_t(ar(b)) << 8) | (11 << 4) | 0xd));   // mov a11, argB
+    emit2(uint16_t((13u << 8) | (10 << 4) | 0xd));                // mov a10, a13

🤖 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/moonlive_asm_xtensa.cpp` around lines 174 - 180, Update
the three-argument emission sequence around the mov instructions to stage
argument A in the non-vreg scratch register a13 before writing a11, then assign
a10 from that staged value. Preserve the existing high-first ordering for
arguments C and B and ensure ar(a) is read before a11 can be overwritten.

---

167-172: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve a14 and a15 across the window rotation.

kXtReg maps virtual registers R10 and R11 to a14 and a15. callx8 rotates the window by 8, so these live values can be overwritten. Save both registers to the entry frame around callx8 and restore them with the other rotate-out values.

🤖 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/moonlive_asm_xtensa.cpp` around lines 167 - 172, Update
the callx8 save/restore sequence around the rotate-out scratch handling to
preserve a14 and a15, which hold virtual registers R10 and R11 via kXtReg. Store
both registers in unused entry-frame slots before callx8 and restore them
alongside a8, a9, and a11 afterward, without changing the existing a10
argument/result handling.

Source: Path instructions

src/core/moonlive/MoonLiveCompiler.cpp (1)

435-440: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Reject lexer errors while scanning the deferred step expression.

If the step contains an unsupported character, such as i = i / 1, Lexer::advance() leaves the lexer at the same character. This loop then never reaches Tok::End or Tok::RParen. Compilation can block the caller indefinitely.

Proposed fix

         int depth = 0;
         while (!failed && lex.kind != Tok::End) {
+            if (lex.kind == Tok::Error) {
+                fail(lex.err);
+                return false;
+            }
             if (lex.kind == Tok::LParen) depth++;
             else if (lex.kind == Tok::RParen) { if (depth == 0) break; depth--; }
             lex.advance();
         }

🤖 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/moonlive/MoonLiveCompiler.cpp` around lines 435 - 440, Update the
deferred step-expression scanning loop in MoonLiveCompiler to detect lexer
errors or lack of advancement after lex.advance(). When the lexer cannot
advance, reject the expression and propagate the existing compilation failure
path instead of continuing to loop; preserve normal stopping at Tok::End and the
matching Tok::RParen.

🤖 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/moonmodules/light/MoonLiveLayout.md`:
- Line 54: Update the random16 explanation near the two-pass addLight
description: state that counts differ only when random output changes loop
bounds or the number of addLight calls, while random coordinate arguments can
produce different placements between passes without changing the count.

In `@src/platform/esp32/moonlive_asm_riscv.cpp`:
- Around line 16-17: Update the register mapping around kRvReg so host
callee-saved registers x18–x21 are preserved across JIT entry and exit:
implement matching save/restore logic in the registered-function prologue() and
epilogue(), including stack handling, or remove the corresponding R14–R17
mappings from kRvReg. Ensure the existing lowerToBytes() call preservation
remains consistent.

In `@test/unit/light/unit_MoonLiveLayout.cpp`:
- Around line 270-284: Extend the regression test for loop-counter preservation
to exercise StoreElem lowering through an effect or engine buffer, using a
six-iteration loop that calls setRGB with the loop index and asserting all six
indexed writes occur. Keep the existing addLight call-based test separately to
retain Call register-preservation coverage, and align the buffer setup and
assertions with the relevant docs/moonmodules specification.

---

Outside diff comments:
In `@src/core/moonlive/MoonLiveCompiler.cpp`:
- Around line 435-440: Update the deferred step-expression scanning loop in
MoonLiveCompiler to detect lexer errors or lack of advancement after
lex.advance(). When the lexer cannot advance, reject the expression and
propagate the existing compilation failure path instead of continuing to loop;
preserve normal stopping at Tok::End and the matching Tok::RParen.

In `@src/platform/esp32/moonlive_asm_xtensa.cpp`:
- Around line 174-180: Update the three-argument emission sequence around the
mov instructions to stage argument A in the non-vreg scratch register a13 before
writing a11, then assign a10 from that staged value. Preserve the existing
high-first ordering for arguments C and B and ensure ar(a) is read before a11
can be overwritten.
- Around line 167-172: Update the callx8 save/restore sequence around the
rotate-out scratch handling to preserve a14 and a15, which hold virtual
registers R10 and R11 via kXtReg. Store both registers in unused entry-frame
slots before callx8 and restore them alongside a8, a9, and a11 afterward,
without changing the existing a10 argument/result handling.

🪄 Autofix

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: d264f38f-11a3-4be8-9bfb-470edcf04c29

📥 Commits

Reviewing files that changed from the base of the PR and between aa2e75f and c28015b.

📒 Files selected for processing (18)

• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/moonmodules/light/MoonLiveLayout.md
• moondeck/moonlive/disasm.py
• moondeck/moonlive/emit_xtensa.cpp
• moonlive/layouts/two-rows.mlv
• src/core/moonlive/MoonLiveCompiler.cpp
• src/core/moonlive/MoonLiveCompiler.h
• src/core/moonlive/MoonLiveIr.h
• src/platform/desktop/moonlive_asm_host.cpp
• src/platform/desktop/moonlive_lower_host.cpp
• src/platform/esp32/moonlive_asm_riscv.cpp
• src/platform/esp32/moonlive_asm_xtensa.cpp
• src/platform/esp32/moonlive_lower_riscv.cpp
• src/platform/esp32/moonlive_lower_xtensa.cpp
• test/scenarios/light/scenario_MoonLive_pipeline.json
• test/unit/light/unit_MoonLiveLayout.cpp
• test/unit/light/unit_MoonLiveScripts.cpp

[coderabbitai]

Actionable comments posted: 14

Caution

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

⚠️ Outside diff range comments (2)

src/core/moonlive/MoonLiveCompiler.cpp (1)

299-302: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Update the stale comment above the arity check.

The comment states that the IR Call op carries a single argument vreg and that a Call builtin must be unary. The code now passes three argument vregs and rejects only argc > 3. Align the comment with the three-argument contract.

♻️ Proposed comment update

-        // The IR Call op carries a single argument vreg, so a Call-kind builtin must be unary.
-        // (Today random16 is the only one.) Reject a multi-arg Call up front rather than silently
-        // dropping args[1..]; a future N-ary helper needs the IR Call contract widened first.
+        // The IR Call op carries three argument vregs, so a Call-kind builtin takes at most three
+        // arguments. Reject a wider Call up front rather than silently dropping args[3..]; a
+        // future N-ary helper needs the IR Call contract widened first.
         if (fn->kind == BuiltinKind::Call && fn->argc > 3) { fail("a call takes at most three arguments"); return; }

🤖 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/moonlive/MoonLiveCompiler.cpp` around lines 299 - 302, Update the
comment above the arity check in the MoonLiveCompiler call-handling logic to
describe the IR Call op’s three-argument-vreg contract and state that Call-kind
builtins support at most three arguments. Keep the existing argc > 3 validation
and failure behavior unchanged.

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

130-143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Stale frame documentation in RiscvAssembler::call. The call frame grew from 64 to 80 bytes and gained argument staging slots, but the surrounding comments still describe the old layout. A reader checking offsets against the comments gets the wrong numbers.

• src/platform/esp32/moonlive_asm_riscv.cpp#L130-L143: change "64-byte frame, 16-byte aligned" to 80-byte, and derive the saved set from kRvReg so the list cannot drift from the register map.
• src/platform/esp32/moonlive_asm_riscv.cpp#L166-L170: change the trailing comments from lw ra, 108(sp) and addi sp, sp, 112 to 76 and 80, matching the emitted encodings.

🤖 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/moonlive_asm_riscv.cpp` around lines 130 - 143, The
RiscvAssembler::call frame documentation and saved-register handling are stale:
in src/platform/esp32/moonlive_asm_riscv.cpp lines 130-143, update the header to
describe the 80-byte frame and derive the saved set from kRvReg so it cannot
drift; in lines 166-170, correct the trailing offset comments to ra at 76(sp)
and stack restoration by 80 bytes.

🤖 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-20260809` - MoonLive scales — right-sized IR, and the
stack as the register overflow.md:
- Around line 9-14: Update all repository-source Markdown links in this plan,
including the affected ranges, to prefix paths with ../../../ so links resolve
from docs/history/plans/ to the repository root; preserve the existing targets
and link text.
- Around line 220-232: Correct the ordered-list numbering in the verification
steps: keep the first item 6, then renumber the subsequent entries sequentially
from 7 through 10 without changing their content.
- Around line 50-55: Keep platform allocation and executable-memory placement
out of core: update the plan at docs/history/plans/Plan-20260809 - MoonLive
scales — right-sized IR, and the stack as the register overflow.md:50-55 to
specify a core-neutral compile/code-placement interface, and update
src/core/moonlive/MoonLive.cpp:61-76 to inject that interface into MoonLive.
Implement the concrete allocation and executable-code placement behind the
interface under src/platform/**, leaving src/core/** platform-independent.

In `@moonlive/README.md`:
- Around line 22-25: Update the README description of the source control’s
capacity to state that it has a fixed 4 KB limit, with the effective script
limit determined by whichever is reached first: the 4 KB source capacity or
available device memory. Remove the claim that script size is bounded only by
free memory or has no fixed budget.

In `@src/core/moonlive/MoonLive.cpp`:
- Around line 61-76: Update the staging allocation in the compile flow around
srcLen, stagingCap, and compileSource so a valid script is not rejected when the
initial estimate is too small. Base capacity on parsed IR or detect output
overflow and retry compilation with a larger buffer before reporting “codegen
failed,” while preserving cleanup on every allocation and retry path.

In `@src/core/moonlive/MoonLiveIr.h`:
- Line 6: Remove the platform/platform.h dependency from IrProgram in
MoonLiveIr.h and make op-array allocation core-neutral. Update
IrProgram::reserve and its callers to receive an injected allocator interface or
caller-owned span, with the light/platform layer supplying allocation and
release; preserve the existing sizing and storage behavior without platform
references in src/core.
- Around line 122-128: Update IrProgram::reserve to reset vregsUsed alongside
count before allocating the new instruction buffer, ensuring reused programs
start with no stale virtual-register usage mark.

In `@src/light/moonlive/MoonLiveEffect.h`:
- Around line 105-110: Update the comment above MoonLiveEffect::source_ to
remove obsolete references to the 512-byte limit, truncation, and any backlog
for increasing capacity; document only the current 4096-byte buffer rationale
and stable-address requirement.

In `@src/platform/esp32/moonlive_asm_riscv.cpp`:
- Around line 166-170: Update the inline comments on the restore instructions in
the surrounding `emit32` sequence to match their actual immediates: document
`encLw(1, 2, 76)` as loading from 76(sp) and `encAddi(2, 2, 80)` as adding 80 to
sp. Do not change the instruction values or surrounding restore logic.

In `@src/platform/esp32/moonlive_asm_xtensa.h`:
- Around line 27-39: Remove the first duplicated constructor documentation
paragraph in the ESP32 Xtensa assembler declaration, preserving the second block
that also documents branches. Apply the same cleanup to the corresponding
assembler documentation in the desktop host and ESP32 RISC-V declarations.

In `@test/unit/light/probe_w.cpp`:
- Around line 14-26: Add doctest assertions to the light-count test around the
existing controls loop and repeated lightCount() calls: verify the initial
count, assert each result equals the expected width multiplied by height, and
assert a, b, and c are equal for every width. Retain the diagnostic output while
making incorrect or inconsistent counts fail the test.
- Line 4: Add the missing <initializer_list> standard header alongside the
existing includes in probe_w.cpp so the range-based initializer-list loop
compiles in sanitizer builds.

In `@test/unit/light/unit_MoonLiveLayout.cpp`:
- Around line 388-418: Gate the code-generation TEST_CASE named “a script is
limited by memory, not by a fixed op or code budget” with
MM_MOONLIVE_HAS_HOST_JIT, since both subcases require a host backend. Move the
existing `#endif` below this TEST_CASE or apply the same conditional pattern used
in unit_MoonLiveScripts.cpp, while preserving both subcases unchanged when JIT
support is available.

In `@test/unit/light/unit_MoonLiveScripts.cpp`:
- Around line 111-115: Update the valid-script assertion in the MoonLive test
around the case variable c so kCodegenFailed is accepted only when
MM_MOONLIVE_HAS_HOST_JIT is unavailable, matching the existing logic around
lines 77-81. Keep the expected-failure assertion unchanged and preserve
acceptance of successful valid scripts on all hosts.

---

Outside diff comments:
In `@src/core/moonlive/MoonLiveCompiler.cpp`:
- Around line 299-302: Update the comment above the arity check in the
MoonLiveCompiler call-handling logic to describe the IR Call op’s
three-argument-vreg contract and state that Call-kind builtins support at most
three arguments. Keep the existing argc > 3 validation and failure behavior
unchanged.

In `@src/platform/esp32/moonlive_asm_riscv.cpp`:
- Around line 130-143: The RiscvAssembler::call frame documentation and
saved-register handling are stale: in src/platform/esp32/moonlive_asm_riscv.cpp
lines 130-143, update the header to describe the 80-byte frame and derive the
saved set from kRvReg so it cannot drift; in lines 166-170, correct the trailing
offset comments to ra at 76(sp) and stack restoration by 80 bytes.

🪄 Autofix

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: a98fcdee-f1ae-4cd6-a4ac-71d1425795d7

📥 Commits

Reviewing files that changed from the base of the PR and between aa2e75f and 32026eb.

📒 Files selected for processing (33)

• docs/backlog/backlog-light.md
• docs/history/plans/Plan-20260809 - MoonLive scales — right-sized IR, and the stack as the register overflow.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/moonmodules/light/MoonLiveLayout.md
• moondeck/moonlive/disasm.py
• moondeck/moonlive/emit_xtensa.cpp
• moonlive/README.md
• moonlive/layouts/two-rows.mlv
• src/core/Scheduler.cpp
• src/core/moonlive/MoonLive.cpp
• src/core/moonlive/MoonLiveCompiler.cpp
• src/core/moonlive/MoonLiveCompiler.h
• src/core/moonlive/MoonLiveIr.h
• src/light/moonlive/MoonLiveEffect.h
• src/light/moonlive/MoonLiveLayout.h
• src/light/moonlive/MoonLiveModifier.h
• src/platform/desktop/moonlive_asm_host.cpp
• src/platform/desktop/moonlive_asm_host.h
• src/platform/desktop/moonlive_lower_host.cpp
• src/platform/esp32/moonlive_asm_riscv.cpp
• src/platform/esp32/moonlive_asm_riscv.h
• src/platform/esp32/moonlive_asm_xtensa.cpp
• src/platform/esp32/moonlive_asm_xtensa.h
• src/platform/esp32/moonlive_lower_riscv.cpp
• src/platform/esp32/moonlive_lower_xtensa.cpp
• test/CMakeLists.txt
• test/scenarios/light/scenario_MoonLiveEffect_livescript.json
• test/scenarios/light/scenario_MoonLive_pipeline.json
• test/unit/light/probe_w.cpp
• test/unit/light/unit_MoonLiveLayout.cpp
• test/unit/light/unit_MoonLiveModifier.cpp
• test/unit/light/unit_MoonLiveScripts.cpp

[coderabbitai]

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

Fix the source-file links.

These relative paths resolve below docs/history/plans/, not at the repository root. For example, src/core/moonlive/MoonLiveIr.h resolves to docs/history/plans/src/core/moonlive/MoonLiveIr.h.

Prefix repository-source links with ../../../, and update all affected links in this plan.

Also applies to: 31-46, 50-52, 67-70, 83-103, 114-126, 171-191

🤖 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-20260809` - MoonLive scales — right-sized IR, and the
stack as the register overflow.md around lines 9 - 14, Update all
repository-source Markdown links in this plan, including the affected ranges, to
prefix paths with ../../../ so links resolve from docs/history/plans/ to the
repository root; preserve the existing targets and link text.

[coderabbitai]

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Keep platform allocation and executable placement outside src/core.

The plan assigns platform::alloc to core code, and MoonLive::compile implements that design. This breaks the required core/platform boundary. It also leaves MoonLive.cpp coupled to platform allocation and executable-memory behavior.

• docs/history/plans/Plan-20260809 - MoonLive scales — right-sized IR, and the stack as the register overflow.md#L50-L55: change the design to use a core-neutral compile and code-placement interface.
• src/core/moonlive/MoonLive.cpp#L61-L76: inject that interface into MoonLive, and move allocation and executable-code placement to src/platform/**.

Based on learnings: src/core must remain platform-independent and executable-memory handling must move behind a core/platform boundary. As per path instructions: src/core/** must be platform-independent, and only src/platform/** may contain platform-specific APIs.

📍 Affects 2 files

• docs/history/plans/Plan-20260809 - MoonLive scales — right-sized IR, and the stack as the register overflow.md#L50-L55 (this comment)
• src/core/moonlive/MoonLive.cpp#L61-L76

🤖 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-20260809` - MoonLive scales — right-sized IR, and the
stack as the register overflow.md around lines 50 - 55, Keep platform allocation
and executable-memory placement out of core: update the plan at
docs/history/plans/Plan-20260809 - MoonLive scales — right-sized IR, and the
stack as the register overflow.md:50-55 to specify a core-neutral
compile/code-placement interface, and update
src/core/moonlive/MoonLive.cpp:61-76 to inject that interface into MoonLive.
Implement the concrete allocation and executable-code placement behind the
interface under src/platform/**, leaving src/core/** platform-independent.

Sources: Path instructions, Learnings

[coderabbitai]

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

Fix the ordered-list numbering.

Line 222 repeats item 6. Renumber the remaining verification steps from 7 through 10.

🧰 Tools

🪛 markdownlint-cli2 (0.23.2)

[warning] 222-222: Ordered list item prefix
Expected: 7; Actual: 6; Style: 1/2/3

(MD029, ol-prefix)

---

[warning] 226-226: Ordered list item prefix
Expected: 8; Actual: 7; Style: 1/2/3

(MD029, ol-prefix)

---

[warning] 229-229: Ordered list item prefix
Expected: 9; Actual: 8; Style: 1/2/3

(MD029, ol-prefix)

---

[warning] 232-232: Ordered list item prefix
Expected: 10; Actual: 9; Style: 1/2/3

(MD029, ol-prefix)

🤖 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-20260809` - MoonLive scales — right-sized IR, and the
stack as the register overflow.md around lines 220 - 232, Correct the
ordered-list numbering in the verification steps: keep the first item 6, then
renumber the subsequent entries sequentially from 7 through 10 without changing
their content.

Source: Linters/SAST tools

[coderabbitai]

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

Describe the fixed source limit.

The source control has a fixed 4096-byte buffer. State that the effective limit is the 4 KB source capacity or available device memory, whichever is reached first. Do not state that script size has no fixed budget.

As per coding guidelines, “Documentation must describe the system as it currently exists.”

🤖 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 `@moonlive/README.md` around lines 22 - 25, Update the README description of
the source control’s capacity to state that it has a fixed 4 KB limit, with the
effective script limit determined by whichever is reached first: the 4 KB source
capacity or available device memory. Remove the claim that script size is
bounded only by free memory or has no fixed budget.

Source: Coding guidelines

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Replace the fixed source-to-code allocation multiplier.

Line 71 allocates 256 + srcLen * 8 bytes. A call also emits argument materialization and the backend call-save sequence. Repeated short valid calls can exceed this capacity. The lowerer then returns zero and reports codegen failed.

Estimate code capacity from parsed IR, or distinguish output overflow and retry with a larger buffer. Do not reject a valid script because the initial staging estimate is too small.

🤖 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/moonlive/MoonLive.cpp` around lines 61 - 76, Update the staging
allocation in the compile flow around srcLen, stagingCap, and compileSource so a
valid script is not rejected when the initial estimate is too small. Base
capacity on parsed IR or detect output overflow and retry compilation with a
larger buffer before reporting “codegen failed,” while preserving cleanup on
every allocation and retry path.

[coderabbitai]

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Add the required standard header.

The range-based initializer-list loop requires <initializer_list>. The sanitizer builds fail before tests run.

Proposed fix

 `#include` <cstring>
+#include <initializer_list>

📝 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

	#include <cstring>
	`#include` <cstring>
	`#include` <initializer_list>

🤖 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/probe_w.cpp` at line 4, Add the missing <initializer_list>
standard header alongside the existing includes in probe_w.cpp so the
range-based initializer-list loop compiles in sanitizer builds.

Source: Pipeline failures

[coderabbitai]

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add assertions for the repeated width changes.

This test only writes diagnostic output. It passes when lightCount() returns incorrect or inconsistent values. Assert the initial count, each expected width * height count, and equality across the repeated queries.

As per path instructions, “Unit and integration tests using doctest. Verify tests cover edge cases and match the specifications.”

🧰 Tools

🪛 Cppcheck (2.21.0)

[error] 23-23: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

🪛 GitHub Actions: Test / 0_sanitizers (address).txt

[error] 18-18: GCC compilation failed: deducing from a brace-enclosed initializer list requires '#include <initializer_list>'. Add the missing header. The 'mm_tests' build failed with exit code 2.

🪛 GitHub Actions: Test / 2_sanitizers (realtime).txt

[error] 18-18: C++ compilation failed: cannot deduce the initializer-list type because <initializer_list> was not included. Add #include <initializer_list>.

🪛 GitHub Actions: Test / 3_sanitizers (thread).txt

[error] 18-18: Compilation failed: deducing from a brace-enclosed initializer list requires '#include <initializer_list>'. Add the missing include before using the initializer-list range in the for loop. Command failed during 'cmake --build build/san --target mm_tests'.

🪛 GitHub Actions: Test / sanitizers (address)

[error] 18-18: GCC compilation failed in the mm_tests build: deducing from a brace-enclosed initializer list requires '#include <initializer_list>' for the range-based loop. Failed command: cmake --build build/san --target mm_tests.

🪛 GitHub Actions: Test / sanitizers (realtime)

[error] 18-18: Build of mm_tests failed: cannot deduce the initializer-list type because <initializer_list> was not included. Add #include <initializer_list> to probe_w.cpp.

🪛 GitHub Actions: Test / sanitizers (thread)

[error] 18-18: Compilation failed: deducing from a brace-enclosed initializer list requires '#include <initializer_list>' for the range-based loop. Add the missing include.

🤖 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/probe_w.cpp` around lines 14 - 26, Add doctest assertions to
the light-count test around the existing controls loop and repeated lightCount()
calls: verify the initial count, assert each result equals the expected width
multiplied by height, and assert a, b, and c are equal for every width. Retain
the diagnostic output while making incorrect or inconsistent counts fail the
test.

Source: Path instructions

[coderabbitai]

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (2)

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

172-181: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Stop branch patching after an emission overflow.

emit32() and emit() set overflow_ without advancing len_. A later branch records a fixup at that full buffer offset. finalize() then calls patchBranches() before lowerToBytes() checks overflowed(). Both implementations can read or write past buf_ during compilation of a valid script that exceeds the fixed code capacity.

• src/platform/esp32/moonlive_asm_riscv.cpp#L172-L181: Return before reading buf_ + f.at when overflow_ is set.
• src/platform/esp32/moonlive_asm_xtensa.cpp#L209-L214: Return before writing buf_[f.at + 2] when overflow_ is set.

Proposed fix

 void RiscvAssembler::patchBranches() {
+    if (overflow_) return;
     for (uint8_t i = 0; i < fixupCount_; i++) {

 void XtensaAssembler::patchBranches() {
+    if (overflow_) return;
     for (uint8_t i = 0; i < fixupCount_; i++) {

🤖 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/moonlive_asm_riscv.cpp` around lines 172 - 181, Stop
branch patching immediately when overflow_ is set: in patchBranches() within
src/platform/esp32/moonlive_asm_riscv.cpp at lines 172-181, return before
reading buf_ + f.at; likewise, in src/platform/esp32/moonlive_asm_xtensa.cpp at
lines 209-214, return before writing buf_[f.at + 2].

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

166-171: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Reject invalid and unbound branch labels.

Line 166 indexes labelPos_ before it validates f.label. An invalid label reads past the fixed table. An unbound label leaves the branch offset at zero and does not set overflow_. A taken conditional branch then targets itself, and lowerToBytes() can return executable code that loops forever.

Set overflow_ and stop patching when f.label >= kMaxLabels or when its target is unbound.

Proposed fix

     for (uint8_t i = 0; i < fixupCount_; i++) {
         const Fixup& f = fixups_[i];
+        if (f.label >= kMaxLabels) {
+            overflow_ = true;
+            return;
+        }
         int32_t target = labelPos_[f.label];
-        if (target < 0) continue;
+        if (target < 0) {
+            overflow_ = true;
+            return;
+        }

🤖 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/moonlive_asm_host.cpp` around lines 166 - 171, Update
the branch-patching loop around labelPos_ to validate f.label against kMaxLabels
before indexing the table, and set overflow_ then stop patching for invalid
labels. Also set overflow_ and stop patching when the validated label’s target
is unbound, rather than leaving the branch unchanged; preserve normal immediate
patching for valid bound labels.

🤖 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/MoonDeck.md`:
- Around line 959-964: Add the text language identifier to the fenced code block
containing the Guru Meditation monitor output in MoonDeck.md, changing the
opening fence to use text while preserving the output content unchanged.

In `@moondeck/run/monitor_esp32.py`:
- Around line 111-130: Store the firmware SHA comparison result from the SHA_RE
validation near the existing warning, and use it to prevent decoding when the
running SHA does not match elf_sha. Keep reporting the mismatch, but skip the
decode(tool, elf, addrs) call in the subsequent frame-processing loop until the
SHA is valid.
- Around line 71-80: Update the firmware setup block around find_addr2line and
elf_sha so OSError from Path(elf).read_bytes() is caught; on failure, reset
tool, elf, and elf_sha to None and print the existing raw-address fallback
message, allowing monitoring to continue to the serial-port setup.

---

Outside diff comments:
In `@src/platform/desktop/moonlive_asm_host.cpp`:
- Around line 166-171: Update the branch-patching loop around labelPos_ to
validate f.label against kMaxLabels before indexing the table, and set overflow_
then stop patching for invalid labels. Also set overflow_ and stop patching when
the validated label’s target is unbound, rather than leaving the branch
unchanged; preserve normal immediate patching for valid bound labels.

In `@src/platform/esp32/moonlive_asm_riscv.cpp`:
- Around line 172-181: Stop branch patching immediately when overflow_ is set:
in patchBranches() within src/platform/esp32/moonlive_asm_riscv.cpp at lines
172-181, return before reading buf_ + f.at; likewise, in
src/platform/esp32/moonlive_asm_xtensa.cpp at lines 209-214, return before
writing buf_[f.at + 2].

🪄 Autofix

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: 0d855154-cc2c-4a70-a59b-ba2bcf2b61cd

📥 Commits

Reviewing files that changed from the base of the PR and between 32026eb and 4806046.

📒 Files selected for processing (35)

• docs/architecture.md
• docs/backlog/backlog-light.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• moondeck/MoonDeck.md
• moondeck/moondeck_config.json
• moondeck/moonlive/disasm.py
• moondeck/run/monitor_esp32.py
• moonlive/README.md
• src/core/moonlive/MoonLive.h
• src/core/moonlive/MoonLiveCompiler.cpp
• src/core/moonlive/MoonLiveCompiler.h
• src/core/moonlive/MoonLiveIr.h
• src/light/layers/BlendMap.h
• src/light/layers/Layer.h
• src/light/moonlive/MoonLiveBuiltins_light.h
• src/light/moonlive/MoonLiveEffect.h
• src/light/moonlive/MoonLiveLayout.h
• src/light/moonlive/MoonLiveModifier.h
• src/platform/desktop/moonlive_asm_host.cpp
• src/platform/desktop/moonlive_asm_host.h
• src/platform/desktop/moonlive_lower_host.cpp
• src/platform/esp32/moonlive_asm_riscv.cpp
• src/platform/esp32/moonlive_asm_riscv.h
• src/platform/esp32/moonlive_asm_xtensa.cpp
• src/platform/esp32/moonlive_asm_xtensa.h
• src/platform/esp32/moonlive_lower_riscv.cpp
• src/platform/esp32/moonlive_lower_xtensa.cpp
• test/CMakeLists.txt
• test/scenarios/light/scenario_MoonLiveEffect_livescript.json
• test/scenarios/light/scenario_MoonLive_pipeline.json
• test/unit/light/unit_BlendMap.cpp
• test/unit/light/unit_MoonLiveLayout.cpp
• test/unit/light/unit_MoonLiveModifier.cpp
• test/unit/light/unit_MoonLiveScripts.cpp

💤 Files with no reviewable changes (6)

• test/CMakeLists.txt
• src/core/moonlive/MoonLiveCompiler.h
• moonlive/README.md
• docs/backlog/backlog-light.md
• moondeck/moonlive/disasm.py
• src/core/moonlive/MoonLiveCompiler.cpp

[coderabbitai]

Actionable comments posted: 8

Caution

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

⚠️ Outside diff range comments (5)

docs/MIGRATING.md (1)

55-55: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Update the manual preset migration.

The documented file edit changes "Layers" to "Effects" but leaves the persisted "role": "layer" value unchanged. That preset still loads without a pad tint, as Line 53 states. Rename the role to "effects" in the manual migration, or instruct users to re-save the preset after the edit.

As per coding guidelines, "**/*.md: Documentation must describe the system as it currently exists."

🤖 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/MIGRATING.md` at line 55, Update the manual preset migration
instructions near the Layers-to-Effects rename to also change the persisted role
value from "layer" to "effects" in each preset JSON, ensuring migrated presets
load with the correct pad tint.

Source: Coding guidelines

docs/moonmodules/light/MoonLiveModifier.md (1)

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

Document for loops as supported syntax. MoonLiveCompiler.cpp now parses braced for loops. A modifier still runs once per coordinate, but its script can contain a loop.

• docs/moonmodules/light/MoonLiveModifier.md#L11-L11: replace “It does not loop” with wording that distinguishes per-coordinate invocation from supported script syntax.
• docs/moonmodules/light/MoonLiveModifier.md#L39-L39: remove for from the unsupported grammar list.

As per coding guidelines, “Documentation must describe the system as it currently exists.”

🤖 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/moonmodules/light/MoonLiveModifier.md` at line 11, Update
docs/moonmodules/light/MoonLiveModifier.md:11 to clarify that the layer invokes
the modifier once per physical light/coordinate while the modifier script itself
may contain supported braced for loops. At
docs/moonmodules/light/MoonLiveModifier.md:39, remove for from the unsupported
grammar list; no other syntax documentation needs changing.

Source: Coding guidelines

src/platform/desktop/moonlive_asm_host.h (1)

59-64: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the host-call documentation to match the three-argument API.

HostAssembler::call now accepts three source registers, but the comment still says d = fn(a). Document d = fn(a, b, c) so the backend contract matches the declaration.

Proposed fix

-    // Call a host built-in: d = fn(a). Preserves the host-arg registers
+    // Call a host built-in: d = fn(a, b, c). Preserves the host-arg registers

🤖 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/moonlive_asm_host.h` around lines 59 - 64, Update the
documentation for HostAssembler::call to describe the three-argument operation
as d = fn(a, b, c), while preserving the existing register-preservation and
live-vreg contract details.

src/core/moonlive/MoonLiveIr.h (1)

87-92: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the branch-label limit wording.

kIrLabels = 16 allocates new label IDs as each loop is parsed; a sequential script can use more than eight for constructs if the compiler stopped there. Reserve two labels using localCount for the actual nesting depth, or change kIrLabels to match the intended per-program label capacity.

🤖 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/moonlive/MoonLiveIr.h` around lines 87 - 92, Correct the
branch-label capacity around kIrLabels and its documentation: account for two
labels per for construct when determining supported nesting depth, using
localCount if that is the intended limit, or set kIrLabels to the actual
per-program label capacity. Ensure the documented limit matches the compiler and
assembler behavior.

src/light/moonlive/MoonLiveBuiltins_light.h (1)

24-27: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Remove mutable process-global state from concurrent built-ins.

Layout scripts can run on more than one thread. mm_light_random16() writes s without synchronization. printBudget() is also decremented and reset without synchronization. These operations cause C++ data races and undefined behavior.

• src/light/moonlive/MoonLiveBuiltins_light.h#L24-L27: make the PRNG state execution-local, such as thread_local, or pass it through a binding-owned context.
• src/light/moonlive/MoonLiveBuiltins_light.h#L111-L124: store diagnostic budget in binding-owned state and synchronize its reset and consumption. Do not share the counter across script executions.

🤖 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/moonlive/MoonLiveBuiltins_light.h` around lines 24 - 27, Remove
mutable process-global state from both concurrent built-ins: update
mm_light_random16() to use execution-local PRNG state, such as thread_local or
binding-owned context, and update printBudget() to use binding-owned diagnostic
state with synchronized reset and consumption; apply the changes at
src/light/moonlive/MoonLiveBuiltins_light.h lines 24-27 and 111-124.

♻️ Duplicate comments (1)

moondeck/moonlive/emit_xtensa.cpp (1)

1-20: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Keep the Xtensa backend in the platform layer.

This translation unit defines __XTENSA__ and includes platform assembler and lowering implementations directly. That violates the platform-layer boundary and can create duplicate backend definitions when normal platform sources are linked. Expose the required lowering API from the platform target and link this utility against it.

🤖 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/moonlive/emit_xtensa.cpp` around lines 1 - 20, Remove the direct
__XTENSA__ definition and included platform implementation files from the
utility translation unit. Expose the required lowerToBytes API through the
platform target, then link this utility against that target while retaining only
the necessary public declarations and IR includes.

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 `@docs/moonmodules/light/MoonLiveEffect.md`:
- Around line 37-38: Update the system-variable documentation for width, height,
depth, and x, y, z to explicitly state their valid 0..255 range, reflecting the
uint8_t storage and saturation/bypass behavior. Do not present these fields as
unrestricted logical-grid dimensions or light coordinates.
- Around line 60-61: Update the builtin range descriptions in the table for
scale(value, n), sin(angle), and cos(angle) to match their implementation:
document scale as 0..n-1 and the trigonometric results as 1..65535. Leave the
surrounding descriptions unchanged.

In `@docs/moonmodules/light/MoonLiveLayout.md`:
- Around line 14-22: Use valid triple-backtick Markdown fences for both code
examples in docs/moonmodules/light/MoonLiveLayout.md: update the opening and
closing delimiters at lines 14-22 and 33-49, preserving each example’s content.

In `@moondeck/moonlive/emit_xtensa.cpp`:
- Around line 28-29: Update the compileSource invocation in the layout sample to
pass layoutSysVars() instead of modifierSysVars(), while keeping lightBuiltins()
and the existing source, buffer, and size arguments unchanged.

In `@moondeck/run/monitor_esp32.py`:
- Around line 125-131: Update the final serial cleanup around the reconnect flow
in monitor_esp32 so it checks whether ser is not None before calling
ser.close(). Preserve the existing cleanup behavior for an active serial
connection while avoiding AttributeError when interruption occurs during the
ser-is-None reconnect loop.

In `@moonlive/modifiers/shift.mlv`:
- Around line 1-5: Update the shift modifier’s setXYZ call so x + amount cannot
truncate to an in-range uint8_t value; preserve out-of-range coordinates through
the modifier runtime or use its established invalid-coordinate representation,
allowing Layer to drop positions beyond the box instead of wrapping them.

In `@src/core/moonlive/MoonLiveBuiltins.h`:
- Around line 144-147: Update SysVar::add to validate offsets before storing a
variable: accept Arena entries only when their offset is at least kMaxCtrls and
below kArenaBytes, and accept Arg entries only when their offset corresponds to
a defined argument VReg. Return false for invalid values while preserving the
existing capacity and null-name checks.

In `@test/unit/light/unit_MoonLiveScripts.cpp`:
- Around line 116-120: Require successful code generation on supported JIT hosts
by guarding the valid-script assertions with MM_MOONLIVE_HAS_HOST_JIT: in
test/unit/light/unit_MoonLiveScripts.cpp lines 116-120, require r.ok instead of
accepting moonlive::kCodegenFailed; in test/unit/core/unit_moonlive_compiler.cpp
lines 227-233, require ok.ok; and in lines 248-257, require both ok.ok and
seq.ok. Preserve the existing fallback acceptance for hosts without JIT support.

---

Outside diff comments:
In `@docs/MIGRATING.md`:
- Line 55: Update the manual preset migration instructions near the
Layers-to-Effects rename to also change the persisted role value from "layer" to
"effects" in each preset JSON, ensuring migrated presets load with the correct
pad tint.

In `@docs/moonmodules/light/MoonLiveModifier.md`:
- Line 11: Update docs/moonmodules/light/MoonLiveModifier.md:11 to clarify that
the layer invokes the modifier once per physical light/coordinate while the
modifier script itself may contain supported braced for loops. At
docs/moonmodules/light/MoonLiveModifier.md:39, remove for from the unsupported
grammar list; no other syntax documentation needs changing.

In `@src/core/moonlive/MoonLiveIr.h`:
- Around line 87-92: Correct the branch-label capacity around kIrLabels and its
documentation: account for two labels per for construct when determining
supported nesting depth, using localCount if that is the intended limit, or set
kIrLabels to the actual per-program label capacity. Ensure the documented limit
matches the compiler and assembler behavior.

In `@src/light/moonlive/MoonLiveBuiltins_light.h`:
- Around line 24-27: Remove mutable process-global state from both concurrent
built-ins: update mm_light_random16() to use execution-local PRNG state, such as
thread_local or binding-owned context, and update printBudget() to use
binding-owned diagnostic state with synchronized reset and consumption; apply
the changes at src/light/moonlive/MoonLiveBuiltins_light.h lines 24-27 and
111-124.

In `@src/platform/desktop/moonlive_asm_host.h`:
- Around line 59-64: Update the documentation for HostAssembler::call to
describe the three-argument operation as d = fn(a, b, c), while preserving the
existing register-preservation and live-vreg contract details.

---

Duplicate comments:
In `@moondeck/moonlive/emit_xtensa.cpp`:
- Around line 1-20: Remove the direct __XTENSA__ definition and included
platform implementation files from the utility translation unit. Expose the
required lowerToBytes API through the platform target, then link this utility
against that target while retaining only the necessary public declarations and
IR includes.

🪄 Autofix

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: e23e433f-42f4-4962-aea4-ee3714d935bf

📥 Commits

Reviewing files that changed from the base of the PR and between 4806046 and 9e5169a.

⛔ Files ignored due to path filters (1)

• moondeck/build/erase_flash_esp32.py is excluded by !**/build/**

📒 Files selected for processing (39)

• docs/MIGRATING.md
• docs/backlog/backlog-light.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/moonmodules/light/MoonLiveEffect.md
• docs/moonmodules/light/MoonLiveLayout.md
• docs/moonmodules/light/MoonLiveModifier.md
• moondeck/MoonDeck.md
• moondeck/moonlive/disasm.py
• moondeck/moonlive/emit_xtensa.cpp
• moondeck/run/monitor_esp32.py
• moonlive/effects/lines.mlv
• moonlive/layouts/grid.mlv
• moonlive/layouts/reversed-row.mlv
• moonlive/layouts/ring.mlv
• moonlive/layouts/two-rows.mlv
• moonlive/modifiers/mirror.mlv
• moonlive/modifiers/shift.mlv
• src/core/math16.h
• src/core/moonlive/MoonLive.cpp
• src/core/moonlive/MoonLive.h
• src/core/moonlive/MoonLiveBuiltins.h
• src/core/moonlive/MoonLiveCompiler.cpp
• src/core/moonlive/MoonLiveCompiler.h
• src/core/moonlive/MoonLiveIr.h
• src/light/moonlive/MoonLiveBuiltins_light.h
• src/light/moonlive/MoonLiveEffect.h
• src/light/moonlive/MoonLiveLayout.h
• src/light/moonlive/MoonLiveModifier.h
• src/platform/desktop/moonlive_asm_host.h
• src/platform/esp32/moonlive_asm_riscv.h
• src/platform/esp32/moonlive_asm_xtensa.h
• test/scenarios/light/scenario_MoonLive_pipeline.json
• test/unit/core/unit_math16.cpp
• test/unit/core/unit_moonlive_compiler.cpp
• test/unit/core/unit_moonlive_fill.cpp
• test/unit/core/unit_moonlive_ir.cpp
• test/unit/light/unit_MoonLiveLayout.cpp
• test/unit/light/unit_MoonLiveScripts.cpp

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Preserve out-of-range coordinates instead of wrapping them.

For a 255-wide box, x + amount can exceed 255. setXYZ stores the low byte. For example, x = 254 and amount = 4 becomes 2. The Layer then receives an in-range coordinate and cannot drop it.

Preserve overflow state through the modifier runtime, or define a non-wrapping invalid-coordinate representation.

🤖 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 `@moonlive/modifiers/shift.mlv` around lines 1 - 5, Update the shift modifier’s
setXYZ call so x + amount cannot truncate to an in-range uint8_t value; preserve
out-of-range coordinates through the modifier runtime or use its established
invalid-coordinate representation, allowing Layer to drop positions beyond the
box instead of wrapping them.

[coderabbitai]

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (5)

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

291-293: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Do not defer synchronous print() on the render path.

Lines 291-293 confirm that print(v) writes directly to serial while an effect runs in tick(). The burst cap limits volume, but it does not make blocking I/O nonblocking. Queue preallocated records and drain them during housekeeping before release.

As per path instructions, render/tick code is nonblocking and must not perform blocking I/O.

🤖 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-light.md` around lines 291 - 293, Update MoonLive’s
synchronous print path so EFFECT execution in tick() never writes directly to
serial or performs blocking I/O. Queue print records using preallocated storage,
then drain the queue from the existing housekeeping/platform-output path before
release while preserving the current burst budget and burst-spent behavior.

Source: Path instructions

docs/moonmodules/light/MoonLiveModifier.md (1)

13-18: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the Markdown fence around the setXYZ examples.

Line 13 uses ``c and Line 18 uses two backticks. Markdown does not render this example as a fenced C block. Use a three-backtick opener and closer.

Proposed fix

-``c
+```c
...
-``
+```

🤖 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/moonmodules/light/MoonLiveModifier.md` around lines 13 - 18, Correct the
Markdown code fence surrounding the setXYZ examples: update the opening and
closing delimiters to use three backticks, preserving the C language annotation
and all example lines unchanged.

docs/moonmodules/light/MoonLiveLayout.md (1)

55-55: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Make the documented layout clock match the runtime.

Line 55 says a layout reads elapsed milliseconds through t. The runScript method in src/light/moonlive/MoonLiveLayout.h currently calls engine_.run(..., 0), so layout scripts receive t == 0 for both passes. Either pass one captured elapsed value to both count and placement passes, or document t as fixed at zero. Otherwise, layouts that use t cannot animate as documented.

As per path instructions, documentation must describe the system as it currently exists.

🤖 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/moonmodules/light/MoonLiveLayout.md` at line 55, The documentation
claims MoonLive layouts receive elapsed milliseconds via t, but runScript
currently supplies 0. Update the MoonLiveLayout documentation to describe t as
fixed at zero, unless the runtime is changed to capture and pass one elapsed
value to both runScript count and placement passes; ensure the documented
behavior matches the implemented system.

Source: Path instructions

src/core/moonlive/MoonLiveCompiler.cpp (2)

457-494: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject trailing tokens in the loop step.

parseExpr() accepts a valid prefix and the parser restores the outer lexer without checking that the next token is Tok::RParen. A source such as for (i = 0; i < 2; i = i + 1 junk) { ... } compiles as if junk were absent.

After parsing stepSrc, require the next token to be the closing parenthesis before emitting IrOp::Mov.

🤖 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/moonlive/MoonLiveCompiler.cpp` around lines 457 - 494, The re-lexed
loop-step parser accepts trailing tokens because it does not verify that parsing
ended at Tok::RParen. In the stepLex block of the for-loop compilation flow,
validate the next token after parseExpr() is Tok::RParen before emitting
IrOp::Mov; reject any other token and avoid emitting the step assignment.

---

479-498: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Preserve loop values across host calls.

A loop body can call a builtin such as addLight(i, 0, 0). The counter and limit registers remain live after that call and are used by Lines 497-498. HostAssembler::call states that caller-saved registers other than R0-R2 must not be live across a call. The loop counter and bound therefore can be clobbered before the branch executes.

Define a cross-call liveness contract for loop values. Spill and restore live loop registers, or make every backend preserve the required virtual registers across IrOp::Call.

🤖 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/moonlive/MoonLiveCompiler.cpp` around lines 479 - 498, Preserve the
loop’s live counter and limit values across builtin calls before the post-body
branches in the for-loop compilation path. Update the loop handling around
parseStatement(), IrOp::Call generation, or HostAssembler::call to establish a
cross-call liveness contract, ensuring counter and limit are restored or
retained before the BranchGe and BranchNe 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 `@moondeck/event/_gates.py`:
- Line 145: Update the command construction in the event gate to use iterable
unpacking for UV instead of list concatenation, preserving the existing
build_desktop.py, --gcc, and --tests arguments while resolving Ruff RUF005.

---

Outside diff comments:
In `@docs/backlog/backlog-light.md`:
- Around line 291-293: Update MoonLive’s synchronous print path so EFFECT
execution in tick() never writes directly to serial or performs blocking I/O.
Queue print records using preallocated storage, then drain the queue from the
existing housekeeping/platform-output path before release while preserving the
current burst budget and burst-spent behavior.

In `@docs/moonmodules/light/MoonLiveLayout.md`:
- Line 55: The documentation claims MoonLive layouts receive elapsed
milliseconds via t, but runScript currently supplies 0. Update the
MoonLiveLayout documentation to describe t as fixed at zero, unless the runtime
is changed to capture and pass one elapsed value to both runScript count and
placement passes; ensure the documented behavior matches the implemented system.

In `@docs/moonmodules/light/MoonLiveModifier.md`:
- Around line 13-18: Correct the Markdown code fence surrounding the setXYZ
examples: update the opening and closing delimiters to use three backticks,
preserving the C language annotation and all example lines unchanged.

In `@src/core/moonlive/MoonLiveCompiler.cpp`:
- Around line 457-494: The re-lexed loop-step parser accepts trailing tokens
because it does not verify that parsing ended at Tok::RParen. In the stepLex
block of the for-loop compilation flow, validate the next token after
parseExpr() is Tok::RParen before emitting IrOp::Mov; reject any other token and
avoid emitting the step assignment.
- Around line 479-498: Preserve the loop’s live counter and limit values across
builtin calls before the post-body branches in the for-loop compilation path.
Update the loop handling around parseStatement(), IrOp::Call generation, or
HostAssembler::call to establish a cross-call liveness contract, ensuring
counter and limit are restored or retained before the BranchGe and BranchNe
instructions.

🪄 Autofix

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: f3f72725-afee-4259-b840-396454c22b2b

📥 Commits

Reviewing files that changed from the base of the PR and between 9e5169a and 746c4e1.

📒 Files selected for processing (19)

• docs/MIGRATING.md
• docs/backlog/backlog-light.md
• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/moonmodules/light/MoonLiveEffect.md
• docs/moonmodules/light/MoonLiveLayout.md
• docs/moonmodules/light/MoonLiveModifier.md
• moondeck/event/_gates.py
• moondeck/moonlive/emit_xtensa.cpp
• moondeck/run/monitor_esp32.py
• moonlive/modifiers/shift.mlv
• src/core/moonlive/MoonLiveBuiltins.h
• src/core/moonlive/MoonLiveCompiler.cpp
• src/core/moonlive/MoonLiveIr.h
• src/platform/desktop/moonlive_asm_host.h
• test/scenarios/light/scenario_GridBlacks_blackpixel.json
• test/unit/core/unit_FilesystemModule_persistence.cpp
• test/unit/core/unit_moonlive_compiler.cpp
• test/unit/light/unit_MoonLiveScripts.cpp

[coderabbitai]

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use iterable unpacking for UV.

Ruff RUF005 flags UV + [...] on Line 145. Replace it with [*UV, "moondeck/build/build_desktop.py", "--gcc", "--tests"] to avoid list concatenation and clear the lint warning.

🧰 Tools

🪛 Ruff (0.16.1)

[warning] 145-145: Consider iterable unpacking instead of concatenation

Replace with iterable unpacking

(RUF005)

🤖 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/event/_gates.py` at line 145, Update the command construction in the
event gate to use iterable unpacking for UV instead of list concatenation,
preserving the existing build_desktop.py, --gcc, and --tests arguments while
resolving Ruff RUF005.

Source: Linters/SAST tools

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (4)

src/light/moonlive/MoonLiveLayout.h (1)

110-120: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

Serialize compilation and execution of the shared engine.

lightCount() and forEachCoord() both call compile(). The runtime can invoke layout walks from the HTTP task and the render task concurrently. A first compile or source update then races on source_, compiled_, and engine_. One task can replace generated code while another task executes it.

Compile on a housekeeping path and publish an immutable program with a safe lifetime. Do not mutate engine_ from concurrent layout walks.

🤖 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/moonlive/MoonLiveLayout.h` around lines 110 - 120, Refactor
MoonLiveLayout::compile and the lightCount/forEachCoord execution path so layout
walks never concurrently mutate or execute the shared engine_. Perform
compilation during a serialized housekeeping/update path, then publish an
immutable compiled program with lifetime protection for readers; have concurrent
walks use that published program instead of source_, compiled_, or engine_
directly. Preserve status and dynamic-byte updates during compilation while
eliminating races on all shared compilation state.

src/core/moonlive/MoonLiveCompiler.cpp (1)

456-458: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

Bound loop work at runtime.

A loop bound can be dynamic. Four nested loops with controls set to 255 execute more than four billion body iterations, even when every step advances. A MoonLiveEffect runs this code from tick(), so a valid script can block rendering for an unbounded time.

Add a shared generated iteration budget that exits the script when exhausted. Apply it to every loop level.

🤖 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/moonlive/MoonLiveCompiler.cpp` around lines 456 - 458, In
MoonLiveCompiler’s loop code generation around the limit expression, add one
shared runtime iteration budget for the entire script execution, initialize it
per run, and decrement/check it on every iteration of every loop level. Exit the
script cleanly when the budget is exhausted, ensuring nested and dynamically
bounded loops cannot run unbounded work.

src/light/moonlive/MoonLiveBuiltins_light.h (2)

97-128: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Make the print budget per execution context.

printBudget() returns one process-global mutable counter. resetPrintBudget() writes it during compilation or preparation while mm_light_print() decrements it during execution. The supplied src/light/moonlive/MoonLiveLayout.h context allows those paths to run on different tasks. This creates a data race and allows one module to reset another module's budget. Store the budget with the module or run context, or use synchronized thread-local state with an explicit contract.

🤖 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/moonlive/MoonLiveBuiltins_light.h` around lines 97 - 128, Make the
print budget execution-context-local instead of process-global: update
printBudget(), resetPrintBudget(), and mm_light_print() to access the owning
module or MoonLiveLayout run context, or an explicitly synchronized thread-local
context. Ensure each context can reset and decrement only its own budget without
data races or affecting other modules.

---

25-29: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Do not share RNG state across concurrent script runs.

mm_light_random16 mutates one function-local static s without synchronization. The supplied src/light/moonlive/MoonLiveLayout.h context allows scripted layout runs from HTTP and render tasks, so simultaneous scripts can race on s. Keep RNG state per engine or run, or use documented thread-local state if cross-thread sequence sharing is not required.

🤖 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/moonlive/MoonLiveBuiltins_light.h` around lines 25 - 29, The
mm_light_random16 function currently shares mutable static RNG state across
concurrent script runs. Replace the function-local static s with RNG state
scoped to the engine or individual run, or use documented thread-local storage
when sequence sharing across threads is unnecessary, while preserving the
existing random16 calculation and zero-input behavior.

♻️ Duplicate comments (1)

src/light/moonlive/MoonLiveBuiltins_light.h (1)

97-128: 🩺 Stability & Availability | 🟠 Major

Remove synchronous output from script execution.

This remains unresolved from the previous review. mm_light_print calls std::printf, and MoonLiveEffect::tick() can execute it through engine_.run(). The call can block the render path and violates the nonblocking hot-path contract. Queue bounded diagnostic records and drain them from a housekeeping path.

As per path instructions: render/tick code must be nonblocking and must not allocate, block, or perform slow I/O.

🤖 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/moonlive/MoonLiveBuiltins_light.h` around lines 97 - 128, Replace
synchronous std::printf calls in mm_light_print with bounded, nonblocking
diagnostic record queuing, preserving the print budget and returned value. Add
or reuse a housekeeping-path drain to perform serial output outside
MoonLiveEffect::tick() and engine_.run(), ensuring the render path performs no
allocation, blocking, or slow I/O.

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 `@docs/moonmodules/light/MoonLiveEffect.md`:
- Line 80: Update the nearby MoonLive and MoonLiveEffect API bullets to document
compile(source, table, sysvars) and state that bindings provide effectSysVars(),
modifierSysVars(), or layoutSysVars(). Keep the new system-variable section
consistent with this current API description.

In `@test/unit/core/unit_moonlive_fill.cpp`:
- Around line 150-168: Add a host-JIT test alongside “a loop counter survives a
call in the body” that passes the supplied t value into random16, then uses t in
a later setRGB argument after the generated call. Run the program with a known t
value and assert the output contains that value, ensuring t survives across the
call.

---

Outside diff comments:
In `@src/core/moonlive/MoonLiveCompiler.cpp`:
- Around line 456-458: In MoonLiveCompiler’s loop code generation around the
limit expression, add one shared runtime iteration budget for the entire script
execution, initialize it per run, and decrement/check it on every iteration of
every loop level. Exit the script cleanly when the budget is exhausted, ensuring
nested and dynamically bounded loops cannot run unbounded work.

In `@src/light/moonlive/MoonLiveBuiltins_light.h`:
- Around line 97-128: Make the print budget execution-context-local instead of
process-global: update printBudget(), resetPrintBudget(), and mm_light_print()
to access the owning module or MoonLiveLayout run context, or an explicitly
synchronized thread-local context. Ensure each context can reset and decrement
only its own budget without data races or affecting other modules.
- Around line 25-29: The mm_light_random16 function currently shares mutable
static RNG state across concurrent script runs. Replace the function-local
static s with RNG state scoped to the engine or individual run, or use
documented thread-local storage when sequence sharing across threads is
unnecessary, while preserving the existing random16 calculation and zero-input
behavior.

In `@src/light/moonlive/MoonLiveLayout.h`:
- Around line 110-120: Refactor MoonLiveLayout::compile and the
lightCount/forEachCoord execution path so layout walks never concurrently mutate
or execute the shared engine_. Perform compilation during a serialized
housekeeping/update path, then publish an immutable compiled program with
lifetime protection for readers; have concurrent walks use that published
program instead of source_, compiled_, or engine_ directly. Preserve status and
dynamic-byte updates during compilation while eliminating races on all shared
compilation state.

---

Duplicate comments:
In `@src/light/moonlive/MoonLiveBuiltins_light.h`:
- Around line 97-128: Replace synchronous std::printf calls in mm_light_print
with bounded, nonblocking diagnostic record queuing, preserving the print budget
and returned value. Add or reuse a housekeeping-path drain to perform serial
output outside MoonLiveEffect::tick() and engine_.run(), ensuring the render
path performs no allocation, blocking, or slow I/O.

🪄 Autofix

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: c87af99a-3632-4e6a-b8d0-8adea569b6b6

📥 Commits

Reviewing files that changed from the base of the PR and between 746c4e1 and 41eb2c0.

📒 Files selected for processing (19)

• docs/metrics/repo-health.json
• docs/metrics/repo-health.md
• docs/moonmodules/light/MoonLiveEffect.md
• docs/moonmodules/light/MoonLiveLayout.md
• docs/moonmodules/light/MoonLiveModifier.md
• moondeck/check/check_specs.py
• moondeck/docs/screenshot_modules.py
• moondeck/docs/update_module_docs.py
• moondeck/scenario/run_live_scenario.py
• moonlive/README.md
• src/core/moonlive/MoonLiveCompiler.cpp
• src/light/drivers/Drivers.h
• src/light/moonlive/MoonLiveBuiltins_light.h
• src/light/moonlive/MoonLiveLayout.h
• src/platform/desktop/moonlive_asm_host.cpp
• src/platform/desktop/moonlive_asm_host.h
• test/unit/core/unit_moonlive_compiler.cpp
• test/unit/core/unit_moonlive_fill.cpp
• test/unit/light/unit_MoonLiveScripts.cpp

💤 Files with no reviewable changes (1)

• moonlive/README.md

[coderabbitai]

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

Update the API bullets for system variables.

The new system-variable section is correct, but the nearby MoonLive and MoonLiveEffect bullets still describe compile(source, table) and say that MoonLiveEffect passes only the builtin table. src/core/moonlive/MoonLive.h requires compile(source, table, sysvars), and the bindings pass effectSysVars(), modifierSysVars(), or layoutSysVars(). Update those bullets so this specification matches the current API.

As per coding guidelines: Documentation must describe the system as it currently exists.

🤖 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/moonmodules/light/MoonLiveEffect.md` at line 80, Update the nearby
MoonLive and MoonLiveEffect API bullets to document compile(source, table,
sysvars) and state that bindings provide effectSysVars(), modifierSysVars(), or
layoutSysVars(). Keep the new system-variable section consistent with this
current API description.

Source: Coding guidelines

[coderabbitai]

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Exercise t across the generated call.

This test keeps i and w live across random16, but it never reads t after that call. It cannot detect a regression in the x3-backed t preservation changed in src/platform/desktop/moonlive_asm_host.cpp. Add a case that passes t to random16, uses t in a later argument, and asserts that the output contains the supplied t.

🤖 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_moonlive_fill.cpp` around lines 150 - 168, Add a host-JIT
test alongside “a loop counter survives a call in the body” that passes the
supplied t value into random16, then uses t in a later setRGB argument after the
generated call. Run the program with a known t value and assert the output
contains that value, ensuring t survives across the call.