diff --git a/docs/reviews/SPEC-REVIEW-582-r1.md b/docs/reviews/SPEC-REVIEW-582-r1.md new file mode 100644 index 00000000..f17e829d --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-582-r1.md @@ -0,0 +1,160 @@ +# SPEC-REVIEW-582-r1 + +**Issue:** [#582](https://github.com/Matysh/houseplan-card/issues/582) — «HA Companion: на плане с масштабом 1 см/точку при pinch появляются фиксированные белые области» +**Этап:** ТЗ на ревью (S4-spec-review) · заход r1 · блокирующих циклов израсходовано 0 из 4 +**Ревьюер:** независимая сессия, без контекста автора ТЗ +**Материал:** тело issue #582, раздел `## ТЗ`, состояние на момент ревью (см. якоря) + +## Скоуп ревью + +Проверяется ТЗ в теле issue #582 по чек-листу PROCESS.md §7.1: обязательные разделы, +однозначность и доказуемость каждого AC, отсутствие непомеченных догадок, +соответствие `docs/SCOPE.md` (какую строку Core user jobs закрывает) и +терминологии `docs/USER-GUIDE.ru.md`. Продуктовый код не менялся и не оценивался +как реализация — только как контекст для проверки технических утверждений ТЗ. + +Задача полного трека (аналитик явно назвал нарушенные критерии лёгкого трека: +влияние на производительность и обязательный touch-контракт, несколько +compositor-подсистем, кросс-браузерные доказательства) — это корректно, автор +ТЗ не пытался обойти классификацию. + +## Как проверялось + +Помимо чтения тела issue и всех комментариев (`gh issue view 582 --json body,comments`), +каждое техническое утверждение ТЗ сверено с действующим кодом и каноническими +документами, а не принято на слово: + +| Утверждение ТЗ | Где проверено | Результат | +|---|---|---| +| Постоянный `will-change: filter` на `.hp-paperg`, введённый #532 | `src/styles/plan.styles.ts:97-105` — правило висит безусловно на `.stage.daycycle .hp-paperg` и `.hp-static-stage.daycycle .hp-paperg`, не зависит от размера бумажного bbox | подтверждено дословно | +| `.hp-static-stage` — это `houseplan-space-card`, риск общий для обеих поверхностей | `src/space-card.ts:650,944` использует тот же селектор `.hp-static-stage` | подтверждено | +| Внутренние SVG-единицы пропорциональны `1/cell_cm` (значит `cell_cm: 1` даёт кратно больший bbox, чем `cell_cm: 5`, при одинаковой физической площади) | `docs/CANVAS.md:43` — `gridVisualScale(cell_cm) = 5 / cell_cm` | подтверждено, числа в ТЗ (4700×4200 vs 932×841) согласуются с формулой | +| Компоситорный контракт #579 («от первого live-кадра до terminal commit — один compositor lifecycle», ownership boundary как источник белого/прозрачного кадра в HA Companion WebView) | `docs/ARCHITECTURE.md:1966-2004` | подтверждено дословно, включая упоминание того же symptom-класса | +| Существующий mutation-свидетель #532 (`daycycle-outline-not-promoted`, снимает `will-change: filter`) | `scripts/mutation-registry.mjs:2613-2624` | найден, согласуется с AC4 («свидетель #532 остаётся зелёным») | +| 4 golden-сцены day-cycle dawn/day/dusk/night уже существуют | `demo/golden/baselines/day-cycle-{dawn,day,dusk,night}-dark.png` | подтверждено, ровно 4 | +| Существующие smoke, названные в тест-плане (`smoke_daycycle_raster.mjs`, `smoke_live_pan_coverage.mjs`) | `demo/smoke_*.mjs` (249 файлов) | оба файла существуют | +| Термин «Следует за Солнцем» — не изобретён | `docs/USER-GUIDE.ru.md:1628` | совпадает дословно | +| Touch-контракт «never flash white, become transparent» применяется к HA Companion WebView | `docs/TOUCH-SUPPORT.md:48-53` | подтверждено — ТЗ решает ровно уже задокументированный release-blocking контракт, не изобретает новый | +| Указанные ролью «внутренняя `.hp-paperg`» — pointer-inert | `src/houseplan-card.ts:11622` (`pointer-events="none"`) | подтверждено | + +Дополнительно сверено разбиение issue: часть «Проблема/Воспроизведение/Ожидание» +выше `## ТЗ` — интейк-материал от заведения issue, формальный ТЗ начинается с +`### Пользовательский сценарий`. Это тот же паттерн, что и в недавнем принятом +ТЗ #580 (сверено `gh issue view 580`), значит не является отклонением от +устоявшейся практики репозитория. + +## Находки + +### Low — AC5 не несёт собственного независимого доказательства (снято ревьюером) + +`AC5 (render contract)` дублирует контракт-пункт 5 («один внешний контур без +внутренних швов») и часть контракта-пункта 7, но категория «render contract» не +входит в перечень доказательств §2.5 (`unit`/`backend`/`smoke`/`golden`/«ревью +кода»). При первом чтении это выглядит как AC без названного способа проверки. + +Проверка тест-плана показала, что это не пробел: раздел «Тест-план и +доказательства» явно называет «unit/structure-проверку наличия отдельной +outline-scene **только там, где она нужна**» — формулировка покрывает и +позитивную часть AC5 (full/kiosk/space-card получают контур), и негативную +(редакторы и изометрия не получают новый filter-layer), а видимый шов между +комнатами — тот же механизм, что уже сегодня закрывает существующий +комментарий `houseplan-card.ts:11617-11619` («One `` around ALL paper shapes +… so adjacent rooms never cast seams»). Иными словами, AC5 полностью +покрывается связкой AC6 (golden) + AC7 (unit/structure), у него просто нет +собственной строки-подписи. + +Снимаю без правки: это вопрос перечисления, а не отсутствующей проверки — +автор может при желании сменить бирку AC5 на `(golden + unit, см. AC6/AC7)`, +но это не блокирует переход в «Готово к разработке». + +## Что проверено и признано корректным + +- Все обязательные по §7.1 содержательные блоки присутствуют (распределены по + подзаголовкам как и в #580): сценарий, видимый до/после, проблема (частично + вынесена в тело issue выше `## ТЗ`, частично — в «Подтверждённая причина»), + контракт поведения, UX/данные/i18n, критерии приёмки с доказательством, + тест-план, риски, откат, release-артефакты. +- Технический диагноз (`will-change: filter` на координатно большой + `.hp-paperg` создаёт compositor-слой пропорционально `cell_cm`) грамотно + отделён от решения: ТЗ явно запрещает наивные обходы («нельзя просто удалить + `will-change`», «нельзя отключить тень», «нельзя подменить фон статичным») и + оставляет техническую реализацию (где именно рисуется вторая сцена, как она + синхронизируется) агентам — это правильная граница §7.1 «всё, чего + пользователь не наблюдает, решают агенты». +- Продуктовых вопросов владельцу нет, и по факту не нужно: единственные + неопределённости (природа «белых областей» как platform-specific потери + тайлов, способ вычисления экранного лимита теста, неизменность параметров + тени) явно помечены разделом «Принятые предположения», а не выданы за факт. + Это именно то разграничение, которое требует PROCESS.md §7.1 — ни одна из + них не является продуктовым вопросом («что видит/делает человек»), поэтому + эскалация владельцу была бы избыточной. +- AC1–AC4, AC6–AC9 однозначны, у каждого назван способ доказательства и он + соответствует существующей инфраструктуре гейтов (CDP LayerTree smoke, + presented-frame/screencast smoke, performance-сравнение, golden, unit/ + structure, quality gates, полевая проверка). AC2 явно требует мутационного + свидетеля и связи с mutation registry — соответствует §2.7 требованию + «таблица чем краснеет» на будущее код-ревью. +- Контракт поведения (пункты 1–7) непротиворечив: не расширяет пользовательский + скоуп (внешний вид, тайминг фаз, параметры тени не меняются), явно + перечисляет незатронутые подсистемы (изометрия, редакторы, солнечные лучи, + Glow, мебель, проёмы, конфиг-схема) — это и есть не-скоуп, просто внутри + «Контракта», а не под отдельным заголовком. +- Откат — один коммит без миграции данных, с явным условием (не откатывать + тихо, без блокировки беты) — проверяемо и соразмерно классу бага. +- Release-артефакты названы: `ARCHITECTURE.md`, `SUN.md`, опционально + `TOUCH-SUPPORT.md`/`TESTING.md`, оба changelog в терминальном коммите при + `User-Visible: yes` — соответствует §2.5 DoR. +- i18n и модель данных корректно помечены «не затрагиваются» и это + подтверждается описанным характером правки (только отрисовка). + +## Чего не проверял + +- Сам продуктовый код фикса не существует (это этап ТЗ, реализации ещё нет) — + соответственно AC1–AC9 не исполнялись, только оценивалась их проверяемость и + соответствие существующей инфраструктуре гейтов. +- Не проверялось реальное поведение в HA Companion WebView на физическом + устройстве — это находится за пределами ревью ТЗ и явно вынесено в AC9 + (полевая проверка после беты), что ТЗ само корректно помечает как + дополняющую, а не блокирующую автоматические AC проверку. +- Не запускал `npm test`/`npm run build`/gates — на этапе ревью ТЗ не + применимо: код не менялся, только текст issue. +- Не проверял issues #532 и #579 построчно по их собственным ТЗ/ревью — + ограничился их итоговым состоянием в `ARCHITECTURE.md`/`SUN.md` и `CLOSED` в + трекере, этого достаточно для проверки согласованности заявленной причины. + +## Вердикт + +Зелёный. AC пронумерованы, каждый имеет способ доказательства, технические +утверждения о причине дефекта подтверждены чтением действующего кода и +канонических документов (не догадка, выданная за факт), продуктовых открытых +вопросов нет, риски и откат описаны. Единственная находка — Low, снята +ревьюером без правки текста (см. раздел «Находки»). + +--- + +## Материал раунда + +- Issue: #582, ветка ТЗ — тело issue, раздел `## ТЗ` (файл в `docs/specs/` не + создаётся, #517). +- Проверено на состоянии репозитория `bae6afaae7bac2dd45bca52b6caf64a16d96f182` + (текущая рабочая копия), файлы-источники истины для сверки: `src/styles/plan.styles.ts`, + `src/space-card.ts`, `src/space-render.ts`, `src/houseplan-card.ts`, + `scripts/mutation-registry.mjs`, `docs/CANVAS.md`, `docs/ARCHITECTURE.md`, + `docs/SUN.md`, `docs/TOUCH-SUPPORT.md`, `docs/USER-GUIDE.ru.md`. +- Текст ТЗ получен `gh issue view 582 --repo Matysh/houseplan-card --json body,comments` + в ходе этого ревью; отдельный SHA/blob тела issue конвейер проставит сам в + публикуемую версию документа. + +--- + + + +## Материал раунда + +- Ветка: `issue/582-webview-large-filter-layers`, коммит `bae6afaae7ba` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `df4dd4b28569cf0269e0ae155d20537080179e07` + ``` + git log --all --format='%H %T' | grep df4dd4b28569 + ``` +- Тело issue: `4c1e1ecfe8ce205f3cfb3c44eb6028a35d0bb4358323e05ba60c6fd85b3b40ba` +- Вердикт конвейера: `green` · High 0