docs: review document for #299

Issue: #299
User-Visible: no

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

@@ -0,0 +1,125 @@
+# SPEC-REVIEW-299-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/299
+- **Этап:** ревью ТЗ (PROCESS.md §2.4)
+- **Документ ТЗ:** `docs/specs/299-mixed-role-wall-records.md`
+- **SHA на котором получен вердикт:** `10f000dc47c391e5d6c18ba6e9b5b46ed4df67a3`
+  (ветка `issue/299-mixed-role-thickness`, коммит «docs: specify role-aware
+  wall compaction»)
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (первый заход, §4
+  бюджет ещё не тратился)
+
+## Скоуп проверки
+
+Первый раунд — разбор полный (PROCESS.md §2.10 применяется только со второго
+раунда). Проверено:
+
+1. `docs/SCOPE.md` — задача закрывает J6 («Keep the plan true as the home
+   evolves»); ADR #282 явно оставляет открытые P1 на живой репрезентации
+   (строка 135: «None of this cancels an open P1»), так что точечная починка
+   в обход стадий ADR легитимна, а не самовольное расширение скоупа.
+2. `AGENTS.md`, `PROCESS.md` §2.4/§2.10/§7.1/§5 — трек не `small` (сложность
+   8/10, риск 9/10, две продуктовые операции, real-plan regression), поэтому
+   файл ТЗ обязателен и он есть; комментарий аналитика подтверждает трек
+   «обычный» и что продуктовых вопросов нет.
+3. Тело issue #299 и три комментария (аналитика → занятие → «ТЗ готово»).
+4. `docs/USER-GUIDE.ru.md` не проверялся отдельно текстом, т.к. ТЗ прямо
+   заявляет «новых кнопок, диалогов, предупреждений и терминов UI нет» (§2) —
+   термины Optimize/«Удалить комнату, оставить стены» уже существующие и не
+   переименовываются.
+5. Канонический документ подсистемы: `docs/WALL-THICKNESS.md` — прочитан
+   целиком, модель, компакция и контракт удаления комнаты сверены построчно.
+6. Связанные закрытые issue #287 (диагностика `checkMixedRoleRecords`, есть в
+   репозитории) и #289 (safe Resize eligibility) — оба `CLOSED`, содержимое их
+   спек (`docs/specs/289-no-mixed-role-resize.md`) явно исключает «исправление
+   уже сохранённых mixed-role records через Optimize» из своего скоупа, что
+   подтверждает отсутствие дублирования с #299. #298 сейчас в `S4-spec-review`
+   (параллельно) — ТЗ #299 корректно называет эту зависимость и требует, чтобы
+   автор перед кодом ребейзился и держал контракт независимым от порядка
+   слияния (принятое предположение №5, §15).
+
+## Как проверялось (техническая верификация утверждений ТЗ)
+
+ТЗ делает несколько фактических утверждений о текущем поведении кода —
+согласно инструкции ревью, догадка, выданная за факт, является находкой. Все
+проверены чтением исходников на SHA выше, не исполнением:
+
+| Утверждение ТЗ | Где проверено | Результат |
+|---|---|---|
+| §3: `normalizeWallIntervals()` группирует соседние атомы только по `cm` и «сплошной», не сравнивая владельцев | `src/wall-thickness.ts:1927-2005`, ключевая проверка `pr.cms[next] !== cm` на строке 1966 — владелец/роль нигде не сравнивается | подтверждено |
+| §3: `checkMixedRoleRecords()` — существующий диагностический инвариант из #287, не product-фикс | `scripts/model-invariants.mjs:518-543` | подтверждено, вызывается только в `main()` отчёта, не в write-пути |
+| §4.4 / #228: exclusive positive-solid intervals удаляемой комнаты → partitions, shared/virtual/zero не материализуются | `src/room-deletion.ts:73-112`, фильтр `interval.kind === 'outer' && !interval.open && interval.cm > 0` | подтверждено |
+| §4.4: Keep walls и Optimize сходятся в одном канонизаторе | `src/houseplan-card.ts:11456` (`this._normalizeWalls(materializedWalls, …)` после удаления комнаты) и `src/plan-optimizer.ts:580` (`normalizeWallIntervals(...)`) — оба вызывают одну и ту же функцию `src/wall-thickness.ts:1927` | подтверждено, один enforcement point действительно один |
+| §15 предположение №1: роль выводится из `WallInterval.roomId`/`kind` ('shared'\|'outer'), пары владельцев ещё не готовы как поле | `src/wall-thickness.ts:1535-1545` (`WallKind = 'shared'\|'outer'`, без id второго владельца) | подтверждено техническое допущение реалистично: пару владельцев для `shared(A,B)` придётся строить геометрическим сопоставлением reversed-копий, а не читать готовое поле — это именно то, что ТЗ явно отдаёт на усмотрение автора/ревьюера в §15.1, а не выдаёт за уже существующее |
+| ADR #282 исключение для P1 | `docs/adr/282-wall-geometry-representation.md:135-137` | подтверждено дословно |
+| Комментарий кода `KNOWN` (`demo/smoke_edit_walk.mjs`) действительно содержит долг `mixed_role_record` на `real-plan-first-floor.json` seed 1 и 3 | `demo/smoke_edit_walk.mjs:82,86` | подтверждено, ссылка «Класс #287/#289» соответствует истории |
+| Фикстура/тестовые файлы, названные в AC, существуют | `test/fixtures/real-plan-first-floor.json`, `test/wall-thickness.test.mjs`, `test/plan-optimizer.test.mjs`, `test/room-deletion.test.mjs` | все существуют |
+| Инфраструктура для AC8 (targeted mutation) реально существует | `scripts/mutation-gate.mjs` (issue #85) | подтверждено, не изобретённый инструмент |
+
+Ни одно проверенное утверждение не оказалось догадкой, выданной за факт.
+
+## §7.1: обязательные разделы
+
+Все обязательные разделы присутствуют и в правильном порядке: сценарий (§1) ·
+что человек увидит (§2) · проблема/причина (§3) · контракт поведения (§4) ·
+scope и не-scope (§5) · модель данных/миграция (§6) · UX/i18n/touch/security
+(§7) · AC1…AC9 с доказательством (§8) · план автотестов (§9) · производительность
+(§10) · риски (§11) · откат (§12) · ожидаемые файлы (§13) · release-артефакты
+(§14) · явный блок принятых технических предположений (§15). Дополнительные
+продуктовые разделы AGENTS.md (персона/поверхность/момент; что человек видит
+одной фразой без терминов реализации) — оба на месте и написаны без терминов
+реализации.
+
+## Находки
+
+Блокирующих (High) находок нет. Medium в скоупе или вне скоупа — нет.
+
+Отмечено, но не является находкой (Low, снято без правки): §15 сознательно
+оставляет способ построения пары владельцев `shared(A,B)` на усмотрение
+реализации («может быть заменено ревьюером на эквивалентное без изменения
+UX») — это ровно тот класс технического решения, который PROCESS.md §7.1
+явно не выносит на продуктовый вопрос владельцу. Снимаю без правки: граница
+между «непроверяемая догадка» и «явно объявленное техническое допущение»
+соблюдена.
+
+## Продуктовые вопросы владельцу
+
+Отсутствуют, и это корректно: единственный продуктовый вопрос («ожидает ли
+пользователь, что кнопка Optimize/Keep walls чинит толщину, а не портит её»)
+уже отвечен самим фактом происхождения задачи — она заведена как P1-баг
+против уже согласованного поведения (J6), а не как новый UX-контракт. Ни один
+технический вопрос не был замаскирован под продуктовый и не был переадресован
+владельцу — оба спорных технических места (owner-signature representation,
+порядок интеграции с #298) явно решены в §15 как «принято предположительно,
+поменять свободно», что и требуется вместо вопроса.
+
+## AC — проверяемость и доказательство
+
+AC1–AC9 однозначны, у каждого назван способ доказательства (unit table-driven,
+real-plan regression, production-bundle smoke, targeted mutation, локальные
+гейты). AC8 (мутационный тест на сам guard) — сильная защита от неполной
+реализации, убивающая как «слишком мягкий» (не сравнивает владельцев), так и
+«слишком строгий» (blanket-disable всей компакции) уклон; вместе с
+позитивными assertions AC2 это заранее закрывает риск того, что фикс превратит
+нормализацию в отключение слияния вообще (см. риск §11 и `AC2`).
+
+## Что не проверялось
+
+- Само исполнение тестов/гейтов — на этапе спек-ревью кода ещё нет, это
+  предмет код-ревью (§2.7); проверка ограничена чтением ТЗ и сверкой с
+  текущим состоянием репозитория.
+- Полнота geometry-инвариантов (`npm run invariants`) не прогонялась — нет
+  ещё изменения геометрии для прогона; будет предметом код-ревью.
+- Не проверялся сам ADR #282 целиком построчно за пределами цитируемого
+  исключения — прочитан фрагмент, относящийся к заявленному основанию.
+- Актуальность `docs/USER-GUIDE.ru.md`/`CANVAS.md`/`UX-MODES.md` не сверялась
+  построчно, так как ТЗ не меняет видимую поверхность/термины (§2, §7); при
+  расхождении это всплывёт на код-ревью, когда будет реальный diff
+  документации.
+
+## Вердикт
+
+Зелёный. ТЗ полно, каждый AC проверяем и привязан к способу доказательства,
+технические предположения явно помечены и не выданы за факт, продуктовых
+вопросов нет и не должно быть, скоуп и не-скоуп чётко разграничены и не
+пересекаются с закрытыми #287/#289 и параллельным #298.