Handle hint and errors in yaml by stvncode · Pull Request #3 · esphome/device-builder

stvncode · 2026-04-28T13:45:00Z

No description provided.

Copilot

Pull request overview

Adds a new EditorController to support live YAML validation by talking to a long-lived esphome vscode --ace subprocess, and wires it into the DeviceBuilder lifecycle/command registry.

Changes:

Introduce EditorController with an editor/validate_yaml WebSocket command.
Start/stop the editor controller alongside existing controllers.
Implement subprocess protocol handling (validate, read_file, result) for structured YAML errors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`esphome_device_builder/device_builder.py`	Registers and lifecycle-manages the new `EditorController`.
`esphome_device_builder/controllers/editor.py`	New controller implementing persistent subprocess-based YAML validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Marcel van der Veldt <m.vanderveldt@outlook.com>

- Reframe module/class docstrings to describe purpose and usage rather than internal mechanics. - Add docstrings to the lifecycle, subprocess management, and validation helpers. - Replace the single editor-wide lock with per-configuration sessions so concurrent edits on different devices no longer queue behind each other. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-authored-by: Marcel van der Veldt <m.vanderveldt@outlook.com>

Three inline comments addressed: - Move NO_WIFI_VARIANTS / BOARDS imports + the lowercase _ESP32_NO_WIFI_VARIANTS derivation under the 'except ImportError' fallback branch. The 'upstream helper present' path now has zero coupling to upstream's implementation-detail tables — once the dep floor moves past the release that lands esphome/esphome#16300 the whole try/except block can collapse to a plain import. Fallback-correctness tests get a skipif marker since the constants are empty when upstream is loaded. - Update _infer_native_wifi docstring: dropped the stale 'Anything else → True' wording and replaced with the explicit allowlist semantics (esp8266 / bk72xx / rtl87xx / ln882x / libretiny + ESP32 / RP2040 dispatch branches → True; nrf52, host, unknown platforms → False). - _make_board docstring: clarified that connectivity=None produces a board with the hardware object present and an empty connectivity list (not a missing hardware block). Plus add 'libretiny' (legacy umbrella key for bk72xx / rtl87xx / ln882x) to the fallback's _FALLBACK_WIFI_FIRST_PLATFORMS and the test parametrisation, matching the upstream PR's LIBRETINY_OLDSTYLE addition.

* On-disk validation of cleanup_ttl_seconds (Copilot). The WS validator on set_settings gates write-through-WS, but the on-disk decode path didn't apply the same not_bool / range check. A sidecar with cleanup_ttl_seconds: true would deserialise as 1 (bool is an int subclass) and the sweep would treat anything older than 1s as cold. Other wrong types (string, float, None) would propagate to the sweep's now - ttl_seconds arithmetic and raise TypeError every cycle. New __post_init__ on RemoteBuildSettings coerces non-int / bool back to DEFAULT_CLEANUP_TTL_SECONDS and clamps the result to [MIN, MAX]. Doesn't reject the row (no ValueError) — the load path stays robust against partially-corrupt sidecars; the operator's last-good enabled toggle survives even if the TTL field is broken. Six new tests in test_config_controller.py pin the coerce-to-default branches, the clamp-to-MIN / clamp-to-MAX branches, and the pass-through-on-valid branch. * Public seam on FirmwareController for in-flight remote-peer jobs (reviewer concern #1). Reaching into firmware._jobs from the cleanup loop coupled the two controllers to the private dict shape — a future refactor (lock-wrapped jobs, QUEUED + RUNNING split into two dicts, indexed view) would silently break this loop. New FirmwareController.active_remote_peer_jobs generator filters to QUEUED / RUNNING + non-empty remote_peer. The cleanup loop now consumes through that seam. Two new tests pin every filter branch (local skip, remote queued / running yielded, remote completed skipped, empty case). * Named constant for the parse_from_configuration tail-segment count (reviewer nit). The +3 magic was folklore; replaced with _TAIL_SEGMENT_COUNT and a docstring explaining the three required tail parts (dashboard_id, device_name, YAML filename under the device subtree). Reviewer concern #2 (RAM-canonical settings) and #4 (rate-limit per cycle) deferred per their "worth a small follow-up" framing. Concern #3 (first-sweep latency) was considered but YAGNI for this PR: the receiver-side cleanup ships ahead of any production user (alpha, ~10 beta testers), so no accumulated subtrees exist before this code's first cycle — and the default 24h TTL means even a fresh receiver has nothing to reclaim for the first day regardless.

Handle hint and errors in yaml

dc33c39

stvncode self-assigned this Apr 28, 2026

Copilot AI review requested due to automatic review settings April 28, 2026 13:45

stvncode added the enhancement Improvement to an existing feature label Apr 28, 2026

Copilot started reviewing on behalf of stvncode April 28, 2026 13:46 View session