docs: review document for #220

Issue: #220
User-Visible: no

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

@@ -0,0 +1,213 @@
+# SPEC-REVIEW-220-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/220
+- **ТЗ:** `docs/specs/220-space-tab-reorder.md` (коммит `0fd2331`)
+- **Ревьюер:** Claude (роль «ревьюер ТЗ», PROCESS.md §2.4)
+- **Цикл:** r1/2 (обычный трек — лимит 4, но фиксирую фактический номер цикла)
+- **Вердикт:** жёлтый · High: 0 · Medium: 2 (обе в скоупе задачи)
+
+## Скоуп ревью
+
+Полный разбор — первый цикл. Проверено:
+
+1. `docs/SCOPE.md` — соответствие job J6, персона «администратор плана».
+2. `AGENTS.md`, `PROCESS.md` §2.4, §2.5, §7.1 — обязательные разделы ТЗ,
+   критерии DoR, формат вердикта.
+3. Тело issue #220 и оба комментария (аналитика → `S3-spec`; хендофф ТЗ →
+   `S4-spec-review`).
+4. `docs/USER-GUIDE.ru.md` — терминология «пространство», «вкладка».
+5. `docs/TOUCH-SUPPORT.md` — канонический документ для editor-only фичи с
+   явным touch-решением владельца.
+6. `docs/CONFIG-COMPATIBILITY.md` — паттерн описания новых/условных полей.
+7. Код на `dev` (`src/houseplan-card.ts`, `src/logic.ts`, `src/devices.ts`) —
+   сверка всех фактических утверждений ТЗ (номера строк, сигнатуры,
+   поведение).
+
+## Как проверялось
+
+- Построчное чтение ТЗ (`docs/specs/220-space-tab-reorder.md`, 236 строк)
+  против требуемых разделов §7.1 — все присутствуют: сценарий, что человек
+  увидит до/после, подтверждённая причина, скоуп/не-скоуп, контракт поведения,
+  данные/i18n/a11y/security, performance, риски, AC1…AC8 с доказательством,
+  план автотестов, мутационный гейт, release-артефакты, откат, блок
+  принятых предположений.
+- Каждая техническая ссылка на код (`houseplan-card.ts:15717/6692/3350/4273/
+  1165`, `devices.ts:1058/1063/1249`, `logic.ts:1870-1879`) сверена с текущим
+  `dev` через `grep`/чтение файлов — расхождение только в точных номерах строк
+  (код на `dev` сдвинулся на десятки строк с момента анализа), сама логика,
+  сигнатуры и поведение подтверждены дословно:
+  - `navigationSpaces = fixed.kind === 'valid' ? [space] : model` (`:15673`,
+    рендер вкладок `:15746`) — подтверждает побочный эффект: при активном
+    `floor` (#210) в панели всегда ровно один таб, значит правило AC5 «одно
+    пространство — обработчики не навешиваются» само по себе накрывает и этот
+    случай, отдельной ветки в контракте не требуется;
+  - `firstSpaceId: this._model[0]?.id || ''` встречается дважды
+    (`houseplan-card.ts:3350`, `:4273`), потребляется в `devices.ts` в
+    `resolveExplicitMarkerPlacement` и в ветке virtual-маркера — подтверждён
+    риск, ради которого написан §8.3/AC3;
+  - `swipeTarget(dx, dy, zoom, spaceIds, current, minPx)` (`logic.ts:1870`)
+    принимает `spaceIds` параметром, вызывающий код передаёт
+    `this._model.map((m) => m.id)` (`houseplan-card.ts:5719`) — заявление §8.4
+    «изменений не требует» подтверждено буквально;
+  - `_fixedFloorState` (`:1165`) действительно имеет ветвь
+    `out-of-range-index` — таблица в §«подтверждённая причина» точна;
+  - `_canEdit` (`:845`, серверный `can_write`/`is_admin`) и `_mode` (тип
+    `'view'|'plan'|'devices'|'decor'`, `mode-transition.ts:1`) — разные
+    независимые гварды, оба перечислены в §8.1 корректно и раздельно;
+  - `_writeConfig`/`expected_rev` (`:6702`) — существующий механизм, на
+    который опирается AC6, подтверждён.
+  - Итог: ни одного факта, выданного за решение без опоры на код или на
+    явный owner-decision, не найдено. Все «подтверждённые причины» в ТЗ
+    подтверждаются.
+- `docs/TOUCH-SUPPORT.md` прочитан целиком — раздел «Documentation rule»
+  (строки 153–165) обязывает каждую спецификацию новой editor-фичи явно
+  указать один из трёх ярлыков: `Touch editor: supported` /
+  `best effort / intentionally degraded` / `not exposed`. В ТЗ #220 такой
+  строки нет (см. находку M1).
+- `docs/CONFIG-COMPATIBILITY.md` прочитан как образец того, как описываются
+  новые/условные поля конфигурации, для сверки с находкой M2.
+- AC1…AC8 проверены на однозначность и наличие способа доказательства —
+  все восемь пронумерованы и каждый называет `unit`/`smoke`/`diff` (см.
+  ниже раздел «AC»).
+- Блок §17 «Принятые предположения» проверен на то, что в нём действительно
+  лежат только технические решения (не продуктовые) — да, все четыре пункта
+  такие; продуктовые вопросы (тач, права, клавиатура) закрыты явными
+  owner-decision в §4, что подтверждается вторым комментарием issue.
+
+## Находки
+
+### M1 — Medium, в скоупе. Отсутствует обязательная touch-классификация
+
+`docs/TOUCH-SUPPORT.md` («Documentation rule», строки 153–165) требует, чтобы
+спецификация новой editor-фичи явно называла один из трёх статусов:
+`Touch editor: supported` / `best effort / intentionally degraded` /
+`not exposed`. Задача #220 добавляет новую editor-фичу (перетаскивание
+вкладок), которая по прямому решению владельца (§4.1 ТЗ) на сенсорных
+экранах не работает вовсе — это классический случай `not exposed`. ТЗ нигде
+не содержит этой строки (проверено `grep -n -i "touch editor"
+docs/specs/220-space-tab-reorder.md` → пусто).
+
+**Почему это не формальность.** Правило существует специально для того, чтобы
+touch-ограничение было явным, а не выведенным читателем ТЗ из контекста: §103
+того же документа («Deliberate degradation rule») требует пяти условий,
+включая «touch limitation is deliberate, described in the user-facing
+limitations» — описание в USER-GUIDE запланировано (§15 ТЗ), но сама
+классификация как акт признания решения — нет.
+
+**Чем закрывается:** одна строка вида `Touch editor: not exposed —
+перетаскивание вкладок работает только мышью в редакторах (§4.1, §7)` в ТЗ,
+рядом с §4 или §9. Не требует новых AC и не меняет скоуп.
+
+### M2 — Medium, в скоупе. §9 утверждает «нет нового поля», хотя §8.3/§17.3 предполагают обратное
+
+§9 («Данные») формулирует как решённый факт: *«только порядок элементов
+массива `spaces`. Ни одно поле не добавляется и не удаляется; миграции и
+compatibility-полей нет»*.
+
+Это противоречит собственному §8.3/§17.3 того же документа. Причина
+противоречия не косметическая, а структурная: сегодня «первое пространство»
+физически **не имеет** другого представления, кроме позиции 0 в массиве —
+отдельного поля-якоря или временной метки создания не существует
+(подтверждено чтением `devices.ts` и `houseplan-card.ts`: `firstSpaceId`
+везде вычисляется как `_model[0]?.id`). Значит, чтобы якорь **пережил
+перезагрузку страницы** — а он обязан её пережить, иначе после reload
+`_model[0]` снова укажет на новый первый элемент (тот, что оказался на месте
+0 после перестановки), и AC3 перестанет выполняться при следующей сессии —
+единственный способ сохранить инвариант «маркер не меняет `space`» состоит в
+том, чтобы **где-то записать исходный id постоянно**. Собственный блок
+предположений автора (§17.3) прямо это и предлагает: «хранить якорь в
+`settings` при первой записи конфигурации».
+
+То есть §9 отвечает «нет» на вопрос, который DoR (`PROCESS.md` §2.5: «миграция
+и compatibility-поля решены по `docs/CONFIG-COMPATIBILITY.md`») требует
+решить, а не задекларировать закрытым текстом, который тут же опровергается
+соседним разделом. Технический выбор (само устройство поля) законно оставлен
+автору — но факт «поле, скорее всего, появится» должен быть проговорён в §9,
+а не спрятан только в необязательном для чтения блоке предположений.
+
+**Последствие, если не поправить.** Если реализация действительно добавит
+поле в `settings` (наиболее вероятный путь по собственному §17.3),
+`docs/CONFIG-COMPATIBILITY.md` не получит записи о новом поле, хотя документ
+существует ровно для таких случаев (сравните с трактовкой `known_devices`,
+`custom_fill`, `bg_mode` — все новые поля `settings` в этом файле
+задокументированы отдельным разделом с историей совместимости).
+
+**Чем закрывается:** переформулировать §9 в духе «если якорь потребует нового
+поля в `settings` — оно аддитивно и forward-compatible: старые клиенты его
+игнорируют, при отсутствии поля fallback остаётся прежним (`_model[0]`)», и
+добавить короткую запись в `docs/CONFIG-COMPATIBILITY.md` в том же коммите,
+где появится реализация (не обязательно сейчас, но контракт должен быть
+проговорён в ТЗ, а не отрицаться).
+
+### Low — не является блокирующим, не требует правки
+
+- Гвард начала drag (`pointerdown` по `.tab`, порог 4px) не оговаривает
+  явно элемент `.tabedit` (шестерёнка настройки пространства) внутри вкладки:
+  текущий обработчик шестерёнки — `@click` с `e.stopPropagation()`
+  (`houseplan-card.ts:15754`), но `pointerdown`, скорее всего, будет повешен
+  на `.tab`, а не на кнопку, и `stopPropagation` в `@click` не остановит
+  всплытие `pointerdown`. Реализация почти наверняка и так исключит `.tabedit`
+  из зоны старта drag (иначе шестерёнка перестанет открывать диалог при
+  случайном микросдвиге курсора) — это тривиальная деталь без продуктовой
+  неоднозначности и без риска потери данных. Снимаю без правки: чисто
+  реализационная деталь, накрываемая кодревью.
+
+## AC — проверка на однозначность и доказуемость
+
+| AC | Однозначен | Способ доказательства указан | Комментарий |
+|---|---|---|---|
+| AC1 | да | `smoke` | конкретное поведение: позиция + переживает reload |
+| AC2 | да | `smoke` | порог 4px, оба исхода (клик/drag) проверяемы |
+| AC3 | да | `unit` (`buildDevices`) | явно требует «тест красный до развязки» — хорошая практика |
+| AC4 | да | `unit`+`smoke` | подтверждено чтением кода: `swipeTarget` не требует правок |
+| AC5 | да | `unit`+`smoke` | четыре условия исключения перечислены явно и проверяемо |
+| AC6 | да | `unit`/`smoke` | опирается на существующий механизм `expected_rev` |
+| AC7 | да | `smoke` | «один раз за сессию» — паттерн уже есть в коде (`_onboardingShown`), не изобретён |
+| AC8 | да | `diff` + сверка бандлов | стандартный release-гейт |
+
+Ни один AC не сформулирован расплывчато, у каждого назван корректный способ
+доказательства применительно к тому, что он утверждает.
+
+## Что проверено и корректно
+
+- Персона и job (`docs/SCOPE.md` J6) подобраны верно, сценарий и «что человек
+  увидит» — в пользовательских терминах, без реализации.
+- Продуктовые решения (§4) действительно закрывают все три продуктовых
+  вопроса из аналитики (тач/права/клавиатура) явной ссылкой на owner-decision
+  2026-08-20, а не на догадку исполнителя.
+- Все три места, где порядок массива несёт скрытый смысл (`firstSpaceId`,
+  `swipeTarget`, числовой `floor`), названы, подтверждены по коду и закрыты
+  разделами контракта (§8.3, §8.4, §8.5) — это тот самый риск, ради которого
+  задача вообще была оценена в сложность 4/10, а не 1/10.
+- Технические предположения (§17) действительно технические — ни один пункт
+  не требует продуктового решения владельца, что соответствует правилу
+  «размытое место не додумывается, а выносится владельцу» из PROCESS.md §7.1
+  (здесь размытых продуктовых мест не осталось).
+- Мутационный гейт целится в самые опасные регрессии (потеря записи, съеденный
+  клик, возврат порядко-зависимого `firstSpaceId`, снятие проверки
+  `pointerType`) — это именно те четыре мутанта, которые действительно ловят
+  типовые ошибки такой фичи.
+- Release-артефакты (§15) и откат (§16) присутствуют и покрывают оба
+  changelog, оба USER-GUIDE, отсутствие влияния на golden/perf.
+
+## Чего не проверял
+
+- Не проверял реализацию — её ещё нет (это ревью ТЗ, код не существует).
+- Не запускал никаких гейтов/тестов: на этапе ревью ТЗ гейты (`typecheck`,
+  `npm test`, `npm run build`) неприменимы — класса A/B изменений нет, только
+  документация (класс C).
+- Не проверял `docs/specs/README.md` на полноту сверх наличия строки про
+  #220 — строка присутствует, этого достаточно для этого этапа.
+- Не оценивал абсолютную величину порога 4px и механику «вставка перед» —
+  оба явно помечены автором как «принято предположительно, поменять
+  свободно» (§17.1–17.2), оспаривать нечего: реализация вправе выбрать другое
+  значение без нового цикла ревью.
+
+## Итог
+
+Два Medium-замечания, обе в скоупе задачи, обе устраняются точечной правкой
+текста ТЗ (без изменения AC, скоупа или контракта поведения). High-находок
+нет. Вердикт — жёлтый: автор правит §4/§9 ТЗ (добавляет touch-классификацию и
+снимает противоречие в разделе «Данные»), правка проходит второй цикл ревью
+ТЗ. Задача не `small`, лимит циклов ревью ТЗ — 4 (`PROCESS.md` §4); этот
+раунд — первый из них.