From 5bebc26aeb4ceda45b40bcd192cc54d47c1931c9 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 07:45:56 +0000 Subject: [PATCH] docs: review document for #137 Issue: #137 User-Visible: no --- docs/reviews/SPEC-REVIEW-137-r1.md | 250 +++++++++++++++++++++++++++++ 1 file changed, 250 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-137-r1.md diff --git a/docs/reviews/SPEC-REVIEW-137-r1.md b/docs/reviews/SPEC-REVIEW-137-r1.md new file mode 100644 index 00000000..f4e212ec --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-137-r1.md @@ -0,0 +1,250 @@ +# 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 не проговорена явно) — ни одна +не блокирует приёмку, все либо правятся косметически, либо снимаются с этой +записью на усмотрение автора без нового цикла.