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

SPEC-REVIEW-244-r1

• Issue: #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).