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

SPEC-REVIEW-137-r1

• Issue: https://github.com/Matysh/houseplan-card/issues/137
• ТЗ под ревью: docs/specs/137-plan-snap-overlay.md (коммит 0ce28b0, ветка issue/137-plan-snap-overlay)
• Роль: ревьюер ТЗ (не автор), этап S4-spec-review
• Трек: обычный (не small/trivial) — сложность и риск 6/10, новый UX-контракт (hover/preview/commit), несколько поверхностей (SVG layering, resolver, performance); лёгкий/короткий трек корректно не применён
• Цикл: r1/4

Скоуп ревью

Проверялось соответствие ТЗ:

• docs/SCOPE.md — попадание в Core user jobs, отсутствие расширения скоупа;
• PROCESS.md §2.4, §2.5 (DoR), §7.1 (обязательные разделы), §3/§12 (запреты);
• AGENTS.md — классы файлов, ветка, трейлеры коммита ТЗ;
• каноническим документам: docs/CANVAS.md §9 (grid-bound/wall-bound snap), docs/WALL-THICKNESS.md (T-соединения, union тел стен), docs/UX-MODES.md (инструменты Plan-редактора), docs/TOUCH-SUPPORT.md (best-effort editors);
• docs/USER-GUIDE.ru.md — терминология инструментов «Контур комнаты» / «Перегородка», существующие toast-тексты;
• фактическому состоянию кода (src/houseplan-card.ts) — на предмет того, что технические утверждения ТЗ и issue-аналитики не являются непроверенной догадкой, выданной за факт.

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

1. Прочитан весь тред issue #137: аналитика S2 с оценками и defaults D1–D5, решение владельца по D1–D5, комментарий автора ТЗ с открытыми вопросами Q1–Q7 (каждый с предлагаемым default), решение владельца «приняты defaults Q1–Q7», и финальный комментарий «ТЗ готово» со ссылкой на коммит.
2. Сверены обязательные разделы ТЗ (§7.1 PROCESS.md) построчно — таблица ниже.
3. Прочитан код src/houseplan-card.ts: подтверждено существование _snapDrawPoint() (:5936), _renderMarkupLayer() (:16532, вызывается на :14499) и _alignCandidates() (:16003) — ровно те механизмы, которые ТЗ называет текущим состоянием и точкой расширения в §3. Технический диагноз не является голословным.
4. Прочитан docs/CANVAS.md §9.3 (WALL-BOUND: snapToWall для openings, snapPointAlongPoly для точек Split на стене) — ссылка ТЗ §7.4 «тот же класс координат, который уже применяется для wall-bound opening и split points» подтверждена дословно, а не изобретена.
5. Прочитан docs/WALL-THICKNESS.md §3 («shared and T junctions show one continuous body with no internal seams», рендер — union тел стен по комнатам) — подтверждает архитектурную достижимость AC4/Q1: новый отрезок может дать геометрическое T-соединение без дробления существующего сегмента, потому что непрерывность тела стены в T уже обеспечена на уровне рендера, а не на уровне топологии одной комнаты.
6. Прочитан custom_components/houseplan/validation.py — подтверждено, что room_drafts и partitions существуют как реальные поля схемы, а не придуманы для ТЗ.
7. Прочитан docs/USER-GUIDE.ru.md (раздел 8, таблица «Инструменты плана» и src/i18n/ru.json): кнопка называется «Контур комнаты» (markup.add), «Перегородка» совпадает дословно (markup.partition). Существующие toast-тексты toast.contour_cannot_close, toast.contour_min_edges, toast.room_overlap подтверждают ссылку ТЗ §7.5 на «существующий toast» как факт, а не догадку.
8. Прочитан demo/performance/README.md: фикстура large-house-v1 реально имеет 60 комнат / 60 перегородок — числа AC12 и §13.4 переиспользуют существующий профиль, а не изобретают новый масштаб.
9. Прочитан demo/golden/matrix.mjs и demo/golden/harness.mjs: golden-гейт на сегодня параметризует тему только как light/dark (page.emulateMedia({ colorScheme })); отдельного forced-colors emulation-пути в харнессе нет.
10. Проверена запись docs/specs/README.md:84 — строка на #137 добавлена в том же коммите, ссылка issue ↔ ТЗ двусторонняя.
11. Проверены трейлеры и class-принадлежность: git diff --stat origin/dev...HEAD показывает только docs/specs/137-plan-snap-overlay.md и docs/specs/README.md (класс C); коммит 0ce28b0 несёт Issue: #137 и User-Visible: no — корректно для документа ТЗ.

Обязательные разделы (§7.1 PROCESS.md)

