houseplan-card/docs/reviews/SPEC-REVIEW-676-r1.md

SPEC-REVIEW-676-r1

• Issue: https://github.com/Matysh/houseplan-card/issues/676
• Этап: S4-spec-review (ревью ТЗ, PROCESS.md §2.4)
• Трек: полный (метки bug, P1, S4-spec-review; аналитик явно зафиксировал «лёгкий трек: нет» — несколько поверхностей, новый UX-контракт размещения протяжкой и подсказки, сложность > 3).
• Материал: тело issue #676, раздел ## ТЗ (редакция 1, 27.09) + 3 комментария: (1) «Взял» аналитика, (2) полная аналитика — причины 9 дефектов по коду и воспроизведению, решение «латать или переписывать», оценка, (3) факт публикации ТЗ и перевод в S4-spec-review.
• Заход: r1 · блокирующих циклов израсходовано 0 из 4
• Роль: ревьюер ТЗ (не автор)

Скоуп ревью

Первая итерация лестниц (#663) даёт 9 дефектов редактирования, воспроизведённых владельцем и подтверждённых аналитиком чтением кода и браузерным пробником (demo/probe_stairs_bugs.mjs, не гейт). Задача переписывает слой редактирования лестниц (src/stairs-editor.ts, stairs-editor-model.ts, рамка узлов в houseplan-card.ts, подсказка в stairs-view.ts, стили) на box-контракт, общий с декором/мебелью — жесты, курсоры, магнит по краям, диалог свойств. Модель stairs.ts, площадь (#669), навигация по клику в Просмотре и 2.5D не трогаются.

SCOPE-проверка (docs/SCOPE.md): сценарий закрывает J6 («keep the plan true», редактирование объектов плана без регресса в сохранённых данных) и J1/J2 через корректную навигацию/подсказку лестницы в Просмотре. Из «никогда не строить» ничего не задевается; из «партиально покрыто» — touch-эргономика редакторов («best effort», не блокирует).

Как проверялось

1. Прочитаны docs/SCOPE.md, docs/process/REVIEWER.md целиком; по ссылкам конспекта открыт PROCESS.md §2.4, §2.5, §4, §7.1, §7.2 (§2.10 не применялся — это r1).
2. Прочитано тело issue #676 целиком (gh issue view 676 --json body) и все 3 комментария.
3. Прочитан канонический docs/STAIRS.md — контракт К1–К8 ему не противоречит; раздел «Implementation boundary» уже называет stairs-editor-model.ts владельцем чистых Plan-трансформаций — задача продолжает существующее разделение, а не изобретает новое.
4. Сверены обязательные разделы §7.1: Сценарий, Что человек увидит до/после, Проблема, Скоуп/Не-скоуп, Контракт поведения (К1–К8), UX, Модель данных и миграция, i18n, Критерии приёмки AC1–AC11 со способом доказательства, План автотестов, Риски, Откат, Release-артефакты — все на месте; первые два — продуктовые, без терминов реализации.
5. Прочитан код, лежащий в основании диагностики «Проблема», чтобы отделить факт от догадки:
   • src/stairs-editor.ts:2,3,5 — подтверждён импорт и вызов resizeFurnitureTransform, snapFurnitureToWall, furnitureWallSurfacesFor для текущего (дефектного) поведения;
   • src/houseplan-editor-runtime.ts:2214-2223,3333-3334 — подтверждён _markupClick/_suppressClick, объясняющий дефекты 1/2/8 (копия при ресайзе, потеря выделения);
   • src/wall-thickness.ts:222-243 — подтверждено, что fieldToCm/ cmToField/clampWallCm зажимают в [WALL_MIN_CM, WALL_MAX_CM] = не [30, 10000], а диапазон толщины стены — дефект 4 («100 см потолок») реален и назван верно;
   • src/houseplan-card.ts:9097,11069-11088 — подтверждён порядок отрисовки: stairs.renderLayer() стоит ДО _renderWallBodies, _renderTextFrame (декор) — ПОСЛЕ; перенос рамки лестницы к _renderTextFrame (Скоуп) действительно решает дефект 3 и AC7;
   • src/stairs-editor.ts:427-435, src/styles/plan.styles.ts:1595-1605 — подтверждён единый класс hp-stair-resize (nwse-resize для всех 8 узлов) и радиус _gridPitch * 0.65 (единицы плана, не экрана) — дефекты 5 и 9 реальны, К2 их закрывает;
   • src/furniture-placement.ts:7 (FURN_WALL_CELLS = 6) и src/furniture.ts:140-141 (FURN_MIN_CM = 1, FURN_MAX_CM = 10000) — константы контракта (STAIR_MAX_CM = 10000, довод магнита 6 клеток) совпадают с источником, на который ссылается ТЗ;
   • src/stairs-editor.ts:317-323 (undoActiveDrag) — Ctrl+Z во время жеста уже реализован и работает (откат к drag.original, сброс жеста); К6 в этой части не меняет контракт, а фиксирует уже верное поведение;
   • i18n: stairs.fixed_floor, history.stair_move уже существуют во всех четырёх словарях (src/i18n/{en,ru,de,fr}.json) — новые ключи (stairs.tooltip_navigate, history.stair_resize, history.stair_rotate) не конфликтуют и следуют тому же нейминг-паттерну; test/i18n.test.mjs действительно проверяет плейсхолдер-паритет всех словарей — AC10 доказуем как заявлено;
   • golden: 4 существующих сцены лестниц подтверждены (stairs-flat-hover- dark, stairs-flat-normal-light, stairs-flat-selected-light, stairs-isometric-dark, demo/golden/baselines/) — AC9 корректно называет ровно одну меняющуюся и три неизменные;
   • test/stairs-editor-model.test.mjs, demo/smoke_stairs.mjs — второй файл существует и расширяется, первый отсутствует и создаётся с нуля, как и заявлено («новый»). Диагностика «Проблема» и вся фактическая база контракта проверена по коду и подтверждена — ни одного случая догадки, выданной за факт, не найдено.
6. Проверена math-корректность сектора курсора К2 (round(θ/45) mod 8, 0°= +x экрана по часовой): раскладка по секторам (0/4→ew, 2/6→ns, 1/5→nwse, 3/7→nesw) арифметически верна для экранной системы координат (y вниз) и соответствует стандартным CSS-именам диагоналей (SE/NW действительно лежат на диагонали nwse-resize, NE/SW — на nesw-resize).
7. Проверено покрытие исходных 9 дефектов отчёта критериями приёмки — все девять сведены к AC1–AC11 (таблица построена вручную, см. «Что проверено и корректно»).
8. Прогон гейтов не выполнялся — см. «Чего не проверял»: на этапе S4-spec-review кода задачи ещё нет, ветка issue/676-* не создана, рабочее дерево на f1a87fba содержит только исходную (дефектную) реализацию #663.

Находки

Medium (в скоупе) — AC8 не называет проверку двух из трёх ветвей подавления подсказки

Что не так. К8 описывает три причины, по которым подсказка Просмотра НЕ показывается: «Лестница без цели, с битой целью или в карточке с закреплённым этажом». Это ровно три состояния stairTargetState() (missing, deleted, fixed — функция уже существует, src/stairs-editor-model.ts:97-102). AC8 же называет доказательством только одну ветвь: «наведение на лестницу с целью garden даёт … ; уход убирает; лестница без цели подсказки не даёт» — про «битую цель» (deleted) и «закреплённый этаж» (fixed) в столбце «Доказательство» нет ни слова.

Почему это находка, а не придирка. Тултип — новый код (существующий _showTip только переиспользуется, сам вызов на новом обработчике наведения в stairs-view.ts — новый), а не старая, уже покрытая гвардом навигация; то, что stairTargetState для missing/deleted/fixed уже протестирован для навигации (не-скоуп), не гарантирует, что тот же guard правильно подключён именно к вызову тултипа — это ровно тот код, который эта задача добавляет. Реализация, читающая AC8 буквально, закроет её одним смоук-шагом («нет цели → нет подсказки») и формально будет «выполнена», оставив нерабочим (или непроверенным) случай удалённой цели и случай floor-карточки — то есть именно тот регресс, который К8 прямо запрещает, останется недоказанным.

Чем закрывается в скоупе задачи. Расширить столбец «Доказательство» AC8 (или явно расщепить его на AC8a/AC8b) двумя смоук-утверждениями: наведение на лестницу с удалённым/несуществующим target_space_id не показывает тултип; наведение на лестницу с валидной целью в карточке, сконфигурированной с floor, не показывает тултип. Технический характер уточнения (какой именно смоук-шаг доказывает уже сформулированное продуктовое поведение) — решение автора ТЗ, не продуктовый вопрос владельцу.

Medium (в скоупе) — риск «рамка перехватывает события под другим инструментом» не привязан ни к одному AC

Что не так. К2 формулирует обязательное условие: «Рамка не перехватывает события под другими инструментами плана». Раздел «Риски» повторяет его же как угрозу регресса: «Рамка в оверлее должна оставаться инертной под инструментами кроме «Выбор»/«Лестница» (К2), иначе перехватит стены». Ни в таблице AC1–AC11, ни в «Плане автотестов» нет ни одной строки, которая бы это проверяла — ни unit, ни smoke.

Почему это находка, а не придирка. Рамка теперь стоит В ВЕРХНЕМ ОВЕРЛЕЕ, выше тел стен (это и есть исправление дефекта 3) — именно геометрическое перекрытие с кликабельной зоной инструмента «Стены» создаёт риск, названный самим автором. DoR требует не просто «риски перечислены», а чтобы задача имела для риска обозначенный путь проверки; риск, названный, но не проверяемый ни одним AC, с равной вероятностью останется бумажным предупреждением и уйдёт в релиз непроверенным — ровно то, ради чего вообще существует колонка «Доказательство».

Чем закрывается в скоупе задачи. Добавить в «План автотестов» один смоук-шаг (или новый AC12): под инструментом «Стены» (или любым другим не-«Выбор»/не-«Лестница» инструментом) клик в точке, где визуально находится узел выделенной лестницы, должен воздействовать на стену/плоскость плана, а не быть перехвачен рамкой лестницы (pointer-events: none на оверлее вне активных инструментов — assert через elementFromPoint или эквивалент, как уже делает demo/smoke_stairs.mjs для гвардов навигации).

Low — не задан tie-break для равного протяжки по обеим осям (снимаю сам)

К1: «длинная ось протяжки — ось подъёма» для случая |B−A|x == |B−A|y (ровно диагональная протяжка под 45°) не называет, какая ось считается доминирующей — угол результата не определён детерминированно текстом ТЗ. Не блокирует: это событие нулевой меры для реального указателя, а в кодовой базе уже есть прецедент детерминированного тай-брейка при похожем равенстве (resizeFurnitureTransform, src/furniture.ts: «Equal deltas deliberately prefer width for deterministic ties»). Снимаю сам с рекомендацией придерживаться того же соглашения (например, при точном равенстip предпочитать +x/-x перед +y/-y) — чисто техническое решение, владельца не касается.

Что проверено и корректно

• Обязательные разделы §7.1 присутствуют и в правильном порядке; «Сценарий» и «Что человек увидит» — продуктовые, без терминов реализации, называют персону 1/2 по docs/SCOPE.md.
• Все 9 дефектов из «Что происходит» сведены к контрактам К1–К8 и покрыты критериями: 1/2 → К6/AC6; 3 → К2/AC7; 4 → К7/AC5; 5 → К2/AC4; 6 → К4/AC3; 7 → К8/AC8; 8 → К3+К4/AC3+AC11; 9 → К2/AC7. Ни один исходный дефект не остался без критерия приёмки.
• Диагностика «Проблема» и числовые/структурные утверждения контракта (константы, файлы, существующие функции) сверены с кодом и подтверждены фактом, а не приняты на веру — см. «Как проверялось», п. 5.
• AC1, AC2, AC3, AC9, AC10, AC11 однозначны, называют способ доказательства и воспроизводимы буквально по тексту.
• Модель данных не меняется, откат («revert коммитов, данные читаются обеими версиями») корректен и согласован с docs/CONFIG-COMPATIBILITY.md (задача явно заявляет, что не задевает этот документ, и это верно — персистентная схема стула не меняется).
• Touch учтён минимально необходимо: К1 включает touch/pen в размещение (курсорный жест общий для указателя любого типа, ничего специфичного не требуется), AC8 явно фиксирует, что касание не показывает тултип (наведения нет) и не меняет навигацию касанием — единственное туч-чувствительное новое поведение проговорено. Экраны редактора вне View — best effort по docs/TOUCH-SUPPORT.md, отдельного пункта не требуется.
• «Принято предположительно» корректно фиксирует семь технических развилок как решённые автором (направление подъёма, поведение клика, шаг вращения, предел доворота магнита 5°, минимальный размер 30 см, база отсчёта магнита, техническая реализация рамки/подавления клика) — ни одна не является продуктовым вопросом, эскалации владельцу не требовалось и не произошло.
• Release-артефакты (changelog RU+EN, docs/STAIRS.md, руководство, golden, «performance — нет, security — нет») названы явно, а не оставлены подразумеваемыми; бюджет бандла назван риском с конкретным запасом (~2 КБ) и существующим гейтом (bundle:budget).

Чего не проверял

• Не запускал npm run typecheck/npm test/npm run build и node scripts/check-docs.mjs — на этапе S4-spec-review кода задачи ещё нет: ветка issue/676-* не создана, рабочее дерево на f1a87fba содержит только исходную реализацию #663 без единой правки этой задачи. Дифф для этих гейтов просто не существует; они относятся к этапу код-ревью.
• Не запускал demo/smoke_stairs.mjs, node scripts/smoke-select.mjs, golden:verify и инварианты модели — по той же причине: браузерные смоки и golden проверяют код, а на этапе ТЗ проверяемый код не существует. Их прогон целиком ложится на код-ревью после реализации.
• Не проверял demo/probe_stairs_bugs.mjs (пробник аналитика) по существу — он явно не гейт и не идёт в коммит; использовал только код продукта (src/*.ts), который он якобы демонстрирует, и подтвердил утверждения чтением этого кода напрямую (см. «Как проверялось», п. 5), а не доверием к выводу пробника.
• Не проверял #663 и #669 по существу (смежные задачи) — использовал только факт их состояния («работают, не трогаются» / «работает, #669 закрыт») как вводную для не-скоупа этой задачи.

Вердикт

Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 2 → в задаче

---

Материал раунда

• Ветка: dev, коммит f1a87fbad9d4 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 4c3048e855e543bdf5ecf9edbdfed8146f31c648

  git log --all --format='%H %T' | grep 4c3048e855e5
  

• Тело issue: fe591d135b54a53f4596f5e657f3139e5fb064331dbd48ff529b63472395175d
• Вердикт конвейера: yellow · High 0