docs: review document for #244

Issue: #244
User-Visible: no

docs/reviews/SPEC-REVIEW-244-r1.md

@@ -0,0 +1,165 @@
+# SPEC-REVIEW-244-r1
+
+- Issue: [#244](https://github.com/Matysh/houseplan-card/issues/244) — «Маркеры остаются привязанными к удалённому id пространства и пропадают с плана; Optimize такую ссылку не лечит»
+- Этап: ТЗ на ревью (PROCESS.md §2.4)
+- Заход: r1 · блокирующих циклов израсходовано 0 из 4
+- ТЗ: `docs/specs/244-orphan-space-references.md` (446 строк), коммит `9c3c7e1b378b9bef9bcb02b9a8c4ef725c99a041`
+- Ветка: `issue/244-orphan-space-references`, `HEAD` на момент ревью = `9c3c7e1`
+- Метки issue на входе: `bug`, `P2`, `S4-spec-review` (лёгкий/короткий трек не применяются — файл ТЗ обязателен, он и создан)
+
+## Скоуп ревью
+
+Первый заход. Разбор полный: сценарий, продуктовая рамка, все 14 AC, термины
+и инварианты §7, контракты Optimize/импорта/удаления/редактора карточки,
+release-артефакты, откат, раздел принятых предположений §18. Диагноз ТЗ (§3)
+сверен построчно с текущим кодом на `HEAD` (не с пересказом автора).
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `PROCESS.md`, тело issue #244 и оба
+   аналитических комментария владельца (подтверждение Q1, затем Q2+Q3),
+   комментарий «Взял: автор ТЗ» и комментарий «ТЗ готово».
+2. Прочитан `docs/USER-GUIDE.ru.md` (разделы про Optimize, `default_floor`,
+   несколько карточек) — сверена терминология: «Оптимизировать планы»,
+   «предпросмотр», «Стартовое/резервное пространство» — ТЗ использует те же
+   слова, не изобретает новые.
+3. Построчно перепроверены все шесть пунктов «Подтверждённого диагноза» (§3)
+   по факту, а не на слово автора:
+   - `resolveExplicitMarkerPlacement()` (`src/devices.ts:1045-1065`) —
+     подтверждено: `(area && areaToSpace[area]) || marker.space || firstSpaceId`,
+     а для `manualRoomWithoutArea` — `marker.space || firstSpaceId` без ветки
+     на Area;
+   - virtual-маркер (`src/devices.ts:1248-1249`) — подтверждено:
+     `m.space || (area && areaToSpace[area]) || firstSpaceId` — `marker.space`
+     действительно приоритетнее Area, ровно как заявлено;
+   - View/kiosk рендер (`src/houseplan-card.ts:16409`,
+     `src/space-render.ts:190`) — подтверждено: фильтр `d.space === space.id`
+     точный, мёртвый id не совпадёт ни с одним пространством — маркер
+     невидим на всех планах, а не «где-то не так»;
+   - `_deleteSpace()` (`src/houseplan-card.ts:14457-14473`) — подтверждено:
+     удаляется только элемент `config.spaces`, `layout`/`markers` не трогаются,
+     запись идёт через общий `houseplan/config/set`, который `layout` не
+     касается вообще (`custom_components/houseplan/websocket_api.py:1123` —
+     `houseplan/config/set` работает с одним стором);
+   - `build_space_merge()` (`custom_components/houseplan/import_export.py:825-995`)
+     — подтверждено: копия всегда получает свежий `space_<old>_<hex>` id,
+     старое пространство при этом способе не удаляется — заявление ТЗ «код не
+     подтверждает, что импорт сам портит корректную конфигурацию» точное, не
+     смягчение;
+   - `resolveInitialSpace()` (`src/initial-load.ts:70-90`) — подтверждено:
+     `hash -> saved -> default -> first`, невалидный `default_floor` тихо
+     проваливается в `first`.
+4. Проверено техническое основание контракта §10 (атомарная запись
+   config+layout под двумя revision-guard'ами и код `conflict`/`space_in_use`)
+   — это не выдумка: тот же паттерн уже реализован для
+   `houseplan/plan/optimize` (`websocket_api.py:1355-1466`, два
+   `expected_*_rev`, единая транзакция под `rt.write_lock`) и для отказа по
+   занятости в `houseplan/plans/delete` (`websocket_api.py:888-931`, код
+   `in_use`). У ТЗ в §15 прямо сказано, что имя новой WS-команды свободно —
+   корректно помечено как техническое решение, не спрятанная догадка.
+5. Проверено техническое основание §8.3 про восстановление `marker.room_id`
+   по «единственной комнате effective Area»: `resolveExplicitMarkerPlacement`
+   отдаёт только `area`/`space`, но клиент уже строит более богатую карту
+   `Area -> {space, room}` (`src/houseplan-card.ts:3551-3555`,
+   `_areaToSpace`), которая площадь для этого способна дать. Контракт
+   реализуем без второго расходящегося resolver, как и требует §7.
+6. Проверена ссылочная семантика импорта одного пространства
+   (`import_export.py:885-990`, ремап `marker.room_id`,
+   `vacuum.segment_map`, `rl_<room_id>`, `layout[*].s`) — контракт §9
+   («применить тот же `id_map` к оставшимся target-ссылкам, если старого id
+   больше нет») ложится на существующий код без противоречий.
+7. `node scripts/check-docs.mjs` — green (диапазон не трогает `src/**`, гейт
+   не обязателен, прогнан для очистки совести).
+8. Класс изменения — C (`docs/specs/**`), `typecheck`/`test`/`build` к
+   ревью ТЗ не относятся и не запускались.
+
+## Находки
+
+### Medium (в скоупе задачи — возвращается автору)
+
+**M1. Не назван пункт DoR «влияние на touch».**
+PROCESS.md §2.5 требует явно назвать влияние на touch по
+`docs/TOUCH-SUPPORT.md` как блокирующий пункт готовности к разработке (View
+и киоск — блокирующие). В ТЗ #244 слово «touch» и «киоск» не встречаются ни
+разу (`grep -in "touch\|киоск" docs/specs/244-orphan-space-references.md` —
+пусто), хотя изменение имеет прямое отношение к View: восстановленный маркер
+после Optimize становится видимым на плане, который открывают в том числе на
+планшете-киоске. Остальные редакторские поверхности (диалог Optimize,
+удаление пространства, поле `default_floor`) подпадают под «desktop-first
+editors, best effort on touch» и, по всей видимости, действительно не требуют
+отдельного контракта — но это должно быть сказано явно, а не восстановлено
+ревьюером по догадке.
+
+*Как воспроизвести отсутствие:* открыть §12 «Данные, compatibility и
+миграция» или §16 «Риски, производительность и security» — оба раздела,
+где такая строка ожидалась бы рядом с «производительность… не нужен»,
+про touch ничего не говорят.
+
+*Почему это не Low:* пункт прямо назван блокирующим в §2.5, и его отсутствие
+делает чек-лист DoR формально невыполненным — не стилистическая придирка.
+
+*Что чинить:* одна-две фразы в §12 или §16: рендер маркера и все три
+изменяемые UI-поверхности не вводят новых интерактивных путей и наследуют
+существующий touch-контракт (View — уже поддерживаемый рендер/тап без
+изменений; редакторы — desktop-first best effort как обычно), либо иной
+явный вывод, если авторы видят особый случай.
+
+## Что проверено и корректно
+
+- Все шесть пунктов диагноза (§3) подтверждены чтением текущего кода, не
+  голословны.
+- Все три продуктовых решения владельца (Q1/Q2/Q3) отражены в контракте
+  дословно и без искажений (§4, §10, §11).
+- Приоритет правил §8.2 (signature → effective Area → detach) корректно
+  наследует пограничные случаи `resolveExplicitMarkerPlacement`, включая
+  `manualRoomWithoutArea` и особый порядок virtual-маркера — я прошёл через
+  оба ветвления вручную и не нашёл случая, где приоритет противоречил бы
+  рантайму.
+- §7 инварианты (1–5) корректно закрывают ровно тот набор проблем, который
+  описан в диагнозе; ничего лишнего не требуется.
+- Раздел §18 «Принятые предположения» — все шесть пунктов действительно
+  технические (не пользовательские) решения: длина stem, поведение
+  tombstone, сохранение unattached layout (обосновано ссылкой на «never
+  delete a user's file on an inference» из `docs/SCOPE.md`), полное удаление
+  позиции при Area/detach, отсутствие отдельного modal, лимит 10 id в UI.
+  Ни один пункт не подменяет продуктовое решение технической догадкой.
+- AC1–AC14 однозначны, у каждого указан способ доказательства (unit /
+  backend / browser smoke / golden / ревью кода), и AC12/AC13 закрывают
+  неизменность входа и производительность отдельно от «счастливого пути».
+- Риски (§16) покрывают именно те точки, где мог случиться ложный автопуш
+  маркера не туда: ограничение сигнатуры, отсутствие переноса координат,
+  атомарность двух Store.
+- Release-артефакты (§17) перечисляют оба changelog, оба User-Guide, три
+  bundle-копии, docs screenshot после `src/**`, `CONFIG-COMPATIBILITY.md`.
+- Открытых продуктовых вопросов к владельцу нет — они закрыты в
+  аналитике/S2 до входа в ТЗ, это не пропуск, а правильный порядок процесса.
+- Продукт: задача закрывает J6 «Keep the plan true as the home evolves» из
+  `docs/SCOPE.md` (устройство не должно тихо пропадать при штатной
+  эксплуатации), никакого расширения скоупа за пределы диагноза не
+  обнаружено — «Не входит» (§6) явно отсекает соседние соблазны (лечение
+  произвольных broken controls, полная миграция full-restore, GC неизвестных
+  layout entries).
+
+## Чего не проверял
+
+- Не проверялась реализация — её ещё нет, стадия чисто ТЗ.
+- Не прогонялись `typecheck`/`test`/`build`/смоки — класс изменения C
+  (docs-only), гейты код-ревью к этому этапу не относятся.
+- Не проверялась точная реализуемость атомарной backend-транзакции глубже
+  чтения существующего кода `houseplan/plan/optimize` — это ревью
+  архитектурного основания контракта, а не написанного кода, которого нет.
+- Не оценивалась производительность на реальных данных — в ТЗ заявлено
+  «MAX_SPACES/MAX_MARKERS/MAX_LAYOUT уже ограничивают объём», это принято на
+  слово как разумная оценка сложности линейного прохода, а не измерено.
+
+## Вердикт
+
+Диагноз точен и перепроверен по коду, все продуктовые решения владельца
+отражены без искажений, AC полны и доказуемы, скоуп не расширен. Единственная
+находка — отсутствие явного пункта про touch/kiosk из обязательного DoR
+чек-листа §2.5; High нет, Medium в скоупе задачи и чинится добавлением одной
+фразы в существующий документ.
+
+**Жёлтый.** Возврат автору на правку M1 в текущем ТЗ (`S3-spec` → доработка →
+повторное ревью, не более 2 циклов сверх этого, если трек останется
+обычным — фактически лимит 4).