docs: review document for #477

Issue: #477
User-Visible: no

docs/reviews/SPEC-REVIEW-477-r1.md

@@ -0,0 +1,274 @@
+# SPEC-REVIEW-477-r1
+
+- **Issue:** #477 — «Оптимизировать планы» не должна требоваться повторно
+- **Этап:** ТЗ на ревью (PROCESS.md §2.4), трек **полный** (не `small`)
+- **Материал:** `docs/specs/477-editor-writer-fixed-point.md` на SHA
+  `aa962ed2c5a03bd2f50b4ae066968761cd053585` (дерево `56e39416...`), ветка
+  `issue/477-optimizer-fixed-point`
+- **Заход:** r1 · блокирующих циклов израсходовано 0/4
+- **Вердикт: зелёный**
+
+## Вердикт
+
+`Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче | · Документ: docs/reviews/SPEC-REVIEW-477-r1.md`
+
+## Скоуп разбора
+
+Первый заход, ТЗ полного трека. Разбирал целиком: `docs/SCOPE.md`, `AGENTS.md`,
+`PROCESS.md` целиком, тело issue #477 и все три комментария владельца
+(повторная аналитика после #478, поправка про off-grid мебель, ссылка на
+готовое ТЗ), сам файл `docs/specs/477-editor-writer-fixed-point.md` (492
+строки) и его запись в `docs/specs/README.md`. Отдельно поднял канонические
+документы затронутых подсистем: `docs/CANVAS.md` (§9.4, §9.5),
+`docs/WALL-THICKNESS.md` (контракт #276/#281/#296), `docs/USER-GUIDE.ru.md`
+(терминология «цепочка», «Оптимизировать планы», `Esc`), `docs/TOUCH-SUPPORT.md`
+(контракт touch-редакторов), `docs/ARCHITECTURE.md`/`docs/CONFIG-COMPATIBILITY.md`
+(контракты #282, #314).
+
+Владелец в третьем комментарии прямо попросил независимое ревью отдельно
+проверить три границы: seeded reconciliation, Undo/Redo после завершения
+wall-chain, исключение continuous transform мебели/изображений из Optimize —
+все три разобраны отдельно ниже (§6.1–6.2, §7, §9 документа ТЗ).
+
+## Как проверялось
+
+Это ревью ТЗ, не код-ревью: гейты `typecheck`/`test`/`build`/`check-docs`
+здесь не применяются — на branch нет продуктового кода (см. ниже), только
+документация. Основной метод — **состязательная фактчек-проверка**: каждое
+техническое утверждение ТЗ, которое можно спутать с догадкой, выданной за
+факт, я сверил прямым чтением текущего `dev`-кода (не поверил заявлению
+автора). Не поверил на слово ни одному месту, где заявлен *текущий* дефект
+(«функция X сейчас не делает Y»), и перепроверил каждое такое место чтением
+тела функции. Список того, что сверено буквально — раздел «Что проверено»
+ниже.
+
+Отдельно проверил состав ветки: `git diff <merge-base d12b1f20>..HEAD` —
+ветка добавляет только `docs/specs/477-editor-writer-fixed-point.md` и одну
+строку в `docs/specs/README.md`. Полный `git diff origin/dev..HEAD` показывает
+больше файлов (`.github/workflows/validate.yml`, `scripts/mutation-gate.mjs`,
+удаление `docs/reviews/CODE-REVIEW-475-r*.md` и др.) — это независимый прогресс
+`dev` после точки расхождения (`origin/dev` и эта ветка разошлись от общего
+предка `d12b1f20`, ни один не является предком другого), а не изменения этой
+задачи; branch не рибейзился на текущий `dev`, но это не создаёт конфликта
+содержимого для ревью ТЗ (класс C, документация, DoR ещё не пройден, кода нет).
+Отмечаю это для протокола, чтобы автор рибейзнул ветку перед переходом к
+`S6-in-progress`, если к тому моменту `dev` уйдёт ещё дальше — блокирующей
+находкой не считаю, т.к. на этапе ТЗ рибейз не требуется.
+
+## §7.1: обязательные разделы — все на месте
+
+| Раздел §7.1 | Где в ТЗ |
+|---|---|
+| Сценарий (персона/поверхность/момент) | §1 |
+| Что человек увидит до/после | §2 |
+| Проблема | §3 |
+| Скоуп и не-скоуп | §5 |
+| Контракт поведения | §6, §7, §8, §9 |
+| UX | §13 |
+| Модель данных и миграция | §12 |
+| i18n | §13 |
+| AC1…ACn с доказательством | §14 |
+| План автотестов | §15 |
+| Риски | §16 |
+| Откат | §17 |
+| Release-артефакты | §18 |
+
+Плюс необязательный, но полезный раздел §19 «Принято предположительно» —
+именно то место, где по правилу §7.1 должны жить технические (не
+продуктовые) допущения автора, доступные для оспаривания ревьюером.
+
+## Продуктовая рамка (SCOPE.md)
+
+Job **J6** — «Keep the plan true as the home evolves» — явно упоминает
+drag/resize, merge/split, оптимистичную блокировку как часть этой работы.
+Задача устраняет технический долг ровно в этой job: план не должен «портиться»
+незаметно после обычного редактирования. Персона (§1 ТЗ) — Home admin,
+единственная персона, которая вообще видит редакторы (SCOPE.md: «View mode is
+the product», редакторы — admin-only). Сценарий и before/after (§1–2)
+отвечают на оба обязательных продуктовых вопроса без терминов реализации в
+разделе «что увидит» — придаточные предложения там пользовательские
+(«завершение цепочки», «после удаления или объединения комнаты»), а не
+кодовые идентификаторы. Технические термины (`_finishWallChain`,
+`mergeCollinearPartitions` и т.п.) появляются только в §3 и далее — это
+правильное место для них.
+
+## Открытый вопрос? Разбор по существу
+
+Инструкция ревью требует: «не бывает сложной задачи без единого открытого
+вопроса». Для этой задачи он был — и уже закрыт **до** написания ТЗ: владелец
+отработал открытые продуктовые вопросы в двух отдельных комментариях анализа
+(«Повторная аналитика после #478» и «Поправка… off-grid геометрия мебели»),
+явно сузив скоуп (исключил `room_drafts`/room-acceptance, разобранные #478) и
+дав прямое продуктовое решение по мебели/изображениям («их transform должен
+сохраняться целиком»). Раздел §19 документирует остаточные **технические**
+развилки (не продуктовые), как и требует процесс: продуктовая неопределённость
+устранена перепиской до захода в `S3-spec`, техническая — явным блоком
+«поменять свободно». Отдельного неотвеченного продуктового вопроса в тексте
+ТЗ не осталось, и это не тихое замалчивание, а прослеживаемый результат.
+
+## Что проверено буквальным чтением кода (не поверил на слово)
+
+Все ссылки — на исходники на SHA `aa962ed2` (текущий `dev`, из которого ТЗ
+исходит: продуктовый код в этой задаче ещё не пишется).
+
+- **`_finishWallChain`** (`src/houseplan-editor-runtime.ts:1474-1484`) —
+  подтверждено: тело только очищает `_path`, `_activeWallChainId`,
+  `_activeWallChainPartitionIds`, `_wallChainSegmentCms`, `_wallChainRedo`,
+  `_closingWallCm`, вызывает `_clearPlanSnapHover()`. Merge/reconcile не
+  вызывается — центральное утверждение проблемы (§3) верно буквально.
+- **`_mergeSpacePartitions`** (`houseplan-editor-runtime.ts:1458`) — существует,
+  принимает `seedIds?: string[]`, оборачивает `mergeCollinearPartitions`; по
+  всему `src/*.ts` вызывается только собственным определением/обёрткой в
+  `houseplan-card.ts` — реально нигде не вызывается в рантайме. Утверждение
+  «фактически не вызывается» — верно.
+- **`mergeCollinearPartitions`** (`src/wall-merge.ts:148`) — контракт seed-scope,
+  epsilons, детерминированный survivor, цикл до fixed point — присутствуют в
+  коде уже сейчас (задел под #229). ТЗ §6.1 описывает использование
+  существующего контракта, а не выдумывает новый.
+- **`commitWallChainSegmentGeometry()`** (`src/draft-live-commit.ts:60`) —
+  прочитан целиком: один local physical check, один junction pass, один
+  `_recordGeometry` + один `_saveConfig()`; ни merge, ни reconcile, ни
+  `optimizePlans` не вызываются. §11 верно буквально.
+- **`_commitPhysicalGeometry`** guard `wallModelOffGridValueCount(after) <=
+  wallModelOffGridValueCount(before, authored)` — подтверждён
+  (`houseplan-editor-runtime.ts:2113-2114`, тот же паттерн в
+  `draft-live-commit.ts:107-108`). `wallModelOffGridValueCount` определена в
+  `src/wall-segment-model.ts:88`.
+- **`_canCommitSpace`** (`houseplan-card.ts:1475`) — admin/authority guard,
+  используется в путях commit — §12 «current writer не расширяет права» не
+  голословно.
+- **`_confirmRoomDelete`** (`houseplan-editor-runtime.ts:5485-5579`) и
+  **`_commitMerge`** (`:6408-6430`) прочитаны целиком: обе фильтруют
+  `sp.rooms`, ни одна не трогает `marker.room_id` или
+  `marker.vacuum.segment_map`. Починка orphan `room_id` сейчас существует
+  только в `space-reference-repair.ts`, вызываемой из `plan-optimizer.ts` — то
+  есть баг реален именно там, где заявлен (§3/§8): чинится только следующим
+  Optimize, а не в момент удаления/слияния.
+- **`resizeFurnitureTransform`** (`src/furniture.ts:524`) — существует,
+  комментарий в коде прямо говорит про continuous resize (#383).
+- **`alignAllToGrid`** (`src/align-grid.ts:145`) — decor-цикл не выделяет
+  `furniture`/`image` в отдельную ветку и гонит их через общий box-snap,
+  инкрementируя `moved`/`maxShiftCm` — подтверждает текущий баг, который §9
+  ТЗ должен закрыть.
+- **`docs/CANVAS.md` §9.4** (строка 512) — «Furniture resize is the explicit
+  exception to positional quantisation… Furniture placement and movement
+  remain grid-bound» — дословно то же, что заявляет §9 ТЗ. Раздел пока не
+  упоминает `image`, но §18 ТЗ обязывает обновить `CANVAS.md` этим же
+  коммитом — не расхождение, а зафиксированный будущий DoD.
+- **`docs/WALL-THICKNESS.md`** (строка 512) — контракт #276/#281/#296 (proven
+  section absorption, deterministic residuals, `max(roomCm, partitionCm)`)
+  присутствует и совпадает с §6.2–6.3 ТЗ.
+- **Performance-бюджеты** (`demo/benchmark_wall_draw_click.mjs:51-57`) —
+  `medianMs: 150, maxMs: 250, remoteMedianMs: medianMs*1.5+20` совпадают с
+  числами §11 ТЗ буквально — не выдуманы.
+- **#282/#314** — реальные, ранее принятые контракты (ADR стабильной
+  идентичности стен и rollback после отказа записи), многократно
+  цитируются в `docs/ARCHITECTURE.md`, `docs/CONFIG-COMPATIBILITY.md`,
+  комментариях кода — не выдуманные номера.
+- **`marker.vacuum.segment_map`** — поле есть в `src/types.ts:140`.
+- **`docs/specs/README.md`** и **#478** — запись про #478 подтверждена,
+  `docs/specs/478-remove-room-drafts.md` и `docs/reviews/CODE-REVIEW-478-r*.md`
+  существуют: #478 реализована и прорецензирована, а не просто задумана — ТЗ
+  #477 корректно опирается на закрытый факт, а не на обещание.
+- **`_routeDepartureHandled`**, `hashchange`-listener в `houseplan-card.ts` —
+  подтверждают, что «route departure» и смена space через hash — существующие
+  механизмы, на которые §6.4 ссылается как на finish-owners.
+- **`async_save_config_state`/`canonicalize_config_geometry`**
+  (`custom_components/houseplan/store.py:200,220`) — вызов канонизации перед
+  записью подтверждён; сам факт («числовая канонизация — на каждой записи»,
+  из тела issue) верен. Не искал отдельно, называется ли обёртка ровно
+  `async_save_config_state` во всех путях сохранения — не критично для ТЗ,
+  которое само по себе на эту функцию не опирается (это констатация уже
+  закрытого класса, не предмет #477).
+
+Отдельно проверил семантическую состоятельность формального инварианта §4:
+`optimizePlans(P).changed === false ⇒ optimizePlans(W(P)).changed === false`
+для одной операции `W` индукцией распространяется на последовательность
+операций (после `W(P)` условие снова выполнено, значит следующая `W'`
+сохраняет его же). Формулировка корректна и не скрывает индукционный шаг.
+
+## Три границы, которые владелец попросил проверить отдельно
+
+1. **Seeded reconciliation (§6.1–6.2).** Scope ограничен «связным
+   merge-компонентом, к которому относится хотя бы один seed» — это
+   осмысленная граница: incidental collinear соседи, физически примыкающие к
+   только что дорисованной цепочке, обязаны мержиться (иначе шов остаётся),
+   а несвязанные партиции плана трогать нельзя (иначе цена unbounded и это
+   прямо запрещено §5.2 «merge partitions всего пространства на каждом
+   клике»). §11 честно называет риск («если нельзя доказать без full-space
+   sweep — назад в S3-spec») вместо того, чтобы тихо платить цену на каждом
+   клике.
+2. **Undo/Redo после finish (§7).** Контракт различает активную цепочку
+   (буквальный snapshot, посегментный Undo) и завершённую (canonical
+   snapshot через тот же lossless finalizer на каждом шаге истории) без
+   добавления скрытой history-команды — соответствует принципу «пользователь
+   рисовал стены, а не запускал скрытую команду» (§6.3) и не открывает второй
+   источник этой же геометрии.
+3. **Furniture/image exclusion (§9).** Формулировка «весь transform… byte-
+   equivalent» и явное решение не пытаться отличить legacy off-grid transform
+   от нового legal continuous resize (провенанс недоступен) — прямое
+   повторение продуктового решения владельца из второго комментария, ничего
+   не додумано.
+
+## Находки
+
+Нет. High: 0, Medium: 0, Low: 0.
+
+Единственное отмеченное выше по разделу «Как проверялось» (branch не
+рибейзнут на текущий `dev`) — не находка, а операционное замечание для
+следующего этапа; на этапе ТЗ (класс C, без кода) оно не блокирует переход в
+`S5-ready`.
+
+## Что не проверял
+
+- Не проверял `python -m pytest` / typecheck / build — на ветке нет
+  продуктового кода, гейты неприменимы к этапу ТЗ.
+- Не проверял, что ровно эта функция называется `async_save_config_state` во
+  всех путях сохранения бэкенда (см. пояснение выше) — не влияет на выводы
+  ТЗ.
+- Не оценивал реализуемость perf-бюджета §11 количественно (насколько
+  реалистичен «не более 1.5× median на удвоении unrelated партиций») —
+  это то, что должен доказать сам код и его perf-witness на код-ревью, а не
+  ТЗ; сам числовой контракт корректно унаследован от #461.
+- Не проводил самостоятельный код-ревью существующих `wall-merge.ts` /
+  `coincident-partitions.ts` на предмет иных дефектов, не связанных с #477 —
+  вне скоупа ревью ТЗ.
+- Единственная умышленно неполная иллюстрация — таблица §10 «минимальная
+  матрица» не перечисляет явно Split Room. Это не пробел ТЗ: сама таблица
+  названа «минимальной», а AC8 требует от реализации exhaustive-перечисления
+  **всех** текущих structural/layout writers с тестом, который краснеет на
+  недостающей строке — split, если у него найдётся долг, будет пойман тем же
+  AC8 на код-ревью. Ни владелец, ни разбор автора не называли Split
+  источником долга, и текущий код Split строит room-граничные стены, а не
+  независимые партиции, так что источника такого класса проблем там
+  структурно нет — но это не проверено построчным чтением `_splitClick`/
+  `splitRoom`, поэтому отмечаю как то, что не проверял, а не как принятое на
+  веру утверждение.
+
+## Итог
+
+ТЗ полное по §7.1, каждый AC проверяем и указывает способ доказательства,
+технические допущения явно обособлены в §19 и оспоримы, продуктовые вопросы
+закрыты владельцем до входа в этап ТЗ, а все проверенные технические
+утверждения о текущем состоянии кода подтвердились буквальным чтением — без
+единой догадки, выданной за факт. Три специально запрошенных владельцем
+границы (seeded reconciliation, Undo/Redo, furniture/image exclusion)
+разобраны предметно и непротиворечиво. Оснований для Medium/High нет.
+
+**Вердикт: зелёный.**
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/477-optimizer-fixed-point`, коммит `aa962ed2c5a0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `56e39416687f5bb6bd1242ed4c5722ca2ba34cc4`
+  ```
+  git log --all --format='%H %T' | grep 56e39416687f
+  ```
+- ТЗ `docs/specs/477-editor-writer-fixed-point.md`, блоб `a07a7248e008229e6e865ef793b213fdb524d5e4`
+  ```
+  git log --all --find-object=a07a7248e008229e6e865ef793b213fdb524d5e4 -- docs/specs/477-editor-writer-fixed-point.md
+  ```