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

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 нет.

Вердикт: зелёный.

---

Материал раунда

• Ветка: 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