docs: review document for #290

Issue: #290
User-Visible: no

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

@@ -0,0 +1,265 @@
+# SPEC-REVIEW-290-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/290
+- **Этап:** spec (PROCESS.md §2.4)
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
+- **Ревьюируемый артефакт:** `docs/specs/290-near-axis-authoring-and-repair.md`,
+  ветка `issue/290-near-axis-authoring-repair`, коммит `dcdb7565`
+- **Вердикт:** жёлтый
+
+## Скоуп ревью
+
+Issue не помечен `small`/`trivial` → обычный трек, ТЗ обязано жить файлом в
+`docs/specs/`. Диапазон изменений от `origin/dev` — ровно один файл
+(`docs/specs/290-near-axis-authoring-and-repair.md`, класс C), что и требуется
+на этапе `S4-spec-review`: продуктовый код не тронут.
+
+Задача — часть цепочки #284 (канонизация координат записи) → #279 (устойчивый
+рендер почти-ортогонального T-стыка) → #290 (не создавать такую геометрию
+молча и дать явный способ её исправить). Продуктовая рамка по `docs/SCOPE.md`
+— J6 («Keep the plan true as the home evolves»): администратор должен видеть и
+контролировать изменение геометрии, Optimize обязан явно сообщать о
+lossy-исправлениях. Это совпадает с собственной оценкой аналитика в issue.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `PROCESS.md` (жизненный цикл, §7.1, §8),
+   `AGENTS.md`.
+2. Прочитано тело issue #290 целиком и все три комментария (аналитика,
+   продуктовые вопросы с defaults, ссылка на готовое ТЗ).
+3. Прочитан весь текст `docs/specs/290-near-axis-authoring-and-repair.md`
+   (313 строк).
+4. Сверено с каноническими документами задетых подсистем: `docs/CANVAS.md`
+   (снап/Shift/hover-resolver, барьер канонизации координат записи, «Оптимизировать
+   планы»), `docs/RESIZE.md` (safe-resize contract, eligibility, pure pipeline,
+   Undo), `docs/WALL-THICKNESS.md` (модель `walls[]`, `wallKey`, rekey после
+   Resize/Optimize), `docs/USER-GUIDE.ru.md` (текущий текст диалога
+   «Оптимизировать планы» и его таблица операций).
+5. Прочитаны связанные issue #279 (полная переписка причины/фикстуры),
+   #277 (safe Resize), #248 (идемпотентность Optimize), #284 (канонизация
+   записи) — все указаны в ТЗ как связанные.
+6. Сверены существующие тестовые фикстуры: `test/fixtures/279-near-orthogonal-junction.json`,
+   `test/fixtures/real-plan-second-floor.json`, `test/fixtures/coordinate-canonicalization.json`
+   — на предмет соответствия тому, что AC4 называет «minimized fixture из #284».
+7. Прочитан существующий продуктовый код на предмет уже введённой константы
+   `MULTI_WALL_NEAR_ORTHOGONAL_MAX_DEGREES = 0.25` (`src/wall-thickness.ts:84`),
+   чтобы проверить, не конфликтует ли предлагаемый общий `NEAR_AXIS_MAX_DEGREES`
+   с уже существующим порогом того же значения.
+8. Для сравнения формата прочитаны полностью аналогичные недавние ТЗ той же
+   геометрической линии — `docs/specs/277-safe-resize.md` и
+   `docs/specs/279-near-orthogonal-junction.md` — как ориентир на «Риски»,
+   «Rollback» и формат таблицы AC/Доказательство, принятый в этом репозитории.
+
+Гейты (`typecheck`/`test`/`build`) не запускались: на этапе `spec` продуктовый
+код не менялся, диапазон diff состоит из одного файла документации, гоны не
+применимы к этому этапу.
+
+## Находки
+
+### Medium (в скоупе задачи — чинится в этом же ТЗ)
+
+**M1. Отсутствуют обязательные разделы §7.1 «риски» и «откат».**
+
+- **Файл:** `docs/specs/290-near-axis-authoring-and-repair.md`
+- **PROCESS.md §7.1:** «Обязательные разделы ТЗ: сценарий · что человек увидит
+  до и после · проблема · скоуп и не-скоуп · контракт поведения · UX · модель
+  данных и миграция · i18n · критерии приёмки AC1…ACn с указанием
+  доказательства · план автотестов · риски · откат · release-артефакты.»
+- **Что не так:** в документе нет ни одного раздела «Риски» и ни одного
+  раздела/абзаца «Откат/Rollback». Единственное упоминание слова «риск» — в
+  шапке, это число самооценки аналитика (9/10), не анализ. Раздел 11
+  («Release») описывает только трейлеры коммита и требование Linux-артефакта
+  для golden — про то, как вернуть изменение назад (revert commit? нужен ли
+  Labs-флаг? что произойдёт со старыми планами, если поведение окажется
+  ошибочным и придётся откатывать) не сказано ни слова.
+- **Почему это не мелочь:** это не формальность, а действующая практика в этой
+  же геометрической линии задач. Оба непосредственных предшественника —
+  `docs/specs/277-safe-resize.md` (§12.1 «Риски и меры», таблица из 10 строк, и
+  §17 «Release-артефакты и rollback» с явным «Rollback — revert implementation
+  commit... После rollback вернётся широкий нестабильный UI, поэтому release
+  rollback должен быть полным и сопровождаться предупреждением») и
+  `docs/specs/279-near-orthogonal-junction.md` (§5 «Совместимость, риски и
+  performance») — оба содержат этот анализ. У #290 риск выше, чем у #277: здесь
+  вводится **автоматическое изменение геометрии при рисовании без обходного
+  модификатора** (§5.1 ТЗ, п. «Специального modifier bypass нет») — то есть
+  редактор будет менять то, что нарисовал пользователь, без явного согласия в
+  моменте. Ровно такой случай (переопределение поведения без spec на риски)
+  — то, ради чего DoR требует «влияние… названо (или явно «нет»)» и «риски
+  перечислены» (PROCESS.md §2.5).
+- **Воспроизведение:** `grep -niE "риск|откат|rollback"` по файлу ТЗ (кроме
+  строки самооценки в шапке и одного упоминания «downgrade не требует
+  migration» в §9) не находит содержательного раздела.
+- **Требуется:** добавить раздел «Риски» (минимум: ложные срабатывания
+  auto-straightening на длинных почти-диагональных стенах около границы
+  0.25°; двойной подсчёт shared-стены между Optimize-кандидатами; конфликт с
+  параллельно идущими #276/#278/#281, которые тоже трогают Optimize-геометрию,
+  как это явно оговорено в #277 §12.1) и раздел «Откат» (что происходит, если
+  после релиза найден дефект: revert коммита, нет миграции схемы, старые планы
+  с уступом остаются как есть до следующего Optimize).
+
+**M2. AC исходного issue про `npm run invariants` на реальных данных и
+видимое снижение числа почти-ортогональных узлов не перенесён в ТЗ.**
+
+- **Файл:** `docs/specs/290-near-axis-authoring-and-repair.md`, раздел 8
+  (AC1–AC10), в сравнении с AC6 тела issue #290.
+- **Что не так:** issue формулирует шестой критерий приёмки явно: «`npm run
+  invariants` на обоих реальных планах — код 0; после выпрямления уступа число
+  почти-ортогональных узлов уменьшается, и это видно в отчёте Optimize». В ТЗ
+  этому соответствует только AC5, но AC5 требует нулевых нарушений
+  `checkWallKeys`/`checkMixedRoleRecords`/preflight на **минимизированной
+  фикстуре**, а не на «обоих реальных планах», и не требует показать
+  уменьшение числа почти-ортогональных узлов ни в выводе
+  `scripts/model-invariants.mjs`, ни в `OptimizeReport`. Раздел 10
+  («Ожидаемые файлы» → Tests/evidence) тоже не называет
+  `node scripts/model-invariants.mjs --config <...>` явно, хотя PROCESS.md §8
+  делает эту команду обязательной ровно для диффов, трогающих геометрию и
+  ссылки на неё (edges, `wall thickness records`, `open_spans`) — а это ядро
+  задачи #290.
+- **Почему это не формальность:** минимизированная фикстура доказывает, что
+  механизм работает на сконструированном случае; она не доказывает, что на
+  реальном плане (с сотнями узлов, где уже случались #253/#258/#259) число
+  почти-ортогональных узлов действительно падает, а не создаётся новый класс
+  случаев в другом месте плана. Именно необходимость такой проверки на
+  реальных данных и породила #279 (реальный экспорт `44.json`) и практику
+  «второго реального плана» в текущей серии коммитов ветки
+  (`test: add a second real plan to the masonry gate`,
+  `test: one thickness record must not describe two wall roles`) — то есть
+  инфраструктура для анонимизированной проверки на реальных/близких к реальным
+  данным в этом репозитории уже есть и используется соседними задачами.
+- **Требуется:** либо явно перенести требование issue в AC (анонимизированная
+  фикстура, произведённая из реального плана, а не полностью синтетическая
+  «minimized»; факт снижения near-orthogonal-count через
+  `scripts/model-invariants.mjs` до/после Optimize), либо явно и обоснованно
+  сузить критерий с указанием, почему полный реальный план как источник
+  проверки не нужен — так же, как #277 §16 прямо объясняет использование
+  анонимизированной регрессионной фикстуры вместо сырого экспорта. Молчаливое
+  сужение критерия приёмки, сформулированного в issue, без объяснения —
+  ровно тот случай, который ревью обязано ловить.
+
+### Low (снимается либо правится, с записью)
+
+**L1. AC4 неточно называет источник fixture.**
+
+- **Файл:** `docs/specs/290-near-axis-authoring-and-repair.md`, AC4: «Minimized
+  fixture из #284 содержит duplicated shared physical edge `316×1`».
+- **Наблюдение:** узел, который АС4 описывает (`316×1`, координаты
+  `(-1.670833333, 2.95)`/`(-0.354166667, 2.954166667)`), — это **буквально**
+  `test/fixtures/279-near-orthogonal-junction.json` (сверено байт в байт: тот
+  же `node`, те же координаты стен). Issue #284 — про барьер канонизации
+  координат записи (`snap = v => Math.round(v*240)/240`), отдельная задача про
+  другой механизм; она не производила эту fixture. Вероятная причина —
+  смешение «issue #284 как общее исследование, откуда взят пример» и «issue
+  #279 как задача, где эта fixture уже закоммичена». Это не блокирует
+  реализацию (fixture физически существует и полностью подходит под описание
+  AC4), но при написании автотеста первое, что сделает исполнитель — будет
+  искать/создавать fixture «из #284», где её нет.
+- **Решение ревьюера:** Low, не эскалируется. Достаточно поправить ссылку на
+  `test/fixtures/279-near-orthogonal-junction.json` (или явно сказать «тот же
+  случай, что и в #279») при следующей правке файла; отдельного цикла ради
+  этого не требуется.
+
+## Что проверено и признано корректным
+
+- **Обязательные продуктовые разделы §7.1** («сценарий», «что человек
+  увидит») присутствуют и содержательны (разделы 1 и 3); ТЗ отвечает на оба
+  вопроса без терминов реализации в пользовательской части.
+- **Продуктовые вопросы владельцу закрыты.** Ровно три вопроса
+  (выравнивать/предупреждать, поведение Optimize, единый допуск) заданы одним
+  комментарием с defaults, каждый — что человек видит/делает, не техническое
+  решение, выданное за продуктовое. Раздел 2 ТЗ фиксирует все три ответа
+  дословно, раздел «Открытых продуктовых вопросов нет» подтверждён — при
+  чтении issue и ТЗ вместе противоречий не найдено.
+- **Единый допуск и его источник (раздел 4)** сформулирован проверяемо:
+  включительная граница `0.25°`, `316×1` классифицируется, `316×2` — нет,
+  zero-length исключён, инвариантность к endpoint order/winding/scale/theme
+  названа явно. AC1 требует единый source threshold и source-guard против
+  отдельных литералов `0.25` — реализуемо и falsifiable (мутант «порог ниже
+  `0.181315°`» и «strict `<`» в AC9 корректно бьют по обеим сторонам границы).
+  То, что предлагается использовать *то же числовое значение*, что уже занято
+  `MULTI_WALL_NEAR_ORTHOGONAL_MAX_DEGREES` в `src/wall-thickness.ts:84` для
+  другого геометрического сравнения (угол между двумя лучами, а не отклонение
+  одного луча от глобальной оси) — техническое решение, а не путаница: раздел
+  4 явно оговаривает, что renderer может продолжать использовать
+  «эквивалентный dot/sine form for orthogonal pairs», то есть общий источник —
+  это значение допуска, а не буквально один и тот же exported symbol. Это
+  подпадает под «принято предположительно, поменять свободно» и не требует
+  отдельного продуктового решения.
+- **Негативный контракт (диагонали не трогаются)** описан симметрично
+  положительному на всех трёх путях (Walls, Resize, Optimize) и покрыт
+  отдельным AC6 плюс мутантами AC9 «считать room copies как две стены» /
+  «repair only one owner shared wall», что закрывает риск двойного счёта
+  расшаренной стены — именно то, из-за чего первоначальный AC issue выделял
+  этот пункт отдельно.
+- **Совместимость с существующими контрактами подсистем.** Раздел 5.2
+  корректно наследует существующий контракт `docs/RESIZE.md` («только
+  numerically horizontal/vertical edge eligible», epsilon поглощает только
+  storage noise) и не пытается расширить Resize eligibility на near-axis
+  случаи — вместо этого явно направляет их через Optimize (раздел 12, пункт
+  2), что не противоречит ни `RESIZE.md`, ни `WALL-THICKNESS.md`. Раздел 6.1
+  корректно ссылается на существующий канонический механизм ключей толщины
+  (`wallKey`, exact-span lookup) из `WALL-THICKNESS.md` вместо изобретения
+  нового формата хранения — толщина не становится отдельным «candidate»,
+  а reprojection'ится из исправленной геометрии существующим pipeline, что
+  соответствует модели `WALL-THICKNESS.md` («key is computed from
+  lattice-stable endpoints»).
+- **Touch/производительность (раздел 9)** соответствует терминологии
+  `docs/TOUCH-SUPPORT.md` дословно («View/kiosk rendering fully supported» ↔
+  «Fully supported» в таблице `TOUCH-SUPPORT.md`), не расширяет editor parity
+  на touch сверх «best effort», не блокирует pinch/pan/pointercancel —
+  соответствует установленному продуктовому решению («editors are
+  desktop-first»). Заявление «Authoring classifier `O(1)`» и «Optimize pass
+  линейный по edges» не противоречит существующим performance-бюджетам
+  `RESIZE.md` и не заявляет числ, которые нечем будет доказать: сохранена
+  ссылка на «full performance gate обязателен».
+- **Никаких догадок, выданных за факт, не найдено** среди технических
+  утверждений о **существующем** поведении: каждая проверенная ссылка на
+  текущий контракт (Shift 45°, safe Resize eligibility, `wallKey`,
+  барьер канонизации записи #284) точно соответствует канону в `CANVAS.md` /
+  `RESIZE.md` / `WALL-THICKNESS.md`. Пункт раздела 12 («Принятые технические
+  предположения») корректно ограничен вопросами, которые пользователь не
+  наблюдает (какой endpoint двигается при равной безопасности кандидатов,
+  что unsafe-repair не обязан чинить любой ценой) — то есть именно тем
+  классом решений, которые PROCESS.md §7.1 разрешает решать автору/ревьюеру
+  без владельца.
+- **i18n, DoR-чеклист.** Раздел 10 называет оба файла (`en.json`/`ru.json`);
+  конкретные ключи на этапе ТЗ не требуются — сверено с #277/#279, которые на
+  этом же этапе тоже ограничиваются общей фразой «RU/EN i18n» без перечисления
+  ключей.
+- **Файл ТЗ лежит в правильном месте и правильно назван** —
+  `docs/specs/290-near-axis-authoring-and-repair.md`, номер совпадает с
+  номером issue, что соответствует PROCESS.md §2.3.
+
+## Чего не проверял
+
+- **Не проверял, что перечисленные в ТЗ файлы (`src/resize.ts`,
+  `src/align-grid.ts`/`src/plan-optimizer.ts`, `src/houseplan-card.ts`)
+  действительно являются правильными точками внедрения** — на этапе ТЗ кода
+  ещё нет, а раздел 10 явно помечен как «Ожидаемые файлы», то есть прогноз, а
+  не обязательство; проверка предметна для код-ревью.
+- **Не оценивал реальную достижимость перфоманс-бюджетов** («Authoring
+  classifier O(1)», сохранение p95 из `RESIZE.md`) — это будет проверяться
+  performance-гейтом перед бетой, не на этапе спецификации.
+- **Не проверял мутационный registry** (AC9) на предмет технической
+  реализуемости каждого конкретного мутанта в текущей кодовой базе — на этом
+  этапе mutation ids не существуют, это работа код-ревью.
+- **Не запускал никаких гейтов** (`typecheck`/`test`/`build`) — diff этого
+  раунда состоит из одного файла документации класса C, гейты к этапу spec не
+  относятся (см. «Как проверялось»).
+- **Не проверял docs/CONFIG-COMPATIBILITY.md registry на конкретные записи** —
+  ТЗ утверждает отсутствие изменений схемы (раздел 9), новых persisted-полей
+  нет, поэтому реестр совместимости не должен получать новых записей; это
+  утверждение принято на веру как техническое (не продуктовое) без
+  дополнительной проверки скрипта `config-field-registry.mjs`, так как задача
+  явно заявляет «schema не меняется».
+
+## Вывод
+
+Продуктовая часть ТЗ (сценарий, видимый результат, продуктовые решения
+владельца, критерии приёмки как таковые, негативный контракт для диагоналей)
+выполнена тщательно и без домыслов, выданных за факт. Причина жёлтого вердикта
+— два содержательных пробела в обязательных разделах и трассируемости AC:
+отсутствие анализа рисков и плана отката (§7.1), и молчаливое сужение AC6
+issue (проверка на реальных данных, видимое снижение числа
+почти-ортогональных узлов) без объяснения. Оба пункта — Medium в скоупе этой
+же задачи и чинятся правкой того же файла, без нового issue и без обращения к
+владельцу.