From 729e2d05afd9dbed34d122b80c52d4f8c038cc18 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 25 Aug 2026 16:40:13 +0000 Subject: [PATCH] docs: review document for #307 Issue: #307 User-Visible: no --- docs/reviews/SPEC-REVIEW-307-r1.md | 139 +++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-307-r1.md diff --git a/docs/reviews/SPEC-REVIEW-307-r1.md b/docs/reviews/SPEC-REVIEW-307-r1.md new file mode 100644 index 00000000..e040f999 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-307-r1.md @@ -0,0 +1,139 @@ +# SPEC-REVIEW-307-r1 + +Issue: #307 · Этап: ТЗ (spec) · Трек: `small` · Заход: r1 · Вердикт: **зелёный** +Блокирующих циклов израсходовано: 0/2 (лимит лёгкого трека, §4) + +## Скоуп + +ТЗ (в теле issue #307, small-трек, шаблон §5: проблема · контракт · AC · откат) +на исправление визуального дефекта инструмента «Стены» в редакторе плана: во +время рисования цепочки уже поставленные (персистнутые) сегменты теряют +осевую линию `.pathline` и узлы `.vertex`, потому что тело стены рисуется +поверх них, а снап-оверлей намеренно исключает активный драфт из своей +геометрии. Решение — вынести ось/узлы/rubber-band активной цепочки +(`.pathline`, `.vertex`, `.active-axis`, `.active-vertex`) из +`_renderMarkupLayer` в отдельный editor-слой между `_renderWallBodies` и +`_renderPlanSnapOverlay`, ничего не меняя в геометрии снапа. + +Затронутая поверхность — Plan editor (`src/houseplan-card.ts`, один модуль), +только режим `_markup`/`_editing`; View не затрагивается. Персона — Home admin +(единственная, кто пользуется редакторами, `docs/SCOPE.md`), контекст — +задача J4/J6 («от нуля до плана» и «поддерживать план актуальным», два +встроенных редактора). Изменение чисто композиционное (порядок отрисовки уже +существующих DOM-узлов), без новых полей конфига, миграций, i18n-ключей. + +## Как проверялось + +Ревью читал ТЗ и тело/комментарий issue без устных пояснений автора, +состязательно — искал, где контракт невыполним или непроверяем, а не +соглашался с текстом. Каждое фактическое утверждение о коде и документации +проверено чтением, а не принято на веру: + +1. `docs/SCOPE.md` — подтвердил, что задача обслуживает J4/J6 и не задевает + инвариант View/lock; продуктового конфликта нет. +2. `docs/USER-GUIDE.ru.md:372-375` — дословно подтверждает цитату ТЗ: «поверх + уже нарисованных стен видны тонкие осевые линии и точки их концов». Значит + ожидаемое поведение — не догадка автора, а уже зафиксированный контракт; + исключение для «ещё не завершённой цепочки» нигде не оговорено. +3. `src/houseplan-card.ts` — построчно сверил все технические утверждения ТЗ: + - `_renderMarkupLayer` (:19960) действительно эмитит `.pathline` (:20029), + `.active-axis`/`.active-vertex` (:20032-20036) и `.vertex` (:20038); + - вызов `_renderMarkupLayer` (:17553) стоит **до** `_renderWallBodies` + (:17565), которая, в свою очередь, стоит **до** `_renderPlanSnapOverlay` + (:17582) — ровно та последовательность, что и в ТЗ; + - `_renderPlanSnapOverlay` (:19843) и геометрия `buildPlanSnapGeometry` + (`src/plan-snap-overlay.ts:253`) действительно пропускают активный драфт + (`if (draft.id === options.activeDraftId) continue`) — второй заявленный + механизм подтверждён, не выдумка; + - `physical-geometry.ts:210-221` подтверждает, что каждый персистнутый + сегмент `room_drafts` немедленно попадает в непрозрачные тела + (`draftSegments`/`drafts`) без исключения активного драфта — то самое + тело, которое перекрывает ось; + - «hit-слой», упомянутый в контракте как остающийся на месте элемент + `_renderMarkupLayer`, — реален: `_renderPhysicalEditorLayer()` (:19711, + вызывается на :20008) рисует `.physical-hit` (прозрачные линии/пути для + перетаскивания драфтов/перегородок/колонн, `pointer-events: none` вне + инструмента «Выбор», `src/styles.ts:2256-2264`) — контракт не подменяет + несуществующим термином существующий механизм; + - цвет `#ffc14d` из AC1 подтверждён как реальный `stroke`/`fill` `.pathline` + и `.vertex` (`src/styles.ts:1803-1857`) — пиксельная проба технически + осмысленна и отличима от штриховки масонри; + - AC3 ссылается на существующий `demo/smoke_plan_snap_overlay.mjs` — файл + существует. +4. `docs/specs/228-plan-drawing-problems.md` — беглая проверка, что + `active-axis`/`active-vertex` действительно история #228 (проекция оси + активного отрезка), заявленная в issue как смежная, не пересекающаяся + работа — согласуется, конфликта скоупа нет. +5. Сверил критерии лёгкого трека (§5 PROCESS.md) построчно: сложность/риск + ≤3 (аналитика — 2/10), одна поверхность, без миграции конфига, поведение + уже описано в USER-GUIDE (не новый UX-контракт), перф/touch не задеты + (перестановка существующих DOM-узлов, ни одного нового элемента). Шаблон + ТЗ соответствует §5 буквально: проблема · контракт · AC · откат, файла в + `docs/specs/` нет — верно для `small`. +6. Комментарии issue — один, аналитика владельца (S2→S4, метка `small` + поставлена и объяснена). Прежнего вердикта ревью ТЗ нет — это первый заход, + раздел «Унаследовано из r0» не применяется. + +## Находки + +Блокирующих (High/Medium) не найдено. + +Один пограничный момент, не тянущий на находку, а не пропущенный молча: +контракт не называет позицию нового слоя относительно +`_renderResizeMeasurements`/`_renderRoomHoverOutline`/диагностического +оверлея скрытых стен/превью размещения проёма — всех элементов, которые +сейчас лежат между `_renderWallBodies` и `_renderPlanSnapOverlay`. Это не +продуктовый вопрос: перечисленные оверлеи активны для других инструментов +(resize/двери-окна) и не пересекаются по времени с активной цепочкой «Стены», +кроме, возможно, подсветки наведённой комнаты (`roomHover`), которая может +сработать параллельно с рисованием. AC2 не требует конкретной позиции внутри +этого промежутка — только «тела → слой цепочки → снап-оверлей», и это +достаточно проверяемо смоком. Оставляю как техническую деталь на усмотрение +реализации, как и позволяет §7.1 («всё, чего пользователь не наблюдает, +агенты решают сами»); не блокирует и не снижает вердикт. + +## Что проверено и корректно + +- Оба продуктовых вопроса (что видит пользователь, какой объём входит в + задачу) не требуют обращения к владельцу: ожидаемое поведение уже + зафиксировано в USER-GUIDE.ru.md, объём — один модуль, одна визуальная + причина с двумя техническими механизмами, обе описаны и обе имеют решение. + Владельцу вопросов не задавалось — и это оправдано, а не пропущено. +- Причина дефекта — не догадка, выданная за факт: каждое утверждение о коде + (номера строк, имена классов/функций, порядок вызовов, поведение + `buildPlanSnapGeometry`) подтверждено чтением текущего `dev` (см. выше). +- Контракт однозначен: что именно переносится (`.pathline`, `.vertex`, + `.active-axis`, `.active-vertex`), откуда, куда (между конкретными двумя + существующими вызовами), что не меняется (геометрия/резолвер снапа, + жёлтая стилистика, всё остальное содержимое markup-слоя). +- AC1-AC3 — каждый проверяем и указывает способ доказательства (`smoke` + новый/существующий, ревью кода). AC1 задаёт конкретные пиксельные точки + (середина оси первого сегмента, центр промежуточного узла) и конкретный + ожидаемый цвет, отличимый от штриховки — тест способен упасть на текущем + `dev`, что и заявлено явно («падает на текущем dev»). AC3 явно фиксирует + границу: `plan-snap-overlay.ts` не входит в дифф. +- Откат тривиален и назван верно: одиночный revert, ни новых полей, ни + миграций, ни persisted-состояний — соответствует характеру чисто + композиционного изменения. +- Критерии лёгкого трека выполнены все одновременно, метка `small` не + оспаривается. +- Не найдено расхождений ТЗ с `docs/SCOPE.md`, `docs/USER-GUIDE.ru.md`, + `docs/UX-MODES.md`: изменение целиком внутри `hp-editor-only-layer`, + гейтится `this._markup`, View не задет, инвариант блокировок (SCOPE.md, + «The lock invariant») не касается геометрии стен. + +## Чего не проверял + +- Не запускал код и не собирал бандл — на этапе ревью ТЗ кода ещё нет, + проверка была по чтению текущего `dev` (`git log` показывает `dev` на + `489f68ee`, что совпадает с SHA, названным в аналитике owner). + `typecheck`/`test`/`build` к этому этапу не относятся (это гейты + код-ревью, §2.7). +- Не проверял, как новый слой будет вести себя при одновременной подсветке + наведённой комнаты (`roomHover`) во время рисования цепочки — отмечено выше + как техническая деталь реализации, не блокирующая ТЗ. +- Не оценивал производительность численно — оценка «перестановка тех же + DOM-узлов» правдоподобна на глаз и совпадает с заявлением owner в + аналитике, но замер не проводился (задача не требует его: §2.5 требует + «влияние на производительность названо», а не измерено, когда оно + очевидно нулевое).