mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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`,
|
||||
имя соответствует `<NN>-<slug>.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) — по усмотрению автора, править или снять записью.
|
||||
Reference in New Issue
Block a user