docs: review document for #126

Issue: #126
User-Visible: no

docs/reviews/SPEC-REVIEW-126-r2.md

@@ -0,0 +1,180 @@
+# SPEC-REVIEW-126-r2
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/126
+- **ТЗ:** `docs/specs/126-ha-area-marker-relocation.md`
+- **Коммит материала:** `cfd9c0fd` (`git rev-parse HEAD` перед подведением итогов; ветка `issue/126-ha-area-marker-relocation`)
+- **Этап:** ТЗ на ревью (PROCESS.md §2.4)
+- **Заход:** r2 · блокирующих циклов израсходовано 1 из 4
+- **Трек:** полный (владелец явно закрыл `small` 2026-08-30)
+
+## SHA предыдущего раунда — примечание
+
+Вердикт r1 назвал материал `8bc8ba66`. Этого объекта в текущей истории репозитория
+нет (`git cat-file -t 8bc8ba66` → `fatal: Not a valid object name`) — SHA был
+назван, но перестал существовать. Причина не в недобросовестности: комментарий
+автора в issue от 2026-08-31T05:40 прямо говорит «Ветка перебазирована на
+актуальный dev@4683a493cca25a4a527f2b96d3d11723aa9da74c», и `git log -1
+--format='%H %P' a3d343ca` подтверждает — родитель коммита ТЗ теперь `4683a493`
+(коммит `docs: review document for #400`), а не старый `dev@6bf39ee9`, на
+который опирался `8bc8ba66`. Это ребейз на ушедший вперёд `dev` (PROCESS.md
+§2.10/§7.2) — по правилу «после ребейза это другой код», разбор ниже полный, а
+не только по дельте текста.
+
+## Скоуп ревью
+
+Диапазон материала — весь актуальный текст `docs/specs/126-ha-area-marker-relocation.md`
+на `cfd9c0fd` плюс проверка того, что ребейз (диапазон `dev@6bf39ee9..dev@4683a493`)
+не задевает ни одного факта, который ТЗ выдаёт за освоенный код. Продуктовый код
+по-прежнему не менялся — issue остаётся в `S3-spec`/до-DoR стадии, только текст
+спецификации.
+
+## Дельта с r1
+
+`git diff da8cb6fa..cfd9c0fd -- docs/specs/126-ha-area-marker-relocation.md`
+(коммит `cfd9c0fd`, +17/−2) — правки по всем трём находкам r1 (M1, L1, L2).
+Дополнительно проверено, что ребейз `a3d343ca`/`da8cb6fa`/`0d92712c`/`cfd9c0fd`
+на `dev@4683a493` не занёс изменений в подсистемы, о которых ТЗ делает
+фактические заявления:
+
+```
+git diff 6bf39ee9..4683a493 --stat -- src/devices.ts src/logic.ts \
+  src/space-geometry.ts src/types.ts \
+  custom_components/houseplan/validation.py custom_components/houseplan/websocket_api.py
+```
+→ пусто, ни один из этих файлов в диапазоне ребейза не тронут.
+
+Ребейз действительно принёс продуктовый код, но только по #400 (полировка
+furniture-редактора): `src/houseplan-card.ts` — порядок отрисовки handle'ов
+selection box у мебели (`HANDLE_PAINT_ORDER`), и `src/houseplan-editor-runtime.ts`
+— исключение перетаскиваемого маркера устройства из align-candidates через
+`_deviceDrag`. Оба куска — про мебель/выравнивание в редакторе плана, не про
+маркеры устройств, Area или layout-запись, на которых стоит ТЗ #126. Ни один
+символ, который ТЗ #126 называет (`_livePos`, `_defaultPositions`,
+`_syncNewDevices`, `diffNewDevices`, `markerPos`, `pointInPolygon`,
+`houseplan/layout/delete`, explicit-override правило #317), в диапазоне ребейза
+не задет. Ребейз не инвалидирует ни один факт из «что проверено и признано
+корректным» r1.
+
+## Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где это видно |
+|---|---|---|
+| **M1** (Medium, в скоупе) — §9 ссылался на несуществующий budget `known_devices` | §9 переписан: вводится собственный лимит `MAX_MARKER_AREA_SNAPSHOT = MAX_KNOWN_DEVICES = 20_000` entries плюс общий `MAX_CONFIG_BYTES = 2 MiB`; то же продублировано в §23 | `docs/specs/126-ha-area-marker-relocation.md:204-206, 479-480` (коммит `cfd9c0fd`). Число `20_000` не выдумано: `custom_components/houseplan/validation.py:1092` реально объявляет `MAX_KNOWN_DEVICES = 20000` и применяет его к `known_devices`/`new_device_ids` через `vol.Length(max=MAX_KNOWN_DEVICES)` (`validation.py:1932-1933`) |
+| **L1** (Low, снята с записью) — `bindingKind:'entity'` не отличает direct entity marker от composite light group | §8 получил абзац-дискриминатор: eligibility требует сохранённый `marker` с exact `binding === entity:${bindingRef}`, id-префикс — только defensive check; §23 повторяет это как явное техническое решение | `docs/specs/126-ha-area-marker-relocation.md:176-180, 482-483`. Поле `marker.binding: string // 'device:<id>' \| 'entity:<eid>' \| 'virtual'` реально существует (`src/types.ts:121`); light-группы markerless и получают синтетический id `'lg_'+eid` (`src/logic.ts:812`, `src/devices.ts:1175`) — у них нет сохранённого `marker.binding`, так что дискриминатор из §8 действительно их отсекает, а не выдуман |
+| **L2** (Low, снята с записью) — нет явной строки о влиянии на производительность | В §19 добавлен абзац «Performance budget»: resolver гоняется только на authoritative rebuild, линейная сложность по markers+rooms, не вызывается из render, не пишет при unchanged input | `docs/specs/126-ha-area-marker-relocation.md:433-435` |
+
+Примечание не по делу закрытия, а по качеству r1: сама находка M1 была верной
+по существу (ТЗ действительно ссылалось на несуществующее *название* бюджета,
+не подкреплённое кодом), но обоснование r1 содержало неточность — r1 утверждал,
+что «явного per-field ограничения... в коде нет», хотя `MAX_KNOWN_DEVICES = 20000`
+(`validation.py:1092`) существовал уже тогда и не менялся в диапазоне ребейза.
+Похоже, r1 грепал `known_devices|new_device_ids` в `src/types.ts` и
+`MAX_CONFIG_BYTES` в `validation.py`, но не искал сам `MAX_KNOWN_DEVICES`. Это
+не отменяет находку (несуществующее было именно то конкретное число/имя,
+которое ТЗ приписывало — и после правки текст ссылается на реальный, верно
+процитированный лимит), поэтому цикл не переоткрывается — только фиксирую для
+трассируемости.
+
+## Унаследовано из r1
+
+Документ `docs/reviews/SPEC-REVIEW-126-r1.md`, SHA материала `8bc8ba66`
+(недоступен в текущей истории — см. раздел выше; содержательно эквивалентен
+`a3d343ca`+`da8cb6fa` до ребейза, что подтверждено отсутствием изменений в
+диапазоне ребейза по всем файлам, которые ТЗ цитирует). Принято без повторной
+построчной проверки в r2, так как текст этих разделов не менялся в дельте
+`da8cb6fa..cfd9c0fd` и ни один цитируемый в них символ не задет ребейзом:
+
+- полнота обязательных разделов §7.1 (сценарий/персона, «что видно до и
+  после» без терминов реализации, проблема/диагноз, скоуп/не-скоуп, контракт
+  поведения, UX/touch/a11y, модель данных и миграция, i18n, AC1–AC12 со
+  способом доказательства, план тестов, риски/откат, release-артефакты);
+- соответствие J6 (`docs/SCOPE.md`);
+- построчная сверка с кодом причины дефекта (`_livePos()`
+  `src/houseplan-card.ts:5174-5190`, `markerPos()`
+  `src/space-geometry.ts:629-638`, `diffNewDevices()` `src/logic.ts:2092-2100`);
+- explicit-override правило #317 (`src/devices.ts:1421-1424`,
+  `resolveExplicitMarkerPlacement()` `src/devices.ts:1066-1086`);
+- реальность эндпойнта `houseplan/layout/delete`
+  (`custom_components/houseplan/websocket_api.py:1099-1106`);
+- переиспользование `pointInPolygon()` (`src/logic.ts:431`) для backfill;
+- терминология по `docs/USER-GUIDE.ru.md` (метка «Новое», Find/поиск её не
+  снимают);
+- `docs/specs/README.md` обновлён корректно;
+- владельцу не задано новых продуктовых вопросов.
+
+## Новая находка r2
+
+### Low — снимается с записью
+
+**L3. Строка 4 документа («Статус документа: актуализировано на
+`dev@6bf39ee9...`») устарела после ребейза на `dev@4683a493`.**
+
+Реальный родитель первого коммита ТЗ в текущей истории —
+`4683a493cca25a4a527f2b96d3d11723aa9da74c`
+(`git log -1 --format='%H %P' a3d343ca`), а не `6bf39ee9`, который документ
+называет своей базой. Само по себе не влияет на содержание ТЗ (диапазон ребейза
+не касается ни одной подсистемы, на которую опирается спецификация — см. раздел
+«Дельта с r1»), но неточная строка провенанса мешает следующему ревьюеру быстро
+проверить актуальность базы, как и в этом раунде пришлось восстанавливать факт
+ребейза из комментария issue, а не из самого документа. **Снимается с
+записью:** не блокирует; рекомендую поправить строку на актуальный `dev@…` при
+следующей правке ТЗ (или прямо при переводе в `S5-ready`), отдельного цикла ради
+этого не открываю.
+
+## Как проверялось
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Typecheck | `npx tsc --noEmit` | green, без вывода |
+| Unit-тесты | `npm test` | green: `tests 1668, pass 1667, fail 0, skipped 1` (тот же известный environment-sensitive пропуск, не относится к дифу) |
+| Build | `npm run build` | green, `dist` собран без ошибок |
+| Сверка трёх копий бандла | `npm run bundle:sync` | green; после запуска `git status --porcelain` пуст — `dist`, `custom_components/houseplan/frontend`, `demo/srv/assets` уже были в синхроне (диф #126 не трогает `src/**`) |
+| `node scripts/check-docs.mjs` | не запускал | диф #126 (`da8cb6fa..cfd9c0fd`) не трогает `src/**`; ребейз принёс изменения в `src/houseplan-card.ts`/`src/houseplan-editor-runtime.ts`, но это уже смёрженный и прошедший свой гейт код #400, не предмет этого ревью |
+| `npm run invariants` / model invariants | не запускал | диф не трогает геометрию, `layout`, `marker.space`, `open_spans` — только текст ТЗ |
+| Browser smoke (`demo/smoke_*`) | не запускал | продуктовый код не менялся ни этим дифом, ни ребейзом в затронутых #126 модулях |
+| `golden:verify`, `performance_smoke`, `pytest tests_backend` | не запускал | визуальные/perf/backend поверхности не тронуты — диф docs-only |
+
+Зелёного Validate на SHA `cfd9c0fd` не найдено, поэтому дешёвые гейты прогнаны
+вручную повторно в этом раунде (код в дереве изменился из-за ребейза, хотя сам
+диф #126 — нет); все зелёные. Тяжёлые гейты не применимы по тем же основаниям,
+что и в r1.
+
+## Что проверено и признано корректным
+
+- Все три находки r1 (M1/L1/L2) закрыты текстом, а не заявлением — см. таблицу
+  «Закрытие раунда r1» с точными строками и перекрёстной проверкой по коду.
+- Ребейз на `dev@4683a493` не заносит правок ни в один файл/символ, на который
+  опирается ТЗ #126 — проверено диапазонным `git diff --stat` по всем ключевым
+  путям.
+- AC1–AC12, разделы §5–§6 (скоуп/не-скоуп), §11–§14 (порядок записи,
+  multi-client, race-guard) и §21 (риски/откат) не менялись в дельте и не
+  задеты ребейзом — наследуются из r1 без повторной построчной проверки.
+- Новый текст §9 корректно цитирует реальный код (`MAX_KNOWN_DEVICES = 20000`,
+  `validation.py:1092`), а не изобретает число.
+- Новый текст §8 корректно опирается на реально существующее поле
+  `marker.binding` (`src/types.ts:121`) как дискриминатор, а не на
+  недостаточный `bindingKind`.
+
+## Чего не проверял
+
+- Не проверял выполнимость резолвера по коду — кода ещё нет, это спек-ревью.
+- Не прогонял golden/performance/browser smoke/backend pytest — диф их не
+  касается (см. таблицу гейтов).
+- Не делал полного повторного построчного код-аудита разделов, унаследованных
+  из r1 (§7.1-состав, J6, #29/#74/#317 контракты, endpoint layout/delete,
+  point-in-polygon, терминология USER-GUIDE) — их текст не менялся в дельте, а
+  ребейз не касается ни одного файла, на который они ссылаются; это явно
+  зафиксировано в разделе «Унаследовано из r1».
+- Не проверял вручную код #400 (мебель/handle paint order, align-candidates) —
+  это чужой уже смёрженный на `dev` контракт вне предмета #126, тронут только
+  ребейзом, не текстом ТЗ.
+
+## Вывод
+
+High-находок нет. Все Medium/Low из r1 закрыты правкой на конкретных строках,
+подтверждённой кодом. Одна новая Low-находка (L3, устаревшая строка провенанса
+в шапке документа) снята с записью, не блокирует.
+
+**Вердикт: зелёный.** ТЗ #126 готово к переводу в «Готово к разработке» (DoR
+§2.5): AC пронумерованы и доказуемы, миграция/compatibility, touch, i18n,
+performance и откат названы явно, открытых продуктовых вопросов нет.