docs: review document for #220

Issue: #220
User-Visible: no

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

@@ -0,0 +1,185 @@
+# SPEC-REVIEW-220-r2
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/220
+- **ТЗ:** `docs/specs/220-space-tab-reorder.md` (текущий коммит `e57e1d9`,
+  «docs: close M1 and M2 from the spec review of #220»)
+- **Ревьюер:** Claude (роль «ревьюер ТЗ», PROCESS.md §2.4)
+- **Заход:** r2 · блокирующих циклов 1/4
+- **Вердикт:** жёлтый · High: 0 · Medium: 1 (в скоупе задачи)
+
+## Скоуп ревью
+
+Разбор по дельте (PROCESS.md §2.9, #214), а не заново — с одним расширением.
+Раунд r1 закрыл два Medium (M1, M2) точечной правкой текста ТЗ, но правка по
+M2 не косметическая: §8.3 полностью переписан — вместо «хранить якорь в
+`settings`» теперь «материализовать привязку в той же записи». Это смена
+контракта поведения (новая нормативная процедура записи конфигурации), а не
+переформулировка факта, поэтому именно этот кусок разобран полно — включая
+сверку с кодом `devices.ts`/`houseplan-card.ts` заново, а не по памяти r1.
+Остальное (§1–§7, §10–§16, AC1/2/4/5/6/7/8, мутационный гейт вне AC3, план
+автотестов) дельта не касается — унаследовано из r1 без повторной проверки
+(раздел ниже).
+
+## Как проверялось
+
+1. Найден вердикт r1 в комментариях issue #220
+   (`https://github.com/Matysh/houseplan-card/issues/220#issuecomment-5361187761`,
+   `2026-08-20T20:17:49Z`) и документ `docs/reviews/SPEC-REVIEW-220-r1.md`,
+   зафиксировавший ТЗ на коммите `0fd2331`.
+2. Объявлена дельта: `git diff 0fd2331..HEAD -- docs/specs/220-space-tab-reorder.md`
+   (HEAD = `e57e1d9`). Дельта — 58 вставок / 19 удалений, только §8.3, §9, AC3,
+   мутационная таблица, §17.3; остальные разделы файла побитово не менялись.
+3. Комментарий владельца о закрытии r1 называет коммит `d0a5bfa` — такого
+   объекта в репозитории нет (`git cat-file -t d0a5bfa` → `fatal: Not a valid
+   object name`), вероятно переписан ребейзом веток issue. Не доверяю
+   названному SHA, проверка сделана по факту — прямым диффом файла между
+   `0fd2331` (зафиксирован в документе r1) и текущим `HEAD` (`e57e1d9`,
+   единственный коммит после `c3278dd`/review-doc r1, который трогает файл
+   ТЗ) — содержимое дословно совпадает с тем, что владелец описал в
+   комментарии, расхождение только в имени SHA в тексте комментария.
+4. Каждая находка r1 (M1, M2) сверена не по заявлению автора, а по строке
+   текста ТЗ — см. таблицу «Закрытие раунда r1».
+5. `docs/TOUCH-SUPPORT.md` (§153–165, «Documentation rule») прочитан повторно,
+   построчно сверена ровно та формулировка ярлыка, которую требует правило.
+6. Новая нормативная процедура §8.3 (материализация в той же записи) сверена
+   с фактическим кодом разрешения `firstSpaceId`:
+   `src/devices.ts:1045-1065` (`resolveExplicitMarkerPlacement`, включая ветку
+   `manualRoomWithoutArea`, ранее не разбиравшуюся отдельно ни в issue, ни в
+   r1) и `src/devices.ts:1249` (виртуальный маркер). Оба случая подтверждают
+   общий критерий ТЗ «нет ни `area`, ведущей в пространство, ни собственного
+   `space`» — он корректно обобщает все три ветки кода, включая ветку
+   marker-без-HA-area из #3, которую ТЗ не называет по номеру, но покрывает
+   по содержанию.
+   Также проверено третье использование `firstSpaceId`
+   (`src/houseplan-card.ts:18818`, черновик маркера в диалоге) — это
+   непостоянный preview, не пишется в конфиг, к обязательству §8.3
+   («ни один маркер не меняет `space`») не относится: диалог всегда
+   пересчитывает его заново по актуальной модели.
+7. AC3 (единственный AC, задетый дельтой) перепроверен на однозначность и
+   доказуемость с учётом новой формулировки. AC1, AC2, AC4–AC8 дельтой не
+   задеты — унаследованы из r1 без повторной проверки.
+
+## Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где это видно |
+|---|---|---|
+| **M1** — нет обязательной touch-классификации по `docs/TOUCH-SUPPORT.md` §153 | В §9 добавлена строка **`Touch editor: not exposed.`** с обоснованием (вкладки живут во View, где переключение — fully supported/release-blocking; жест на вкладке рискует съесть тап) и отдельно — что safety floor §69 соблюдён по построению | `docs/specs/220-space-tab-reorder.md:156-165`; формулировка ярлыка совпадает с требуемой буква в букву (сверено с `docs/TOUCH-SUPPORT.md:160-162`) |
+| **M2** — §9 отрицал новое поле конфигурации, §8.3/§17.3 предполагали якорь в `settings` | §8.3 переписан: вместо якоря — материализация существующего поля `marker.space` в той же записи, что и порядок; §9 теперь говорит «новых полей конфигурации не появляется... см. §8.3», и это уже не противоречит остальному документу, а согласуется с ним; §17.3 переписан в терминах материализации, старое предположение про якорь явно помечено отвергнутым | `docs/specs/220-space-tab-reorder.md:107-139` (§8.3), `:167-170` (§9), `:268-271` (§17.3); AC3 (`:202-206`) и мутационная таблица (`:240-241`) синхронно обновлены под новую механику |
+
+Обе находки закрыты не декларативно: правка убирает саму причину
+противоречия (M2) и добавляет предметное содержание, а не просто ярлык (M1).
+Новых полей конфигурации в изменённом варианте действительно не появляется —
+проверено по коду: материализация пишет уже существующее поле маркера
+`space`, `CONFIG_SCHEMA` и `scripts/config-field-registry.mjs` в этой задаче
+не упоминаются как изменяемые, и это утверждение больше не противоречит §9.
+
+## Унаследовано из r1
+
+Принято без повторной проверки в этом раунде — проверено и подтверждено в
+`docs/reviews/SPEC-REVIEW-220-r1.md` на коммите `0fd2331`, дельта эти разделы
+не касается:
+
+- Персона, сценарий и job `docs/SCOPE.md` J6 (§1–§2 ТЗ).
+- Продуктовые решения владельца §4 (тач/права/клавиатура) и их соответствие
+  owner-decision в комментариях issue.
+- §8.1, §8.2, §8.4, §8.5 контракта (порог 4px, `.tabadd`, запись через
+  `_writeConfig`/`expected_rev`, `swipeTarget`, тост о числовом `floor`).
+- AC1, AC2, AC4, AC5, AC6, AC7, AC8 — однозначность и способ доказательства.
+- Мутанты `tab-reorder-not-persisted`, `tab-reorder-eats-click`,
+  `tab-reorder-ignores-pointer-type` (не переписаны в дельте).
+- §11 (риски), §15 (release-артефакты), §16 (откат) кроме уже отражённых в
+  дельте формулировок.
+- Low-находка r1 про `.tabedit`/`pointerdown` — оставлена без правки
+  экспертным решением r1 («реализационная деталь, накрываемая кодревью»),
+  дельта её не касается, пересматривать нет причины.
+
+## Находки
+
+### M3 — Medium, в скоупе. §17.3 маркирует нормативное требование как «свободно меняемое»
+
+Заголовок §17 — «Принятые предположения (**техническое, менять свободно**)».
+Пункт 3 внутри него (`docs/specs/220-space-tab-reorder.md:268-271`) гласит:
+«Материализация привязки (§8.3) выполняется в том же `config/set`, что и
+порядок, а не отдельной записью: две записи дали бы окно, в котором порядок
+уже новый, а привязка ещё старая».
+
+Но именно это же самое требование в §8.3 названо не предположением, а
+**«Норматив»** (`:113`, буквально это слово стоит заголовком абзаца) — и оно
+уже вошло в тестируемый контракт: AC3 требует «маркер получает явное `space`
+**в той же записи**» (`:203-204`), а мутант `reorder-skips-materialization`
+(`:240`) специально ловит его нарушение.
+
+**Почему это не формальность.** §17.3 своим же текстом объясняет, почему
+атомарность нельзя менять свободно: если порядок и материализация уйдут
+двумя разными записями `config/set`, то в промежутке между ними
+`firstSpaceId = model[0]?.id` уже пересчитан по новому порядку — маркер,
+ещё не материализованный, в это окно резолвится в **не то** пространство.
+Если исполнитель прочитает заголовок §17 буквально («менять свободно») и
+раздельными записями, а не заголовок §8.3 («Норматив»), он получит ровно тот
+риск, ради которого писан весь раздел 8.3 и ради которого задача оценена в
+сложность 4/10 (см. §11, риск №2 «Маркеры уезжают» — главный риск задачи).
+Опасность усугубляется тем, что при таком (неверном) выборе реализации
+собственный юнит-тест AC3, написанный тем же исполнителем под ту же
+(неверную) модель, скорее всего будет проверять «согласованность после двух
+записей», а не «атомарность одной записи» — то есть мутационный гейт
+`reorder-skips-materialization` перестанет быть надёжным барьером именно в
+том сценарии, для которого его писали.
+
+**Чем закрывается:** убрать пункт 3 из §17 (он не является свободным
+предположением — норма уже сформулирована в §8.3 как обязательная и
+проверяется AC3), либо явно пометить его как исключение из «менять свободно»
+с отсылкой на §8.3/AC3. Правка текстовая, AC и скоуп не меняются.
+
+## AC — что перепроверено дельтой
+
+| AC | Задет дельтой | Однозначен | Доказательство | Комментарий |
+|---|---|---|---|---|
+| AC1, AC2, AC4, AC5, AC6, AC7, AC8 | нет | — | — | унаследованы из r1 без повторной проверки |
+| AC3 | да | да | `unit` (`buildDevices` + запись) | формулировка усилена («маркеры с `area`/`space` — побитово прежними»), однозначна и доказуема; ровно вокруг него — находка M3 (не сам AC, а соседний раздел §17.3, который создаёт риск неверной трактовки реализации, проверяемой этим же AC) |
+
+## Что проверено и корректно
+
+- M1 и M2 закрыты предметно, а не декларативно — см. таблицу выше; §9 больше
+  не противоречит §8.3.
+- Новая механика §8.3 (материализация) корректно обобщает все три реальных
+  пути резолюции `firstSpaceId` в `devices.ts`, включая ветку
+  `manualRoomWithoutArea` (маркер ручной комнаты без HA area, контракт #3),
+  которую ни issue, ни r1 не разбирали пофамильно — критерий «нет `area`,
+  ведущей в пространство, и нет собственного `space`» покрывает её без
+  исключений.
+- Материализация не задевает preview-путь черновика маркера
+  (`houseplan-card.ts:18818`) — он не персистентный, обязательство §8.3 на
+  него не распространяется, и в ТЗ об этом ничего лишнего не заявлено.
+- Новая пара мутантов (`reorder-skips-materialization`,
+  `materialization-touches-bound-markers`) корректно закрывает ровно два
+  направления ошибки материализации (пропуск и избыточность), заменив собой
+  устаревший `first-space-follows-order`.
+- Владелец сам в комментарии честно отделил решение от подгонки: назвал две
+  альтернативы снятия противоречия M2 («признать поле» / «обойтись без
+  него») и выбрал вторую с объяснением — это ровно то, что PROCESS требует
+  от продуктовых/технических решений, выносимых в ревью.
+
+## Чего не проверял
+
+- Не проверял реализацию — кода ещё нет, это ревью ТЗ.
+- Не запускал `tsc`/`npm test`/`npm run build` — класс изменения C
+  (документация), гейты неприменимы, как и в r1.
+- Не пересматривал разделы, не тронутые дельтой (§1–§7, §10–§16, AC1/2/4-8,
+  три из пяти мутантов) — они унаследованы из r1 на коммите `0fd2331`, дельта
+  их не меняла ни байтом.
+- Не проверял точность SHA `d0a5bfa`, названного владельцем в комментарии о
+  закрытии r1 — объект не существует в репозитории; вместо этого верификация
+  сделана по прямому диффу файла до `HEAD` (`e57e1d9`), см. «Как
+  проверялось», п.3. Расхождение SHA не влияет на вывод ревью, но зафиксировано
+  как наблюдение.
+
+## Итог
+
+Обе находки r1 закрыты предметно. Правка §8.3 (материализация вместо якоря)
+— смена контракта поведения, а не косметика, и именно в ней найдена новая
+находка M3: §17 маркирует как «свободно меняемое» требование, которое сам же
+документ в §8.3 называет обязательным норматив и проверяет тестируемым AC3.
+Находка в скоупе, чинится точечной правкой текста (перенос/каветирование
+пункта 17.3), без изменения AC, скоупа или контракта поведения. High-находок
+нет. Вердикт — жёлтый: правка проходит третий цикл ревью ТЗ (заход r3, лимит
+циклов ревью ТЗ — 4).