Раздел	Есть	Комментарий
Сценарий (персона/поверхность/момент)	✅	§1
Что человек увидит до/после	✅	§2 — по абзацу на «до»/«после», не строго одна фраза (см. Low-2)
Проблема	✅	§3, с указанием конкретных существующих методов
Скоуп / не-скоуп	✅	§4 / §5
Контракт поведения	✅	§6–§8
Модель данных и миграция	✅	§9
UX и i18n	✅	§10
AC1…ACn с доказательством	✅	§12, 14 штук, каждый с типом
План автотестов	✅	§13
Риски	✅	§16
Откат	✅	§17
Release-артефакты	✅	§15

Все обязательные разделы присутствуют и содержательны, не формальные заглушки. Дополнительно есть архитектурный/performance-контракт (§11) и явный блок «принятые технические предположения» (§18), не требуемый §7.1 буквально, но соответствующий духу PROCESS.md §7.1 о записи технических решений.

Находки

Находок уровня High и Medium нет — правки в отдельный issue заводить не требуется.

Low-1 — терминология инструмента «Контур» вместо «Контур комнаты»

Файл: docs/specs/137-plan-snap-overlay.md (используется повсеместно, например §1, §4, §6.1, §7.3, AC1, AC3, AC8)

ТЗ последовательно называет инструмент «Контур». В docs/USER-GUIDE.ru.md:259,283 и src/i18n/ru.json (markup.add) кнопка называется «Контур комнаты». Внутри самого ТЗ сокращение употребляется единообразно и не создаёт неоднозначности для читателя документа. Но §10 ТЗ прямо обязывает будущую правку docs/USER-GUIDE.ru.md, а AGENTS.md требует, чтобы формулировки поведения брались из пользовательского словаря, а не изобретались заново; сокращённое имя, попав как есть в реализацию или в код-ревью, может разойтись с уже принятым пользовательским термином.

Решение ревьюера: Low, не блокирует. При правке docs/USER-GUIDE.ru.md и при код-ревью использовать полное «Контур комнаты» там, где текст обращён к пользователю; в ТЗ можно оставить как есть или поправить одним словом при следующей редакции — на усмотрение автора.

Low-2 — «что человек увидит» длиннее одной фразы

Файл: docs/specs/137-plan-snap-overlay.md:33-39 (§2)

PROCESS.md §7.1 требует «одной фразой, без терминов реализации». Раздел написан двумя короткими абзацами («До:» / «После:»), по одному предложению каждый — по существу требование выполнено (нет терминов реализации, ясно и конкретно), но формально это не «одна фраза», а две. Не влияет на проверяемость AC и не создаёт риска неоднозначности.

Решение ревьюера: Low, не блокирует. Косметическая правка на усмотрение автора.

Low-3 — типы доказательства AC12–AC14 не входят буквально в перечень §2.5

Файл: docs/specs/137-plan-snap-overlay.md:319-328 (AC12–AC14)

DoR (PROCESS.md §2.5) перечисляет типы доказательства как unit / backend / smoke / golden / «ревью кода». AC12 использует performance + ревью кода, AC13 — unit + backend/schema review, AC14 — typecheck + unit + build + documentation review. По существу каждый критерий проверяем: performance ссылается на существующий release-blocking performance_smoke/large-house benchmark (§8 PROCESS.md), а не вводит шестой вид проверки; typecheck/build — существующие обязательные гейты (§8); «schema review» и «documentation review» по факту являются тем же «ревью кода», уточнённым по предмету. Тот же класс находки уже фиксировался как Low в SPEC-REVIEW-123-r1 и не блокировал приёмку.

Решение ревьюера: Low, не блокирует. Можно свести формулировки к пяти каноническим типам при следующей правке ТЗ либо оставить как есть — критерии не теряют проверяемости.

Low-4 — план golden не описывает явно, чем доказывается forced-colours

Файл: docs/specs/137-plan-snap-overlay.md:122-133, 285-288, 373-382 (§6.3, AC2, §13.3)

§6.3 и AC2 требуют, чтобы линия/обычная/активная точка различались в светлой, тёмной и forced-colours теме, а доказательством AC2 указаны unit + golden. Но план golden (§13.3) перечисляет только два кадра — light и dark; demo/golden/matrix.mjs/harness.mjs на сегодня умеют эмулировать только colorScheme (light/dark), отдельного forced-colors emulation-пути в харнессе нет. Формально ТЗ не лжёт — оно не утверждает, что forced-colours проверяется golden-кадром, а «unit + golden» как раз позволяет прочитать это как «visual layering — golden, а forced-colours CSS-правило — unit». Но явно эта граница не проведена, и читатель может ожидать third golden frame, которого план не обещает и харнесс не поддерживает.

Решение ревьюера: Low, не блокирует. При написании ТЗ для code review достаточно, чтобы разработчик явно указал в PR/хендоффе, каким именно тестом (unit CSS-assertion или golden) доказана строка про forced-colours — это станет предметом код-ревью AC2, а не спец-ревью.

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

