From dc68868bd8548ae128aaba9eb279b72d6077172f Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:09:10 +0000 Subject: [PATCH] docs: review document for #307 Issue: #307 User-Visible: no --- docs/reviews/CODE-REVIEW-307-r1.md | 171 +++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-307-r1.md diff --git a/docs/reviews/CODE-REVIEW-307-r1.md b/docs/reviews/CODE-REVIEW-307-r1.md new file mode 100644 index 00000000..da0940c9 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-307-r1.md @@ -0,0 +1,171 @@ +# CODE-REVIEW-307-r1 + +Issue: #307 · «Рисование стен: осевые линии и узлы не видны на уже поставленных +сегментах цепочки до её завершения» · трек `small` (ТЗ в теле issue) · заход r1 +(первый цикл) · бюджет циклов лёгкого трека 0/2 израсходовано. + +Ветка приведена к `dev` конвейером до ревью (893a9aa0 → 11343454, +1 коммит +dev). Разбор полный (это первый заход, вопрос «дельта vs полный разбор» из +§2.10 к r1 не применяется). + +Коммит под ревью: `11343454` — `fix: keep the active chain axis and nodes +above the wall bodies (#307)`. Единственный коммит в диапазоне +`origin/dev..HEAD`. + +## Скоуп + +Симптом: во время рисования цепочки стен (Plan → «Стены») уже поставленные +сегменты не показывают осевую линию и узлы — их закрашивает непрозрачное тело +свежеперсистнутого драфта. Видна только резинка (`.active-axis`/ +`.active-vertex`) активного отрезка. Причина зафиксирована в issue двумя +пунктами: (1) `.pathline`/`.vertex` рисовались в `_renderMarkupLayer`, который +вставлен в композицию **до** `_renderWallBodies`; (2) снап-оверлей рисуется +**после** тел, но намеренно исключает активный драфт из геометрии — это +осознанное решение, не трогается. + +Заявленное решение (вариант A из issue): вынести разметку активной цепочки в +отдельный editor-слой, вставленный **после** тел стен и **до** снап-оверлея. + +Соответствует Core user job из `docs/SCOPE.md`: J6 «Keep the plan true as the +home evolves» (два редактора, drag/resize) — сам инструмент рисования стен; +чинится визуальная обратная связь редактора, не продуктовая функция. Вне +скоупа продукта задача не выходит: чисто editor-only правка рендеринга, +никакого нового поведения, конфига или контракта. + +## Как проверялось + +| Гейт | Результат | Команда | +|---|---|---| +| typecheck | зелёный, без вывода | `npx tsc --noEmit` | +| unit-тесты | 1304 pass / 0 fail / 1 skipped | `npm test` | +| build | собран, 14 с | `npm run build` | +| сверка 3 копий бандла | идентичны после пересборки (`md5sum` совпал с закоммиченными `dist/` и `custom_components/.../frontend/`, `bundle:sync` пересоздал недостающую `demo/srv/assets/` копию) | `npm run build && npm run bundle:sync` | +| docs (`docs` job) | зелёный, отпечаток скриншотов совпал | `node scripts/check-docs.mjs` → «Documentation checks passed (7 files, 10 external links)» | +| новый смок `demo/smoke_active_chain_ink.mjs` | **OK** (пиксельные пробы `axisMidYellow: true`, `nodeYellow: true`, RGB `[255,193,77]`/`[129,87,23]`) | `node demo/smoke_active_chain_ink.mjs` | +| тот же смок на коде **до** фикса (`HEAD~1:src/houseplan-card.ts`, пересобран) | **падает**: `inkAboveBodies: false`, `axisMidYellow: false`, `nodeYellow: false` (пиксели `[255,255,255]` — фон вместо оси) — тест умеет ловить регрессию | `node demo/smoke_active_chain_ink.mjs` на откаченном src, дерево затем полностью восстановлено до `HEAD` | +| выборка смежных смоков (`smoke-select.mjs`, 14 прямых совпадений по `_path`/`_cursorPt`/`_markup`/`_contourClosed`/`Axis`) | все 14 — **OK** | см. список ниже | +| `npm run golden:verify` (полная матрица, 127 сценариев) | **все `passed`**, включая 4 сценария из группы риска | `npm run golden:verify` | +| `npm run invariants` | не прогонялся — не применимо, см. «Чего не проверял» | — | +| `python -m pytest tests_backend` | не прогонялся — diff не касается `custom_components/**/*.py` | — | + +Прямые совпадения `smoke-select.mjs`, прогнанные все 14 — все **OK**: +`smoke_draw_wall_thickness`, `smoke_align_guides`, `smoke_editor_gestures`, +`smoke_plan_drawing_repairs`, `smoke_plan_snap_overlay` (AC3 явно требует его), +`smoke_room_autoclose`, `smoke_unified_wall_tool`, `smoke_wall_junctions`, +`smoke_edit_walk`, `smoke_merge_split`, `smoke_optimize_coincident_partition`, +`smoke_resize_audit_1550`, `smoke_room_resize`, `smoke_split_nonsnap`. +«Слабую связь» (18 смоков, одно распространённое имя `_path`/`_cursorPt`) не +гонял — ни один не выглядит специфичным к слоям композиции плана, а прямые +совпадения уже покрывают draw/junction/snap/resize/merge-split поверхность. + +`npm run golden:verify` запускался несмотря на то, что ТЗ его не называет, +по прямому предписанию процесса: diff — это переупорядочивание editor-only +SVG-слоёв (изменился z-order `.pathline`/`.vertex`/`.active-axis`/ +`.active-vertex` относительно тел стен), то есть «может изменить видимый +результат: слои». Нашёл 4 golden-сценария в plan-режиме, которые выставляют +`card._path`/`_tool='draw'` поверх настоящей геометрии стен +(`plan-snap-endpoint-light`, `plan-snap-line-gaps-dark`, +`wall-junctions-plan-preview-light`, `wall-junctions-plan-t-dark` — +`demo/golden/matrix.mjs:266-285`, `demo/golden/harness.mjs:965-1017`) — это +ровно тот класс состояния (резинка/поставленные точки цепочки поверх +реальных стен), для которого правка меняет порядок отрисовки. Полная матрица +прошла зелёной без единого расхождения — риск проверен и снят, а не просто +предположен. + +## Находки + +Отсутствуют. High: 0, Medium: 0, Low: 0. + +## Что проверено и корректно + +**AC1 (ось и узлы видны на поставленных сегментах).** Доказано автотестом +(`demo/smoke_active_chain_ink.mjs`), тест умеет падать — прогнан на коде до +фикса, три проверки красные с ожидаемым фоновым цветом вместо жёлтого. На +текущем коде все проверки зелёные, включая точные RGB-пробы в середине оси +первого сегмента и в узле. + +**AC2 (слой цепочки между телами и снап-оверлеем, резинка не регрессирует).** +Доказано чтением кода и тем же смоком: +- `src/houseplan-card.ts:17565` (`_renderWallBodies`) → + `:17585-17586` (новый `` вокруг + `_renderActiveChainInk()`) → `:17587-17588` (`_renderPlanSnapOverlay`) — + порядок в композиции совпадает с контрактом ТЗ «после тел, до снап-оверлея». +- Смок проверяет тот же факт через `compareDocumentPosition` в DOM + (`inkAboveBodies`, `inkBelowSnapOverlay`) — оба true. +- Резинка (`.active-axis`/`.active-vertex`) перенесена в тот же новый метод + `_renderActiveChainInk` (`:20057-20074`) без изменения условий рендера — + идентичный JSX-блок, только другое место вставки; смок подтверждает + присутствие обоих элементов при активном курсоре. +- Инструмент «Split» использует свои собственные `.pathline`/`.vertex` внутри + `_renderMarkupLayer` (`:20033-20040`) — их ТЗ явно оставляет на месте, и + диф их не трогает: отдельный блок кода, не задет переносом. + +**AC3 (снап не меняется).** `src/plan-snap-overlay.ts` не входит в diff +(проверено — пустой `git diff` по файлу). `smoke_plan_snap_overlay` зелёный. +Существующие юнит-тесты снапа входят в общий зелёный прогон `npm test` (1305 +тестов, ни один не упал). Проверено чтением + существующим смоком, как и +требует AC. + +**CSS не завязан на структуру родителя.** `.pathline`, `.vertex`, +`.active-axis`, `.active-vertex` — глобальные селекторы классов в +`src/styles.ts:1803-1863`, не скоупятся через потомство конкретного ``. +Перенос элементов в новый `` (тот же паттерн, +что уже используют align-guides, hidden-wall-diagnostics, +opening-placement-preview, снап-оверлей — `opacity="${modeVisual?.editorWeight +?? 1}"`) не требует правок стилей и не ломает `editorWeight`-затемнение в +неактивных режимах. + +**Трейлеры и changelog.** Единственный коммит несёт `Issue: #307` и +`User-Visible: yes`; оба `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` правлены в +том же коммите (проверено `git show --stat`, `git diff` по обоим файлам — +согласованные формулировки на двух языках, ссылка на issue). + +**Формулировка ТЗ vs UI.** ТЗ цитирует `docs/USER-GUIDE.ru.md:372-374` +дословно («поверх уже нарисованных стен видны тонкие осевые линии и точки их +концов») — сверено, совпадает буквально. + +**Откат.** Один коммит, без новых полей конфига, миграций или +persisted-состояний — подтверждено диффстатом (только `src/houseplan-card.ts` ++ сгенерированные копии + смок + changelog + отпечаток скриншотов). + +**«Одно число — один источник».** Не применимо: диф не добавляет и не +меняет ни одной пользователю видимой величины (числа, площади, подписи) — +только z-order editor-only SVG-элементов. `test/single-source-numbers.test.mjs` +входит в общий зелёный `npm test`. + +## Чего не проверял + +- **`npm run invariants`** — не прогонял. Diff не касается геометрической + модели: нет правок `layout`, записей толщины, `marker.space`, `open_spans`, + `physical-geometry.ts`, `plan-snap-overlay.ts`. Изменение — это + переупорядочивание editor-only SVG-слоёв внутри одного компонента + рендеринга; ни одна из трёх инвариантных проверок (запись толщины, + разрешимость ссылок, ключ записи = ключ решёточного ребра) не имеет + отношения к этому диффу. +- **`python -m pytest tests_backend`** — не прогонял. Diff не касается + `custom_components/houseplan/**/*.py` (только сгенерированная копия фронтенд- + бандла в `custom_components/houseplan/frontend/houseplan-card.js`, класс D). +- **Полный набор `demo/smoke_*.mjs`** — не гонял все ~70+ смоков, только 1 + новый + 14 прямых совпадений (см. таблицу и обоснование выше). Слабые связи + (18 смоков с одним общим именем `_path`/`_cursorPt`) не прогонял — по + выводу инструмента это решение ревьюера, и просмотр списка (drag/pan/split/ + opening-preview на разных, не связанных с композицией плана поверхностях) + не выявил кандидатов, специфичных именно к порядку SVG-слоёв. +- **`performance_smoke`** — не прогонял. Не назван в AC, диф не трогает + чувствительные к перфу пути (рендер не добавляет новых вычислений — тот же + шаблон переставлен в другое место композиции, число DOM-узлов не меняется). +- **Ручное тестирование в браузере** — не проводилось (в цикле code-review + ручного тестирования нет по регламенту); функциональность подтверждена + смоком с пиксельными пробами и golden-верификацией композитного рендера. + +## Вердикт + +Все 3 AC доказаны: AC1/AC2 — автотестом, который умеет падать (проверено на +коде до фикса), AC3 — чтением кода (файл снапа вне диффа) плюс существующим +зелёным смоком. Дешёвые гейты (typecheck/test/build/bundle-sync/docs) зелёные. +Прямые смежные смоки (14) зелёные. Полная golden-матрица (127 сценариев, +включая 4 сценария из группы прямого риска — резинка/точки цепочки поверх +настоящих стен в plan-режиме) зелёная без единого расхождения. Трейлеры и оба +changelog корректны. Находок нет. + +**Зелёный.**