diff --git a/docs/reviews/SPEC-REVIEW-228-r1.md b/docs/reviews/SPEC-REVIEW-228-r1.md new file mode 100644 index 00000000..950885ca --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-228-r1.md @@ -0,0 +1,151 @@ +# SPEC-REVIEW-228-r1 + +- Issue: [#228](https://github.com/Matysh/houseplan-card/issues/228) — «Проблемы при рисовании плана» +- Этап: spec (PROCESS.md §2.4) +- Заход: r1 · блокирующих циклов израсходовано 0/4 +- ТЗ: `docs/specs/228-plan-drawing-problems.md`, коммит `dca4ed2`, ветка `issue/228-plan-drawing-problems`, HEAD совпадает с этим коммитом +- Вердикт: **зелёный** + +## Скоуп + +Задача объединяет шесть наблюдений владельца по инструменту «Стены»: (1) +отсутствие оси/узла на активном отрезке, (2) зубцы на углах из-за неверного +snap, (3) ложное «незамкнутая комната» при разрывах ≤ несколько см, (4) +диалог удаления комнаты не спрашивает про стены, (5) невозможность создать +комнату из готового замкнутого контура, (6) `Shift` не даёт строгого угла +после snap, а подпись остаётся зелёной при 90,1°. Owner подтвердил на 2026-08-19 +(issue #227) правило бюджета циклов и держит все шесть пунктов в одной задаче +(закрывает J4/J6, `docs/SCOPE.md`). + +Это первый заход — прежнего вердикта нет, деление на дельту (§2.10) не +применяется, разбор полный. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1–§4, §7.1, §7.2, + §12), тело issue #228 и все 4 комментария (аналитика, продуктовые вопросы + с defaults, решения владельца, хендофф «ТЗ готово»). +2. Прочитаны канонические документы подсистемы: `docs/CANVAS.md` (разделы + «Architectural connection overlay», «Planar wall faces», §9 snap/Shift), + `docs/WALL-THICKNESS.md` (модель partitions/openings, диапазон `cm` 1–100), + `docs/TOUCH-SUPPORT.md` (контракт деградации), `docs/USER-GUIDE.ru.md` + (терминология «Стены», «независимая стена», «проём»). +3. Каждое фактическое утверждение раздела 3 ТЗ («подтверждённое текущее + состояние») сверено построчно с актуальным кодом на HEAD: + - `.pathline`/`.vertex` строятся из `path` (`houseplan-card.ts:18658`, + `:18664`), `previewD`/`previewPts` включают курсор (`:18497–18519`) — + подтверждает пункт 1 (нет оси/узла на активном конце); + - `_resolvePlanDrawPoint` (`houseplan-card.ts:6752–6772`): `lock45` + используется только в ветке `this._snapDrawPoint(raw, lock45)`, при + наличии `candidate` возвращается `[...candidate.point]` безусловно — + подтверждает пункт 6 дословно; + - `is45(deg, tol = 0.5)` (`logic.ts:1931`) — подтверждает вторую половину + пункта 6 (широкий допуск красит 90,1° зелёным); + - `_deleteRoomClick` (`houseplan-card.ts:10898`): нативный `confirm()`, + `sp.rooms = sp.rooms.filter(...)` без материализации эксклюзивных стен — + подтверждает пункт 4; + - `wall-face-graph.ts`: `DEFAULT_EPSILON = 0.001`, `findNewWallFaces` ищет + только грани, появившиеся «by one accepted source segment» — подтверждает + пункты 2/3/5 (малый geometry epsilon, контракт #173 не меняется). + Все шесть утверждений подтвердились точно, ни одной неверной или + додуманной ссылки не найдено. +4. Проверено существование и содержание связанных issue: #137 (closed, + узлы привязки при рисовании), #141 (closed, зубцы стыков перегородок), + #173 (closed, единый инструмент стен → предложение комнаты), #232 (open, + S1-new, подсветка оси под курсором) — все ссылки в ТЗ корректны и не + переписывают чужой контракт сверх заявленных точек расширения. +5. Проверена терминология: «независимая стена» (`USER-GUIDE.ru.md:492`), + «проём» (`:59`), «Стены» (`:326`) — используются в ТЗ так же, как в + пользовательском гайде, ничего не изобретено. +6. Проверено, что `hp-dialog` с кнопками `.btn.danger`/`.btn.ghost` уже + существует как паттерн (например, `_renderDecorEraseConfirm`, + `houseplan-card.ts:10237–10249`) — предложение §8.7 (primary/danger/Cancel) + не вводит новый визуальный язык, а расширяет существующий на одну кнопку. +7. Проверен фикстур-профиль AC15: «60-room/60-partition» дословно совпадает + с каноническим `npm run benchmark:large-house` (`docs/TESTING.md:1560`, + 60 комнат/60 partitions/200 устройств/100 проёмов) — не выдуманное число. +8. Проверены обязательные разделы ТЗ по PROCESS.md §7.1: сценарий (§1) · + что человек увидит (§2) · проблема (§3) · скоуп/не-скоуп (§5/§6) · + контракт поведения (§7/§8) · UX (§8) · модель данных и миграция (§9/§10) + · i18n (§11.1) · AC1…AC17 с методом доказательства (§13) · план автотестов + (§14) · риски (§18) · откат (§19) · release-артефакты (§17). Все на месте. +9. Проверено, что раздел 20 («принятые технические предположения») содержит + только технические решения (пороги, приоритет tie-break, переиспользование + внутренних API) — ни одно не меняет то, что видит или делает пользователь + сверх уже утверждённых владельцем 5 defaults. +10. `git show dca4ed2 -- docs/specs/228-plan-drawing-problems.md | git diff + --check` — чисто. Трейлеры коммита: `Issue: #228`, `User-Visible: no` — + корректно для чисто документационного изменения (класс C, докстадия). + `docs/specs/README.md` обновлён линком на файл. + +## Находки + +Блокирующих (High) находок нет. Находок уровня Medium нет. + +Одно наблюдение уровня Low, не требующее правки: + +- §8.5 описывает 7 шагов для случая «клик внутри существующей грани», но не + проговаривает явно «иначе — обычное начало новой цепочки»; это читается + по контексту (`docs/CANVAS.md`, действующее поведение инструмента «Стены» + не отменяется, редефинируется только ветка «клик внутри готовой пустой + области»), и AC5 отдельно фиксирует, что snap-zone click рисует стену, + occupied/duplicate/partial face не предлагаются. Реального риска + неоднозначной реализации не вижу — оставляю без правки. + +## Что проверено и корректно + +- Все шесть исходных наблюдений владельца покрыты сценарием/AC один в один + (см. таблицу соответствия ниже), без тихих пропусков. +- Все пять продуктовых решений владельца (Q1–Q5, комментарий «Решения + владельца») дословно перенесены в §4 и раскрыты в UX-контракте §8.2–§8.7, + без переинтерпретации. +- Ambiguity/2-см-repair/удаление комнаты — везде явно описаны инварианты + «no mutation on hover», «fail-closed», «один Undo», совпадающие с + §7 «Инварианты» и не противоречащие `docs/CANVAS.md`/`WALL-THICKNESS.md`. +- Non-scope (§6) корректно исключает смежные контракты (#141, #173, #232, + toolbar #148, глобальное выравнивание, touch-паритет) — никакого + расползания скоупа. +- Touch-раздел (§11.3) использует ровно одну из трёх канонических формул + `docs/TOUCH-SUPPORT.md` («best effort / intentionally degraded») и + соблюдает safety floor (никакой мутации на pinch/pointercancel/suppressed + tap — то же самое требует AC14). +- Compatibility (§10): новых persisted полей нет, диапазон `cm` 1–100 для + partitions соответствует `WALL-THICKNESS.md` §9, откат (§19) — простой + revert коммита без обратной миграции данных. +- Release-артефакты (§17) перечисляют оба changelog, все канонические + документы подсистемы и три копии бандла — соответствует §2.5 DoR. +- Ни одной догадки, выданной за факт: все утверждения о текущем поведении + либо процитированы с номером строки и подтверждены чтением кода, либо + явно помечены в §20 как «принято предположительно, поменять свободно». + +### Таблица соответствия наблюдение → decision → AC + +| # набл. | Owner decision (Q) | Раздел УХ | AC | +|---|---|---|---| +| 1 (нет оси/узла) | — (баг, не вопрос) | §8.1 | AC1 | +| 2 (зубцы) | Q1 | §8.2 | AC2 | +| 3 (ложный разрыв) | Q2 | §8.6 | AC6, AC7 | +| 4 (удаление комнаты) | Q3 | §8.7 | AC9–AC12 | +| 5 (комната из контура) | Q4 | §8.5 | AC5, AC8 | +| 6 (Shift/подпись) | Q5 | §8.3, §8.4 | AC3, AC4 | + +## Чего не проверял + +- Реализацию — на этапе spec её нет; продуктовый код не менялся (подтверждено + `git show --stat dca4ed2`: только два файла в `docs/specs/`). +- Тяжёлые гейты (`golden`, `performance`, полный backend-harness, + `demo/smoke_*`) — не запускал: на spec-этапе кода для прогона нет, они + относятся к `S6`/пре-релизу. Легковесные тексто/структурные проверки + (`git diff --check`, сверка ссылок и AC) выполнены вручную вместо + `npm run typecheck`/`test`/`build` — они бессмысленны для этой задачи + (изменения только в `docs/`, ни один `.ts`/тестовый файл не тронут). +- Точные будущие имена helper/test/smoke файлов и i18n-ключей — согласно + §20 п.9 ТЗ и PROCESS.md §7.1, это техническая свобода автора, не предмет + ревью ТЗ. + +## Итог + +ТЗ полное, однозначное, каждый AC пронумерован и имеет способ доказательства, +продуктовая неопределённость закрыта владельцем до написания документа, +технические предположения промаркированы и не подменяют продуктовые решения. +Готово к переходу в «Готово к разработке».