• Соответствие docs/SCOPE.md: задача закрывает J4 (план без внешнего редактора — точное соединение отрезков без Inkscape) и J6 (плану оставаться точным без микрозазоров при развитии геометрии). Обе строки в статусе Closed — это улучшение внутри уже принятой функциональности, не расширение продукта.
• Легитимность полного трека: сложность/риск 6/10, новый UX-контракт с hover/pointer-state — критерии small (§5 PROCESS.md, сложность ≤3, нет нового UX-контракта) не выполняются; лёгкий/короткий трек корректно не применён.
• Владелец лично принял D1–D5 (аналитика) и Q1–Q7 (ТЗ) — открытых продуктовых вопросов в финальной редакции нет, и это корректно: вопросы заданы batched, каждый с предложенным default, и закрыты явным решением владельца 2026-08-14, а не додуманы автором. Ни одна догадка не выдана за факт без пометки — раздел §18 отдельно перечисляет технически свободные решения и явно фиксирует «нет открытых продуктовых вопросов».
• Технические утверждения о текущем коде (_snapDrawPoint, _renderMarkupLayer, _alignCandidates, wall-bound контракт CANVAS.md §9.3, T-junction union WALL-THICKNESS.md §3, реальные поля room_drafts/partitions) подтверждены чтением исходников и канона, а не являются голословными.
• Не-скоуп (§5) корректно отсекает смежные соблазны: колонны/проёмы как snap-кандидаты, привязка к продолжениям линий/центрам, автоматическое дробление существующей геометрии при T-соединении, полноценный touch hover-паритет, новая схема/backend/i18n — типичные места, где скоуп мог бы незаметно расшириться, явно исключены.
• Touch-контракт сформулирован по канону docs/TOUCH-SUPPORT.md: явное best effort / intentionally degraded (§8 ТЗ), View/kiosk не создают DOM оверлея, tap повторно решает кандидата — соответствует «safety floor» (никакой геометрии без явного commit) дословно.
• AC1–AC14 однозначны, у каждого указан тип доказательства и конкретный наблюдаемый результат (DOM-присутствие, радиус, координата commit, приоритет кандидата, отсутствие записи в config/storage, DOM-порядок, bounded cache/DOM). План автотестов (§13) даёт конкретный маршрут для каждого пункта и явно требует, чтобы «каждый тест умел падать отдельно» (§13.1, список конкретных регрессий, которые должны красить конкретный тест) — критерий, защищающий от неспособного падать теста.
• Release-артефакты (§15) перечисляют реальные файлы, включая docs/CANVAS.md (для wall-bound контракта) и docs/USER-GUIDE.ru.md. Golden принимается только через npm run golden:accept -- --reviewed по полному Linux-артефакту (§13.3) — соответствует §13 PROCESS.md. Perf/golden-artефакты корректно отнесены к пре-релизному, а не implementation-гейту (§8 PROCESS.md, §11.4). Откат (§17) корректно опирается на отсутствие миграции данных: новая геометрия — валидная существующая геометрия и для старой версии кода.
• Трассируемость: docs/specs/README.md:84 обновлён тем же коммитом (0ce28b0), ссылка issue ↔ ТЗ двусторонняя; ветка issue/137-plan-snap- overlay и трейлеры коммита (Issue: #137, User-Visible: no) корректны для документа класса C, который сам не меняет поведение.
• Diff origin/dev...HEAD не содержит ни одного файла класса A — код не тронут до S5-ready, что соответствует правилу №1.

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

• Не проверял реализуемость resolver'а как чистой функции с описанным в §18 API — это по правилам ТЗ свободно изменяемое техническое предположение автора кода, не предмет ревью ТЗ.
• Не запускал автотесты, golden, performance или browser-смоки — на этапе spec это не требуется; существование референсных фикстур (large-house-v1, toast-строк, wall-bound кода) проверено чтением, а не исполнением.
• Не проверял, что forced-colours CSS-правило технически осуществимо в текущей палитре токенов editor accent/contrast (§18 п.6-7) — это открытое для автора кода техническое решение, отмеченное в ТЗ как свободно изменяемое.
• Не проверял корректность конкретных числовых оценок аналитики (7/10 / 5/10 / 6/10, P2) по существу — это поле владельца (PROCESS.md §2.2), и они уже приняты явным решением владельца до написания ТЗ.

Вердикт

Зелёный. High: 0, Medium: 0. Четыре находки Low (терминология «Контур» vs «Контур комнаты»; «что человек увидит» длиннее одной фразы; типы доказательства AC12–AC14 вне буквального перечня §2.5; граница unit/golden-доказательства для forced-colours не проговорена явно) — ни одна не блокирует приёмку, все либо правятся косметически, либо снимаются с этой записью на усмотрение автора без нового цикла.