mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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). Задача может
|
||||
двигаться в «Готово к разработке».
|
||||
Reference in New Issue
Block a user