From cd17a0b00f9ca45b15b959077bdee4a5fa3f8154 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 17:19:51 +0000 Subject: [PATCH] docs: spec review r1 for optimizer micro-interval cleanup (#198) Issue: #198 User-Visible: no --- docs/reviews/SPEC-REVIEW-198-r1.md | 211 +++++++++++++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-198-r1.md diff --git a/docs/reviews/SPEC-REVIEW-198-r1.md b/docs/reviews/SPEC-REVIEW-198-r1.md new file mode 100644 index 00000000..7fec6eed --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-198-r1.md @@ -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 → в задаче**