mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
docs: spec review r1 for optimizer micro-interval cleanup (#198)
Issue: #198 User-Visible: no
This commit is contained in:
committed by
Sergey Matyunin
parent
3a68efa62f
commit
cd17a0b00f
@@ -0,0 +1,211 @@
|
||||
# Ревью ТЗ — issue #198, цикл r1
|
||||
|
||||
- Этап: `S4-spec-review` (PROCESS.md §2.4)
|
||||
- Артефакт ТЗ: [`docs/specs/198-optimize-micro-interval.md`](../specs/198-optimize-micro-interval.md),
|
||||
коммит `9d6cd8b` на ветке `issue/198-optimize-micro-interval`
|
||||
- Issue: [#198](https://github.com/Matysh/houseplan-card/issues/198)
|
||||
- Ревьюер: Claude (роль «ревьюер ТЗ», отдельная сессия от аналитика/автора)
|
||||
- Трек: обычный (не `small`) — совпадает с метками issue (`bug`, `P3`,
|
||||
`S4-spec-review`, без `small`) и с явной отметкой аналитики «лёгкий трек:
|
||||
нет»; сложность/риск оценены аналитикой в 5/10 и 7/10, что и оправдывает
|
||||
обычный трек с лимитом ревью 4 цикла (не 2, как на лёгком/коротком).
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Оценивалось ТЗ `docs/specs/198-optimize-micro-interval.md` целиком: наличие
|
||||
обязательных разделов §7.1 PROCESS.md, однозначность и доказуемость AC1–AC10,
|
||||
отсутствие догадок, выданных за решённый факт, соответствие `docs/SCOPE.md`
|
||||
(job J6 «Keep the plan true as the home evolves»), `docs/WALL-THICKNESS.md`,
|
||||
`docs/USER-GUIDE.ru.md`, `docs/CONFIG-COMPATIBILITY.md` и текущему коду
|
||||
(`src/plan-optimizer.ts`, `src/wall-thickness.ts`,
|
||||
`custom_components/houseplan/websocket_api.py`,
|
||||
`custom_components/houseplan/validation.py`). Продуктовый код не менялся и не
|
||||
мог быть изменён (задача на этапе ТЗ) — диапазон коммитов
|
||||
`origin/dev...origin/issue/198-optimize-micro-interval` содержит только
|
||||
`docs/specs/198-optimize-micro-interval.md` и одну строку в
|
||||
`docs/specs/README.md`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком.
|
||||
2. Прочитано тело issue #198 и оба комментария через `gh issue view --json`,
|
||||
включая полную историю меток (`gh api .../events`) — аналитика с
|
||||
продуктовыми вопросами Q1–Q3 под `blocked`+`S3-spec` (14:46) и последующий
|
||||
комментарий автора «ТЗ готово к ревью» со ссылкой на defaults владельца
|
||||
(17:10), между которыми снята метка `blocked` (17:08). Проверено, что все
|
||||
три вопроса имеют явный default в тексте аналитики и что итоговое ТЗ (§6)
|
||||
реализует ровно эти default'ы, а не какой-то другой вариант.
|
||||
3. Прочитан весь текст ТЗ построчно, сверен с §7.1 (обязательные разделы) и
|
||||
§2.5 (DoR-чеклист).
|
||||
4. Прочитан канонический `docs/WALL-THICKNESS.md` целиком — модель `walls`
|
||||
(§1: «one physical stretch has one thickness», exact endpoints как
|
||||
продуктовая граница толщины, «Normalisation merges consecutive solid
|
||||
pieces into each maximal run of equal thickness»), чтобы проверить, что
|
||||
ТЗ не переопределяет lossless-контракт, а вводит узкое исключение только
|
||||
для явного Optimize.
|
||||
5. Проверены заявленные идентификаторы и функции чтением продуктового кода
|
||||
`src/plan-optimizer.ts` (`optimizePlans()` целиком, строки 248–407) и
|
||||
`src/wall-thickness.ts` (`export function degradeWalls` :274,
|
||||
`rekeyWallsAfterMove` :368, `wallIntervals` :1186,
|
||||
`normalizeWallIntervals` :1258) и `src/space-geometry.ts`
|
||||
(`NORM_W=1000`, `GRID_PITCH=NORM_W/GRID_N`, `GRID_STEP_N=1/GRID_N`).
|
||||
Подтверждено: все константы и функции, на которые ссылается ТЗ, существуют
|
||||
и называются именно так; заявленный порядок вызова в `optimizePlans()` —
|
||||
`rekeyWallsAfterMove()` → `normalizeWallIntervals()` → `degradeWalls()`
|
||||
(строки 353–364) — соответствует месту вставки, которое ТЗ предлагает
|
||||
(«после rekey… и до финальной lossless-нормализации»): новый helper встаёт
|
||||
между rekey и `normalizeWallIntervals()`, чтобы последняя сама собрала
|
||||
уже одинаковые по толщине три записи в один максимальный пролёт — именно
|
||||
так, как описывает §6 ТЗ.
|
||||
6. Проверено происхождение `canonicalized`/`wallsMerged` (строки 280–373,
|
||||
401–402 `plan-optimizer.ts`): `canonicalized` инкрементируется при
|
||||
изменении JSON-снимка spans/links/walls пространства, `wallsMerged`
|
||||
считает разницу числа записей до/после — ТЗ корректно заявляет
|
||||
переиспользование существующих полей отчёта без нового счётчика (§4, §8).
|
||||
7. Проверено соответствие терминологии `docs/USER-GUIDE.ru.md` §19
|
||||
(«Общие настройки → Оптимизировать планы», «серверная отмена», строка
|
||||
таблицы «Стены | Одинаковые соседние участки толщины объединяются»,
|
||||
`gs.optimize_undo`/`gs.optimize_undone` в `src/i18n/ru.json`/`en.json`,
|
||||
реальный UI-код `_optimizeUndoBusy`/`_canOptimizeUndo`/
|
||||
`houseplan/plan/optimize_undo` в `src/houseplan-card.ts`) — ТЗ не изобретает
|
||||
новый UI-текст и корректно описывает уточнение уже существующей строки
|
||||
таблицы, а не новую функцию.
|
||||
8. Проверено, что backend не требует изменений (§12 ТЗ): `ws_plan_optimize`
|
||||
в `custom_components/houseplan/websocket_api.py` (строки 1323–1441)
|
||||
принимает уже готовый `config`/`layout` от фронтенда как непрозрачный
|
||||
блок и делает только атомарную запись + одноглубинный backup для Undo;
|
||||
`validation.py` (строки 460–842) уже валидирует записи `walls` с `a`/`b`
|
||||
(точные endpoints) — существующая с #197. Новых полей схемы, миграции
|
||||
`model_version` или изменений `CONFIG_SCHEMA` не требуется, что совпадает
|
||||
с заявлением ТЗ.
|
||||
9. Прочитан `docs/CONFIG-COMPATIBILITY.md` целиком — подтверждено отсутствие
|
||||
необходимости новой записи реестра: результат Optimize — один валидный
|
||||
`WallEntry{cm,a,b}` в уже существующем формате (используется с #197),
|
||||
readable старыми клиентами тем же механизмом key-fallback, что и любая
|
||||
другая exact-endpoint запись; ТЗ (§7, §12) корректно не заявляет отдельный
|
||||
compatibility-артефакт.
|
||||
10. Проверено существующее browser-покрытие Optimize UI: `demo/smoke_grid_snap.mjs`
|
||||
(строка 195, проверяет ровно один вызов `houseplan/plan/optimize`) и
|
||||
`demo/smoke_warm_dialogs.mjs` (диалог `_alignDialog`) — ни один не
|
||||
прогоняет полный цикл Preview→Apply→Undo→Cancel. Заявление ТЗ (§10)
|
||||
«Targeted smoke расширяет существующий Optimize smoke либо получает
|
||||
отдельное имя» корректно не переоценивает существующее покрытие и
|
||||
оставляет выбор конкретного файла реализации (§13 п.3).
|
||||
11. Проверена реестровая практика `mutation-gate` (`scripts/mutation-gate.mjs`,
|
||||
`test/mutation-gate.test.mjs`, `.github/workflows/mutation-gate.yml`,
|
||||
аналогичные ссылки в специфика́циях #200/#201/#203) — AC9 ссылается на
|
||||
реально существующий, а не изобретённый инструмент.
|
||||
12. Проверено, что `docs/specs/README.md` получил ровно одну новую строку в
|
||||
таблице «Issue ↔ ТЗ» (без добавления запрещённой §7.3 колонки «Статус
|
||||
ТЗ»), и что коммит `9d6cd8b` несёт корректные трейлеры `Issue: #198` /
|
||||
`User-Visible: no` (документация без изменения поведения на этой стадии,
|
||||
класс C).
|
||||
|
||||
Код не менялся, чтения было достаточно: вопрос ревью ТЗ — «выполнимо и
|
||||
проверяемо ли», а не «работает ли реализация». Гейты (`typecheck`/`test`/
|
||||
`build`/smoke) не запускались — на этапе ТЗ они неприменимы, продуктовый код
|
||||
отсутствует.
|
||||
|
||||
## Проверено и корректно
|
||||
|
||||
- **Все обязательные разделы §7.1 присутствуют**: сценарий и персона (§1), что
|
||||
человек увидит до/после без терминов реализации (§2), проблема/подтверждённая
|
||||
причина (§3), scope/non-scope (§4–5), контракт поведения (§6),
|
||||
preview/запись/Undo/данные (§7), UX/i18n/touch (§8), AC1–AC10 с
|
||||
доказательством (§9), план автотестов (§10), риски и производительность
|
||||
(§11), release-артефакты и rollback (§12), явный блок принятых предположений
|
||||
(§13).
|
||||
- **Продуктовые вопросы Q1–Q3 закрыты по существу, а не молча.** Аналитика
|
||||
владельца сформулировала три вопроса с default'ами (изолированный островок
|
||||
короче `0.5×pitch`; торец/разные соседи — всегда сохранять; очистка только
|
||||
в explicit Optimize); ТЗ §6 реализует ровно эти default'ы пункт за пунктом
|
||||
(условия 1–6), а §13 п.5 явно фиксирует «продуктовых вопросов больше нет».
|
||||
Открытых догадок, выданных за факт, не найдено — все нетривиальные
|
||||
утверждения (о происхождении дефекта, о поведении `rekeyWallsAfterMove()`,
|
||||
о том, что Optimize не создаёт островок из равномерного пролёта)
|
||||
дословно совпадают с уже подтверждённым исполнением на fixture #197 из
|
||||
комментария аналитики, а не являются новой гипотезой автора ТЗ.
|
||||
- **Контракт §6 предметно защищает документированный lossless-инвариант.**
|
||||
Все шесть условий (строгий порог длины, оба соседа collinear/solid/равной
|
||||
толщины, отличие центральной толщины, запрет на любой топологический разрыв
|
||||
на границах центра, принадлежность одной физической линии) вместе не дают
|
||||
схлопнуть намеренную границу толщины, торцевой интервал или разнородных
|
||||
соседей — соответствует Q2 defaults и §1 `WALL-THICKNESS.md`
|
||||
(«exact endpoints make a thickness boundary independent…»).
|
||||
Non-cascading правило (кандидаты по исходному, не по уже изменённому
|
||||
snapshot; конфликтующий interval не трогается) закрывает реальный риск
|
||||
«каскад съедает длинную зону», явно перечисленный в §11.
|
||||
- **Все AC пронумерованы, однозначны и несут явный способ доказательства**
|
||||
(unit/matrix/existing regression/browser smoke/mutation gate/gates),
|
||||
выполняется требование DoR §2.5. AC1–AC2 задают точную числовую границу в
|
||||
обоих масштабах, AC3–AC5 — исчерпывающую негативную матрицу и
|
||||
permutation/immutability, AC7 — сквозной UI-цикл Preview/Apply/Undo/Cancel,
|
||||
AC9 явно требует падающего мутанта.
|
||||
- **Non-scope точен и не оставляет двойного толкования** (§5): явно исключены
|
||||
универсальный минимум длины, фоновая очистка при Save/render/редактировании,
|
||||
изменение lossless-хелперов и schema, починка #197/#199/#201, эвристики по
|
||||
большинству/максимуму — каждый пункт реальная граница, подтверждённая
|
||||
чтением кода (backend/validation не требуют изменений — see «Как
|
||||
проверялось» п.8–9).
|
||||
- **Backend и compatibility корректно исключены из release-артефактов**:
|
||||
подтверждено чтением `websocket_api.py`/`validation.py`, что `config`
|
||||
передаётся фронтендом уже готовым и валидируется существующей с #197
|
||||
схемой `WallEntry{cm,a,b}`; `docs/CONFIG-COMPATIBILITY.md` не требует новой
|
||||
записи, потому что результат Optimize — не новый формат данных.
|
||||
- **UX/i18n/touch раздел корректно пуст** (§8): новых controls, строк и
|
||||
touch-путей нет, что подтверждено чтением реального UI-кода Optimize/Undo
|
||||
(`_canOptimizeUndo`, `gs.optimize_undo`) — фиче не нужна отдельная
|
||||
поверхность, значит `docs/TOUCH-SUPPORT.md` не нарушается по построению.
|
||||
- **Технические решения корректно помечены как предположения** (§13):
|
||||
определение «геометрического узла», момент проверки соседства (после
|
||||
rekey), точное расположение helper'а и имя smoke-файла. Ревьюер эти
|
||||
предположения не оспаривает — они разумны, не влияют на AC и явно оставлены
|
||||
свободными для реализации.
|
||||
- **Release-артефакты названы явно** (§12): оба changelog при
|
||||
`User-Visible: yes` в implementation-коммите, точная строка в
|
||||
`docs/USER-GUIDE.ru.md` §19, уточнение lossless-контракта в
|
||||
`docs/WALL-THICKNESS.md`, обновление `docs/TESTING.md`, unit/smoke/mutation;
|
||||
golden/i18n/backend/security/performance-артефакты корректно исключены с
|
||||
объяснением почему (данные без нового рендер-пути, backend не тронут,
|
||||
профиль малых списков стен).
|
||||
|
||||
## Находки
|
||||
|
||||
Находок High, Medium или Low не выявлено. Спецификация грамотно опирается на
|
||||
уже подтверждённое исполнением (аналитика владельца) и на реально
|
||||
существующий код/документацию; ни одно нетривиальное продуктовое или
|
||||
техническое утверждение не осталось непроверенным при чтении кода.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверялась реализация — на этапе ТЗ её не существует; код-ревью будет
|
||||
отдельным циклом (`S7-code-review`) после написания кода.
|
||||
- Не запускались `npm test`/`npm run build`/смоки/gates: код не менялся, гейты
|
||||
ТЗ не требуют их прогона на этом этапе (документация — класс C, продуктовый
|
||||
код не тронут).
|
||||
- Не проверялась конкретная реализация helper'а (структура функции,
|
||||
сигнатура, точное имя файла unit/smoke) — ТЗ корректно оставляет это
|
||||
технической свободой реализации (§13 п.3) при неизменных AC.
|
||||
- Не оценивалась производительность на реальном большом плане — риск в §11
|
||||
обоснованно сведён к «полиномиальная проверка на малых wall lists, вне
|
||||
render/state tick»; это утверждение проверяется код-ревью, а не
|
||||
спецификацией.
|
||||
- Не проверялось независимо, кем по факту было принято решение владельца по
|
||||
Q1–Q3 (аккаунт `Matysh`, тип `User`, без GitHub App, снял `blocked` и
|
||||
оставил комментарий) — это происходит за пределами репозитория
|
||||
(см. `docs/SCOPE.md`: Telegram — основной канал обратной связи владельца),
|
||||
и проверка этого факта вне полномочий и возможностей ревью ТЗ; сам текст
|
||||
defaults и их реализация в ТЗ взаимно согласованы, что и является предметом
|
||||
этого ревью.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Все обязательные разделы на месте, все AC однозначны и снабжены способом
|
||||
доказательства, причина дефекта и предлагаемый контракт подтверждены чтением
|
||||
кода и совпадают с уже исполненной диагностикой аналитики (а не являются
|
||||
новой непроверенной гипотезой), lossless-инвариант `WALL-THICKNESS.md` явно
|
||||
защищён шестью условиями и негативной матрицей, non-scope точен, backend и
|
||||
config-compatibility корректно исключены с подтверждением по коду. Открытых
|
||||
продуктовых вопросов нет, Q1–Q3 закрыты по существу. Находок нет.
|
||||
|
||||
**Вердикт: зелёный · цикл r1/4 · High: 0 · Medium: 0 → в задаче**
|
||||
Reference in New Issue
Block a user