From ef3e7a54408ba6ae896b56bf78a63ce501202c27 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 2 Oct 2026 20:15:34 +0300 Subject: [PATCH] fix(config): an ordinary save without led_strips keeps the stored shapes (#780) r1 H1: a previous frontend sends every space without the unknown field and the ordinary write path replaced the document with it, erasing every LED strip. The shared ordinary-writer helper now copies the stored shapes of a space the payload omits (config/set and Optimize); an explicit list from a new client still deletes, a removed space takes its strips, and a link to a marker the same write deleted unbinds by the usual rule. config/set reports the normalisation counters after the preservation. Issue: #780 User-Visible: no --- custom_components/houseplan/led_strips.py | 28 +++++++++ custom_components/houseplan/validation.py | 6 +- custom_components/houseplan/websocket_api.py | 5 +- docs/CONFIG-COMPATIBILITY.md | 10 ++- scripts/mutation-registry.mjs | 10 +++ tests_backend/test_ha_websocket.py | 66 ++++++++++++++++++++ tests_backend/test_led_strips.py | 42 +++++++++++++ 7 files changed, 163 insertions(+), 4 deletions(-) diff --git a/custom_components/houseplan/led_strips.py b/custom_components/houseplan/led_strips.py index 19b0546c..4c69a017 100755 --- a/custom_components/houseplan/led_strips.py +++ b/custom_components/houseplan/led_strips.py @@ -28,6 +28,7 @@ Write-path order (ТЗ #780 §9): """ from __future__ import annotations +import copy import math from collections.abc import Iterator from typing import Any @@ -161,6 +162,33 @@ def _strips(config: dict[str, Any]) -> Iterator[tuple[dict[str, Any], dict[str, yield space, strip +def preserve_led_strips(config: dict[str, Any], previous: dict[str, Any] | None) -> int: + """An ordinary writer that omits ``led_strips`` keeps the stored shapes (#780 r1 H1). + + A previous frontend does not know the field and sends every space without + it; the omission is not a deletion. A new client deletes shapes by sending + an explicit list (``[]`` removes all of them). Applies to spaces that keep + their id; a space the writer removed takes its strips with it. Runs on the + ordinary write path before normalisation, so a preserved link to a marker + the same write deleted becomes an unbound strip by the usual rule. + Returns how many spaces received their stored shapes back. + """ + stored = { + space.get("id"): space[LED_KEY] + for space in (previous or {}).get("spaces") or [] + if isinstance(space, dict) and isinstance(space.get(LED_KEY), list) and space[LED_KEY] + } + restored = 0 + for space in config.get("spaces") or []: + if not isinstance(space, dict) or LED_KEY in space: + continue + shapes = stored.get(space.get("id")) + if shapes: + space[LED_KEY] = copy.deepcopy(shapes) + restored += 1 + return restored + + def led_strip_link_report(config: dict[str, Any]) -> dict[str, int]: """What the normaliser would change, without changing anything. diff --git a/custom_components/houseplan/validation.py b/custom_components/houseplan/validation.py index af8a6e7c..c326e3fb 100644 --- a/custom_components/houseplan/validation.py +++ b/custom_components/houseplan/validation.py @@ -20,6 +20,7 @@ from custom_components.houseplan.coordinate_canonicalization import ( from custom_components.houseplan.led_strips import ( LED_STRIPS_SCHEMA, config_led_strip_links, + preserve_led_strips, ) from custom_components.houseplan.vacuum_routes import validate_marker_routes @@ -2274,13 +2275,16 @@ def prepare_ordinary_summary_candidate( readable_entity_ids: set[str], normalize, ) -> dict: - """Apply the one summary-panel contract shared by ordinary config writers. + """Apply the one preservation contract shared by ordinary config writers. `normalize` owns the writer-specific schema/migration sequence. Preservation must happen before it; change-aware reference validation must happen after it. Authoritative full import/restore intentionally does not use this helper. + Preserved: the summary panel namespace (#437) and the LED strip shapes of a + space an older writer sent without the field (#780). """ preserve_summary_panel_namespace(candidate, previous) + preserve_led_strips(candidate, previous) checked = normalize(candidate) validate_summary_panel_references(checked, previous, readable_entity_ids) return checked diff --git a/custom_components/houseplan/websocket_api.py b/custom_components/houseplan/websocket_api.py index f294ceed..3a0ae827 100755 --- a/custom_components/houseplan/websocket_api.py +++ b/custom_components/houseplan/websocket_api.py @@ -70,7 +70,7 @@ from .import_export import ( revalidate_candidate, ) from .junction_limits import JunctionLimitError, validate_junction_limits -from .led_strips import led_strip_link_report +from .led_strips import led_strip_link_report, preserve_led_strips from .plans import ( QuotaError, collect_attachments, @@ -1685,6 +1685,9 @@ async def ws_config_set(hass: HomeAssistant, connection, msg: dict[str, Any]) -> # #780: what the write path will normalise in LED strip links (an # orphan becomes unbound, an empty marker.space adopts the strip's # space). Reported back so a new client re-reads; old ones ignore it. + # An older writer omits the field: its stored shapes are kept first + # (r1 H1), so the report also counts their links. + preserve_led_strips(msg["config"], data.get("config")) led_report = led_strip_link_report(msg["config"]) def _validate_config_cpu(): diff --git a/docs/CONFIG-COMPATIBILITY.md b/docs/CONFIG-COMPATIBILITY.md index 9861adb8..0e575b23 100644 --- a/docs/CONFIG-COMPATIBILITY.md +++ b/docs/CONFIG-COMPATIBILITY.md @@ -1139,8 +1139,14 @@ led_strips?: Array<{ id: string; points: [number, number][]; marker: string | nu adopts the strip's space. A non-empty foreign `space` or a second link still rejects the whole write. `config/set` reports the counts `{led_strips: {unbound, space_adopted}}`; older clients may ignore them. -- **Older clients** keep the unknown array on an ordinary save; deleting a - bound marker succeeds and leaves an unbound strip. +- **Older clients.** An ordinary writer (`config/set`, Optimize) that omits + the field keeps the stored shapes of every space it still sends: the backend + copies them from the stored revision before normalisation + (`preserve_led_strips`, r1 H1). Omission is never a deletion; a new client + deletes shapes only with an explicit list (`[]` removes all). Deleting a + bound marker in the same write succeeds and leaves an unbound strip; a + removed space takes its strips with it. Full import/restore is authoritative + and does not preserve. - **Transfer.** Full export/import keeps geometry, links and `active`. A space import remaps links through the same marker-id map as the devices; a strip whose marker did not travel (skipped duplicate, absent) arrives diff --git a/scripts/mutation-registry.mjs b/scripts/mutation-registry.mjs index 9045b444..e300c7b1 100644 --- a/scripts/mutation-registry.mjs +++ b/scripts/mutation-registry.mjs @@ -14374,6 +14374,16 @@ const MUTANT_DEFINITIONS = [ replace: ' unbound_led_strips = unbind_strips(space, remap={\n old_id: old_id for old_id, new_id in marker_map.items()', }], }, + { + id: 'led-old-writer-drops-strips', + guard: 'node scripts/backend-test-guard.mjs old_writer tests_backend/test_led_strips.py', + because: '#780 r1 H1: an ordinary save without the unknown field keeps the stored shapes; only an explicit list deletes', + patches: [{ + file: 'custom_components/houseplan/validation.py', + find: ' preserve_summary_panel_namespace(candidate, previous)\n preserve_led_strips(candidate, previous)', + replace: ' preserve_summary_panel_namespace(candidate, previous)', + }], + }, ]; const mutationCardSource = readFileSync(join(repoRoot, 'src/houseplan-card.ts'), 'utf8'); diff --git a/tests_backend/test_ha_websocket.py b/tests_backend/test_ha_websocket.py index 953bb2e5..ffd80643 100644 --- a/tests_backend/test_ha_websocket.py +++ b/tests_backend/test_ha_websocket.py @@ -4485,3 +4485,69 @@ async def test_issue_780_old_client_deleting_a_bound_marker_still_saves( await client.send_json_auto_id({"type": "houseplan/config/get"}) unchanged = (await client.receive_json())["result"] assert unchanged["rev"] == second["result"]["rev"], "a rejected write leaves the revision" + + +async def test_issue_780_old_writer_omitting_led_strips_keeps_the_shapes( + hass: HomeAssistant, hass_ws_client: WebSocketGenerator, +) -> None: + """r1 H1: a previous frontend does not know the field and sends every space + without it. The omission is not a deletion: the stored shapes survive, a + link to a marker the same write deleted becomes unbound, and only an + explicit list from a new client removes them. Judged by a fresh connection.""" + await _setup(hass) + client = await hass_ws_client(hass) + space = _space("f1", "r1") + shapes = [ + {"id": "led-1", "points": [[0.1, 0.1], [0.6, 0.1], [0.6, 0.5]], "marker": "lamp", "active": False}, + {"id": "led-2", "points": [[0.2, 0.2], [0.5, 0.2]], "marker": None, "active": True}, + ] + config = { + "spaces": [{**space, "led_strips": copy.deepcopy(shapes)}, _space("f2", "r2")], + "markers": [{"id": "lamp", "binding": "entity:light.kitchen", "space": "f1"}], + "settings": {}, + } + await client.send_json_auto_id({"type": "houseplan/config/set", "config": config, "expected_rev": 0}) + first = await client.receive_json() + assert first["success"], first + + # The old writer: the same plan, no `led_strips` key anywhere. + old_writer = copy.deepcopy(config) + for item in old_writer["spaces"]: + item.pop("led_strips", None) + old_writer["markers"][0]["name"] = "Kitchen strip" + await client.send_json_auto_id({ + "type": "houseplan/config/set", "config": old_writer, "expected_rev": first["result"]["rev"], + }) + kept = await client.receive_json() + assert kept["success"], kept + assert "led_strips" not in kept["result"], "nothing to normalise: the shapes came back as stored" + + # The old writer deletes the bound marker: the save stands, the kept shape unbinds. + deleting = copy.deepcopy(old_writer) + deleting["markers"] = [] + await client.send_json_auto_id({ + "type": "houseplan/config/set", "config": deleting, "expected_rev": kept["result"]["rev"], + }) + unbound = await client.receive_json() + assert unbound["success"], unbound + assert unbound["result"]["led_strips"] == {"unbound": 1, "space_adopted": 0} + + fresh = await hass_ws_client(hass) + await fresh.send_json_auto_id({"type": "houseplan/config/get"}) + stored = (await fresh.receive_json())["result"] + assert stored["config"]["spaces"][0]["led_strips"] == [ + {**shapes[0], "marker": None, "active": True}, shapes[1], + ] + assert "led_strips" not in stored["config"]["spaces"][1] + + # A new client deletes every shape with an explicit empty list. + explicit = copy.deepcopy(stored["config"]) + explicit["spaces"][0]["led_strips"] = [] + await client.send_json_auto_id({ + "type": "houseplan/config/set", "config": explicit, "expected_rev": stored["rev"], + }) + assert (await client.receive_json())["success"] + reader = await hass_ws_client(hass) + await reader.send_json_auto_id({"type": "houseplan/config/get"}) + emptied = (await reader.receive_json())["result"] + assert emptied["config"]["spaces"][0].get("led_strips") == [] diff --git a/tests_backend/test_led_strips.py b/tests_backend/test_led_strips.py index 62454720..49c5e896 100755 --- a/tests_backend/test_led_strips.py +++ b/tests_backend/test_led_strips.py @@ -185,6 +185,48 @@ def test_deleted_marker_leaves_an_unbound_strip_with_its_geometry(active) -> Non assert report == {"unbound": 1, "space_adopted": 0} +def _old_writer(config: dict) -> dict: + """The payload of a frontend that does not know the field (r1 H1).""" + payload = copy.deepcopy(config) + for space in payload["spaces"]: + space.pop("led_strips", None) + return payload + + +def _ordinary(candidate: dict, previous: dict) -> dict: + return v.prepare_ordinary_summary_candidate(copy.deepcopy(candidate), previous, set(), v.CONFIG_SCHEMA) + + +@pytest.mark.parametrize("active", [True, False]) +def test_old_writer_omitting_the_field_keeps_the_stored_shapes(active) -> None: + """r1 H1: an omitted ``led_strips`` is not a deletion on the ordinary write path.""" + previous = _check(_config([_strip(active=active)])) + checked = _ordinary(_old_writer(previous), previous) + assert _strips_of(checked) == _strips_of(previous) + + +def test_old_writer_deleting_the_marker_unbinds_the_kept_shape() -> None: + previous = _check(_config([_strip(active=False)])) + payload = _old_writer(previous) + payload["markers"] = [] + strip = _strips_of(_ordinary(payload, previous))[0] + assert strip == {**_strip(), "marker": None, "active": True} + + +def test_explicit_empty_list_deletes_and_a_removed_space_takes_its_shapes() -> None: + previous = _check(_config([_strip()], extra_spaces=("upper",))) + explicit = copy.deepcopy(previous) + explicit["spaces"][0]["led_strips"] = [] + assert _strips_of(_ordinary(explicit, previous)) == [] + removed = _old_writer(previous) + removed["spaces"] = [space for space in removed["spaces"] if space["id"] != "ground"] + removed["markers"] = [] + checked = _ordinary(removed, previous) + assert all("led_strips" not in space for space in checked["spaces"]) + # The preservation never invents a key for a space that had none. + assert "led_strips" not in _ordinary(_old_writer(_check(_config())), _check(_config()))["spaces"][0] + + def test_tombstoned_marker_is_not_live() -> None: markers = [{"id": "lamp", "binding": "entity:light.kitchen", "space": "ground", "removed": True}] strip = _strips_of(_check(_config([_strip()], markers)))[0]