mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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` (новый `<g class="hp-editor-only-layer">` вокруг
|
||||
`_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`, не скоупятся через потомство конкретного `<g>`.
|
||||
Перенос элементов в новый `<g class="hp-editor-only-layer">` (тот же паттерн,
|
||||
что уже используют 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 корректны. Находок нет.
|
||||
|
||||
**Зелёный.**
|
||||
Reference in New Issue
Block a user