docs: review document for #298

Issue: #298
User-Visible: no

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

@@ -0,0 +1,198 @@
+# SPEC-REVIEW-298-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/298
+- **ТЗ:** `docs/specs/298-resize-wall-thickness-carrier.md` @ commit `45066631aee4997ac1f033dbe75918b5b2d439d4`
+- **Этап:** ТЗ на ревью (PROCESS.md §2.4)
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (лимит лёгкого трека не действует — issue без метки `small`)
+- **Ревьюер:** Claude (сессия ревью ТЗ), автор ≠ ревьюер
+
+## Скоуп ревью
+
+Полный разбор — это первый заход, дельты нет. Читаю ТЗ состязательно: ищу, где
+оно невыполнимо, непроверяемо, либо выдаёт догадку за решение продукта.
+
+## Как проверялось
+
+1. `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` — целиком, перед тем как судить.
+2. Тело issue #298 и три комментария (аналитика, занятие, передача ТЗ) — через
+   `gh issue view 298 --json ...` (MCP-инструменты GitHub были недоступны без
+   подтверждения, поэтому использован `gh` CLI).
+3. Полный текст `docs/specs/298-resize-wall-thickness-carrier.md`.
+4. Канонические документы затронутой подсистемы: `docs/RESIZE.md`,
+   `docs/WALL-THICKNESS.md`, разделы `docs/TOUCH-SUPPORT.md` про safety floor
+   редакторов.
+5. Существование артефактов, на которые ссылается ТЗ, проверено в дереве
+   репозитория, а не на слово автора:
+   - `test/fixtures/real-plan-second-floor.json`,
+     `test/fixtures/real-plan-first-floor.json` — существуют;
+   - `LATTICE_NOISE_STEPS` — существует, `src/coordinate-canonicalization.ts:11`,
+     значение `1e-4`, используется в `scripts/model-invariants.mjs` и
+     мутационном гейте;
+   - `rekeyWallsAfterMove()` — существует, `src/wall-thickness.ts:533`; прочитан
+     код: помеченная в issue пропорциональная проекция (`mapPoint`, строки
+     574–582 — `t` считается от старого ребра и переносится на новое без учёта
+     реального соответствия вершин) реально в проде — корневая причина
+     подтверждена чтением, а не поверена на слово;
+   - `off_lattice_coordinate` / `wall_carrier` — существуют как виды нарушений
+     в `scripts/model-invariants.mjs` и таблице `KNOWN` `demo/smoke_edit_walk.mjs`;
+     текущая таблица `KNOWN` (строки 73–87) уже регистрирует ровно те находки,
+     которые AC6 требует убрать (`off_lattice_coordinate`, `wall_carrier` для
+     обеих фикстур), и отдельно `mixed_role_record` — долг #299, который ТЗ
+     прямо исключает из скоупа и просит не трогать в `KNOWN`. Согласуется.
+6. Проверено существование и состояние всех связанных issue, на которые ссылается
+   ТЗ: #253, #277, #289, #291, #293, #297 — закрыты; #299 — открыт, как и
+   утверждает ТЗ (не дубликат, отдельный долг). Ни одна ссылка не битая и не
+   искажена.
+7. Сверены формулировки с канонoм: раздел 3.3 (legacy fallback, запрет
+   proportional-midpoint) согласуется с `WALL-THICKNESS.md` («Legacy entries
+   without endpoints keep the unambiguous whole-key/midpoint fallback and are
+   never split by inventing a length»); раздел 8 (touch safety floor) согласуется
+   с разделом `TOUCH-SUPPORT.md` «Safety floor that still applies to touch
+   editors»; §5 (disabled handle до pointer capture) согласуется с существующим
+   текстом `RESIZE.md` про eligibility и disabled-handle. Ни одного места, где
+   автор выдаёт непроверяемую догадку за факт, не найдено — все нетривиальные
+   технические решения (§12 «Принятые предположения») либо выводятся из уже
+   принятого контракта #277, либо явно помечены как предположение.
+
+**Не проверялось** (стадия ТЗ этого не требует): выполнение гейтов
+`typecheck`/`test`/`build` — кода к задаче ещё нет; сам код ещё не написан.
+
+## Находки
+
+### Medium (в скоупе задачи) — отсутствуют обязательные продуктовые разделы §7.1
+
+`PROCESS.md` §7.1 требует в ТЗ два продуктовых раздела первыми: «какая персона
+встречает изменение, на какой поверхности, в какой момент» и «что человек
+увидит до и после — одной фразой, без терминов реализации». `AGENTS.md`
+повторяет это как обязательное дополнение к §7.1. Ни то, ни другое в тексте
+ТЗ не выделено.
+
+- §1 («Сценарий и подтверждённая причина») называет только техническую цепочку
+  (`rekeyWallsAfterMove()`, endpoint, topology boundary) — персона, поверхность
+  и момент («администратор дома, десктоп, Plan editor / Resize, после
+  нескольких правок за дни») были прямо названы в комментарии аналитики к
+  issue, но не перенесены в сам документ ТЗ.
+- §2 («Пользовательский результат») — единственный кандидат на «одну фразу без
+  терминов реализации», но весь абзац написан в терминах реализации: «wall
+  endpoints», «topology vertex», «lossless correspondence», «config write»,
+  «Undo entry». Читатель без контекста кода не поймёт по нему, что видит
+  пользователь.
+
+Дефект не про содержание решения — оно верное и подтверждено кодом — а про
+форму ТЗ, которая обязана доказывать, что это изменение продукта, а не работа
+над кодом (PROCESS.md §7.1: «ТЗ, которое не может ответить на эти два вопроса,
+описывает работу, а не изменение продукта»). Правка дешёвая: одна-две фразы в
+начале документа, без пересмотра контракта.
+
+**Предлагаемая правка** (пример, автор волен сформулировать иначе):
+> Персона — администратор дома (`docs/SCOPE.md`), поверхность — desktop Plan
+> editor, инструмент Resize; момент — любой ресайз стены, эффект копится
+> незаметно и проявляется через дни на другой стене.
+> Видимо: сегодня после серии обычных ресайзов случайная стена в другом месте
+> плана вдруг рисуется другой толщиной или теряет рабочую ручку ресайза, хотя
+> её никто не трогал. После исправления обычный ресайз либо проходит как
+> раньше, либо (в редком неоднозначном случае) заканчивается тем же самым
+> сообщением об ошибке без изменения плана — но никогда не портит стену, которую
+> пользователь не двигал.
+
+Это Medium: без High-находок вердикт жёлтый, правка делается автором в этом же
+issue, повторного полного разбора не требует — фактическая архитектура решения
+ревью не оспаривает.
+
+### Low (снято ревьюером с записью) — способ доказательства не назван явно у двух AC
+
+DoR-чеклист (`PROCESS.md` §2.5) требует у каждого AC явного указания, чем он
+доказывается (`unit`/`backend`/`smoke`/`golden`/«ревью кода»). У AC1, AC2, AC4,
+AC6, AC8, AC9 это явно есть (или очевидно из перечисленных команд). У **AC5**
+(carrier preflight) и **AC7** (preview/commit/Undo атомарны) отсутствует строка
+«Доказательство: …» — способ проверки не назван словом, хотя из контекста
+понятен: AC5 — чистая функция преflight из §4, доказывается unit-тестом; AC7 —
+поведение pointer-жеста, доказывается production-bundle smoke по аналогии с уже
+существующим `demo/smoke_room_resize.mjs` (см. `RESIZE.md` «Verification»).
+
+Снимаю без возврата на цикл: неоднозначности в том, *что* проверяется, нет — их
+формулировки уже настолько конкретны (перечислены positive/negative кейсы,
+атомарность записи/Undo), что тип теста восстанавливается однозначно. Автору
+стоит добавить явную строку при реализации ради единообразия с остальными AC,
+но это не блокирует переход в «Готово к разработке».
+
+### Low (снято ревьюером с записью) — разделы «UX» и «i18n» не выделены явно
+
+`PROCESS.md` §7.1 перечисляет UX и i18n как обязательные разделы. В документе
+нет разделов с этими заголовками; содержание по факту размазано — UX-семантика
+(disabled handle, единственное существующее сообщение об ошибке, отсутствие
+нового UX) описана в §5 и подтверждена явным предположением №3 в §12; i18n
+покрыт тем же предположением №3 («новый persisted reason или новый UX в этой
+задаче не вводится» ⇒ новых ключей нет).
+
+Снимаю: по существу оба вопроса закрыты (нет нового текста, нет новых ключей),
+и это прямо написано, просто не под ожидаемым заголовком. Chisto формальный
+момент, не влияющий на проверяемость AC.
+
+## Что проверено и корректно
+
+- Корневая причина в §1 подтверждена чтением `rekeyWallsAfterMove()` —
+  реальный код делает ровно ту пропорциональную проекцию (`mapPoint`), о которой
+  говорит issue и ТЗ; это не пересказ чужих слов.
+- Оба воспроизведения (AC1/AC2) содержат точные числа из issue
+  (`-85 → -82.457`, `59.538`, вершины `17/52/57/101`) и совпадают с текстом
+  issue буквально — не пересказаны с искажением.
+- Скоуп «не входит» (§6) корректно исключает #289/#299 (смешанные роли),
+  ADR #282 (integer storage), eligibility/#277/#293 (topology) — все со ссылками
+  на реально существующие issue в правильном статусе.
+- AC6 корректно ссылается на реальную структуру `KNOWN` в
+  `demo/smoke_edit_walk.mjs` и требует убрать только те два вида находок, которые
+  там сейчас действительно зарегистрированы для этой пары фикстур, не трогая
+  независимый долг #299 (`mixed_role_record`) — проверено построчно.
+- Ни одна ссылка на связанные issue (#253, #277, #289, #291, #293, #297, #299)
+  не оказалась битой, дублирующей или искажающей состояние (закрыт/открыт).
+- Технические решения раздела 3 (correspondence-таблица, конфликт нескольких
+  destinations, атомарное разбиение длинной записи) и раздела 4 (carrier
+  preflight по всему span, не только по концам/midpoint) не изобретают новое
+  поведение — они последовательно продолжают уже принятый контракт #277 и модель
+  данных `WALL-THICKNESS.md` («exact endpoints… independent of whichever room
+  topology later happens to split the same straight line»), а не выдают догадку
+  за факт.
+- Раздел 3.3 (legacy fallback, fail-closed при неоднозначном whole-edge
+  соответствии) прямо согласован с уже задокументированным в `WALL-THICKNESS.md`
+  запретом «изобретать длину» для legacy-записей без `a/b`.
+- Touch/safety floor в §8 корректно ссылается на существующий раздел
+  `TOUCH-SUPPORT.md` «Safety floor that still applies to touch editors», а не
+  придумывает новое правило.
+- Откат (§10) реалистичен: чистый revert коммита, миграции нет, потому что схема
+  не меняется — согласуется с §8.
+- Продуктовых вопросов владельцу в этом ТЗ нет, и по факту разбора это
+  оправданно: спорные места (§5 п.7 про предсказуемый конфликт до pointer
+  capture) читаются как следствие уже принятого принципа fail-closed
+  (`RESIZE.md`: «If neither step is safe, the handle remains… disabled»), а не
+  как новое расширение eligibility — само ТЗ явно исключает изменение eligibility
+  из скоупа (§6 «Не входит»), и текст §5 п.7 с этим не расходится при точном
+  чтении («если конфликт можно доказать в eligibility» — то есть уже
+  существующими проверками, а не новыми).
+- Ни один найденный ранее класс дефектов (#253, #258/#259, #287/#289) не
+  игнорируется задним числом — ТЗ явно проговаривает отличие от каждого в
+  разделе «Почему это дорого» issue и в §6/§12 ТЗ.
+
+## Чего не проверял
+
+- Само исправление (кода ещё нет — задача в `S4-spec-review`, реализация не
+  началась). AC1–AC9 разбирались на выполнимость и проверяемость формулировки,
+  а не на то, будущий код им действительно удовлетворит — это работа
+  код-ревью.
+- Полный текст `docs/CANVAS.md` — раздел на который ссылается ТЗ, но задача не
+  меняет canvas-рендер напрямую (только данные, которые он потребляет); беглая
+  проверка показала, что ссылка не противоречит модели данных `walls[*]`.
+- Все шесть комбинаций `smoke_edit_walk` не прогонялись — на этой стадии нет
+  кода для прогона; сверка ограничилась статическим соответствием таблицы
+  `KNOWN` тому, что требует AC6.
+- Производительность (AC9) — числовые бюджеты `RESIZE.md` не пересчитывались,
+  только сверено само требование «сохраняется p95 budget» с текстом канона.
+
+## Вывод
+
+Единственная содержательная находка — Medium, отсутствие обязательных
+продуктовых разделов §7.1 (персона/поверхность/момент + «одна фраза без
+терминов реализации»). Остальное — два Low, снятых с запиской. High-находок
+нет: контракт технически выполним, каждый AC проверяем, ни одна ссылка не
+искажена, ни одна догадка не выдана за решённый факт. Вердикт жёлтый —
+формально из-за Medium, а не из-за сомнений в самом решении.