docs: review document for #509

Issue: #509
User-Visible: no

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

@@ -0,0 +1,312 @@
+# SPEC-REVIEW-509-r1
+
+Issue: [#509](https://github.com/Matysh/houseplan-card/issues/509) — «Сводная
+панель: при первом показе подвисание и «Source unavailable» вместо значений
+до загрузки metrics-чанка».
+Этап: spec (PROCESS.md §2.4). Заход: r1. Блокирующих циклов израсходовано 0 из 4.
+Материал: тело issue #509, раздел `## ТЗ` (owner-решение #517, 2026-09-10 —
+ТЗ полного трека тоже живёт в теле issue, файл `docs/specs/509-*.md` не
+создаётся), на момент перехода `S3-spec → S4-spec-review`
+(`unlabeled: S3-spec` / `labeled: S4-spec-review` в
+`2026-09-10T07:56:11Z`, тот же момент несёт единственную правку тела после
+S2-комментария). Рабочая копия `dev` на `d204bf5d` — код по этой задаче ещё
+не существует, ветки `issue/509-*` на origin нет; ревью целиком по тексту.
+
+## 1. Скоуп ревью
+
+Трек — полный: метки issue — `bug`, `S4-spec-review`, `small` отсутствует.
+Явного предложения «трек: small — нет, критерий не пройден» я в S2-комментарии
+не нашёл дословно, но по существу критерий §5 «нет влияния на
+производительность» действительно не пройден — вся задача построена вокруг
+измеренных миллисекунд и мутационных гардов на перф-путь, что и оправдывает
+полный трек фактически (см. Low-1 ниже: это гигиена формы, не решение).
+
+Ревью охватывает:
+- тело issue #509 целиком: секцию «Симптом», секцию «Что видно в коде (для
+  S2)», комментарий S2-аналитики (владелец/автор, `2026-09-10T07:49:33Z`) и
+  секцию `## ТЗ`;
+- соответствие `docs/SCOPE.md` (персона, поверхность, job), `PROCESS.md`
+  §7.1/§7.2/§5, `AGENTS.md`;
+- фактические утверждения ТЗ о текущем коде — сверены чтением, не приняты на
+  слово;
+- реализуемость каждого AC заявленным способом доказательства.
+
+Это первый заход — раздела «Унаследовано из r0» не требуется, разбор полный.
+
+## 2. Как проверялось
+
+- Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (включая §2.2,
+  §2.4, §2.10, §4, §5, §7.1, §7.2, §8).
+- Прочитано тело issue #509 и его единственный комментарий через
+  `gh issue view 509 --json body,comments,labels,state`.
+- Прогон таймлайна issue (`gh api .../issues/509/timeline`) и
+  `userContentEdits` (GraphQL) — подтверждено: `S1-new → S2-analysis` в
+  `07:44:05`, комментарий-аналитика в `07:49:33`, переход в `S3-spec` уже в
+  `07:49:35` (через 2 секунды после своего же комментария), переход в
+  `S4-spec-review` в `07:56:11` — единственная правка тела issue приходится на
+  этот же момент (второй `userContentEdits`-узел). Значит между вопросом
+  владельцу («что видно в первые ~200 мс», вариант A по умолчанию) и записью
+  ТЗ отдельного ответа владельца физически не было — редакция ТЗ добавлена в
+  теле той же ~7-минутной сессией, которая задала вопрос. Это не идёт вразрез
+  с §2.2 (аналитик не обязан ждать подтверждения и явно объявил «если ответа
+  не будет — реализую A»), но заголовок ТЗ «решение владельца по плейсхолдеру
+  — в комментарии S2» точнее было бы читать как «дефолт автора с явным
+  объявлением», а не как отдельное решение владельца — содержательно на
+  вердикт не влияет (вариант A разумен, задокументирован, проверяем AC1/AC6),
+  поэтому не заводится отдельной находкой, но фиксирую здесь для
+  прослеживаемости следующего раунда.
+- Каждое фактическое утверждение ТЗ и S2-комментария о текущем коде
+  перепроверено чтением на `dev` `d204bf5d`, а не принято на слово:
+  - `LoadedSummaryPanelRuntime.value()` (`src/summary-panel-runtime-loaded.ts:715-720`):
+    подтверждено — `resolved ?? this.t('summary.unavailable')`, и `resolved`
+    равно `null` как при отсутствующем `module` (`metricsModule` ещё не
+    загружен), так и при легитимно недоступном источнике — состояния
+    неразличимы в возвращаемом значении, ровно как описывает симптом.
+  - `metrics()` (`:687-712`): подтверждено — вызывается синхронно, мемо по
+    `cfgEpoch`/`layoutRev`/`registryRev` (device) и `cfgEpoch` (area), при
+    первом вычислении после загрузки модуля выполняется целиком в кадре.
+  - `totalCleanFloorAreaM2` (`src/summary-panel-metrics.ts:59-84`):
+    подтверждено — цикл по `space.rooms` вызывает `innerContourForRoom` без
+    `sharedRoomWallGeometry`/`sharedMultiWallNodes` на каждой комнате; сама
+    `innerContourForRoom` (`src/wall-thickness.ts:2750-2779`) в этом случае
+    заново вызывает `wallBodiesGeometry`/`multiWallNodesForGeometry` — точно
+    описанная причина 7,5-кратной разницы.
+  - Утверждение «карточка уже передаёт общую геометрию через `_innerContour`»
+    подтверждено: `_innerRoomContour` (`src/houseplan-card.ts:9219-9240`)
+    передаёт `roomWalls`/`multiWallNodes` из `_wallUnionGeometry()` (общий,
+    закешированный по эпохе объект) — карточка действительно не пересчитывает
+    геометрию стен на каждой комнате, а сводная панель — пересчитывает.
+    Не выдумка, реальный прецедент в том же файле.
+  - `SummaryPanelPresentation`/анимация 190 мс (issue #505) — подтверждено:
+    класс существует и подключён (`src/summary-panel-runtime-loaded.ts:23,93`),
+    #505 закрыт и смёржен (`gh issue view 505` → `state: CLOSED`, статусных
+    меток нет) — AC5 действительно проверяет уже существующий, но
+    (по симптому) заблокированный синхронным расчётом путь, а не выдуманную
+    фичу.
+  - `summary.unavailable` — реальный существующий ключ
+    (`src/summary-panel-i18n.ts:16,84,152,220`, en/ru/de/fr) — заявление
+    «новых i18n-ключей нет» корректно, скелет действительно чисто визуальный.
+  - Персона «житель дома» — используемый в проекте перевод `Household
+    members` из `docs/SCOPE.md` (совпадает с формулировкой в
+    `SPEC-REVIEW-212-r1.md`, `SPEC-REVIEW-374-r1.md`, `SPEC-REVIEW-505-r2.md`)
+    — не изобретённый термин.
+- Проверена техническая реализуемость AC3 («один вызов… через инъекцию
+  счётчика»): `wallBodiesGeometry`/`multiWallNodesForGeometry` — экспортируемые
+  чистые функции (`src/wall-thickness.ts:2434,3662`), `innerContourForRoom`
+  уже принимает их как необязательные параметры — заявленный план (передать
+  общую геометрию один раз на пространство) технически реализуем без нового
+  паттерна, здесь это чистая инженерная перестановка вызовов, а не гипотеза.
+- Проверено соответствие обязательных разделов §7.1 (таблица ниже).
+- Гейты (`typecheck`/`test`/`build`) не прогонялись — кода в материале нет
+  (ветки `issue/509-*` не существует), на этапе `spec` эти гейты неприменимы
+  к пустому диффу; смотри также §5 «Чего не проверял».
+
+### Проверка обязательных разделов (§7.1)
+
+| Требование | Где в теле issue |
+|---|---|
+| Сценарий | `## ТЗ` → «Сценарий»: персона «житель дома», поверхность View + kiosk, момент — первое открытие/включение панели |
+| Что человек увидит до/после | «Что человек увидит до и после» — одной фразой, без терминов реализации |
+| Проблема | Секция «Симптом» + «Что видно в коде» (тело issue, до `## ТЗ`) |
+| Скоуп и не-скоуп | «Контракт» (объём правки) + «Не-скоуп» |
+| Контракт поведения | «Контракт», пп. 1–5 |
+| UX | Явного отдельного раздела нет, но по существу покрыт («Контракт» пп.1,2,5 — нет новых элементов управления, только визуальные состояния уже существующих строк) |
+| Модель данных и миграция | Преамбула ТЗ: «Модель данных и схема конфигурации не меняются» |
+| i18n | Преамбула: «Новых i18n-ключей нет» — подтверждено чтением, см. выше |
+| AC1…ACn с доказательством | «AC», 7 пунктов, у каждого назван unit/smoke |
+| План автотестов | Не вынесен отдельным разделом, но покрыт AC + таблицей «Чем краснеет» (тот же уровень, что и прецедент #506, признанный там достаточным) |
+| Риски | «Риски», 2 пункта |
+| Откат | «Откат» |
+| Release-артефакты | «Release-артефакты» |
+
+Все обязательные разделы присутствуют по существу; отсутствие отдельного
+финального блока «Принятые технические предположения» разобрано как
+Medium-1 ниже.
+
+## 3. Находки
+
+### Medium-1 (в скоупе) — отсутствует раздел «Принятые технические предположения» (PROCESS.md §7.1)
+
+ТЗ делает по ходу текста несколько односторонних технических решений, которые
+пользователь не наблюдает, но которые прямо влияют на то, что именно будет
+реализовано, и ни одно не вынесено в явный финальный блок «принято
+предположительно, поменять свободно» — том самом формате, который PROCESS.md
+§7.1 требует именно для того, чтобы ревьюер мог их оспорить, а не читать как
+уже окончательное решение. Конкретно:
+
+1. **Механизм переноса расчёта «вне кадра» назван как альтернатива, не
+   решение.** П.4 контракта: «расчёт запускается после кадра
+   (микрозадача/`requestIdleCallback` с фолбэком на таймер)». Это не один
+   механизм с уточнением, а два содержательно разных: микрозадача выполняется
+   до следующей отрисовки (гарантированно быстро), `requestIdleCallback`
+   ждёт простоя главного потока и при устойчиво занятом потоке может
+   откладываться на неопределённое время — то есть ровно то поведение,
+   которое приводило бы к варианту, где готовое число появляется через
+   секунды после показа скелета на медленной машине. Ни один AC не
+   ограничивает, сколько именно может пройти между показом скелета и
+   заменой его значением (см. также Medium-2) — то есть от выбора между
+   этими двумя механизмами наблюдаемо зависит поведение, а формулировка ТЗ
+   не решает, какой из них, а перечисляет оба через слэш.
+2. Способ технической инъекции счётчика вызовов для AC3 (как именно тест
+   заменит/оборачивает `wallBodiesGeometry`/`multiWallNodesForGeometry`,
+   учитывая, что они — обычные экспортируемые функции ES-модуля, а не
+   параметры) не назван ни как решённый вопрос, ни как «на усмотрение
+   реализации».
+3. Форма хранения «прежнего значения» при инвалидации мемо (п.4 контракта:
+   «показывает прежнее значение до прихода нового») — остаётся ли это тем же
+   полем `deviceMemo`/`areaMemo`, что и сегодня, или требует нового состояния
+   («последнее успешно вычисленное» отдельно от «текущего валидного») — не
+   названо явно.
+
+Прецедент того же приёма находки, для той же подсистемы, у того же автора:
+`SPEC-REVIEW-505-r1.md`, Medium «отсутствует обязательный раздел «Принятые
+технические предположения»», со ссылкой на `docs/specs/493-...md` как
+образец, где формат уже применялся. В #506 то же требование было закрыто
+записью Low, а не Medium, потому что там содержание уже было распределено
+по тексту («предпочтительный дизайн… точные имена и границы модуля
+допускается уточнить») — здесь же пункт 1 выше не просто не назван свободным,
+а прямо оставляет открытым выбор между двумя семантически разными
+механизмами, поэтому не дотягивает даже до уровня #506 и остаётся Medium.
+
+**Как чинится:** добавить в конец ТЗ раздел «Принятые технические
+предположения» и явно решить (или явно пометить «на усмотрение реализации,
+не меняет ни один AC») три пункта выше — в первую очередь пункт 1, так как
+он пересекается с Medium-2.
+
+### Medium-2 (в скоупе) — контракт п.4 (прежнее значение при инвалидации мемо, без скелета) не доказан ни одним AC
+
+П.4 контракта содержит отдельное поведенческое обязательство: «при
+инвалидации панель показывает **прежнее** значение до прихода нового (не
+скелет — скелет только когда значения ещё не было)». Это прямая защита
+именно от того паттерна мигания, из-за которого заведена задача (значение
+подменяется на «недоступно»/пусто и обратно) — то есть по существу это
+самый близкий к первоначальному симптому кусок контракта, а не второстепенная
+деталь.
+
+Ни один из AC1–AC7 его не проверяет:
+- AC1/AC2 — про состояние «`metricsModule` ещё не загружен» vs «источник
+  недоступен», не про инвалидацию уже загруженного модуля;
+- AC3 — про идентичность площади и число вызовов геометрии, не про то, что
+  показывается в UI между инвалидацией и пересчётом;
+- AC4 — про длительность первого показа, не про повторные показы после смены
+  конфигурации;
+- AC5/AC6 — про анимацию появления/исчезновения панели и уважение
+  `prefers-reduced-motion`, не про эту конкретную защиту;
+- AC7 — про трейлеры/i18n/golden.
+
+**Сценарий проявления:** реализация меняет `cfgEpoch` (правка конфигурации,
+редактор) и на время пересчёта (после п.4, теперь асинхронного) ошибочно
+показывает скелет вместо уже известного старого числа — ни один автотест
+здесь не покраснеет, регрессия обнаружится только ручным глазом на
+следующей задаче, ровно как было с #234 (не этот баг конкретно, но тот же
+класс: незамеченная регрессия смежного поведения, которую в этом процессе
+призвана ловить именно связка «AC + автотест»).
+
+**Как чинится:** добавить AC (или явно расширить формулировку одного из
+существующих, например AC1), проверяющий: `cfgEpoch` меняется →
+`deviceMemo`/`areaMemo` уже когда-то были посчитаны → `value()`/рендер строки
+показывает старое число, а не скелет, пока не готово новое. Разумный
+свидетель — unit на `metrics()`/`value()` с последовательностью
+{вычислено → `cfgEpoch++` → запрошено снова до завершения асинхронного
+пересчёта}.
+
+### Low-1 (не блокирует, для протокола) — критерий отказа от лёгкого трека не назван явно
+
+PROCESS.md §2.2 п.8 требует при выборе полного трека называть конкретный
+критерий §5, который задача не проходит («обычный трек» без названного
+критерия обоснованием не является). S2-комментарий переходит в `S3-spec`
+без такой явной фразы (в отличие от прецедента `SPEC-REVIEW-506-r1`, где
+аналитик прямо процитировал «критерий small «нет влияния на
+производительность» заведомо не выполнен»). По существу критерий
+действительно не пройден — вся задача измеряет и бюджетирует
+производительность (AC3–AC5, таблица «Чем краснеет», мутанты на перф-путь),
+и никакого другого вывода из содержания ТЗ сделать нельзя, поэтому это
+дефект формы аналитического комментария, а не двусмысленность самого ТЗ.
+Не блокирует зелёный вердикт по этой задаче; отдельного действия не требует
+(снимается записью). Упоминается ради единообразия с прецедентами #506/#505.
+
+## 4. Что проверено и корректно
+
+- **Формат этапа:** полный трек, ТЗ — в теле issue, что верно для задач,
+  открытых после решения #517 (2026-09-10); ссылки issue↔ТЗ на месте
+  (единственный документ и есть issue).
+- **Персона и SCOPE.md:** «житель дома» (Household members) на View/kiosk —
+  укладывается в J1 («live spatial overview») напрямую; никакого конфликта с
+  `docs/SCOPE.md` не найдено, задача не расширяет функциональность, а
+  устраняет деградацию отзывчивости и вводящий в заблуждение текст ошибки.
+- **Технический диагноз подтверждён чтением, не переписан со слов автора**:
+  все пять цитируемых мест кода (`value()`, `metrics()`,
+  `totalCleanFloorAreaM2`, `_innerRoomContour`, `SummaryPanelPresentation`)
+  проверены построчно на `dev` `d204bf5d` и совпадают с описанием ТЗ и
+  S2-комментария; числа (11 045 → 1 488 мс, 306.3 м²) взяты из измерения
+  того же комментария, а не изобретены заново в ТЗ.
+- **AC3 технически реализуем** заявленным способом (общие экспортируемые
+  функции, `innerContourForRoom` уже принимает shared-параметры) — не
+  гипотеза без опоры на код.
+- **AC5 не выдумывает новую фичу**: #505 (анимация 190 мс) уже смёржена и
+  закрыта, AC5 проверяет, что расчистка главного потока (пп. 3–4) даёт этой
+  уже существующей анимации реально отрисоваться, а не изобретает
+  анимацию заново.
+- **i18n:** новых ключей действительно нет, `summary.unavailable` — реальный
+  существующий ключ на 4 языках; заявление ТЗ точное.
+- **Модель данных / миграция / touch:** корректно помечены «не меняются» /
+  «не относится» — правка не касается конфигурации, хранения или touch-целей.
+- **Риски** называют реальный источник флейка (порог AC4 меряется на CI с
+  запасом, не сравнивается между машинами) — соответствует уже принятому в
+  проекте паттерну perf-смоков (`PROCESS.md` §8).
+- **Откат** тривиален и точен: правка локальна для `summary-panel-*` плюс
+  тесты, данных и миграций нет.
+- Продуктовых вопросов владельцу, кроме уже заданного и закрытого дефолтом
+  (плейсхолдер = вариант A), в тексте не осталось; ни одна догадка о
+  видимом поведении не подана как факт без пометки (единственная зона
+  неопределённости — п.4/AC-разрыв — разобрана явно как находка, а не как
+  скрытая догадка).
+
+## 5. Чего не проверял
+
+- Не прогонялись `typecheck`/`test`/`build`/`process-gate` — кода в
+  материале нет, ветки `issue/509-*` не существует; это гейты этапа `code`,
+  не `spec`.
+- Не запускались браузерные смоки/`golden`/`performance_smoke` — на этапе
+  ТЗ реализации не существует; они станут обязанностью код-ревью, когда
+  появятся файлы `demo/smoke_*`, названные в AC1/AC4/AC5/AC6.
+- Не проверялась синтаксическая корректность будущего кода
+  (`requestIdleCallback`-план, структура нового состояния «прежнее
+  значение») — его ещё не существует; оценивалась только согласованность
+  плана с уже существующим кодом, на котором план основан.
+- Не оценивалось качество будущих мутационных гардов из таблицы «Чем
+  краснеет» сверх того, что они названы и привязаны к конкретным функциям —
+  реализация появится в коде.
+- Не проверялся сам механизм подсчёта хеша тела issue в блоке якорей
+  документа (`scripts/review-doc-guard.mjs`/`process.yml`, #517) — это
+  инфраструктура конвейера, а не предмет ревью ТЗ конкретной задачи.
+
+## 6. Вердикт
+
+High: 0. Medium: 2, обе в скоупе задачи, чинятся правкой того же текста без
+нового технического анализа (обе опираются на уже написанные автором факты
+и код, который я перепроверил). Low: 1, не блокирует, снимается записью.
+
+Обязательные разделы присутствуют по существу; технические утверждения о
+текущем коде подтверждены чтением, а не приняты на слово; единственный
+реальный пробел — контракт описывает защиту от повторного мигания при
+инвалидации мемо (ровно то, из-за чего заведена задача), но не проверяет её
+ни одним AC, и один из технических путей реализации (микрозадача vs
+`requestIdleCallback`) оставлен как «или-или» без решения и без пометки
+«на усмотрение реализации».
+
+Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 2 → в задаче
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `d204bf5d7737` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `cf36bd3b2a3141c427a40f9e10aac607faf02a1f`
+  ```
+  git log --all --format='%H %T' | grep cf36bd3b2a31
+  ```
+- Тело issue: `8d514ac196bbc453d5b25b352421dc27f9925b8668a89e44c00971f304f2c3e4`
+- Вердикт конвейера: `yellow` · High 0