diff --git a/docs/reviews/SPEC-REVIEW-89-r1.md b/docs/reviews/SPEC-REVIEW-89-r1.md new file mode 100644 index 00000000..ba3bc823 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-89-r1.md @@ -0,0 +1,232 @@ +# SPEC-REVIEW-89-r1 — #89, этап 1: объёмный вид за флагом Labs + +- Issue: [#89](https://github.com/Matysh/houseplan-card/issues/89) +- Этап: `spec` (PROCESS.md §2.4) +- ТЗ под ревью: [`docs/specs/089-isometric-view-stage1.md`](../specs/089-isometric-view-stage1.md), ревизия 3 +- Диапазон: `origin/dev...74b08df` (ветка `issue/89-isometric-stage1`, детач `HEAD`) +- Цикл: **r1/4** (первый независимый ревью-артефакт этой задачи в `docs/reviews/`; + «историческое ревью ревизии 1», упомянутое в теле issue и в самом ТЗ, честно + помечено автором как недоступный артефакт репозитория и не засчитывается) +- Вердикт: **зелёный** + +## Скоуп ревью + +Диапазон коммитов `origin/dev...HEAD` содержит только: + +- `docs/SCOPE.md` — узкое исключение для #89 в разделе Out-of-scope; +- `docs/specs/089-isometric-view-stage1.md` — нормативное ТЗ этапа 1 (ревизия 3); +- `docs/specs/README.md` — обновление строки статуса ТЗ. + +Продуктовый код, тесты, бандлы и changelog не менялись (`git diff --stat`). +Коммит `74b08df` несёт трейлеры `Issue: #89` / `User-Visible: no`, что верно для +документации без изменения поведения. + +Референсный контекст, прочитанный до вердикта: `docs/SCOPE.md`, `AGENTS.md`, +`PROCESS.md` (§1–§9, §12), тело issue #89 и все 7 комментариев (включая +`PSEUDO_3D_SPECIFICATION.md`, приложенный владельцем, и решения владельца +Q1–Q6/O1–O6), `docs/CANVAS.md`, `docs/WALL-THICKNESS.md`, `docs/LIGHT.md`, +`docs/UX-MODES.md`, `docs/TOUCH-SUPPORT.md`, `docs/WARM-REMOUNT.md`, +`docs/CONFIG-COMPATIBILITY.md`, родительский `docs/specs/089-isometric-view.md`. + +## Как проверялось + +Это ревью ТЗ, не ревью кода: гейты `npm test`/`npm run build`/`npm ci` не +запускались — изменение класса C (документация), продуктовый и тестовый код не +затронуты, самостоятельный техдолг-issue на гейт не требуется. Проверено: + +| Проверка | Результат | +|---|---| +| `git diff --check origin/dev...HEAD` | green (0 конфликтов/пробельных ошибок) | +| Провенанс коммита `74b08df` (`Issue:`/`User-Visible:`) | корректны для класса C | +| Обязательные разделы ТЗ по PROCESS.md §7.1 | см. таблицу ниже | +| Каждый AC1–AC15 — однозначность + способ доказательства | см. «AC» ниже | +| Технические утверждения ТЗ против реального кода/канонических доков | см. «Верификация фактов» — заняло основную часть ревью | +| Ссылки на issue (#82, #73, #50, #85, #83, #52, #92, #93, #95) | все существуют, тематика совпадает с тем, как на них ссылается ТЗ | +| Признаки «догадка выдана за решение» без пометки предположения | целевой поиск, см. ниже | + +### Обязательные разделы ТЗ (PROCESS.md §7.1) + +| Раздел | Есть | Где | +|---|---|---| +| Проблема | да (через job/персону/issue) | §1, тело issue | +| Скоуп и не-скоуп | да | §1 (входит/не входит), §14 | +| Контракт поведения | да, избыточно подробно | §2–§9 | +| UX | да (сведён к минимуму по решению D1) | §3, §7 | +| Модель данных и миграция | да | §10 | +| i18n | да | §13.4 | +| AC1…ACn с доказательством | да, с отдельной матрицей проверки ревьюера | §11.6, §12 | +| План автотестов | да, очень подробный (unit/smoke/golden/mutation/a11y) | §11 | +| Риски | **распределены по документу, не собраны в отдельный раздел** — см. находку L1 | §0.2 (B1–B8), §8.2, §9, §13.5 | +| Откат | да, полный (флаг + code revert) | §13.5 | +| Release-артефакты | да (внутренние — согласовано с D1/D7) | §13.2, §13.3 | + +### Верификация фактов (проверка «догадка выдана за решение») + +ТЗ делает десятки конкретных технических утверждений о существующем коде и +канонических документах. Каждое проверено, поскольку неверное утверждение, +поданное как факт, — именно тот дефект, который должен убить ревью: + +- `.hdr.kioskhide { display: none }` — подтверждено, `src/styles.ts:976`, + `src/houseplan-card.ts:13686`. Утверждение B5 («кнопки в киоске нет и быть не + может») технически верно. +- `_hashSpace()` и её единственный regex-парсер — подтверждено, + `src/houseplan-card.ts:1607` и использования далее. +- `mix-blend-mode: screen` для Glow — подтверждено, `src/styles.ts:550`, + `src/glow-blend.ts`. +- `filter: brightness(...)` на `.zoomwrap` для day/night — подтверждено по духу + (`src/houseplan-card.ts:13785`, переходная яркость на том же слое); формулировка + ТЗ не переоценивает механизм. +- `wallBodiesGeometry()` в `src/wall-thickness.ts`, используется в + `src/houseplan-card.ts` и разделяется с моделью света — подтверждено, совпадает + с `docs/LIGHT.md` («barriers… wallBodiesGeometry»). +- `NORM_W = 1000` в `src/space-geometry.ts`, pivot `[NORM_W/2, NORM_W/2]` — + константа подтверждена; выбор пивота обоснован ровно тем риском, который + описывает `docs/CANVAS.md` (`_showFar`/`_baseVb()` меняют кадр, а не + геометрию). +- `_cfgEpoch`, `_showFar`, `_svgPoint()` — все подтверждены в + `src/houseplan-card.ts`; предупреждение D5 («кэш по эпохе уже ломался на + барьерах света») — прямая цитата реального прошлого дефекта, не спекуляция. +- `localStorage` ключи-соседи `houseplan_card_layout_v1`, `houseplan_card_cfg_v1` + — подтверждены в `src/houseplan-card.ts` / `src/config-store.ts`. +- `docs/WARM-REMOUNT.md`: `_view`, `_viewModeSnap`, `_showFar`, `warmBoot`, + правило «воскрешаем черновик, не воскрешаем решение» — расширение контракта в + §7.1 ТЗ (проекция + логический центр в слоте) непротиворечиво накладывается на + существующую модель слотов, не переопределяет её. +- `docs/CANVAS.md`: диапазон `±5000` (`CANVAS_LIMIT`), `_baseVb()`/`spaceFrame()` + — оба факта, на которые ссылается ТЗ (B1: «`_baseVb()` не знает про поднятые + грани»), подтверждаются текстом канона. +- `docs/TOUCH-SUPPORT.md`: требование явно писать «Touch editor: + supported/best effort/not exposed» в спеке нового фичи — ТЗ соблюдает это + буквально (§7: «Touch editor: не exposed»). +- Отсутствие зависимости `semver` в проекте (утверждение §2.4, M3) — + подтверждено по `package.json`: только `lit`, `polyclip-ts` в + `dependencies`. +- `demo/performance/budgets-*.json`, `demo/golden/harness.mjs`, + `GOLDEN_MATRIX_VERSION` (`demo/golden/matrix.mjs:4`), `test/golden-matrix.test.mjs` + — все существуют; переиспользование инфраструктуры (M6, M7) не изобретает + несуществующие точки расширения. +- Ссылки на #82 (P2 feature, камера/zoom-fit), #73 (закрыт, мигание при + возврате на вкладку), #50 (закрыт, экспорт/импорт конфигурации), #85 + (тесты/infra, «смок должен уметь падать»), #83 (P2 feature, «Сводка дома», + кандидат на Labs), #52 (S4-spec-review, размеры на плане, кандидат на Labs) — + все существуют и совпадают по теме с тем, как на них ссылается ТЗ. +- Камера `rotDeg = 0`, наклон `18–22°` (§4.2) — это не догадка автора, а + буквальная рекомендация из `PSEUDO_3D_SPECIFICATION.md`, приложенного + владельцем к issue (комментарий владельца от 2026-08-13, раздел 6: + «Рекомендуемый наклон от вида строго сверху: 18–22°»). Формулировка ТЗ + корректно оставляет точное значение внутри диапазона решению ADR (§13.1), + не выдаёт его за уже принятое число. + +Не найдено ни одного утверждения о поведении карточки, которое не имело бы +основания в коде, в каноническом документе или в материалах владельца и не +было бы при этом явно помечено как решение ADR/предположение (§15). Это +отличает документ от типичного ТЗ такого объёма. + +## Находки + +Ни одной **High**. Ни одной **Medium**. Две **Low** — обе разобраны и сняты +здесь же с записью, отдельные issue не заводятся (PROCESS.md §2.4: на этапе ТЗ +Low «либо правится, либо снимается решением ревьюера с записью в документе»). + +### L1 — риски не собраны в отдельный раздел + +PROCESS.md §7.1 перечисляет «риски» как самостоятельный обязательный раздел +ТЗ, отдельно от AC и от «откат». В `089-isometric-view-stage1.md` риски не +собраны под одним заголовком: они распределены по §0.2 (таблица находок +B1–B8, каждая — с формулировкой риска и тем, куда внесено исправление), §8.2 +(риск деградации perf на больших планах), §9 (риск падения renderer) и §13.5 +(риск, что этап 1 сломает flat-путь — с явным следствием «issue возвращается в +S6, бета блокируется»). Содержательно риски перечислены и снабжены мерами; +не хватает одного места, где их видно списком. + +**Вердикт по находке:** снимается с записью. Материал присутствует и +проверяем; отсутствие заголовка не делает ТЗ неисполнимым или +непроверяемым — это единственный критерий блокировки на этом этапе. +Рекомендация автору на будущее (не обязательна для этой ревизии): свести +таблицу B1–B8 плюс §8.2/§9/§13.5 в один поднумерованный список «Риски», +чтобы не заставлять читателя собирать его по документу вручную. + +### L2 — «нормативная» таблица слоёв формально противоречит собственному запрету ТЗ на новый оконный элемент + +§6 ТЗ открывается фразой «Поведение слоёв — таблица §6 исследования +[`089-isometric-view.md`], она нормативна», а затем добавляет булиты «как +обязательный контракт Stage 1». Строка «Окно» в этой внешней таблице +буквально обещает «Разрыв объёма с **отдельным стилизованным оконным +элементом**»; булиты этажа 1 сразу же запрещают именно это: «вертикальные +створки, светлые оконные вставки и **новый оконный свет не появляются**». +Решение владельца O5 (комментарий от 2026-08-13) и AC13 подтверждают, что +запрет — это то, что реально должно быть реализовано; формулировка «таблица +нормативна» без явной оговорки «кроме строки Окно, которую переопределяет +§6/O5/AC13» создаёт секундное разночтение для читателя, который сверяется +только с внешней таблицей. + +**Вердикт по находке:** снимается с записью, поскольку фактическое +поведение однозначно зафиксировано трижды в самом ТЗ (булиты §6, AC13, §14 +п.3 «Полировка дверей, окон и ворот… — этап 2») и в решении владельца O5 — не +только в спорной внешней таблице. Реализация и код-ревью не могут выбрать +неверный вариант, читая только ТЗ. Рекомендация автору на будущее: одна +фраза («кроме строки Окно/Ворота — см. булиты ниже») сняла бы вопрос совсем. + +## Что проверено и корректно + +- Обязательные разделы ТЗ по PROCESS.md §7.1 присутствуют (кроме + консолидации рисков — L1, снята). +- Все 15 AC пронумерованы, для каждого указан способ доказательства + (`unit`/`smoke`/`golden`/`performance`/код-ревью), и матрица §11.6 + дополнительно предписывает ревьюеру кода конкретную проверку исполнением — + это закрывает будущий провал типа T1 («смок должен уметь падать»), на + который прямо ссылается #85. +- Каждое технически-конкретное утверждение (имена функций, констант, CSS + правил, файлов инфраструктуры, номера issue) проверено против реального + кода/документов и подтвердилось — см. «Верификация фактов». Догадок, не + помеченных как предположение, не найдено. +- Границы этапа (D1, O1–O6) корректно и без потери смысла перенесены из + решений владельца (issue-комментарии Q1–Q6/ответы) в нормативный текст ТЗ. +- Раздел «Принятые предположения» (§15) корректно отделяет то, что автор + вправе менять свободно (имена модулей, точные числовые константы внутри + диапазона, версионирование localStorage-ключей), от продуктовых решений. + Технические вопросы в issue не встречаются — автор не выносил владельцу то, + что должен решать сам (в отличие от типичной ошибки, которую просит искать + этот процесс). +- Touch/kiosk-контракт сформулирован по правилам `docs/TOUCH-SUPPORT.md` + (явная строка `Touch editor: не exposed`, конкретный touch target 44×44 CSS + px, release-blocking пункт для View/kiosk). +- Совместимость с `docs/CONFIG-COMPATIBILITY.md`: формат конфигурации не + меняется, новых полей нет, `localStorage`-ключ версионирован суффиксом + `_v1` — не требует записи в реестр совместимости (реестр покрывает только + серверный конфиг). +- Откат (§13.5) полный: аварийный пользовательский путь без пересборки + (`?hp-labs=-iso`/`off`) и полный code rollback (revert + удаление флага из + реестра) без обратной миграции данных. +- Именование файла ТЗ (`089-isometric-view-stage1.md`) соответствует правилу + PROCESS.md §2.3 для многоэтапной задачи (`--stage.md`). +- Не найдено расширение скоупа сверх решённого владельцем: этап 1 явно + ограничен Stage 1 Labs/spike+renderer (O4), Stage 2 вынесен в будущий issue. + +## Чего не проверял + +- Гейты `npm ci`/`npm test`/`npm run build`/`golden`/`performance` не + запускались — на этом коммите нет изменений класса A/B/D, запускать их + было бы проверкой пустого множества. +- Не проверялась реализуемость точных числовых бюджетов perf (§8.2, «20% + относительный допуск») на реальном большом плане — это по определению + проверяется на этапе code review профилем `large-house-isometric-v1`, + которого пока не существует. +- Не проверялось поведение в Safari/WebKit и Firefox для заявленных + filter/clip/mix-blend рисков (§13.1 ADR) — согласно самому ТЗ, это предмет + ADR/spike, а не текста спеки. +- Не оценивался остаточный риск того, что технический пользователь найдёт + `?hp-labs=iso` в живом бандле раньше Stage 2 (сам механизм URL-флага по + конструкции обнаруживаем через чтение бандла/консоли). Это осознанный + побочный эффект многократно переиспользуемого механизма Labs (D2, O3), + одобренного владельцем для будущих #82/#83/#52 — не специфичный для #89 + риск, который стоило бы решать в рамках этой задачи. + +## Вердикт + +**Зелёный · цикл r1/4 · High: 0 · Medium: 0 → нет новых issue.** + +ТЗ проверяемо, однозначно по каждому AC, границы этапа зафиксированы решением +владельца, а технические утверждения выдерживают проверку против кода и +канонических документов. Обе Low-находки сняты с записью в этом документе. +Следующий статус — «Готово к разработке» (`S5-к-разработке` / `S5-ready`).