docs: spec review document for #176

Issue: #176
User-Visible: no

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

@@ -0,0 +1,259 @@
+# SPEC-REVIEW-176-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/176
+- **ТЗ под ревью:** тело issue #176 (лёгкий трек, файл `docs/specs/` не
+  создаётся — метка `small` подтверждена аналитиком и не оспорена владельцем)
+- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
+- **Трек:** лёгкий (`small`), лимит циклов ревью ТЗ — 2 (§4 PROCESS.md)
+- **Цикл:** r1/2
+
+## Скоуп ревью
+
+ТЗ #176 удаляет мёртвый внутренний tool-state `'partition'` из
+`src/houseplan-card.ts` (значение `MarkupTool`, `MARKUP_TOOLS`, метод
+`_partitionClick`, ~15 условных веток `this._tool === 'partition'`) и три
+осиротевших i18n-ключа, оставленных после #173 (замена one-shot Partition
+единым Walls-инструментом). Persisted-модель независимых стен
+(`space.partitions`, `PartitionCfg`, `kind: 'partition'` у selection/opening
+host, весь UI работы с уже созданными стенами) не меняется.
+`User-Visible: no`; changelog не требуется.
+
+Находка изначально заведена самим ревьюером кода на #173
+(`docs/reviews/CODE-REVIEW-173-r1.md`, Medium-1) и подтверждена владельцем как
+issue — проверка «issue создан явным решением владельца» выполнена (первая
+запись в issue — команда владельца на вход в `S2-analysis`).
+
+Не в скоупе ревью: продуктовый код не написан (issue в `S4-spec-review`;
+ветки `issue/176-*` в репозитории нет — проверено `git branch -a`), поэтому
+реализация, тесты и гейты — предмет будущего код-ревью (PROCESS.md §2.7).
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (актуальная
+   редакция, включая §2.4/§2.5/§5/§7.1/§8) и тело issue #176 со всеми
+   комментариями (аналитика владельца, передача на ревью, отметка о сбое
+   автоматического прогона).
+2. Построчно сверены технические утверждения ТЗ с текущим `dev`
+   (`src/houseplan-card.ts`, `grep -in partition`, полный список ~140
+   вхождений):
+   - `type MarkupTool = ... | 'partition' | ...` — строка 516, подтверждено;
+   - `'partition'` в `MARKUP_TOOLS` — строка 547, подтверждено;
+   - `private _partitionClick(...)` — строка 7143, реализует «два клика → одна
+     перегородка» через `_recordGeometry('history.partition_add', …)`;
+     подтверждено;
+   - диспетчер клика: `if (this._tool === 'partition') { this._partitionClick(...); }`
+     — строка 6834-6835, единственная точка вызова метода — подтверждено, что
+     `_partitionClick` не достижим никаким иным путём;
+   - ветки `this._tool === 'draw' || this._tool === 'partition'` (лимиты,
+     snap, hints, Undo/Escape, рендер превью) — найдены на строках 2241,
+     5362, 5429, 5450, 5487, 6364, 6601, 17202, 17923, 18039, 18041, 18091,
+     9597; одиночные `this._tool === 'partition'` — строки 2270, 2323, 6834,
+     9559, 9598, 11908 — количество и характер веток соответствуют заявленным
+     в issue «порядка 15».
+3. Проверена причина недостижимости `this._tool === 'partition'` — ключевое
+   техническое утверждение issue, а не декларация:
+   - `normalizeMarkupTool` (`houseplan-card.ts:551-562`) — единственное место,
+     присваивающее `this._tool` из внешнего/сохранённого значения
+     (`this._tool = normalizeMarkupTool(vp.tool)` при восстановлении warm
+     viewport, строка 2656);
+   - `normalizeMarkupTool` безусловно вызывает
+     `value = normalizeUnifiedWallTool(value)` **до** проверки
+     `MARKUP_TOOLS.has(...)` (строка 555);
+   - `normalizeUnifiedWallTool` (`src/wall-face-graph.ts:53-55`):
+     `return value === 'partition' ? 'draw' : value;` — литерал `'partition'`
+     маппится в `'draw'` безусловно.
+   - Других обработчиков, присваивающих `this._tool = 'partition'` из
+     пользовательского ввода, не найдено (единственное текстовое совпадение
+     — сравнения `===`, не присваивания). Вывод issue «код недостижим, а не
+     редко используем» подтверждён чтением, не декларацией автора.
+4. Проверено, что тип `MarkupTool` и `MARKUP_TOOLS` не используются больше
+   нигде в `src/` (`grep -rn "MarkupTool\|MARKUP_TOOLS" src/`) — оба
+   контейнера частные для `houseplan-card.ts`; удаление литерала не пробивает
+   границу модуля.
+5. Проверена граница «мёртвый код инструмента» vs «живая persisted-модель»
+   построчно: `kind: 'partition'` в типах selection/drag/dialog (строки 1225,
+   1228, 1234, 1243, 7196, 7257, 7283, 7374, 7401, 7415, 7434, 7462, 7505,
+   11485, 12005, 17840, 17863, 17881, 17981), `space.partitions`/`PartitionCfg`,
+   `_partitionDeleteDialog`/`_confirmPartitionDelete`,
+   `_partitionOpeningCuts`/`resolvePartitionOpeningCompat` и весь модуль
+   `partition-openings.ts` — ни один из них не завязан на `this._tool`, все
+   работают с уже существующими объектами независимо от активного
+   инструмента. Граница, которую ТЗ объявляет неприкосновенной (контракт п.3),
+   реально проходит там, где заявлено.
+6. Проверены i18n-ключи (`src/i18n/en.json`, `src/i18n/ru.json`):
+   `title.markup_partition`, `markup.hint_partition`, `physical.partition_size_title`
+   — все три существуют в обоих locale и (`grep -rn` по `src/`) используются
+   только через `this._tool === 'partition'` (строки 9559, 9598) либо нигде
+   не используются вне собственного определения (`title.markup_partition`) —
+   подтверждено, что это ровно осиротевший набор, ни ключом больше, ни
+   меньше. Ключи `markup.partition`, `physical.partition_properties`,
+   `history.partition_add`, `opening.host_partition`,
+   `confirm.delete_partition_openings_*`, `opening.rebind_partition` — заняты
+   persisted-объектом и диалогами, ТЗ верно не включает их в список удаления.
+7. Проверен `test/golden-matrix.test.mjs:72`:
+   `assert.equal(['draw', 'partition'].includes(scenario.planSnap.tool), true, …)`
+   — сейчас допускает оба значения. Проверены реальные сценарии
+   `demo/golden/matrix.mjs` (`planSnap: {...}`, строки 73-80) — оба
+   существующих `planSnap`-сценария уже используют `tool: 'draw'`; ни один
+   golden-сценарий не задаёт `'partition'`. Сужение assert до одного `'draw'`
+   не меняет ни одного изображения — заявление ТЗ «визуальные эталоны не
+   меняются» подтверждено чтением фикстур, а не предположением.
+8. Проверены названные в плане автотестов файлы:
+   `demo/smoke_free_walls.mjs` (существует; строки 57-60, 177-181 напрямую
+   ставят `c._tool = 'partition'` и зовут `c._partitionClick(...)` в обход
+   публичного dispatch — ровно то приватное использование, которое ТЗ обязуется
+   перевести на публичный flow), `demo/smoke_plan_snap_overlay.mjs`
+   (существует; строка 185 `card._tool = 'partition'` перед `_markupClick`,
+   что действительно проходит через диспетчер строки 6834-6835), и
+   `demo/smoke_unified_wall_tool.mjs` (существует; не использует приватный
+   tool-state — уже написан против публичного `_tool = 'draw'` +
+   `_keepClosedAsPartitions()`, строка 128 — служит рабочим образцом того,
+   как должны выглядеть переписанные сценарии).
+9. Проверено существование паттерна «source-contract test», на который
+   ссылается AC1 как способ доказательства: `test/isometric-contract.test.mjs`,
+   `open-passage-contract.test.mjs`, `optional-space-model-contract.test.mjs`,
+   `release-contract.test.mjs` и другие — это существующая практика репозитория
+   (в т.ч. для внутренних инвариантов), а не изобретённый под это ТЗ механизм.
+10. Проверено `docs/USER-GUIDE.ru.md:343` — таблица инструментов Плана уже
+    фиксирует «отдельного инструмента «Перегородка» нет» на пользовательском
+    уровне; терминология ТЗ («persisted-объект остаётся «Перегородка»,
+    внутренний tool-token удаляется») не расходится и не переизобретает то,
+    что задокументировано.
+11. Проверено `docs/TESTING.md:486-497` («Room markup editor») и
+    `docs/STATUS.md` (`grep -in partition` — без совпадений) — ни один
+    текущий пункт не описывает приватный `_tool='partition'`; изменение кода
+    не требует правки формулировок этих файлов по существу, перечисление их
+    в «Затрагиваемые поверхности» — корректная страховка на случай появления
+    новых заметок в реализации, не декларация обязательной правки.
+12. Проверено, что issue #176 не оспорил `small`: сложность/риск 3/10, одна
+    поверхность (`src/houseplan-card.ts` + производные тесты/i18n), нет
+    миграции, нового UX-контракта, влияния на perf/touch — все критерии §5
+    выполнены одновременно; `trivial` корректно не применён (тип `tech-debt`,
+    не `bug`).
+13. Проверено отсутствие ветки `issue/176-*` (`git branch -a | grep 176` —
+    пусто) и отсутствие более раннего `docs/reviews/SPEC-REVIEW-176-*.md`
+    (`ls docs/reviews | grep 176` — пусто) — это первый цикл, r1/2.
+
+Гейты (`typecheck`/`test`/`build`) не прогонялись — на этапе ревью ТЗ
+продуктового кода нет; прогон гейтов не относится к этому этапу (PROCESS.md
+§2.7/§8).
+
+## Обязательные разделы (§7.1 PROCESS.md, лёгкий трек — тело issue)
+
+| Раздел | Есть | Комментарий |
+|---|---|---|
+| Сценарий (персона/поверхность/момент) | N/A, обоснованно | задача не создаёт наблюдаемого изменения продукта (`User-Visible: no`, контракт п.6); §7.1 формулирует эти два раздела как продуктовые именно для отделения «работы» от «изменения продукта» — здесь по существу нет стороны-наблюдателя, это подтверждено чтением кода (см. п.3), а не заявлено голословно |
+| Что человек увидит до/после | N/A, обоснованно | то же: «до» и «после» с точки зрения пользователя идентичны — задача про внутреннюю модель, не про интерфейс |
+| Проблема | ✅ | подтверждена чтением `houseplan-card.ts` и `wall-face-graph.ts` (см. «Как проверялось» п.2-3), а не декларацией автора |
+| Контракт (скоуп/не-скоуп внутри пунктов 1-3) | ✅ | граница мёртвый-код/persisted-модель проверена построчно (п.5) и проходит ровно там, где заявлено |
+| Модель данных и миграция | ✅ | «persisted-модель не меняется» — подтверждено (п.5); миграции нет, потому что нет изменения хранимой схемы |
+| i18n | ✅ | ровно три ключа, ни больше ни меньше — подтверждено построчной сверкой (п.6) |
+| AC1…ACn с доказательством | ✅ | 3 штуки, у каждого назван реалистичный и проверяемый способ доказательства (typecheck+source-contract test / unit+targeted smokes / locale unit+build diff+golden:verify) |
+| План автотестов | ✅ | ссылается на реально существующие файлы (п.7-8), включая точную строку смока, которая сейчас использует приватный API и должна быть переписана |
+| Риски | частично | явного раздела «Риски» нет, но риск («будущий контрибьютор вернёт мёртвую ветку по аналогии») назван в теле issue и закрывается самим фактом удаления кода — для лёгкого трека §5 отдельного раздела «Риски» не требует (шаблон: проблема · контракт · AC · откат) |
+| Откат | ✅ | один revert, без миграции данных — обоснованно, так как persisted-схема не тронута |
+| Release-артефакты | ✅ | явно и корректно: `User-Visible: no` → changelog не требуется; три bundle snapshot синхронизируются по стандартному процессу сборки |
+
+Раздел «Принятые предположения» присутствует и корректно отделяет то, что
+уже прочно установлено чтением кода (`partition` в persisted-типах — не
+мёртвый код; legacy-нормализатор остаётся намеренно), от того, что не
+является продуктовым вопросом вовсе (нет UI, нет changelog) — ни одно
+утверждение раздела не выдаёт техническую догадку за факт.
+
+## Находки
+
+Отсутствуют. Ниже — то, что специально проверено на предмет типичных для
+этого класса дефектов (пропущенный call site, недосказанная граница
+скоупа, домысленное поведение) и не подтвердилось.
+
+- **Домыслы вместо решения не найдены.** Единственное потенциально спорное
+  техническое утверждение — «код недостижим, а не редко используем» — не
+  принято на веру, а прослежено до кода нормализации (`normalizeUnifiedWallTool`,
+  «Как проверялось» п.3): это не догадка автора, а проверяемый факт.
+- **Скоуп i18n не занижен и не завышен.** Отдельно проверено, что среди трёх
+  удаляемых ключей есть `physical.partition_size_title`, которого не было в
+  исходной находке код-ревью #173 (там названы только `title.markup_partition`
+  и `markup.hint_partition`) — авторская аналитика нашла его самостоятельно и
+  подтвердила чтением (`this._tool === 'partition' ? 'physical.partition_size_title'`,
+  строка 9559); ключ действительно используется только приватным
+  tool-state и нигде в persisted-UI.
+- **Golden-эталоны не сузятся ошибочно.** Проверено, что сужение
+  `test/golden-matrix.test.mjs:72` до одного `'draw'` не затрагивает ни один
+  реальный сценарий `demo/golden/matrix.mjs` — оба `planSnap`-сценария уже
+  на `'draw'`.
+
+## Что проверено и корректно
+
+- **Соответствие `docs/SCOPE.md`:** задача не создаёт нового Core user job и
+  не расширяет ни один из них напрямую — это ожидаемо для `tech-debt`.
+  Косвенная связь с J4/J6 (поддержание единственного тестируемого пути Plan
+  editor, без альтернативного нетестируемого API) не притянута: старый
+  `_partitionClick` действительно обходит crash-safe/finish/limit-контракт,
+  установленный #173 для «unified Walls chain» (J6 — «keep the plan true as
+  the home evolves»).
+- **Легитимность лёгкого трека:** сложность 3/10, одна поверхность, нет
+  миграции/нового UX-контракта/влияния на perf или touch — критерии §5
+  выполнены все одновременно; аналитик верно не предложил `trivial` (тип
+  `tech-debt`, критерий §5.1 требует `bug`).
+- **Продуктовых вопросов действительно нет.** Единственный класс вопросов,
+  который вообще уместен на этом этапе — объём видимого изменения — здесь
+  вырожден: видимого изменения нет вовсе (`User-Visible: no`), и это не
+  недосмотр автора, а прямое следствие того, что удаляется недостижимый код.
+  Технические решения (source-contract test, перевод смоков на публичный
+  flow) корректно оставлены на усмотрение разработчика записью в разделе
+  «Принятые предположения», как и предписывает §7.1 PROCESS.md.
+- **Граница мёртвый-код / persisted-модель проведена точно и без пропусков**
+  — построчная проверка (см. «Как проверялось» п.5) не нашла ни одного
+  места, где persisted `kind: 'partition'` зависел бы от `this._tool`, и ни
+  одного места, где `this._tool === 'partition'` относился бы к чему-то,
+  кроме активного инструмента разметки.
+- **Ссылки на существующие файлы не выдуманы** — все три названных в плане
+  автотестов смока (`smoke_free_walls.mjs`, `smoke_plan_snap_overlay.mjs`,
+  `smoke_unified_wall_tool.mjs`) существуют, и первые два действительно
+  используют приватный `_tool = 'partition'`/`_partitionClick` способом,
+  который ТЗ обязуется устранить — заявление в AC2 не голословно.
+- **Обратная совместимость не сужается.** Legacy warm-viewport токен
+  `'partition'` продолжает нормализоваться в `'draw'` уже существующим кодом
+  (`normalizeUnifiedWallTool`), который ТЗ явно не трогает и защищает
+  unit-тестом (контракт п.2) — соответствует `docs/CONFIG-COMPATIBILITY.md`
+  в части «не сужать уже принятый вход», хотя формально warm viewport —
+  page-memory, а не persisted storage schema, которую документ описывает
+  напрямую.
+- **Откат тривиален и корректен** — один revert без миграции данных,
+  поскольку persisted-схема не меняется ни в одной части.
+
+## Чего не проверял
+
+- Реализацию — её нет: issue в `S4-spec-review`, ветки `issue/176-*` не
+  существует (`git branch -a` пуст по этому номеру) — проверено.
+- Гейты `typecheck`/`test`/`build`/browser smoke/golden — не относятся к
+  этапу ревью ТЗ; предмет будущего код-ревью (PROCESS.md §2.7/§8).
+- `python -m pytest tests_backend` — backend (`custom_components/**/*.py`) не
+  упомянут ни в issue, ни в затронутых файлах, и не содержит `partition`
+  как tool-state (только независимая от frontend-инструмента persisted-схема,
+  если вообще есть — не проверялась специально, так как ТЗ прямо заявляет
+  «Backend … не меняется», а весь код-путь — frontend-only `MarkupTool`).
+- Точный будущий текст source-contract теста и точный способ переписать
+  `smoke_free_walls.mjs`/`smoke_plan_snap_overlay.mjs` на публичный
+  draw/finish flow — это техническое решение реализации (§7.1: «всё, чего
+  пользователь не наблюдает, агенты решают сами»), а не предмет ревью ТЗ.
+- Реальный визуальный результат `npm run golden:verify` — CSS/геометрия не
+  меняются по заявлению ТЗ и по чтению фикстур (см. «Как проверялось» п.7),
+  но фактический прогон гейта относится к реализации/пре-релизу, не к этому
+  этапу.
+- Точность числовых оценок аналитики (ценность 1/10, сложность 3/10, P3) по
+  существу — поле владельца (PROCESS.md §2.2), не предмет ревью ТЗ.
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0, Low: 0. ТЗ описывает точно очерченную,
+доказанную чтением кода (не декларированную) границу между мёртвым
+tool-state и живой persisted-моделью независимых стен; все технические
+утверждения (недостижимость `this._tool === 'partition'`, точный список
+осиротевших i18n-ключей, безопасность сужения golden-matrix assert)
+проверены построчно и подтвердились. Продуктовых вопросов нет обоснованно —
+задача не создаёt наблюдаемого изменения, а не умалчивает о нём. AC однозначны
+и снабжены доказуемыми способами проверки.
+
+**Вердикт: зелёный · цикл r1/2 · High: 0 · Medium: 0 → нет · Документ:
+docs/reviews/SPEC-REVIEW-176-r1.md**