From 9d6a4e34fd1dee915deed820d5291cbff263488b Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 23 Aug 2026 08:30:43 +0000 Subject: [PATCH] docs: review document for #253 Issue: #253 User-Visible: no --- docs/reviews/SPEC-REVIEW-253-r1.md | 171 +++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-253-r1.md diff --git a/docs/reviews/SPEC-REVIEW-253-r1.md b/docs/reviews/SPEC-REVIEW-253-r1.md new file mode 100644 index 00000000..83d3c823 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-253-r1.md @@ -0,0 +1,171 @@ +# SPEC-REVIEW-253-r1 + +- Issue: [#253](https://github.com/Matysh/houseplan-card/issues/253) — «Ресайз в некоторых случаях теряет толщину стен: перемещённые рёбра остаются осевыми линиями» +- Этап: ТЗ на ревью (PROCESS.md §2.4) +- Заход: r1 · блокирующих циклов израсходовано 0 из 4 +- Артефакт под ревью: `docs/specs/253-resize-wall-thickness.md` (+ строка в `docs/specs/README.md`) +- Ветка: `issue/253-resize-wall-thickness` +- SHA ревью: `d9f7861` (`git diff origin/dev..d9f7861` — единственная дельта, новый файл ТЗ + запись в реестре) +- Вердикт: **зелёный** + +## Скоуп ревью + +Первый заход (r1), раздела «Унаследовано из r0» и «Закрытие раунда r0» не требуется — +предыдущего вердикта не существует. Разбор — полный документ ТЗ по PROCESS.md §7.1 +и §2.4, плюс верификация технических утверждений автора по фактическому коду `dev` +(а не по слову автора), поскольку ТЗ построено на числовом разборе конкретного +дефекта, и любая неточность в диагнозе сделала бы контракт нереализуемым. + +## Как проверялось + +1. Прочитано тело issue #253 и все 5 комментариев (owner) — включая исходный + отчёт, воспроизведение на приложенном экспорте (24→23 записи, потеря 33 см) и + финальную аналитику с «Требуемым поведением» и «Принятыми предположениями». +2. Прочитан `docs/SCOPE.md` (job J6), `AGENTS.md`, `PROCESS.md` §2.4/§2.10/§7.1, + `docs/WALL-THICKNESS.md`, `docs/USER-GUIDE.ru.md` (строка 456: контракт «интервалы + толщины и виртуальности переносятся вместе со стеной» уже документирован — + фикс восстанавливает существующее обещание, а не придумывает новое). +3. Полностью прочитан `docs/specs/253-resize-wall-thickness.md` (401 строка, + разделы §1–17) и сверен с обязательным списком PROCESS.md §7.1 — все разделы + на месте (сценарий, что человек увидит до/после, проблема, скоуп/не-скоуп, + контракт поведения, UX/touch, модель данных/миграция, i18n, AC1…AC10 с + доказательством, план автотестов, риски, производительность/безопасность, + откат, release-артефакты, принятые технические предположения). +4. Прочитан текущий код на `dev`, чтобы убедиться, что диагноз ТЗ (§3) не + догадка, а факт: + - `src/wall-thickness.ts:438` `rekeyWallsAfterMove()` — подтверждено: запись с + точными `a/b` переносится только когда **оба** конца лежат в допуске **одного** + старого ребра (`distToSeg(...) > tol → continue`), без разбиения на частичном + пересечении; коллизия `used.has(nk)` действительно тихо выбрасывает запись + (строка 512) без сравнения геометрии/толщины. Ровно то, что описывает §3 ТЗ. + - Проверена смежная гипотеза, которую ТЗ **не рассматривает явно**: + `_rszApplyPreview()` (`src/houseplan-card.ts:8323`) пропускает `oldSpans/newSpans` + комнаты целиком при `oldR.poly.length !== newPoly.length`, а `shiftSharedSpans()` + (`src/resize.ts:124`) прямо документирован как «the neighbour may become + L-shaped» — то есть вставляет вершину соседу при частичном перекрытии, меняя + число вершин. Прослежено до конца: `sharedSpansWith`/`shiftSharedSpans` клипуют + перекрытие соседа диапазоном `[0, L]` длины **основного** перетаскиваемого + ребра, поэтому перемещаемый интервал стены всегда целиком покрыт собственным + ребром «своей» комнаты (`plan.roomId`, чей `poly.length` при edge-drag не + меняется — `movePolyEdge` двигает только 2 вершины) и при corner-scale + (`applyRoomScale`: `room.poly.map(scalePt)`, вершины только линейно + преобразуются, число тоже не меняется). Пропуск соседа с изменившимся + `poly.length` избыточен, но не создаёт потери покрытия — не дефект. + Не в отчёте как находка, потому что после трассировки подтверждено отсутствие + эффекта; фиксирую здесь, чтобы дельта следующего раунда не переоткрывала тот же + путь. + - `src/coordinate-canonicalization.ts` — подтверждено существование + nine-decimal canonicalization (`COORDINATE_DECIMALS = 9`) и что `space.walls[].a/b` + уже входят в её allow-list (строки 95–98). Ссылка ТЗ (§6.4) на этот механизм не + фантазия. + - `scripts/model-invariants.mjs:163` `checkWallRecordsPreserved()` — подтверждено: + существующая инфраструктура #254 сравнивает мультимножество значений `cm` и + именно её комментарий цитирует дефект #253 как мотивирующий пример. Ссылка + ТЗ (§12.3) точна. + - `scripts/mutation-gate.mjs` — подтверждено: реестр мутантов, куда автор обязан + добавить новый мутант под #253; механизм существует и работает, как описано. + - `custom_components/houseplan/validation.py:802` `WALL_SCHEMA` / + `MAX_WALLS = 500` — подтверждено: схема не требует уникальности `key`, значит + "разные записи с одинаковым key" (контракт §6.4 ТЗ) не будет отвергнуто + backend-валидацией; заявление АС9 «persisted schema/backend неизменны» + корректно. +5. Самостоятельно прогнаны заявленные в хендоффе гейты ТЗ (не поверил на слово): + - `node scripts/check-docs.mjs --external` → `Documentation checks passed (7 files, 10 external links).` + - `node scripts/process-gate.mjs --range origin/dev..HEAD --issues` → `гейт пройден, предупреждений 0` + - `git diff --check origin/dev..HEAD` → чисто (exit 0) + - `docs/specs/README.md` — строка `#253` добавлена, ссылка на файл корректна. + +## Находки + +Нет находок уровня High или Medium (ни в скоупе, ни вне скоупа). + +### Low (снимается ревьюером, с записью) + +**L1 — риск `MAX_WALLS=500` от роста числа записей не упомянут в разделе рисков.** +Новый контракт (§6.2) может **увеличивать** число записей `walls[]` на ресайзе +(разбиение частично пересекающегося интервала на покрытый и непокрытый фрагмент) — +раньше число записей могло только оставаться неизменным или уменьшаться (склейка). +Таблица рисков (§14) не называет верхнюю границу `MAX_WALLS = 500` +(`custom_components/houseplan/validation.py:483`) и не описывает поведение при её +достижении (что произойдёт с записью — тем самым интервалом, который фикс обязан +не терять, если backend отвергнет весь `walls[]` целиком по `vol.Length(max=500)`). +Оставляю как Low: практический предел варианта не близок за один жест (нужны сотни +уже существующих записей ещё до правки), поэтому не блокирую — но при реализации +стоит явно решить (тест или защитный код), что происходит, если разбиение подведёт +план к границе. Снимается без правки ТЗ: авто-тесты плана (§12.1) и так покрывают +табличные случаи; граница `MAX_WALLS` не является частью контракта этой задачи и не +меняется ею — упоминание было бы уместно, но его отсутствие не делает ни один AC +непроверяемым. + +## Что проверено и корректно + +- **Продуктовая рамка.** §1 явно привязывает задачу к J6 `docs/SCOPE.md` + («Keep the plan true as the home evolves»), персона/поверхность/момент указаны + конкретно (админ дома, desktop Plan editor, Resize верхней стены сауны). + «Что человек увидит» (§2) — одной парой фраз, без терминов реализации. +- **Диагноз — не догадка.** §3 ТЗ воспроизводит числовой разбор владельца + (24→23 записи, потеря записи 33 см) и код-трассировку `rekeyWallsAfterMove`; + оба независимо подтверждены чтением `src/wall-thickness.ts` (см. «Как + проверялось» выше). Ни одно поведенческое утверждение о текущей системе не + висит без опоры на код или на комментарий владельца. +- **Скоуп/не-скоуп** (§5) — явный и непротиворечивый: `open_spans`/UX/handles/ + инструмент «Толщина»/миграция/Optimize/схема исключены с конкретной причиной + каждый раз, ничего не тянется «раз уж мы здесь». +- **Контракт интервалов** (§6) специфицирован как детерминированный алгоритм + (партиционирование по границам пересечений → перенос покрытых фрагментов по + линейному `t` → сохранение непокрытых): в нём нет шага, оставленного на догадку + реализатора. Коллизия `key` явно не может удалять запись (§6.4), что закрывает + корневую причину из §3 пункт 2. +- **AC1…AC10** (§11) — каждый с однозначным способом доказательства (unit-таблица, + production-bundle smoke на минимизированной fixture, model-invariant gate, + compatibility/docs-тесты); AC1 сформулирован числами реального сценария (24 + записи, конкретные `x/y`), а не общими словами. +- **Модель данных/миграция** (§9) — схема не меняется, legacy key-only записи не + переписываются фоном, потеря уже случившаяся в старых версиях не восстанавливается + «из воздуха» — соответствует стоящему правилу `docs/SCOPE.md` не чинить то, для + чего нет исходных данных. +- **UX/touch** (§8) — новых контролов нет, desktop-first Resize остаётся таким, + общий safety floor (Esc/pointercancel не пишут) переносится без изменений; + согласуется с `TOUCH-SUPPORT.md`/`UX-MODES.md`. +- **i18n/документация/changelog** (§10, §16) — новых строк интерфейса нет; RU/EN + changelog и оба канонических документа (`WALL-THICKNESS.md`, `ARCHITECTURE.md`) + назначены к обновлению в том же коммите, как требует правило «документация в том + же коммите, что поведение». +- **Откат** (§15) — один revert продуктового коммита, без миграции данных; старая + версия читает результат как обычные atomic-записи (согласуется с моделью + `docs/WALL-THICKNESS.md`). +- **Принятые технические предположения** (§17) — все 6 пунктов действительно + невидимы пользователю или являются защитной веткой «не должно происходить при + валидных операциях» с собственной красной unit-диагностикой (§6.3); ни один не + маскирует продуктовый вопрос под техническое решение. +- **Гейты ТЗ.** `check-docs.mjs --external`, `process-gate.mjs --issues`, + `git diff --check` — все три перепрогнаны мной на SHA `d9f7861` и зелёные (см. + «Как проверялось»); запись в `docs/specs/README.md` соответствует требованию + PROCESS.md §7.3. + +## Чего не проверял + +- Не проверялась реализация — она не существует, задача в `S4-spec-review`, код + не тронут ни строкой (только `docs/specs/**` и `docs/specs/README.md`, класс C). +- Не запускал `npm run typecheck`/`npm test`/`npm run build` — на этой дельте + (только документация) они не относятся к предмету ревью ТЗ; дешёвые гейты кода + будут обязательны на этапе код-ревью. +- Не проверял `demo/smoke_resize_wall_thickness.mjs`, `test/wall-thickness.test.mjs` + расширения, новый мутант в `scripts/mutation-gate.mjs` — их не существует до + реализации; план для них (§12) оценён на полноту и однозначность, не на + исполнение. +- Не оценивал производительность/`O(W × E log E)` эмпирически — оценка сложности в + §13 принята как разумная асимптотика для входных размеров реального проекта + (десятки-сотни стен), измерение относится к код-ревью. +- Не связывался с владельцем — открытых продуктовых вопросов в ТЗ нет, и в ходе + ревью я не нашёл ни одного пограничного случая, требующего продуктового решения + (а не технического); единственная находка (L1) — техническая и снимается без + эскалации. + +## Итог + +ТЗ полностью соответствует PROCESS.md §7.1 и §2.4: обязательные разделы на месте, +диагноз проверен по коду и не является догадкой, AC однозначны и у каждого назван +способ доказательства, скоуп/не-скоуп непротиворечивы, откат и release-артефакты +описаны. Единственная находка — Low, снимается с запиской (L1). Задача может +двигаться в «Готово к разработке».