From ffe0f0c7882647a789b56b4cfc623c09d36731e3 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 05:06:03 +0000 Subject: [PATCH] docs: review document for #313 Issue: #313 User-Visible: no --- docs/reviews/SPEC-REVIEW-313-r1.md | 147 +++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-313-r1.md diff --git a/docs/reviews/SPEC-REVIEW-313-r1.md b/docs/reviews/SPEC-REVIEW-313-r1.md new file mode 100644 index 00000000..cf8d5b93 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-313-r1.md @@ -0,0 +1,147 @@ +# SPEC-REVIEW-313-r1 + +Issue: #313 · этап: ТЗ (spec) · трек: `small` (лёгкий) · заход: r1 · лимит циклов ТЗ: 2 · израсходовано: 0 + +## Скоуп + +Инструмент «Толщина» (`_wallThickHit`/`_wallThickHover`/`_wallThickClick`/`_wallThickApply`, +`src/houseplan-card.ts`) сейчас видит только атомарные интервалы контуров комнат +(`wallIntervals(space.rooms, …)`) и не видит независимую кладку — перегородки +(`space.partitions`) и сегменты сохранённых черновиков (`space.room_drafts[].points`). +ТЗ расширяет резолвер, hover, диалог и запись на все три источника, с приоритетом +независимой кладки при наложении и без кнопки «на всю комнату» для неё. Модуль один, +файл один — критерии `small` (сложность/риск ≤3, одна поверхность, без миграции, +без нового UX-контракта, без влияния на перф/touch) соответствуют содержанию задачи. + +Первый вопрос по `docs/SCOPE.md`: чью работу закрывает правка. Инструмент правки +геометрии Plan-редактора — часть J6 («Keep the plan true as the home evolves», +два редактора, drag/resize/merge/split). Конфликта со SCOPE нет: это устранение +дыры в уже существующем, одобренном инструменте, а не новая функциональность. + +## Как проверялось + +Ревью ТЗ этапа `S4-spec-review`, r1 — полный разбор (первый заход, дельты нет). +Читал: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1–§12), тело issue #313 и +единственный комментарий (аналитика + решения владельца от 2026-08-26), канонический +документ подсистемы `docs/WALL-THICKNESS.md`, срез `docs/USER-GUIDE.ru.md` по +инструменту «Толщина», связанные issues #308 (закрыт), #306 (open, S3-spec), +#229 (закрыт), #303 (закрыт). + +Код читался в дереве `dev @ a78deb7c` (текущий HEAD), не исполнялся — это ревью ТЗ, +не код-ревью, автотестов ещё нет. Каждое фактическое утверждение ТЗ (сигнатуры, +существование функций/полей, номера строк, тексты тостов, содержимое смоков) +проверено чтением исходников и `docs/WALL-THICKNESS.md`, а не принято на слово: + +- `_wallThickHit` (:11915) действительно перебирает только `wallIntervals(space.rooms, …)` — + подтверждено чтением, совпадает с описанием «Причина (код)». +- `_wallThickApply` (:11978) действительно пишет только через `sp.walls` / + `setWallThicknessForRoom` — writer для `partition.cm`/`draft.segments[i].cm` + отсутствует, подтверждено. +- `wallThickHoverHalfUnits(hit.cm, …)` уже принимает `cm` (:11946, импорт :312) — + подтверждено, hover-контракт из #303 переиспользуется без изменений. +- `PartitionCfg` (`src/types.ts:64`) и `RoomDraftCfg.segments[i]` (`src/types.ts:56-61`) + уже содержат обязательное числовое поле `cm` — модель не меняется, миграция не нужна. +- `docs/WALL-THICKNESS.md` §9 УЖЕ фиксирует инвариант «1–100 см для перегородок и + сегментов драфта, ноль не существует» и «Invalid input blocks the commit and + reports the valid range» — решение владельца №2 (запретить 0/пусто для независимой + кладки) не догадка, а совпадение с задокументированным контрактом модели. + Это же объясняет, почему у комнатных стен 0 легитимен (§6 канона: «empty/0 clears»), + а у независимой кладки — нет: два разных, уже описанных контракта, ТЗ их не путает. +- `toast.wallthick_pick` / `toast.wallthick_open` / `toast.physical_range` (`src/i18n/ru.json:533-537`) + существуют и достаточно общие для переиспользования на перегородках/драфтах — + новых ключей i18n действительно не требуется, как заявлено в аналитике. +- `_commitPhysicalGeometry` + `history.wall_thickness` (:12017) — существующая + физическая транзакция с одним Undo, переиспользуется корректно. +- Правило приоритета независимой кладки над контуром комнаты уже реализовано и + используется в смежном коде — `_boundaryBlocked` (:11285-11309, комментарий + «Independent masonry owns its hit zone…») перебирает `wall_columns`, `partitions` + и `room_drafts` именно в этом порядке приоритета для операций границы/select. + Формулировка ТЗ «согласуется с select-инструментом» подтверждена, а не голословна. +- `_activeDraftId` (:1548) переживает переключение инструмента/пространства + (:1291, :2188) — «активный драфт инструменту не виден» описывает реальную + достижимую ситуацию (недорисованная цепочка, отложенная переключением тула), + а не искусственный кейс; исключение активного драфта уже так работает у snap-геометрии + (`excludeDraftId`, :6746; комментарий :20349) — переиспользование обосновано. +- Три смока, названные в AC4, существуют: `demo/smoke_wall_thickness.mjs`, + `demo/smoke_wallthick_hover_width.mjs`, `demo/smoke_resize_wall_thickness.mjs`. +- Кнопка «на всю комнату» (`wallthick.apply_room`, :12558-12560) сейчас рендерится + безусловно — условный рендер по «есть ли `roomId`/kind `room`» технически + тривиален, контракт (п.3) реализуем как описано. +- #308 (пример наложения перегородки на ребро комнаты, фикстура-экспорт в + приложении к отчёту) — issue **закрыт** без комментария к закрытию и без метки + `rejected`, при этом сохраняет статусную метку `S1-new` (в нарушение §2.9/§9 + PROCESS.md — закрытый issue не должен нести `S*`). Это не блокирует #313: своя + AC3 (приоритет при наложении) самодостаточна и не зависит от того, будет ли + когда-нибудь сделана «согласованная правка обоих источников» из решения + владельца №3. Но пункт «остаётся скоупом #308» из решения владельца сейчас + ссылается в никуда — фиксирую это как наблюдение, не как находку против #313. + +Гейты кода не гонялись — на этапе ТЗ кода ещё нет, тестировать нечего +(`typecheck`/`test`/`build`/`check-docs` относятся к код-ревью, §2.7/§8). + +## Находки + +Ни одной High/Medium. Одна Low, снимаю с записью (не блокирует, автору можно +поправить попутно при написании ТЗ AC/PR, отдельного цикла не требует): + +- **Low — неверный номер строки в цитате.** В разделе «Что нужно» (п.1) и в + разделе «Причина» issue правило «Independent masonry owns its hit zone» + указано по адресу `:11104`. На `dev @ a78deb7c` по этому адресу лежит фрагмент + диалога decor-текста (цветовой пикер), правило по факту находится в + `_boundaryBlocked`, `src/houseplan-card.ts:11285`. Раздел «Контракт» (та часть + ТЗ, что реально нормативна) этот номер не повторяет — там же цитата дана без + строки, поэтому имплементация не пострадает. Правлю с записью: верный адрес — + `src/houseplan-card.ts:11285`. + +## Что проверено и корректно + +- Трек `small` выбран обоснованно, все пять критериев §5 выполнены (одна + поверхность/один файл, риск/сложность ≤3, без миграции, без нового + UX-контракта — кнопка `apply_room` просто скрывается там, где нет комнаты, + остальной диалог тот же, — без влияния на touch/perf). +- Обязательные для light-трека разделы (проблема · контракт · AC · откат) + присутствуют, ни один не пуст. +- Три продуктовых решения владельца (комментарий 2026-08-26) корректно перенесены + в «Контракт» и не переизобретены заново. +- Все 4 AC пронумерованы, каждый называет способ доказательства (`smoke`/`unit`), + AC1/AC2 явно фиксируют текущее красное состояние («падает на текущем dev — + hit = null»), AC3 задаёт конкретную регрессионную фикстуру (#308), AC4 защищает + от регрессии по существующим гейтам — тест на «AC умеет упасть» проходит на + уровне формулировки (для AC1/AC2 это прямо написано). +- Ни одного продуктового вопроса не осталось открытым: три развилки, требовавшие + решения владельца (кнопка диалога, ноль/пусто, приоритет при наложении), закрыты + явными решениями в комментарии аналитики, а не угаданы автором. +- Утверждений о поведении, не подтверждённых документом/кодом и не помеченных как + предположение, не найдено — единственная фактическая неточность (строка :11104) + разобрана выше как Low. +- Откат («один revert, без миграций и новых полей конфига») соответствует факту: + модель данных не меняется, меняется только резолвер/writer инструмента. +- Конфликта со `docs/SCOPE.md` и `docs/TOUCH-SUPPORT.md` не нашёл: правка + редактора Plan (десктоп-первый инструмент), в View не протекает. + +## Чего не проверял + +- Не проверял по коду, что hit-резолвер после правки не даст обратной регрессии + для контуров комнат при наличии рядом коллинеарной перегородки НЕ на кейсе + #308 (общий случай «рядом, но не точно на ребре») — в ТЗ такой AC не заявлен + явно за пределами AC3/AC4, это забота код-ревью и, при необходимости, находка + для отдельного цикла на этапе кода, а не пробел ТЗ (AC4 достаточно узко и + корректно защищает именно регрессию контуров). +- Не проверял столбы (`space.wall_columns`) на предмет того, должен ли инструмент + «Толщина» также брать их — симптом, аналитика и AC ни разу их не упоминают, + а в `docs/USER-GUIDE.ru.md` у колонны уже отдельный диалог размера через select + (не через «Толщина»); границу считаю намеренной и не отношу к находкам. +- Не прогонял никаких автотестов/смоков — на этапе ТЗ кода нет, гонять нечего; + сами тексты AC1/AC2 содержат обязательство, что новый смок «падает на текущем + dev», это будет предметом код-ревью, а не этого документа. +- Не проверял `#306` (замена виртуальных стен стенами cm=0) по существу — он + открыт, в `S3-spec`, `blocked`, и в контракт #313 входит только требованием + «точка записи не должна требовать переписывания под будущую ветку 0», что + является технической, а не продуктовой договорённостью и не подлежит проверке + на этом этапе. + +## Вердикт + +Готово к разработке. Both AC-доказательства и продуктовые развилки закрыты +решениями владельца; единственная находка — Low, снята с запиской в этом +документе, отдельного цикла не требует.