docs: review document for #273

Issue: #273
User-Visible: no

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

@@ -0,0 +1,92 @@
+# SPEC-REVIEW-273-r1
+
+- Issue: [#273](https://github.com/Matysh/houseplan-card/issues/273) — Optimize сохраняет sub-grid островок толщины у T-узла: 22→15→22
+- Этап: ТЗ на ревью (`S4-spec-review`), заход **r1** (первый; предыдущих раундов нет)
+- Артефакт ТЗ: `docs/specs/273-optimize-topology-island.md`
+- Проверено на SHA: `b5fbe633a60f8a2091552204fc7ff9e3da67fb06` (HEAD ветки `issue/273-optimize-topology-island` на момент ревью)
+- Лёгкий/короткий трек: нет (`small`/`trivial` не назначены) — ТЗ обязано жить файлом, что соблюдено
+- Вердикт: **жёлтый**
+
+## Скоуп
+
+Явное расширение guard'а `collapseIsolatedWallThicknessIslands()` (#198, `src/plan-optimizer.ts:99-200`): разрешить схлопывание sub-half-grid островка толщины, когда ровно один его конец — реальный room/T topology node, а второй — синтетическая off-grid точка на том же прямом parent edge, при равных соседних толщинах. Изменение затрагивает только explicit Optimize; runtime/editor нормализация остаётся lossless (не входит в скоуп).
+
+Job по `docs/SCOPE.md`: **J6** («Keep the plan true as the home evolves», explicit maintenance должен приводить план к физически осмысленному и идемпотентному виду) — названо в ТЗ явно (§1), обоснование корректно.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (обязательный порядок чтения).
+2. Прочитано тело issue #273 и оба комментария (аналитика владельца, хендофф автора «ТЗ готово к ревью»). Второго ревью-раунда в истории issue нет — это первый заход, разбор полный, не по дельте (§2.10 неприменим).
+3. Прочитан канонический документ подсистемы `docs/WALL-THICKNESS.md`, в частности §3 (уже документированный контракт #198 — «neither endpoint is a room vertex or resolved opening endpoint»), чтобы сверить, что ТЗ предлагает именно точечное расширение, а не переписывание контракта.
+4. Прочитан сам ТЗ-файл `docs/specs/273-optimize-topology-island.md` целиком, сверены обязательные разделы §7.1 PROCESS.md.
+5. Прочитан текущий код `src/plan-optimizer.ts` (`collapseIsolatedWallThicknessIslands`, guard `isTopologyNode`, сбор `nodes` из `roomPoly()` и `openCuts`) и `src/wall-thickness.ts` (`roomWallProfile`, `atomicPolyForRoom`, `floorFootprintGeometry` — комментарий «Independent partitions/columns are deliberately not accepted here»), чтобы проверить техническую состоятельность заявленных в ТЗ гарантий, а не поверить формулировкам на слово.
+6. Сверены названные в ТЗ файлы/инструменты на существование: `demo/smoke_optimize_micro_interval.mjs`, `scripts/mutation-gate.mjs`, `wallBodiesGeometry()`, `docs/TESTING.md`, `docs/CONFIG-COMPATIBILITY.md` — все существуют, ссылки не фиктивны.
+7. Сверен список «Обязательные регресс-тесты» из тела issue против AC1–AC9 ТЗ — построчное соответствие (см. таблицу ниже).
+8. Гейты кода в этом раунде не гоняются: на этапе ТЗ нет кода для типecheck/test/build — это ожидаемо и не является упущением.
+
+### Соответствие регресс-тестов issue → AC ТЗ
+
+| Требование issue | AC ТЗ |
+|---|---|
+| 1. Minimized fixture `22→15→22`, T-узел слева, off-grid справа | AC1 |
+| 2a. Оба конца топологические | AC3 |
+| 2b. Opening endpoints | AC3 |
+| 2c. Соседи различаются | AC4 |
+| 2d. Conflicting exact owners | AC4 |
+| 2e. Цепочка micro-островков | AC4 |
+| 3. Идемпотентность, независимость от порядка/направления | AC5, AC6 |
+| 4. Не только JSON — effective `wallIntervals` и render-толщина | AC1 (wallIntervals), AC2 (wallBodiesGeometry) |
+| 5. Мутант старого `a \|\| b` guard обязан падать | AC8 |
+
+Все пункты, которые владелец назвал обязательными в issue, отражены в AC — пропусков нет.
+
+## Находки
+
+### Medium (в скоупе задачи) — заявленная защита от partition/draft-границы не имеет ни доказательства, ни видимого механизма реализации
+
+**Файл:** `docs/specs/273-optimize-topology-island.md`, §6.2 (пункт «второй endpoint не является... endpoint другого physical axis») и §6.3 (пункт «второй endpoint совпадает с отдельной room/partition/draft axis boundary»).
+
+**В чём проблема.** ТЗ формулирует это как обязательное блокирующее условие наравне с «оба конца топологические» и «opening/open-span endpoint» — но в отличие от них:
+
+- ни в AC3, ни в AC4 нет negative-фикстуры на этот случай (там перечислены только: оба topology/opening endpoints, разные соседи, missing/zero/open neighbor, chain/overlapping, conflicting owners, offset/perpendicular/parallel coincidence, intentional thickness change on topology boundary — партиций/drafts нет);
+- текущий код не имеет механизма её обнаружить. `collapseIsolatedWallThicknessIslands()` строит список topology-узлов (`nodes`, `src/plan-optimizer.ts:111-119`) только из `roomPoly(room)` **всех** комнат и точек `openCuts` (это `open_spans`, а не двери/окна — терминология ТЗ здесь совпадает с существующими тестами #198, это я проверил отдельно и не считаю дефектом). Независимые перегородки (`space.partitions`) и drafts в эту функцию вообще не передаются как аргумент и нигде не читаются. Комментарий в `src/wall-thickness.ts:2311-2313` прямо формулирует принцип модуля: *«Independent partitions/columns are deliberately not accepted here, so they can never enlarge the slab perimeter»* — то есть весь код, от которого наследует `collapseIsolatedWallThicknessIslands`, сознательно не видит partition/draft-геометрию.
+
+**Конкретный сценарий поломки.** Перегородка обычно примыкает к стене комнаты не в вершине полигона комнаты, а в произвольной точке вдоль ребра (T-примыкание). Если синтетическая (некандидатная) граница `b` микро-островка на ребре комнаты случайно совпадёт именно с такой точкой примыкания перегородки — а не с вершиной какой-либо комнаты и не с концом `open_span` — сегодняшний `isTopologyNode` её не увидит вообще, ни до, ни после этого ТЗ: она не входит в `nodes` ни как room-vertex, ни как open-cut endpoint. Формулировка §6.3 обещает, что такой случай «всегда блокирует», но: (а) реализация этого не делает и в разделе «Ожидаемые файлы» (§10) нет ни новой структуры данных, ни изменения сигнатуры `collapseIsolatedWallThicknessIslands` для передачи partition/draft-геометрии; (б) ни один AC не проверяет этот случай, значит регресс тут никогда не будет пойман тестом даже если решение владельца — «пусть остаётся как есть».
+
+Это именно тот класс дефекта, о котором предупреждает процесс: *«утверждение о поведении, которого нет ни в одном документе и которое не помечено как предположение»*. Раздел §14 «Принятые технические предположения» перечисляет 5 пунктов и явно не содержит допущения по этому случаю — то есть автор не пометил его как предположение, а выдал как решённый и гарантированный факт.
+
+**Почему это не High.** Основной репродуцированный дефект (T-node одной комнаты + синтетическая точка на том же parent edge, без перегородок) специфицирован полно, тестируем и напрямую закрывает исходный баг-репорт — партиционный случай гипотетический, не из живого репро, и в худшем случае приводит не к потере геометрии, а к слиянию двух смежных data-записей с одинаковым `cm` в точке, где могла бы быть полезна отдельная граница для будущего редактирования. Это ухудшение data-model гранулярности, а не пользовательский регресс толщины/геометрии.
+
+**Что нужно поправить** (любое из трёх, на усмотрение автора — технический спор решается в следующем раунде, не эскалируется владельцу):
+1. убрать пункт про partition/draft-boundary из §6.2/§6.3 как отдельную категорию и явно перенести его в §14 как «принято предположительно: коллизия с partition/draft-примыканием посередине ребра считается настолько редкой, что не тестируется в этом issue»; либо
+2. добавить AC и negative-фикстуру, доказывающую, что такая коллизия действительно блокируется (и, если для этого нужно новое обнаружение — назвать это явно в §10 «Ожидаемые файлы», это меняет и оценку сложности); либо
+3. явно доказать (комментарием в ревью-цикле), что геометрически такая коллизия на этом code path невозможна — если у автора есть довод сильнее прочитанного мной кода, он снимает находку.
+
+### Low — не все AC называют способ доказательства явным словом по шаблону §2.5
+
+**Файл:** `docs/specs/273-optimize-topology-island.md`, §9, AC3, AC4, AC5, AC7.
+
+AC1/AC2/AC6/AC8 явно называют механизм доказательства (`test/plan-optimizer.test.mjs` + assertion / production-bundle smoke / mutation). AC3, AC4, AC5, AC7 описывают, что именно проверяется, но не произносят слово «unit» и не называют файл — это восстанавливается из контекста и §10 «Ожидаемые файлы» (`test/plan-optimizer.test.mjs`), поэтому не блокирует, но формально DoR требует «у каждого [AC] указано, чем он доказывается: unit / backend / smoke / golden / ревью кода» (§2.5). Рекомендация: при правке ТЗ по Medium-находке заодно дописать по одной строке «Доказательство: unit, `test/plan-optimizer.test.mjs`» к этим четырём AC — цена правки минимальна, откладывать в отдельный цикл не имеет смысла.
+
+## Что проверено и корректно
+
+- Все обязательные разделы §7.1 PROCESS.md присутствуют: сценарий (§1), что человек увидит до/после (§4), проблема и подтверждённая причина (§2–§3), скоуп/не-скоуп (§5), контракт поведения (§6), UX/accessibility/touch/security/perf (§8), модель данных и совместимость (§7, явно: без изменения `model_version`/схемы, старый клиент читает как один obычный wall entry), i18n (§8, явно «новых locale keys нет»), AC1–AC9 (§9), риски (§12), откат (§13), release-артефакты (§11).
+- Технический контракт §6.1–6.2 (базовые условия #198 + разрешённый один T-endpoint) корректно описывает существующий код: реальный guard в `src/plan-optimizer.ts:155` — ровно `isTopologyNode(a) || isTopologyNode(b)`, как и заявлено в §3 «Подтверждённая причина»; арифметика порога (`GRID_PITCH = 4.166667`, `0.5 × GRID_PITCH = 2.083333 > 1.381904`) сходится с числами из реального экспорта.
+- Термин «opening/open-span» в ТЗ корректно соответствует существующему словарю кода и тестов (`openCuts` = `open_spans`, не маркеры дверей/окон) — я отдельно перепроверил это по `test/plan-optimizer.test.mjs:236-266`, где `atOpening`-кейс тестирует именно `openCuts`; расхождения терминологии с каноном нет.
+- Каждый обязательный регресс-тест из тела issue имеет соответствующий AC (таблица выше) — пропусков нет, автор не сузил скоуп относительно требования владельца.
+- Не-скоуп (§5) корректно исключает runtime/editor cleanup, изменение persisted-схемы, #271 (finite-ray) — задачи не смешаны.
+- Продуктовых вопросов владельцу нет, и это оправдано: единственная развилка («когда именно можно схлопывать») полностью решена техническим контрактом §6.1–6.3, разночтений в пользовательском сценарии я не нашёл.
+- Rollback и release-артефакты реалистичны и не описывают несуществующую миграцию.
+- Проверка «одно число — один источник» (PROCESS.md §8): новых пользовательских чисел не вводится, `wallsMerged` переиспользуется как есть (§7) — вопросов нет.
+
+## Чего не проверял
+
+- Не проверял фактическую реализацию (её ещё нет на этом этапе) — только заявленный контракт и его согласованность с текущим кодом `collapseIsolatedWallThicknessIslands`/`roomWallProfile`/`atomicPolyForRoom`.
+- Не запускал `npm test`/`typecheck`/`build` — на этапе ТЗ гонять их не над чем, продуктовый код не менялся.
+- Не проверял golden/smoke/performance — они относятся к код-ревью и пре-релизному гейту, не к ревью ТЗ.
+- Не проверял, насколько часто на реальных планах перегородка примыкает к стене вне вершины комнаты (частотность сценария из Medium-находки) — эмпирических данных нет, вывод сделан только из чтения кода и комментариев модуля.
+- Не проверял вручную конкретный приватный экспорт `1.json` — он не в репозитории, что соответствует правилу «не добавлять приватный файл»; воспроизвёл только числовую арифметику по описанию.
+
+## Унаследовано из r0
+
+Неприменимо — это первый заход (r1), предыдущего цикла нет, наследовать нечего.