From 2a6e6b5e960491ef30c1b2ba2eba38b15a719dd9 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 22 Aug 2026 19:00:34 +0000 Subject: [PATCH] docs: review document for #244 Issue: #244 User-Visible: no --- docs/reviews/SPEC-REVIEW-244-r1.md | 165 +++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-244-r1.md diff --git a/docs/reviews/SPEC-REVIEW-244-r1.md b/docs/reviews/SPEC-REVIEW-244-r1.md new file mode 100644 index 00000000..c2eba5ad --- /dev/null +++ b/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__` 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_`, `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).