mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,200 @@
|
||||
# SPEC-REVIEW-224-r1
|
||||
|
||||
- Issue: [#224](https://github.com/Matysh/houseplan-card/issues/224) — «Канонизировать координаты при записи конфига»
|
||||
- Этап: spec (PROCESS.md §2.4)
|
||||
- Заход: r1 · блокирующих циклов израсходовано 0 из 4
|
||||
- Артефакт ТЗ: `docs/specs/224-config-coordinate-canonicalization.md`
|
||||
- Ветка: `issue/224-config-coordinate-canonicalization`, SHA на момент ревью: `d5725c2fab2c93c007f9a81b7891fef3311ee1dc`
|
||||
- Ревьюер: Claude, роль «ревьюер ТЗ» (не автор)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Первый заход — разбор полный, дельты предыдущего раунда нет. Диапазон:
|
||||
тело issue #224 и все три комментария владельца (аналитика → ТЗ), файл
|
||||
`docs/specs/224-config-coordinate-canonicalization.md` целиком, сверка с
|
||||
фактическим кодом (`custom_components/houseplan/validation.py`,
|
||||
`src/houseplan-card.ts`, `src/align-grid.ts`, `scripts/mutation-gate.mjs`),
|
||||
с `docs/SCOPE.md`, `docs/CANVAS.md`, `docs/CONFIG-COMPATIBILITY.md`,
|
||||
`docs/TESTING.md`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком.
|
||||
2. Прочитано тело issue #224 и все комментарии (аналитика, ТЗ, ссылка на файл).
|
||||
3. Прочитан ТЗ-файл целиком на ветке задачи (`git show
|
||||
origin/issue/224-config-coordinate-canonicalization:docs/specs/224-...md`).
|
||||
4. Факты ТЗ сверены с реальным кодом, а не приняты на слово:
|
||||
- `_COORD`/`_GEOM`/`POS_SCHEMA`/`LAYOUT_SCHEMA`/`CONFIG_SCHEMA` —
|
||||
`custom_components/houseplan/validation.py:522-1130` — подтверждают
|
||||
заявление §3 о том, что схема лишь проверяет диапазон/конечность и
|
||||
возвращает исходный `float`.
|
||||
- `validation.py:593` (`_dedupe_open_spans`, `round(a[0], 6)`) —
|
||||
подтверждает ссылку issue на прецедент округления в схеме.
|
||||
- `PLAN_SCALE_MIN/MAX`, `plan_angle` (`±360`), `CANVAS_LIMIT = 5000.0` —
|
||||
подтверждают допущение §6.1 о том, что `abs(v)*1e9` не подходит
|
||||
к границе `Number.MAX_SAFE_INTEGER`.
|
||||
- `MARKER_SCHEMA` (`validation.py:1025-1124`) — подтверждает, что у
|
||||
маркера в конфиге есть только `angle`, а не `x/y`; `x/y` живёт в
|
||||
отдельном layout-сторе — таблица allowlist §6.2 разводит их верно.
|
||||
- `_writeConfig`/`_persistLayout` в `src/houseplan-card.ts` существуют
|
||||
и соответствуют описанному месту вставки конвергенции (§8).
|
||||
- `async_save_config_state`/`async_save_layout_state` — подтверждены в
|
||||
`websocket_api.py`, `store.py`, `__init__.py` (общий storage barrier
|
||||
из §7.2 адресует реальные точки записи, а не придуманные).
|
||||
- `snapN()` в `src/align-grid.ts` — подтверждает границу не-скоупа
|
||||
(«это не snap к решётке», §5) как отдельный, не переиспользуемый
|
||||
механизм.
|
||||
- `docs/TESTING.md:2093` — строка «server Undo restores the original
|
||||
noisy bits» существует буквально; ТЗ §9 корректно определяет её как
|
||||
подлежащую замене формулировку, а не выдумывает несуществующий текст.
|
||||
- `scripts/mutation-gate.mjs` существует и поддерживает добавление
|
||||
новых id — мутационные пункты §13 реализуемы.
|
||||
5. `git diff origin/dev...origin/issue/224-config-coordinate-canonicalization`
|
||||
— только `docs/specs/224-....md` (360 строк) и одна строка реестра в
|
||||
`docs/specs/README.md`. Продуктовый код не тронут — ожидаемо для стадии
|
||||
spec review, нарушений класса файлов нет.
|
||||
|
||||
Гейты кода на этапе ревью ТЗ не запускались — работа состоит из одного
|
||||
документа, продуктового кода в ветке нет, весь код-гейт неприменим на этой
|
||||
стадии процесса (PROCESS.md §2.4 предметом ревью называет ТЗ, а не гейты).
|
||||
|
||||
## Проверка обязательных разделов (PROCESS.md §7.1)
|
||||
|
||||
Все обязательные разделы присутствуют и в правильном порядке — сценарий и
|
||||
«что человек увидит» идут первыми:
|
||||
|
||||
| Требование §7.1 | Раздел ТЗ |
|
||||
|---|---|
|
||||
| Сценарий (персона/поверхность/момент) | §1 |
|
||||
| Что человек увидит до/после | §2 |
|
||||
| Проблема | §3 |
|
||||
| Скоуп и не-скоуп | §4, §5 |
|
||||
| Контракт поведения | §6, §7, §8, §9 |
|
||||
| UX | явно закрыт констатацией «нового UI/уведомления/настройки нет» в §2 и §11 (нет визуальных/интерактивных изменений — раздела по существу не требуется) |
|
||||
| Модель данных и миграция | §10 |
|
||||
| i18n | §11 (явное «новых строк нет») |
|
||||
| AC1…ACn с доказательством | §12 |
|
||||
| План автотестов | §13 |
|
||||
| Риски | §14 |
|
||||
| Откат | §14 (последний абзац) |
|
||||
| Release-артефакты | §15 |
|
||||
|
||||
Блок принятых технических предположений (§7.1, «размытое место... явным
|
||||
блоком») присутствует — §16, 5 пунктов, каждый явно помечен «принято
|
||||
предположительно, поменять свободно».
|
||||
|
||||
## Проверка каждого AC на однозначность и способ доказательства
|
||||
|
||||
AC1–AC13 (§12) — каждый сформулирован как проверяемое утверждение с
|
||||
конкретным способом доказательства (fixture, unit, mutation id), не общими
|
||||
словами вида «работает корректно». Ключевые:
|
||||
|
||||
- AC1 (побитовое совпадение Python/TS) — доказывается общей fixture,
|
||||
тест умеет упасть при расхождении округления/tie-правил.
|
||||
- AC2/AC11 — allowlist и negative-contract разведены на два разных теста
|
||||
(позитивный + мутант `quantization-hits-allowlist`), что не даёт
|
||||
«зелёному тесту без падения» проскочить.
|
||||
- AC4 — no-op-контракт явно завязан на CAS-проверку **до** сравнения
|
||||
payload (§7.3 п.5), что не даёт no-op стать обходом optimistic locking —
|
||||
формулировка исключает неоднозначность «а что если revision совпадает
|
||||
случайно».
|
||||
- AC7/AC9 — привязаны к реальному регрессу (#218) и к реальному мутанту
|
||||
(`import-path-bypasses-schema`), не абстрактны.
|
||||
- AC10 — граница «диагональные объекты не прилипают к сетке» имеет числовой
|
||||
порог (максимальный сдвиг из §6.1), проверка не оставлена на усмотрение.
|
||||
|
||||
Ни один AC не описывает недоказуемое субъективное состояние («выглядит
|
||||
нормально», «работает быстро» без числа).
|
||||
|
||||
## Проверка «не выдана ли догадка за решение»
|
||||
|
||||
Утверждения о поведении, которые в ТЗ выглядят как факт, сверены с кодом
|
||||
(см. «Как проверялось», п.4) — расхождений не найдено. Технические решения,
|
||||
не наблюдаемые пользователем (точность 9 знаков, единая формула round-half-
|
||||
away-from-zero, деление allowlist на «геометрию» и «presentation/calibration»,
|
||||
обязательность no-op, отказ от фоновой миграции) явно вынесены в §16 как
|
||||
предположения, а не встроены в текст как свершившийся факт — граница
|
||||
между «решено» и «предположено» в тексте видна.
|
||||
|
||||
Продуктовых вопросов владельцу в ТЗ нет — и это корректно: единственная
|
||||
персона (admin, единственный, кто пишет геометрию), единственный сценарий,
|
||||
пользовательский результат явно нулевой («внешний вид и точность размещения
|
||||
не меняются», §2). Открытых продуктовых развилок (какая персона важнее,
|
||||
что считать приемлемой деградацией) в задаче нет — тип задачи это позволяет:
|
||||
чисто внутренний storage-инвариант без нового UI.
|
||||
|
||||
## Соответствие docs/SCOPE.md
|
||||
|
||||
Задача закрывает J6 («Keep the plan true as the home evolves») — план не
|
||||
должен статистически ломаться после обычных правок. Не пересекает out-of-
|
||||
scope списки SCOPE.md. Явно не трогает vacuum calibration (SCOPE.md не
|
||||
упоминает калибровку впрямую, но задача сама явно выносит её в negative
|
||||
contract — корректная осторожность).
|
||||
|
||||
## Согласованность с #218 и #223
|
||||
|
||||
- С #218: регрессия используется как эталонный AC (AC9), причём именно на
|
||||
проблемном наборе координат из issue.
|
||||
- С #223: ТЗ явно называет изменение формулировки Undo-контракта
|
||||
(«restores the original noisy bits» → «restores the original geometry in
|
||||
canonical representation») и относит правку `docs/TESTING.md` в
|
||||
release-артефакты (§15) — значит, задача не оставляет устаревшее
|
||||
утверждение висеть в чужом документе, а берёт его обновление в свой DoD.
|
||||
Это корректно по §12 запрещённого списка PROCESS.md («Оставили в тексте
|
||||
ревью» не считается закрытием — здесь другой случай: правка чужого
|
||||
упоминания через собственный release-артефакт, а не оставление находки
|
||||
без действия).
|
||||
|
||||
## Находки
|
||||
|
||||
Не найдено. High — 0, Medium — 0, Low — 0.
|
||||
|
||||
Рассмотренные, но не подтвердившиеся сомнения (для прозрачности разбора):
|
||||
|
||||
- *Не размечены все 4 mutation id к конкретным AC-строкам §12 (только 2 из
|
||||
4 — `import-path-bypasses-schema` и `quantization-hits-allowlist` —
|
||||
явно упомянуты в тексте AC).* Не является дефектом: `schema-quantization-
|
||||
removed` и `frontend-writes-raw-coords` — мутанты уровня «сломать всю
|
||||
канонизацию целиком», их естественно ловит позитивная проверка AC2/AC6
|
||||
вместе с обычным test suite; §13 явно называет добавление всех четырёх
|
||||
мутаций отдельным пунктом плана реализации. Не блокирует ТЗ — это
|
||||
плотность документа, не пробел в доказательстве.
|
||||
- *Числовой предел безопасности (`abs(v)*factor < MAX_SAFE_INTEGER`)
|
||||
сформулирован как факт, а не измерен на всех полях allowlist.* Проверено
|
||||
выборочно по действующим `vol.Range` для `plan_scale`, `plan_angle`,
|
||||
`_COORD`/`_GEOM` (±5000) — во всех случаях margin на порядки больше
|
||||
необходимого. Не найдено поле allowlist, способное нарушить это
|
||||
утверждение.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Полнота обязательных разделов §7.1.
|
||||
- Однозначность и доказуемость каждого AC.
|
||||
- Фактическая точность утверждений ТЗ о текущем коде (схемы, storage
|
||||
barrier, frontend write path, snap-механизм, мутационный гейт).
|
||||
- Явное разведение presentation/calibration и geometry в allowlist —
|
||||
сверено с реальными схемами (`MARKER_SCHEMA`, `vacuum.calibration`,
|
||||
`plan_aspect`/`view_box`).
|
||||
- Отсутствие незаявленных догадок — все непродуктовые решения вынесены в
|
||||
явный блок §16.
|
||||
- Отсутствие изменений продуктового кода в ветке на этой стадии (класс A
|
||||
не тронут — только класс C, `docs/specs/**`).
|
||||
- Согласованность с #218 (регрессия) и #223 (обновление устаревшей
|
||||
формулировки Undo в общем документе).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Реализацию — её ещё нет, это стадия spec review.
|
||||
- Работоспособность будущей общей fixture Python/TypeScript — она ещё не
|
||||
написана; корректность будет предметом код-ревью.
|
||||
- Полный список числовых полей allowlist на предмет опечаток/пропусков
|
||||
сверх выборочной проверки (markers, plan_*, walls, openings, decor,
|
||||
room_drafts, partitions, wall_columns, open_spans) — сверены ключевые
|
||||
представители каждой категории, не каждое поле построчно по схеме.
|
||||
- Golden/smoke/performance — неприменимо на этой стадии, кода нет.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. ТЗ полное, однозначное, каждый AC проверяем и привязан к способу
|
||||
доказательства, факты о коде подтверждены чтением исходников, догадки не
|
||||
выданы за решения. Задача уходит в «Готово к разработке».
|
||||
Reference in New Issue
Block a user