Skip to content

Extract shared deserialize helpers on Device#2076

Draft
frenckatron wants to merge 2 commits into
mainfrom
frenckatron/refactor-device-deserialize-helpers
Draft

Extract shared deserialize helpers on Device#2076
frenckatron wants to merge 2 commits into
mainfrom
frenckatron/refactor-device-deserialize-helpers

Conversation

@frenckatron
Copy link
Copy Markdown
Collaborator

@frenckatron frenckatron commented May 20, 2026

What

Extract three shared helpers (_filter_effects, _build_palette_dict,
_split_presets_playlists) on Device and call them from both
__pre_deserialize__ and update_from_dict.

Why

The two methods carried near-identical logic for effects filtering,
palette synthesis, and preset/playlist splitting. Any fix landing in one
risked silently diverging from the other — a real dual-maintenance
hazard. This is the refactor specced for src/wled/models.py:865-1011.

How

  • Helpers live next to the existing _build_custom_palettes /
    _build_usermod_palettes statics. _build_palette_dict is a
    @classmethod because it composes the other two; the rest are
    @staticmethod.
  • All helpers return raw dict[str, Any]. The pre-deserialize path
    hands the dicts to mashumaro directly; update_from_dict wraps them
    with Effect(**…), Palette(**…), and Preset.from_dict /
    Playlist.from_dict. One source of truth, two output shapes.
  • Behavior preserved: RSVD skip, null-palette fallback for
    less-capable devices, defensive _presets.copy(), and pop(0, None)
    on both presets and playlists. Version-check / WLEDUnsupportedVersionError
    stays inside __pre_deserialize__ since it's deserialization-specific.

Testing

  • Full suite: 222 passed (37 snapshots passed), no behavior change.
  • ruff check src/wled/models.py clean.

Quality Report

Changes: 1 file changed, 118 insertions(+), 72 deletions(-)

Code scan: clean

Tests: failed (command not found)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

Both `Device.__pre_deserialize__` and `Device.update_from_dict` carried
near-identical logic for effects filtering, palette synthesis, and
preset/playlist splitting. Any fix on one side risked silently drifting
from the other.

Introduce three helpers — `_filter_effects`, `_build_palette_dict`, and
`_split_presets_playlists` — alongside the existing palette builders.
They return raw `dict[str, Any]` so the pre-deserialize path (which hands
off to mashumaro) and the `update_from_dict` path (which constructs
typed objects via `from_dict` / `**kwargs`) can share the same source of
truth. Behavior is preserved: RSVD skip, null-palette fallback, defensive
copy of the presets payload, and `pop(0, None)` on both maps.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

Important

Review skipped

Draft detected.

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

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 35e6f89f-8976-4910-84f0-e43fb6675875

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

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch frenckatron/refactor-device-deserialize-helpers

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@frenckatron frenckatron added the refactor Improvement of existing code, not introducing new features. label May 20, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Improvement of existing code, not introducing new features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant