docs: review document for #669

Issue: #669
User-Visible: no

docs/reviews/INDEX.md

@@ -1,10 +1,11 @@
 # Индекс ревью
 
-Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1093, issue: 388. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
+Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1094, issue: 389. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
 
 | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
 |---|---|---|---|---:|---:|---|---|
 | #671 | [CODE-REVIEW-671-r1.md](CODE-REVIEW-671-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
+| #669 | [SPEC-REVIEW-669-r1.md](SPEC-REVIEW-669-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | AC3 не гарантирует заявленную цель issue; фактическая неточность в «Проблема» (не блокирует) | — |
 | #667 | [CODE-REVIEW-667-r1.md](CODE-REVIEW-667-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
 | #666 | [CODE-REVIEW-666-r1.md](CODE-REVIEW-666-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
 | #665 | [SPEC-REVIEW-665-r1.md](SPEC-REVIEW-665-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | заявленная правка docs/ISOMETRIC.md | `docs/ISOMETRIC.md` `docs/adr/160-isometric-stage3-overlays.md` `check-docs.mjs` `ISOMETRIC.md` |

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

@@ -0,0 +1,193 @@
+# SPEC-REVIEW-669-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/669
+- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
+- **Трек:** полный (метки `bug`, `P1`, `S4-spec-review`; ни `small`, ни
+  `trivial` не выставлены — документ, а не только комментарий).
+- **Материал:** тело issue #669, раздел `## ТЗ`, + 3 комментария: (1) «Взял»
+  автора с планом A/B-замера, (2) аналитика — таблица замеров, причина
+  регрессии, оценка, решение «вопросов владельцу нет», ссылка на новый #675,
+  (3) поправка к одной строке таблицы замеров.
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
+- **Роль:** ревьюер ТЗ (не автор)
+
+## Скоуп ревью
+
+Регрессия производительности в `cleanFloorForRoom` (`src/clean-floor.ts:47`):
+каждый промах кэша чистого пола сразу вычитает **все** лестницы пространства
+(`geometryAreaMinusStairs` → `geometryMinusStairs`, `src/stairs.ts:234-250`) —
+хотя площадь читают только два потребителя (подсказка комнаты, PDF), а путь
+пола (нужный пяти... по факту четырём местам рендера) вычитания не требует.
+Задача переносит вычисление площади на момент первого чтения (лениво,
+мемоизировано в том же кэше) и добавляет отбор лестниц по пересечению bbox
+перед polyclip. Это разблокирует `v1.78.0-beta.5`, у которой красный
+performance-smoke на точном SHA `09873251`.
+
+**SCOPE-проверка (`docs/SCOPE.md`):** чисто внутренняя оптимизация без нового
+видимого поведения — под J1 («показать дом сейчас», включая скорость первого
+кадра) как выполнение уже принятого бюджета, а не новый job. Ничего из
+«никогда не строить» не задевается.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `docs/process/REVIEWER.md` целиком; по ссылкам
+   конспекта открыт PROCESS.md §2.3, §2.4, §2.5, §4, §7.1, §7.2 (§2.10 не
+   применялся — это r1).
+2. Прочитано тело issue #669 целиком и все 3 комментария (`gh issue view 669
+   --json body,comments`).
+3. Сверены обязательные разделы §7.1: Сценарий, Что человек увидит до/после,
+   Проблема, Скоуп/Не-скоуп, Контракт поведения, UX/модель данных и
+   миграция/i18n/touch/производительность, Критерии приёмки AC1–AC4 со
+   способом доказательства, План автотестов, Риски, Откат, Release-артефакты,
+   Затронутые файлы, «Принято предположительно» — все на месте, первые два
+   раздела продуктовые и без терминов реализации.
+4. Прочитан текущий код `src/clean-floor.ts` и `src/stairs.ts` целиком, чтобы
+   проверить, что описание проблемы соответствует коду, а предложенный скоуп
+   реализуем без переписывания контракта:
+   - подтверждено: `path`/`geom` в `cleanFloorForRoom` вычитают только
+     физические тела (`floorMinusBodies`), лестницы в путь никогда не попадали
+     — вычитание лестниц целиком живёт в поле `area`. Значит лениво откладывать
+     нужно именно и только его; контракт «путь не меняется» (AC1, «Контракт
+     поведения») уже выполняется today и задача его не трогает — только
+     переносит момент вычисления `area`.
+   - подтверждено количество мест, читающих только `.path`: `grep -n
+     "_cleanFloor("` даёт 4 попадания (`src/houseplan-card.ts:9059, 9229,
+     10459, 11011`), а не 5, как сказано в разделе «Проблема». Пятое место
+     (`:11873`) читает весь объект ради `.area` (подсказка комнаты). Второй
+     потребитель площади — `resolveRoomArea` (`:10184`) → та же
+     `._cleanFloor(...).area`. Отдельно в `src/pdf/pdf-scene.ts:311` есть
+     independent fallback-вызов `geometryAreaMinusStairs` в обход
+     `cleanFloorForRoom` (когда `resolveRoomArea` не вернул число) — он не
+     проходит через ленивый кэш и в ТЗ не упомянут, но и не входит в
+     «Затронутые файлы»; поскольку это редкий fallback-путь, а не типовой рендер
+     кадра, на бюджет AC3 он не давит — оставляю без находки, только для
+     полноты цепочки.
+   - подтверждено, что `geometryMinusStairsSteps` (сводная панель) реализован
+     отдельно от `geometryMinusStairs` и не переиспользует его — фильтр по
+     bbox внутри `geometryMinusStairs` (место, названное в «Принято
+     предположительно») не заденет сводную панель автоматически, как и
+     требует «Не-скоуп».
+   - сверены оба файла бюджетов `demo/performance/budgets-*-smoke.json`:
+     `spaceSwitchMs` — обязательная (`hardMaxMs`) метрика обоих профилей, а не
+     информационная; лимиты (`modelReadyMs` 3000/2500, `firstStableRenderMs`
+     3500/3000, `spaceSwitchMs` 1800/1500) совпадают с числами из таблицы
+     наблюдений в issue.
+5. Сверены данные аналитики (таблица A/B) с самой формулировкой AC3 — см.
+   находку M1 ниже.
+6. Проверена логика ролбэка/release-артефактов на согласованность с политикой
+   `docs/STATUS.md` (STABLE-тело исключает баги, которые «introduced and fixed
+   strictly inside the beta line»): `v1.78.0` ещё не имеет стабильного релиза
+   (`git tag -l 'v1.78.0*'` — только беты 1–4), значит «changelog — нет»
+   обоснованно, регрессия #663 нигде не выходила стабильным релизом.
+
+## Находки
+
+### Medium (в скоупе) — AC3 не гарантирует заявленную цель issue
+
+**Что не так.** «Ожидаемое поведение» (преамбула issue) и AC3 обещают: «После
+исправления полный Validate точного release-candidate SHA проходит с первого
+воспроизводимого запуска» / «Полный Validate кандидата слияния и кандидата
+беты: `performance_smoke` обоих профилей … проходит `modelReadyMs` и
+`firstStableRenderMs` с первого прогона при неизменных бюджетах». Формулировка
+называет только эти две метрики, но `performance_smoke` — единый job с
+несколькими обязательными (`hardMaxMs`) метриками в том же прогоне, включая
+`spaceSwitchMs`. По собственным замерам аналитики в этом же issue
+`spaceSwitchMs` **уже проваливается** на изометрическом профиле точного SHA
+`09873251` (1821.2 мс и 1813.5 мс против лимита 1800 мс — два из трёх
+зафиксированных провальных прогонов), и признаётся «не относящейся к
+лестницам» — не-скоуп, вынесено в отдельный **#675**. Ни ТЗ, ни преамбула не
+говорят, что делать с этим фактом: если #675 не сольётся раньше или вместе с
+кандидатом beta.5, тот же самый exact-SHA `performance_smoke` может остаться
+красным на `spaceSwitchMs` даже после идеального выполнения AC1–AC4 этой
+задачи — то есть заявленная цель issue («кандидат стабильно проходит
+действующие performance-бюджеты… полный Validate… проходит с первого
+воспроизводимого запуска») **не гарантируется** тем, что реально проверяет
+AC3.
+
+**Почему это находка, а не придирка к тексту.** Разработчик и последующий
+код-ревьюер будут закрывать именно AC3 как написано — двумя названными
+метриками. Формально «AC3 выполнен» при этом возможен ровно в момент, когда
+release по-прежнему заблокирован тем же job'ом на другой метрике; никто не
+обязан заметить разрыв между «AC выполнен» и «issue решает свою же
+преамбулу», а этот разрыв прямо запрещён REVIEWER.md («жёлтый вердикт
+допустим и при выполненных AC, если изменение не решает заявленный
+сценарий»).
+
+**Чем закрывается в скоупе задачи (без нового инженерного объёма).**
+Переформулировать AC3 и/или преамбулу «Ожидаемое поведение», выбрав одно из:
+(a) явно ограничить обещание двумя названными метриками и добавить в
+«Не-скоуп»/«Влияние» прямую фразу «для полностью зелёного `performance_smoke`
+кандидата требуется также #675»; либо (b) если у автора/владельца есть
+основания считать `spaceSwitchMs` эпизодически проходящим независимо от
+данной задачи (шум, а не устойчивый провал) — явно это утверждать со ссылкой
+на замер, а не оставлять предположение читателю. Технический характер этого
+уточнения (не продуктовый вопрос) означает, что owner не привлекается —
+решение принимает автор ТЗ, я его снимаю как открытый вопрос и фиксирую сюда.
+
+### Low — фактическая неточность в «Проблема» (не блокирует)
+
+Раздел «Проблема» утверждает: «пять мест рендера запрашивают только путь
+пола». Чтением кода (`grep -n "_cleanFloor(" src/houseplan-card.ts`)
+подтверждаю **четыре** таких места (`:9059, :9229, :10459, :11011`); пятый
+вызов (`:11873`) читает `.area`, а не только `.path`, и относится к другой
+категории потребителей (подсказка комнаты), уже перечисленной отдельно.
+Утверждение не входит ни в один AC и не влияет на реализуемость — рекомендую
+исправить число при следующей правке ТЗ, не считаю это блокером и снимаю сам.
+
+## Что проверено и корректно
+
+- Обязательные разделы §7.1 присутствуют, в правильном порядке; первые два —
+  продуктовые, без терминов реализации.
+- AC1 и AC2 однозначны, имеют явную пару «доказательство / чем краснеет»,
+  тестовый план реализуем без изменения контракта (`geom`/`path` уже не
+  зависят от лестниц; ленивость нужна только для `area`).
+- AC4 и контракт поведения («в пределах относительной погрешности 1e-9»)
+  корректно фиксируют неизменность видимого числа площади.
+- «Не-скоуп» корректно ограждает `geometryMinusStairsSteps` (независимая
+  реализация — трогать `geometryMinusStairs` её не заденет) и 2.5D-рендер
+  лестниц, с явным условием возврата в ТЗ, если остаточных издержек окажется
+  достаточно, чтобы провалить AC3 на изометрии — хорошая гигиена против
+  молчаливого расширения скоупа.
+- UX/модель данных/i18n/touch — «нет изменений», согласовано с кодом
+  (никаких новых полей конфигурации, никакого нового текста).
+- Release-артефакты («changelog — нет») обоснованы политикой
+  `docs/STATUS.md` о STABLE-теле: `v1.78.0` ещё ни разу не выходил стабильным
+  релизом (только beta.1–4), регрессия #663 нигде не публиковалась стабильно.
+- Риск «устаревшая площадь при копировании объекта» реалистично описан и
+  сейчас не имеет ни одного нарушителя (`grep` на spread/`Object.assign`
+  результата `_cleanFloor`/`cleanFloorForRoom` — пусто), т.е. риск корректно
+  назван, а не выдуман.
+- «Принято предположительно» корректно помечает две технические развилки
+  (геттер vs `areaOf`, место фильтра bbox) как свободно изменяемые — не
+  требует ответа владельца, соответствует §7.1 (не продуктовый вопрос).
+- Открытых продуктовых вопросов к владельцу нет — согласен с автором:
+  видимое поведение не меняется, различие в AC3 (находка Medium выше) —
+  техническая формулировка, а не продуктовый выбор.
+
+## Чего не проверял
+
+- Не запускал `npm test`/`typecheck`/`build` — на этапе spec кода ещё нет,
+  реализация не начата (ветка `issue/669-*` не создана).
+- Не запускал perf-смок и не проверял поведение реального `polyclip-ts` на
+  250 лестницах — AC3 по определению доказывается на кандидате в код-ревью,
+  не на этапе ТЗ.
+- Не проверял `#675` по существу (отдельная задача); использовал только факт
+  её существования и формулировку не-скоупа для находки Medium выше.
+
+## Вердикт
+
+Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 1 → в задаче
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `c9cd6cbf1f5b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `2c7a4c124868cf7499d7b5c725ee028d3810eb62`
+  ```
+  git log --all --format='%H %T' | grep 2c7a4c124868
+  ```
+- Тело issue: `1500ed85bc2e116a29201cc0f77d438e9e6381195fffc70053acc9f250e1dff6`
+- Вердикт конвейера: `yellow` · High 0