Make two effect IDs mean what their comments claim (#132) #133

Merged

claude-bot merged 1 commit from fix/132-effect-ids into main

2026-09-05 01:13:16 +00:00

claude-bot commented

2026-09-05 00:56:27 +00:00

Contributor

Closes #132.

Found while starting #14, by checking the codebase's hardcoded effect IDs against a real 16.0.0 controller's /json/eff instead of against the comments next to them.

The aurora scheme pushed the wrong animation — my bug

aurora.py set "fx": 44 under a comment I wrote in #20:

Effect 44 is Aurora in stock WLED, which is exactly the thing being depicted.

44 is Tetrix — falling blocks. Aurora is 38. The feature is env-gated and off by default so nobody has seen it, but anyone who set AURORA_ENABLED would have got a puzzle game during a geomagnetic storm. I asserted the number rather than reading it — the same mistake as leds.segs in #62 and the transition range in #52.

The AI's fallback catalogue had five of seven pairs wrong

Used whenever no controller is reachable, which is the path a new user hits before wiring hardware — and the default experience #63's demo mode will create.

Sent to the model	Actually is
`{id: 9, name: "Chase"}`	Rainbow
`{id: 65, name: "Colorful"}`	Palette
`{id: 66, name: "Traffic"}`	Fire 2012
`{id: 57, name: "Fire 2012"}`	Lightning
`{id: 108, name: "Twinkle"}`	Sine

The model picks an effect by name — "Twinkle suits a winter scene" — and gets the number beside it. So it asked for Sine and meant Twinkle. Replaced with ten verified pairs, chosen to span genuinely different looks so the model has something to choose between.

The guard is the point of the change

An effect ID is an opaque integer everywhere except the one place a human decides what it means. Nothing in the suite could ever have caught either bug — the check has to be against the device's own answer.

tests/fixtures/wled/16.0.0/eff.json is that answer, captured verbatim from the real controller back in #62. That fixture already existed; this is what makes it load-bearing rather than decorative.

I reintroduced both bugs to confirm the test fails with the right diagnosis before keeping the fixes:

assert 'Tetrix' == 'Aurora'
assert ["id 108 is labelled 'Twinkle' but is 'Sine'"] == []

schemes/builtin.yaml was audited at the same time and is clean — all eight IDs it uses (0, 2, 9, 21, 45, 57, 65, 66) are real effects and plausible for their schemes. A range check now keeps it that way, plus a check that the walker actually finds something, so it can't pass on an empty set.

Verification

10 new tests, ruff/mypy clean, 759 backend tests passing.

🤖 Generated with Claude Code

Closes #132. Found while starting #14, by checking the codebase's hardcoded effect IDs against a real 16.0.0 controller's `/json/eff` instead of against the comments next to them. ## The aurora scheme pushed the wrong animation — my bug `aurora.py` set `"fx": 44` under a comment I wrote in #20: > Effect 44 is Aurora in stock WLED, which is exactly the thing being depicted. **44 is `Tetrix`** — falling blocks. Aurora is **38**. The feature is env-gated and off by default so nobody has seen it, but anyone who set `AURORA_ENABLED` would have got a puzzle game during a geomagnetic storm. I asserted the number rather than reading it — the same mistake as `leds.segs` in #62 and the transition range in #52. ## The AI's fallback catalogue had five of seven pairs wrong Used whenever no controller is reachable, which is the path a new user hits **before wiring hardware** — and the default experience #63's demo mode will create. | Sent to the model | Actually is | |---|---| | `{id: 9, name: "Chase"}` | Rainbow | | `{id: 65, name: "Colorful"}` | Palette | | `{id: 66, name: "Traffic"}` | Fire 2012 | | `{id: 57, name: "Fire 2012"}` | Lightning | | `{id: 108, name: "Twinkle"}` | Sine | The model picks an effect by **name** — "Twinkle suits a winter scene" — and gets the number beside it. So it asked for Sine and meant Twinkle. Replaced with ten verified pairs, chosen to span genuinely different looks so the model has something to choose between. ## The guard is the point of the change An effect ID is an opaque integer *everywhere except* the one place a human decides what it means. Nothing in the suite could ever have caught either bug — the check has to be against the device's own answer. `tests/fixtures/wled/16.0.0/eff.json` is that answer, captured verbatim from the real controller back in #62. **That fixture already existed; this is what makes it load-bearing rather than decorative.** I reintroduced both bugs to confirm the test fails with the right diagnosis before keeping the fixes: ``` assert 'Tetrix' == 'Aurora' assert ["id 108 is labelled 'Twinkle' but is 'Sine'"] == [] ``` `schemes/builtin.yaml` was audited at the same time and is **clean** — all eight IDs it uses (0, 2, 9, 21, 45, 57, 65, 66) are real effects and plausible for their schemes. A range check now keeps it that way, plus a check that the walker actually finds something, so it can't pass on an empty set. ## Verification 10 new tests, `ruff`/`mypy` clean, **759 backend tests passing**. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit

2026-09-05 00:56:28 +00:00

Make two effect IDs mean what their comments claim (#132 )

CI / Python lint & type-check (pull_request) Successful in 1m21s

Details

CI / Dockerfile lint (pull_request) Successful in 8s

Details

CI / Alembic migration check (pull_request) Successful in 1m42s

Details

CI / Pre-commit hooks (pull_request) Successful in 2m21s

Details

CI / Frontend lint, test & build (pull_request) Successful in 1m14s

Details

CI / Python tests (pull_request) Successful in 4m21s

Details

CI / Docker build, health smoke & E2E (pull_request) Successful in 2m18s

Details

73f4cdf729

Found while starting #14, by checking the codebase's hardcoded effect IDs
against a real 16.0.0 controller's /json/eff instead of against the
comments next to them.

The aurora scheme pushed effect 44 with a comment of mine from #20 saying
"Effect 44 is Aurora in stock WLED". 44 is Tetrix -- falling blocks.
Aurora is 38. The feature is env-gated and off by default so nobody has
seen it, but anyone who enabled AURORA_ENABLED would have got the wrong
animation during a geomagnetic storm. I asserted the number rather than
reading it, which is the same mistake as leds.segs in #62 and the
transition range in #52.

The AI's fallback effect catalogue -- used whenever no controller is
reachable, which is the path a new user hits before wiring hardware and
the default in demo mode -- had five of seven pairs wrong. The model
picks an effect by name and gets the number beside it, so it asked for
Sine and meant Twinkle, Rainbow and meant Chase, Lightning and meant Fire
2012. It is now ten verified pairs rather than seven mostly-wrong ones.

The guard is the point of the change. An effect ID is an opaque integer
everywhere except where a human decides what it means, so nothing in the
suite could ever have caught either bug -- the check has to be against
the device's own answer. tests/fixtures/wled/16.0.0/eff.json is that
answer, captured verbatim in #62; this is what makes it load-bearing
rather than decorative. Both bugs were reintroduced to confirm the test
fails with the right diagnosis before the fixes were kept.

schemes/builtin.yaml was audited at the same time and is clean: all eight
IDs it uses are real effects and plausible for their schemes. A range
check now keeps it that way, plus a check that the walker actually finds
something, so it cannot pass on an empty set.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>