From 5c2c2578e19739a166e64830d3a4c27786a962fa Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 01:55:39 +0000 Subject: [PATCH] docs: review document for #276 Issue: #276 User-Visible: no --- docs/reviews/SPEC-REVIEW-276-r1.md | 213 +++++++++++++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-276-r1.md diff --git a/docs/reviews/SPEC-REVIEW-276-r1.md b/docs/reviews/SPEC-REVIEW-276-r1.md new file mode 100644 index 00000000..bdd21117 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-276-r1.md @@ -0,0 +1,213 @@ +# SPEC-REVIEW-276-r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/276 +- **Этап:** ТЗ на ревью (PROCESS.md §2.4) +- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 +- **ТЗ:** `docs/specs/276-coincident-partition-reconciliation.md` + (commit `a06c94b9`, ветка `issue/276-reconcile-coincident-partition`) +- **Вердикт:** красный · High: 1 · Medium: 3 (все в скоупе задачи) + +## Скоуп + +Обычный трек (не `small`), ТЗ — отдельный файл `docs/specs/276-*.md`. Диффом +этого раунда является **вся первая редакция** ТЗ (это r1, дельта не +применяется — §2.10 не в силе). Продуктовый код не менялся, диапазон правок — +только `docs/specs/276-coincident-partition-reconciliation.md` и +`docs/specs/README.md` (commit `a06c94b9`, 249 добавленных строк). + +## Как проверялось + +- Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2, §4, §7.1, §7.2, + §2.5 (DoR). +- Прочитано тело issue #276 и все 4 комментария (аналитика, связанные #277/ + #278, claim, хендофф). +- Прочитан целиком сам документ ТЗ (`docs/specs/276-coincident-partition- + reconciliation.md`, 249 строк). +- Сверка терминологии и заявленной причины с канонической документацией: + `docs/WALL-THICKNESS.md` (модель independent partitions, §9), фрагменты + `docs/USER-GUIDE.ru.md` про инструмент «Граница», «Оптимизировать планы» и + проёмы на совпадающей независимой стене, `docs/ARCHITECTURE.md` (Undo/Redo + стек, Optimize/model-v7). +- Сверка упомянутых в ТЗ функций и артефактов с фактическим деревом + (`grep`/`find` по `src/**`, `docs/**`): `physicalBodyParts()`, + `_boundaryBlocked()`, `_wallUnionGeometry()`, `mergeCollinearPartitions()`, + `optimizePlans()`, `PLAN_MODEL_VERSION`, `resolveOpeningWallAssociation`, + `test/plan-geometry-preflight.test.mjs` — все существуют, заявленное + поведение соответствует коду. +- Прочитан код Optimize Undo/Apply (`src/houseplan-card.ts:15120–15208`) для + проверки заявленного контракта Undo/Redo (см. находку H1). +- Docs-гейты для этого дифф-скоупа (spec-only, класс C) — не запускались: + правки не трогают `src/**`, `node scripts/check-docs.mjs` не относится к + этому раунду (автор уже отчитался о зелёном прогоне в хендоффе, что + достаточно для docs-диффа). `tsc`/`test`/`build` неприменимы — код не + менялся. + +## Находки + +### H1 (High, блокирует). Заявленный Redo для серверной отмены Optimize не существует и не заявлен как новая работа + +**Где:** §6 «UI и отчёт Optimize», абзац «После Apply»: «Undo одним действием +восстанавливает partition и исходный hosted `host`, **Redo снова +канонизирует**.» Тот же контракт закреплён в §12 AC7: «Apply — один WS call, +Undo/Redo восстанавливают обе формы.» + +**Как воспроизвести противоречие:** Optimize использует не общий 50-командный +`_geometryHistory` (`Ctrl+Z`/`Ctrl+Shift+Z`), а отдельный one-shot серверный +snapshot: + +- `_alignOptimizedConfirm()` (`src/houseplan-card.ts:15120–15150`) после Apply + **очищает** `_geometryHistory.clear()` и включает единственный флаг + `_canOptimizeUndo` по ответу сервера (`can_undo`); +- `_undoPlanOptimization()` (`src/houseplan-card.ts:15169–15208`) на + `houseplan/plan/optimize_undo` **снова** зовёт `_geometryHistory.clear()` и + ставит `_canOptimizeUndo = false` — после отмены никакого «Redo» не + существует: кнопка в блоке Undo Optimize (`src/houseplan-card.ts:16434– + 16437`) только одна, `mdi:undo-variant`, без парного redo-действия; +- `docs/USER-GUIDE.ru.md:1444` независимо описывает тот же контракт словами + для пользователя: «После оптимизации доступна **одна** серверная отмена, но + только до следующего изменения плана… Новый edit делает резервную копию + оптимизации устаревшей.» Ни там, ни в `docs/ARCHITECTURE.md` нет упоминания + Redo применительно к Optimize. + +Итого: заявление «Redo снова канонизирует» не соответствует ни одному +существующему механизму и не помечено в ТЗ как предположение или как новая +работа (в §7 «В скоупе» / §9 «Архитектурный контракт» о добавлении Redo-пути +для Optimize нет ни слова, в §8 «Не входит» — тоже). Это ровно тот случай, +который процесс называет замечанием: «утверждение о поведении, которого нет +ни в одном документе и которое не помечено как предположение». AC7 в текущей +форме либо недоказуем (Redo нечем закрыть без изобретения новой +функциональности за пределами заявленного скоупа), либо реализация втихую +досоздаст Redo-путь для Optimize, чего никто не решал. + +**Требуется:** либо убрать пункт про Redo из §6/AC7 и явно описать, что после +Undo Optimize канонизация не восстанавливается автоматически (пользователь +может только повторно нажать «Оптимизировать»), либо — если Redo для Optimize +действительно нужен продукту — вынести это отдельным продуктовым вопросом +владельцу (это расширение существующего контракта one-shot undo, а не деталь +реализации). + +### M1 (Medium, в скоупе). Раздел «Риски» отсутствует — обязательный пункт DoR + +PROCESS.md §2.5 требует явно: «открытых продуктовых вопросов нет; **риски +перечислены**» — как отдельный обязательный пункт перед переводом в +`S5-ready`. В документе нет ни заголовка, ни абзаца, который перечислял бы +риски и меры (аналог §11 «Риски и меры» в предыдущем спеке той же подсистемы, +`docs/specs/275-multiwall-strip-containment.md:402`). §10 «Производительность» +задаёт бюджет как контракт, а не как риск с митигацией; §14 обсуждает только +rollback. Ничего не сказано, например, про риск того, что реальные экспорты +(на которые ссылается сама issue) содержат ещё не описанные комбинации +(частичное перекрытие, разная толщина), которые попадут в «неоднозначный +случай» и останутся неисправленными молча — это стоит явно назвать риском с +принятой мерой (отчёт Optimize должен это показывать/не показывать?). + +**Требуется:** добавить раздел «Риски» с перечислением и принятыми мерами. + +### M2 (Medium, в скоупе). Не перечислены затронутые файлы и модули — обязательный пункт DoR + +PROCESS.md §2.5: «перечислены затронутые файлы и модули» — обязательный +пункт. В документе нет ни одного упоминания `src/*.ts` (проверено: `grep -n +"src/\|\.ts\b"` по файлу спецификации — 0 совпадений), хотя аналитика в +комментарии issue #276 уже называет конкретные функции (`physicalBodyParts()`, +`_boundaryBlocked()`, `_wallUnionGeometry()`, `optimizePlans()`, +`mergeCollinearPartitions()`) и, значит, авторы уже знают, где лежит код. +Раздел 9 «Архитектурный контракт» говорит только расплывчато: «Pure helper +живёт рядом с Optimize/wall merge либо в отдельном модуле» — этого +недостаточно для DoR-чеклиста, который требует список файлов/модулей, а не +намерение. + +**Требуется:** явный список (`src/plan-optimizer.ts`, `src/wall-merge.ts`, +`src/physical-geometry.ts`, `src/houseplan-card.ts` — или что решит автор) — +хотя бы приблизительный, он и так «принято предположительно, поменять +свободно» по §7.1. + +### M3 (Medium, в скоупе). Ссылка на несуществующий канонический документ `docs/PLAN-OPTIMIZE.md` + +Файл упомянут дважды: в шапке («Связано: …, `docs/WALL-THICKNESS.md`, +`docs/PLAN-OPTIMIZE.md`», строка 12) и в §14 «Release-артефакты и rollback» +как документ, который коммит реализации обязан обновить (строка 228). +Проверено: `find docs -iname "*optimiz*"` не находит такого файла — в +репозитории существуют только спеки `docs/specs/{198,199,223,248,252,273}- +optimize-*.md`, а канонический материал по Optimize фактически распределён +между `docs/USER-GUIDE.ru.md` (раздел про «Общие настройки → Оптимизировать +планы», строки ~1392–1449) и `docs/ARCHITECTURE.md` (строка 1036: «Optimize +Plans performs the equivalent idempotent model-v7 migration»). Утверждение о +существовании отдельного канонического файла — факт, поданный без пометки +«предположение», и он неверен уже сейчас: если раздел §14 будет исполнен +буквально, автор реализации не найдёт файл для правки. + +**Требуется:** заменить ссылку на реально существующие документы +(`docs/USER-GUIDE.ru.md`, `docs/ARCHITECTURE.md`) либо явно решить создать +новый `docs/PLAN-OPTIMIZE.md` и пометить это как отдельное техническое +решение автора (§7.1 разрешает такие решения принимать самостоятельно, но не +молча выдавать несуществующее за существующее). + +## Что проверено и корректно + +- **Первые два продуктовых раздела по сути отвечают на нужные вопросы**, хотя + и не оформлены отдельным «Что человек увидит до и после» (как это сделано в + `docs/specs/275-*.md`, §4): §1 даёт сценарий и персону, §6 «После Apply» + перечисляет наблюдаемый результат (толщина сразу видна, Boundary разрешает + virtual, проём не двигается). Контент присутствует, просто размазан по двум + разделам и не сведён в одну фразу без терминов реализации — граница между + этим и полноценным замечанием тонкая, решил не заводить отдельным пунктом + поверх H1/M1–M3, но авторам стоит это поправить попутно. +- **Причина дефекта подтверждена документами, а не выдумана.** Root-cause в + §2 совпадает и с issue-аналитикой, и с `docs/WALL-THICKNESS.md` §9 («A + precisely collinear room wall covering the same interval is cut as a + composite; a crossing/nearby body is not») и `docs/USER-GUIDE.ru.md:555` + («Если независимая стена точно совпадает со стеной комнаты, House Plan + использует её как явную привязку и прорезает единое составное тело») — то + есть рендер/cut уже корректно строит составное тело, а баг именно там, где + указано: thickness edit, `_boundaryBlocked()`, Optimize dedup. Ни одна + упомянутая функция/константа не выдумана — все существуют в `src/`. + проверено чтением, не исполнением. +- **Условие безопасной канонизации (§4) консервативно и покрыто негативной + матрицей** (AC2): partial/longer/shorter/composite/non-shared/virtual/ + different-cm/ambiguous-extra остаются без изменений. Это разумный default, + не требующий отдельного вопроса владельцу — граница технической, а не + продуктовой ответственности (§7.1). +- **Скоуп согласован с `docs/SCOPE.md`.** Комментарий аналитики верно относит + задачу к J4/J6; View mode не получает новых интеракций, touch-контракт не + расширяется (Plan editor остаётся desktop-first) — соответствует + `TOUCH-SUPPORT.md` и §7.1 разделу 11 самого ТЗ. +- **Связанные issue корректно разведены.** #277 (Resize ambiguity) и #278 + (общий wall-union fallback при исчезновении кладки) верно исключены из + скоупа (§8) и совпадают с формулировками в комментарии владельца к #276. +- **Все AC1–AC14 указывают способ доказательства** (unit/negative-matrix/ + golden/smoke/benchmark/review), ни один не оставлен без «Доказательство». +- **Производительность и touch закрывают DoR-пункты** явными числами (§10: + ≤15% p95 / ≤25 ms) и явным «новых жестов нет» (§11) — нет необходимости + возвращать как замечание. +- **Preflight-механизм (#199), на который опирается §4.8 и AC9, реально + существует и тестируем** (`src/plan-geometry-preflight.ts`, + `test/plan-geometry-preflight.test.mjs`) — проверено чтением, план + «Injectable unit» правдоподобен. +- **i18n:** формат отчёта (два RU/EN счётчика, нулевые строки скрыты) + совпадает с уже принятым паттерном Optimize preview в `USER-GUIDE.ru.md`. + Конкретные ключи (`gs.*`) не названы буквально — это Low, не блокирует; + автор решает при реализации. + +## Чего не проверял + +- Не запускал `npx tsc --noEmit` / `npm test` / `npm run build` / + `node scripts/check-docs.mjs` — диапазон коммита r1 не трогает `src/**` и + не является кодом; гейты реализации к спек-ревью неприменимы, продуктовый + код не менялся. +- Не запускал browser smoke / golden / инварианты модели — geometry/wall/ + layout ещё не менялись, это фикстуры и код будущей реализации. +- Не проверял реальный пользовательский экспорт, упомянутый в issue (владелец + не приложил его из-за пользовательских данных) — верю описанию из + комментария аналитики, синтетическая fixture в ТЗ (§7, §13) — это + корректная замена по правилам issue. +- Не оценивал сложность/трудозатраты реализации (7/10 в аналитике) — вне + компетенции спек-ревью. + +## Вывод + +High-находка (H1) блокирует переход в `S5-ready`: контракт Redo для Optimize +Undo не существует и не заявлен как новая работа, из-за чего AC7 недоказуем +как написано. Три Medium-находки (M1–M3) — все в скоупе задачи и чинятся тем +же автором в этом же ТЗ без нового issue: отсутствующий раздел «Риски», +отсутствующий список затронутых файлов/модулей (оба — обязательные пункты DoR +§2.5) и ссылка на несуществующий `docs/PLAN-OPTIMIZE.md`. Открытых продуктовых +вопросов владельцу нет — все находки технические, снимаются автором ТЗ.