From d67bb11de0a42db82d4a086db207cca81aed4886 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:37:13 +0000 Subject: [PATCH] docs: review document for #329 Issue: #329 User-Visible: no --- docs/reviews/SPEC-REVIEW-329-r1.md | 203 +++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-329-r1.md diff --git a/docs/reviews/SPEC-REVIEW-329-r1.md b/docs/reviews/SPEC-REVIEW-329-r1.md new file mode 100644 index 00000000..6d6b68a8 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-329-r1.md @@ -0,0 +1,203 @@ +# SPEC-REVIEW-329-r1 + +Issue: #329 · Этап: spec (S4-spec-review) · Заход: r1 · Трек: обычный +ТЗ: `docs/specs/329-junction-limits.md`, коммит `8dce8453975121a89263d44243e49059f7000da7` +(докоммит: `docs: spec #329 — wall junction limits and an honest sharp apex`, только +класс C — `docs/specs/**`, продуктовый код не тронут). + +## Скоуп ревью + +Изменение — новый файл ТЗ на 151 строку, без сопутствующих правок кода. Задача +не помечена `small`/`trivial`, поэтому полноценный файл ТЗ обязателен — +формально верно. Ревью охватывает весь документ целиком (заход первый, +дельта = документ полностью). + +Продуктовая рамка (`docs/SCOPE.md`): задача — правка визуального дефекта +масонри (некорректный рендер стыка стен) плюс новый барьер на запись +геометрии. Это не новая функция, а восстановление корректности существующей +геометрической модели (примыкает к J6 «keep the plan true» и общей +целостности рендера стен, на которой держатся J1/J5/J7). Конфликта со SCOPE +не вижу, владелец решение принял явно в чате issue. + +## Как проверялось + +- Прочитаны `docs/SCOPE.md`, `PROCESS.md` (весь, включая §2.4, §2.10, §7.1), + `AGENTS.md`. +- Прочитано тело issue #329 и оба комментария владельца (`gh issue view 329 + --comments`) — сверены пять нормативных ограничений П1–П5 из ТЗ дословно с + решением владельца в чате: совпадают без искажений. +- Прочитан канонический `docs/WALL-THICKNESS.md` целиком, включая разделы про + #309 (visual mitre limit / chamfer) и #310 (pair apex / butt-end trim), на + которые ссылается §4 ТЗ — ссылки корректны, терминология («мitre», + «биссектриса», «внутренние грани») не расходится с каноном. +- Прочитан `docs/USER-GUIDE.ru.md`: раздел «Инструменты плана», таблица + Resize, раздел про геометрическую проверку/диагностику при Optimize — искал + установленную терминологию для UX отказа записи (см. находку M1). +- Проверено `git show --stat` на коммит ТЗ: тронут только + `docs/specs/329-junction-limits.md`, продуктовый код и гейты не задеты — + прогон `typecheck`/`test`/`build` для чисто документационного коммита класса + C не требуется (PROCESS.md §1 таблица классов, §8 — гейты код-ревью), не + прогонял. +- Grep по `custom_components/houseplan/validation.py` — подтверждён + прецедент семантической дельта-валидации (`invalid_partition_opening_jamb_margin`), + на который ссылается §5 ТЗ: ссылка не выдумана, паттерн существует. +- Grep по `src/grid-scale.ts`, `src/plan-optimizer.ts` — подтверждено, что + `cell_cm` — реальный настраиваемый параметр (диапазон 0.1–1000 см, по + умолчанию 5 см), не константа — относится к находке L1. + +## Находки + +### M1 (Medium, в скоупе) — UX отказа записи заявлен как факт, а не как решение, и не согласован с уже существующим поведением Resize + +`docs/specs/329-junction-limits.md`, §2 (нормативные ограничения, преамбула): +«Нарушение = отказ всей записи, план не изменён, **тост** называет конкретное +правило.» §3 распространяет это на все пять поверхностей записи: рисование и +завершение контура, черновики, независимые стены/перегородки, **Resize**, +инструмент **«Толщина»**, merge/split. + +Это утверждение о видимом поведении — то есть продуктовый факт, а не +техническая деталь. Но `docs/USER-GUIDE.ru.md` уже фиксирует для Resize +контракт отказа, который тостом не является: + +- строка 482: «Боковая стена перестала бы быть общей... → **ручка + объясняет**: «Нельзя сдвинуть только часть общей стены»»; +- строка 483: «Общая граница частичная... → **ручка остаётся видимой, но + приглушена; наведение, фокус или нажатие объясняет запрет**»; +- строка 492–493: «Неоднозначный результат **отклоняется целиком**...» — без + упоминания тоста; +- для пакетной геометрической проверки Optimize (строка 1438) отказ + оформлен отдельным **диалогом** с «Скопировать диагностику», тоже не + тостом. + +Т.е. в продукте уже как минимум два разных established UI-паттерна отказа +структурной записи (приглушённая ручка с текстом на фокус/hover — для +Resize; диалог с диагностикой — для пакетной геометрической проверки), и ни +один не является тостом. ТЗ утверждает единый тост для всех пяти +поверхностей, не упоминая эти прецеденты и не помечая выбор как +предположение в §9. AC7 (Resize/«Толщина») тоже не называет способ обратной +связи — только «отказ fail-closed, план байт-неизменен», что расходится с +общей формулировкой §2. + +Это ровно тот класс дефекта, о котором предупреждает процесс: «утверждение о +поведении, которого нет ни в одном документе и которое не помечено как +предположение». Здесь оно к тому же противоречит документированному +поведению Resize. + +**Чем это грозит.** Реализация по букве §2 добавит тост поверх уже +существующей ручки-объяснения на Resize — пользователь увидит два +одновременных, возможно противоречащих, сообщения об одном и том же отказе, +либо автор в процессе кодирования тихо перепишет UX Resize под тост, что +является расширением скоупа (смена established UX-контракта отдельного +инструмента) без отдельного продуктового решения. + +**Что нужно сделать.** Явно расписать, какой канал обратной связи +используется на КАЖДОЙ из пяти поверхностей (§3), сверяясь с уже +задокументированным поведением каждого инструмента: тост уместен там, где +сегодня нет собственного канала (рисование/завершение контура, черновики, +merge/split), а для Resize — либо переиспользовать существующий +приглушённая-ручка-и-объяснение канал (без тоста), либо явно зафиксировать +как новое продуктовое решение и передать вопрос владельцу по правилам §7.1 +(«что человек видит», с предлагаемым вариантом по умолчанию). + +### M2 (Medium, в скоупе) — AC5 описывает два разных теста одной фразой и непроверяем в текущей формулировке + +`docs/specs/329-junction-limits.md`, §8, AC5: +> «Повторение «шпиля» из фикстуры issue рисованием — отказ по просвету (даже +> если каждый угол ≥ 15°, но толщины съедают комнату).» + +Фикстура issue (`houseplan-space-test2-2026-08-27_16-14-48.json`, комната +`rmtbq3k5e-0`) имеет вершину ≈9.9° — она нарушает П1 (угол < 15°) сама по +себе. Отказ по этой фикстуре AC1 уже проверяет (угол). Формулировка AC5 +одновременно (а) ссылается на эту же фикстуру и (б) оговаривает «даже если +каждый угол ≥ 15°» — то есть описывает СОВЕРШЕННО ДРУГУЮ геометрию (все углы +проходят П1, но толщины стен всё равно съедают просвет комнаты ниже 25 см²). +Это два разных теста, слитых в один AC, и как единое проверяемое утверждение +он не воспроизводим: не названо, каким конкретно углам/длинам/толщинам +соответствует случай «угол ≥15°, но просвет < 25 см²» — то есть ровно то +несогласованное место, где догадка выдаётся за факт, если разработчик сам +подберёт числа. + +**Что нужно сделать.** Разделить на AC5a (повтор фикстуры issue — отказ, +допустимо по любому из П1/П5, тест уже частично покрыт AC1) и AC5b — +отдельная синтетическая геометрия с конкретными числами (углы, длины, +толщины), где все углы ≥15° проходят П1, но просвет всё равно < 25 см², и +именно она доказывает независимость П5 от П1. + +### L1 (Low) — параллель «5 см = 1 клетка» вводит в заблуждение при нестандартном `cell_cm` + +`docs/specs/329-junction-limits.md`, §2, П4 и П5: «не ближе **5 см** (1 +клетка)» и «≥ **25 см²** (1 клетка 5×5)». `cell_cm` — реальный настраиваемый +параметр пространства (0.1–1000 см, `src/grid-scale.ts`, +`src/plan-optimizer.ts`), а не константа. Числа П4/П5 читаются как абсолютные +сантиметровые пороги (не масштабируются с `cell_cm`) — это, видимо, и есть +решение владельца (в его формулировке те же 5 см/25 см² даны как абсолютные +величины, «клетка» — только пояснение при дефолтной сетке). Но +параллель «(1 клетка)» рядом с абсолютным числом может подтолкнуть +реализацию к ошибочному масштабированию порога на `cell_cm` пространства, +отличном от 5 см. Снимается редакционно: убрать скобку «(1 клетка...)» из +нормативного текста П4/П5 либо явно дописать «абсолютная величина, не +зависит от `cell_cm` пространства». Не блокирует — правится или снимается +решением ревьюера; фиксирую как Low, автор может снять с той же +формулировкой. + +## Что проверено и корректно + +- Пять численных ограничений П1–П5 в ТЗ дословно совпадают с решением + владельца в чате issue — искажений или добавленных «от себя» цифр нет. +- Граница применения (§3): проверки только на записи, изменяющей + геометрию; легаси/импорт/восстановление/миграция не блокируются; + Optimize не обязан чинить легаси, но не должен создавать новых нарушений — + сформулировано как явный пост-условный AC (AC10), проверяемо. +- §4 (рендер легаси-острия) корректно опирается на существующий канон: + #309 (visual mitre limit / chamfer) и #310 (pair apex, «нода из ровно двух + лучей») в `docs/WALL-THICKNESS.md` действительно описывают именно тот + механизм, который вырождается в «трезубец» при полном перекрытии тел — + ссылки точны, не выдуманы. +- Геометрический контракт фаски §4 явно и корректно продублирован в §9 как + «принято предположительно, поменять свободно» — не выдаётся за + окончательное решение, ревьюер вправе оспорить: не оспариваю, реализация + разумна. +- §5 (бэкенд): ссылка на существующий паттерн семантической + дельта-валидации (`invalid_partition_opening_jamb_margin` в + `custom_components/houseplan/validation.py`) подтверждена чтением кода — + прецедент реален, а не придуман для ТЗ. +- Обязательные разделы §7.1 присутствуют по содержанию (сценарий, что видит + пользователь, проблема, скоуп/не-скоуп — слиты в §1/§3, но содержательно + покрыты, контракт поведения §2/§4, модель данных/миграция §3/§6, i18n §6, + AC1–AC10 с указанием способа доказательства, план автотестов §6, риски + §10, откат §11, release-артефакты §6). Формальных пропусков разделов нет. +- Новых полей конфига нет, `config-field-registry` не трогается — заявлено + явно (§6), согласуется с `docs/CONFIG-COMPATIBILITY.md` (миграция/импорт + легаси не блокируются, что и требует этот документ). +- AC1–AC4, AC6–AC10 однозначны, у каждого названы конкретные граничные числа + и способ доказательства (unit/smoke/golden/backend), тест умеет + различать pass/fail на границе — реализуемо без дополнительных догадок. +- Открытых догадок, выданных за факт технических решений, не найдено за + пределами M1 (продуктовый вопрос UX) — все технические допущения явно + оформлены в §9 «принято предположительно». +- Артефакт корректного класса: файл `docs/specs/329-junction-limits.md`, + имя соответствует `-.md`, задача не `small`, полноценный файл + обязателен и присутствует. + +## Чего не проверял + +- Код не менялся в этом коммите (только `docs/specs/**`, класс C) — гейты + `typecheck`/`test`/`build`/`check-docs`/инварианты модели не прогонял: они + не относятся к чисто документационной правке на этапе spec-review и + прогоняются на этапе код-ревью. +- Не проверял, как именно физтела считаются в кэше для П5 (реализация ещё не + существует) — это заявлено как assumed в §9, оценка технической реализации + относится к код-ревью. +- Не проверял golden-матрицу на предмет «нет вершин острее 15°» — ТЗ само + требует подтвердить это прогоном на этапе реализации (§6), это не задача + spec-ревью. +- Не запрашивал у владельца дополнительных продуктовых уточнений — вопрос + M1 возвращается автору ТЗ как находка, а не выносится владельцу напрямую + (по регламенту дуб. технических/продуктовых вопросов на этапе ревью решает + ревьюер вплоть до исчерпания лимита циклов). + +## Вердикт + +Жёлтый. Два Medium в скоупе задачи (M1, M2), High-находок нет. Оба чинятся +в тексте того же ТЗ без смены его архитектуры; отдельный issue не заводится +(#202). Low (L1) — по усмотрению автора, править или снять записью.