docs: review document for #256

Issue: #256
User-Visible: no

docs/reviews/CODE-REVIEW-256-r1.md

@@ -0,0 +1,171 @@
+# CODE-REVIEW — issue #256 — заход r1
+
+**Скоуп задачи:** `houseplan/config/get` и `houseplan/layout/get` получают
+необязательные параметры проекции (`space_id`, `fields`, `marker_fields` /
+`space_id`), не меняя ответ по умолчанию. Лёгкий трек (`small`), ТЗ — в теле
+issue. Заход первый: `git log --oneline origin/dev..HEAD` даёт один коммит
+(`a952f5f`), это полный разбор, дельты по предыдущим раундам нет.
+
+Диапазон материала: `git diff origin/dev...HEAD` — 3 файла, +269/−5:
+`custom_components/houseplan/projection.py` (новый, 95 строк), правки
+`custom_components/houseplan/websocket_api.py` (+41/−5), `tests_backend/
+test_projection.py` (новый, 138 строк, 11 тестов).
+
+## Как проверялось
+
+Прочитаны в порядке из инструкции: `docs/SCOPE.md`, `AGENTS.md`, тело issue
+#256 и оба комментария (спека и отчёт автора), `PROCESS.md` §2.7/§2.10/§4.
+Изменение не трогает видимое поведение (`User-Visible: no`, подтверждено
+диффом — нет правок в `docs/CHANGELOG*`), поэтому `docs/USER-GUIDE.ru.md` и
+канонические документы подсистем не задействованы: меняется только
+серверный WS-протокол для диагностических клиентов, UI не тронут.
+
+### Гейты — что прогнал сам и с каким результатом
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Typecheck | `npx tsc --noEmit` | чисто, exit 0 |
+| JS-тесты | `npm test` | 1136/1136, 0 fail |
+| Сборка + сверка бандла | `npm run build`, затем `diff dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | сборка ок; diff пустой (файлы идентичны) — ожидаемо, диффа в `src/**` нет |
+| Backend pure-тесты (новые) | `python3 -m pytest tests_backend/test_projection.py -v` (после `pip3 install --user pytest pytest-asyncio`, песочница не имела pytest) | **11/11 passed** |
+| Backend pure-подмножество целиком | `python3 -m pytest tests_backend -q --ignore=tests_backend/test_coordinate_canonicalization.py --ignore=tests_backend/test_validation.py` | **51 passed** — совпадает с числом, заявленным автором |
+| Дисциплина «тест умеет падать» | вручную сломал `keep = {"id", *names}` → `{*names}` в копии `projection.py`, перезапустил `test_projection.py`: **4 теста упали** (потеря `id`), включая `test_marker_fields_keep_id_even_when_not_asked` и `test_markers_survive_unexpected_entries`. Отдельно занулил условие раннего возврата (`if False: return config`) — упали **2 теста инварианта идентичности** (`test_no_parameters_change_nothing`, `test_empty_or_malformed_lists_mean_no_projection`). Оба раза файл восстановлен из бэкапа, `git status` чист | тесты не ватные |
+| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | — | «Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут). Browser-smoke этим диффом не выбираются — выбирать нечего.» |
+
+Полный вывод `git status --short` после всех экспериментов — пусто, рабочее
+дерево осталось чистым.
+
+### Что не прогонял и почему
+
+- **`node scripts/check-docs.mjs`** — не прогонял. Условие запуска — diff
+  трогает `src/**`; этот diff `src/**` не трогает (подтверждено и
+  `git diff --stat`, и выводом `smoke-select.mjs`). Отпечаток документации
+  считается по `src/**`, изменение в него не входит.
+- **`npm run invariants -- --config <...>`** — не прогонял. Diff действительно
+  упоминает `layout` и `marker.space`, но только как *ключи фильтрации при
+  чтении* — ни один путь записи (`config/set`, `layout/set`, `plan/optimize`)
+  не тронут, хранимый документ проекция не мутирует (проверено чтением и
+  тестами `test_no_parameters_change_nothing`, `is CONFIG`/`is LAYOUT`).
+  Инварианты модели (#254) — про целостность *хранимых* данных; здесь модель
+  не меняется, поэтому гейт неприменим, а не пропущен.
+- **`golden:verify`, browser-смоки** — не прогонял; рендер, геометрия и стили
+  не затронуты, `smoke-select.mjs` подтверждает: выбирать нечего.
+- **HA-харнесс (`test_ha_websocket.py` и соседние)** — не прогонял. В
+  песочнице нет `homeassistant` (тот же пробел независимо воспроизведён на
+  `origin/dev` — `test_coordinate_canonicalization.py`/`test_validation.py`
+  падают с `ModuleNotFoundError` и там, это не регрессия задачи). Проводка
+  двух обработчиков (`ws_config_get`, `ws_layout_get`) разобрана **чтением, не
+  исполнением** — см. AC2/AC4 ниже; фактический прогон харнесса — за CI.
+- **`hassfest`/HACS** — манифест не менялся, гейт неприменим.
+- **Perf-профили** — не названы в AC, путь не относится к чувствительным к
+  перфу (проекция — плоская фильтрация словаря/списка на чтении, O(n) от
+  размера уже загруженного документа).
+- **Одно число — один источник** — неприменимо: изменение не добавляет и не
+  меняет пользовательскую величину, это диагностический протокол, не UI.
+
+## AC — доказательства
+
+1. **`config/get` без параметров — ответ прежний.** Доказано тестом
+   `test_no_parameters_change_nothing` (проверяет тождество объекта `is
+   CONFIG`, не только равенство) и чтением кода: `ws_config_get` строит
+   `config` ровно как раньше, вызывает `project_config(config, space_id=None,
+   fields=None, marker_fields=None)`, а та при всех трёх `None` возвращает
+   входной объект без копии (`projection.py:56-58`). Тест умеет падать —
+   проверено (см. таблицу гейтов).
+2. **`space_id` возвращает одно пространство, `markers`/`settings` не
+   урезаны.** Доказано `test_space_id_narrows_spaces_only` и
+   `test_unknown_space_returns_empty_list_not_an_error`. Плюс чтением:
+   `project_config` трогает только ключ `spaces`, остальные ключи копируются
+   как есть (`dict(config)`).
+3. **`marker_fields: ["binding","space"]` оставляет `id`+два поля.** Доказано
+   `test_marker_fields_keep_id_even_when_not_asked` (сверка точного словаря) и
+   разрушающим экспериментом выше (тест ловит потерю `id`).
+4. **`layout/get` с `space_id` — только позиции этого пространства.**
+   Доказано `test_layout_space_filter`. Проводка в `ws_layout_get` —
+   **проверено чтением, не исполнением**: `project_layout(data.get("layout",
+   {}), space_id=msg.get("space_id"))` подставлена на место прежнего
+   `data.get("layout", {})`, `rev`/`can_optimize_undo`/`undo_kind` считаются
+   из `config_data`/`data`, которые проекции не касаются — инвариант «флаги
+   считаются до проекции» не может быть задет этой правкой хотя бы потому, что
+   строки, их вычисляющие, физически не изменены (diff это подтверждает).
+
+Инвариант 2 из ТЗ («`rev`, `can_write`, `virtual_lights`, `can_optimize_undo`,
+`undo_kind` не зависят от проекции») — проверено чтением: в `ws_config_get`
+`virtual_lights` считается **до** вызова `project_config`, из непроецированного
+`config`; `can_write`/`can_optimize_undo`/`undo_kind` используют `data`/
+`config_data`, не `config`. Порядок операций в диффе это гарантирует
+структурно, не только по намерению.
+
+Инвариант 3 («проекция только для чтения») — проверено чтением: ни один из
+трёх обработчиков записи (`config/set`, `layout/set`, `plan/optimize`) не
+импортирует и не вызывает `projection.py` — `grep` по diff и по текущему
+`websocket_api.py` подтверждает единственную точку импорта.
+
+Инвариант 4 (неизвестное имя поля — не ошибка; неизвестный `space_id` — пустой
+список) — доказано `test_unknown_space_returns_empty_list_not_an_error`,
+`test_unknown_marker_field_adds_nothing`.
+
+## Разобрано и корректно
+
+- Проекция не мутирует исходный документ: `project_config` делает `dict(config)`
+  (мелкое копирование) и никогда не пишет в `config[...]` до этого; `spaces`/
+  `markers` заменяются новыми списками/словарями, а не редактируются на месте.
+  Подтверждено и тестами (`CONFIG == original` после вызова), и разрушающей
+  проверкой не потребовалось — код тривиально проверяем чтением.
+- Порядок применения фильтров (`space_id` → `marker_fields` → `fields`)
+  корректен даже в комбинации «`fields` не включает `markers`, но
+  `marker_fields` задан»: `marker_fields` спроецирует `markers` до того, как
+  `fields` вырежет сам ключ `markers` — лишняя работа, не баг.
+- Решение «пустой список = отсутствие проекции, а не проекция в ноль полей»
+  автор явно пометил как оспоримое. Он не наблюдаем ни одной персоной
+  (доступен только диагностическим клиентам, не описан в AC) — это техническое
+  решение в духе PROCESS.md §7 («решай и записывай»), а не догадка, выданная
+  за продуктовый факт. Принимаю: альтернатива (пустой список = «убрать все
+  поля») не более очевидна и нигде не запрошена.
+- `id` всегда добавляется в `marker_fields` — соответствует AC3 буквально и
+  сопровождается тестом и комментарием, объясняющим «почему».
+- Схемы `vol.Optional`/`vol.All(...vol.Length(...))` на `space_id`
+  (1–200 символов), `fields`/`marker_fields` (список строк 1–100 символов,
+  максимум 50 элементов) — разумные защитные пределы, симметричные между
+  `config/get` и `layout/get`.
+- Загрузка модуля в тестах по пути (`importlib.util.spec_from_file_location`),
+  а не через пакет — повторяет приём из `test_virtual_lights.py` и обосновано
+  тем же #135; сам `projection.py` не импортирует ничего, кроме `typing`, что
+  и позволяет тесту работать без `homeassistant`. Подтверждено эмпирически:
+  `test_projection.py` собрался и прошёл в песочнице без HA/voluptuous, тогда
+  как соседние файлы, тянущие пакет, — нет.
+- Трейлеры коммита `a952f5f`: `Issue: #256`, `User-Visible: no` — корректно,
+  изменение не даёт пользователю ничего нового, changelog обоснованно не
+  тронут.
+- Продуктовая рамка не нарушена: `docs/SCOPE.md` не содержит прямого пункта
+  под диагностические WS-параметры, но задача явно заведена самим ревью
+  (аудит инфраструктуры 2026-08-23) и принята владельцем на уровне issue —
+  скоуп решён на входе, пересматривать на этапе кода нечего.
+
+## Находки
+
+Нет находок High или Medium. Ниже — то, что рассмотрено и сознательно не
+поднято до находки:
+
+- **Low, не блокирует, не правлю:** порядок `space_id`→`marker_fields`→
+  `fields` в `project_config` иногда строит `markers`-проекцию, которую тут же
+  выбрасывает `fields`-фильтр (см. выше). Чисто эффективность на маленьких
+  документах (максимум пара сотен маркеров), не корректность. Не стоит цикла
+  ревью.
+- Рассмотрел как потенциальную Medium-находку: решение «пустой список =
+  нет проекции» могло быть продуктовым вопросом («хочу узнать только `rev`,
+  без конфигурации вообще»), а не техническим. Отклоняю как находку: ни один
+  реальный клиент (карточка, диагностика) не отправляет пустой список сегодня,
+  случай гипотетический, вариант автора не хуже альтернативы, и не наблюдаем
+  ни одной из трёх персон продукта — чисто внутренний контракт диагностического
+  API. Не эскалирую владельцу: это не вопрос «что видит пользователь».
+
+## Вердикт
+
+Все 4 AC доказаны автотестами, тесты проверены на способность падать. Оба
+инварианта совместимости (байт-в-байт без параметров; `rev`/флаги считаются до
+проекции) проверены и чтением, и тестами. Запись не затронута. Гейты,
+соразмерные объёму задачи (typecheck, JS-тесты, build+bundle-сверка, pure
+backend-тесты), зелёные; неприменимые гейты названы с обоснованием.
+
+**Зелёный.**