docs: review document for #520

Issue: #520
User-Visible: no

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

@@ -0,0 +1,280 @@
+# SPEC-REVIEW-520-r1
+
+## Якоря
+
+- Issue: [#520](https://github.com/Matysh/houseplan-card/issues/520)
+  «Первый устойчивый кадр большого дома вырос до 3.36 с при потолке 3.0 с —
+  регрессия после #509/#500», метка `S4-spec-review` (подтверждено `gh issue
+  view 520 --json labels`, 2026-09-10).
+- ТЗ: тело issue #520, раздел `## ТЗ (полный трек — критерий §5 ...)`,
+  строки 30–67 полученного тела (см. ниже).
+- sha256 нормализованного тела issue (как получено `gh issue view 520
+  --repo Matysh/houseplan-card --json body -q .body`, без дополнительной
+  нормализации переводов строк):
+  `5477d052e0e87182f51a9b3b05d37fce0a7ada3d9dc66c10f927f64564ccc6e4`
+- Материал ветки: `issue/520-first-slug` → фактически `issue/520-first-frame`
+  на `27ea23c1ec25e6421542c9901eb6e8e2484d0665` — по собственному признанию
+  автора в комментарии `2026-09-10T15:09:06Z`: «пока только диагностика
+  бенчмарка, продуктовой правки в ней ещё нет; она появится после
+  `S5-ready`». Подтверждено чтением `git log`/`git show --stat` на трёх
+  последних коммитах — все три с префиксом `test(perf):`, правок в
+  `src/**` нет.
+- Заход r1. Бюджет циклов §4 до этого ревью: 0/4. Вердикт ниже —
+  жёлтый, поэтому расходует цикл: после этого ревью бюджет **1/4**.
+
+**Примечание по маршрутизации.** Шапка задания содержит блок инструкций
+для этапа `code` (диапазон SHA, требование не делать `git fetch`,
+`Зелёного Validate на этом SHA нет` и т.д.) и фразу «Для этапа spec: ТЗ
+живёт в теле issue (#517)», где `#517` — не опечатка про номер текущей
+задачи, а ссылка на решение владельца от 2026-09-10 (issue #517),
+которым сам архив `docs/specs/` закрыт для новых ТЗ (см. PROCESS.md
+§2.3, строка 125). Реальный этап подтверждён меткой `S4-spec-review` на
+#520 — далее в разборе применяются правила §2.4/§7.1, а раздел про
+`git diff origin/dev...HEAD`/SHA к этому раунду не относится: код
+продукта ещё не менялся.
+
+## Скоуп ревью
+
+Ревью ТЗ (PROCESS.md §2.4) для полного трека — задача сама называет
+нарушенный критерий лёгкого трека («нет влияния на производительность»),
+выбор трека верен. Предмет: раздел `## ТЗ` тела issue #520 — контракт,
+AC1…AC5, откат, release-артефакты — против §7.1 (обязательные разделы),
+§2.5 (DoR) и `docs/SCOPE.md` (продуктовая рамка).
+
+## Как проверялось
+
+1. Прочитано тело issue #520 целиком и оба комментария (`S2-анализ`,
+   «Причина найдена…», переход в `S3-spec`) — понадобилось для контекста
+   диагностики, на которой стоит контракт.
+2. Сверено с PROCESS.md §1 (класс A), §2.3–2.5, §4, §5, §7.1.
+3. Сверено с `docs/SCOPE.md`: персона/поверхность ТЗ (администратор,
+   первый показ большого дома) сопоставлены с J1 и таблицей Target
+   audience («large flat, 20–200 devices»; фикстура 60 комнат/200
+   устройств — то же самое).
+4. **Технические утверждения о поведении Lit перепроверены чтением
+   исходника**, а не приняты на веру — это и есть весь фундамент
+   контракта:
+   - `node_modules/@lit/reactive-element/development/reactive-element.js:249-252`
+     — `wrapped = true` действительно ставится в `createProperty`, когда
+     `this.prototype.hasOwnProperty(name)`, **до** проверки `noAccessor`
+     (строка чуть ниже `if (!options.noAccessor)`) — подтверждает пункт 2
+     контракта дословно;
+   - `:880-886` — принудительная запись `wrapped`-свойства в
+     `changedProperties` со значением `undefined` действительно происходит
+     внутри блока первого обновления (`hasUpdated` ещё не выставлен;
+     комментарий в самом Lit: «only for the case of properties created via
+     `createProperty` on accessors») — подтверждает механизм лишней эпохи;
+   - `:339-341` (`getPropertyOptions`) и `:697-712` (`requestUpdate`) —
+     необъявленное имя свойства действительно получает
+     `defaultPropertyDeclaration`, сравнивается через `notEqual` и уходит в
+     `_$changeProperty` наравне с объявленным — подтверждает пункт 1
+     контракта («`requestUpdate` работает с необъявленным именем»).
+5. Цитаты по `src/houseplan-card.ts` сверены построчно с текущим файлом:
+   аксессоры `_layout`/`_serverCfg` (923–940), `static properties` со
+   `_layout`/`_serverCfg: { state: true }` (2556, 2560), `willUpdate` и
+   `_cfgEpoch++`/`preserveGeometry` (4104–4114), ключ памятки модели
+   `` `${_cfgEpoch}|${_cfgFingerprint()}` `` (3919), ключ
+   `cache.entries.cleanFloor` `` `${space.id}|${_cfgEpoch}|${roomKey}` ``
+   (9916) — все точны, расхождений с текстом ТЗ нет.
+6. Оба файла бюджетов из AC5 (`demo/performance/budgets-interaction-smoke.json`,
+   `demo/performance/budgets-large-house-interaction.json`) прочитаны:
+   `cleanFloor: 180` в обоих подтверждён, других budgets-файлов с
+   `cleanFloor` в дереве нет — список AC5 полон и точен. Дополнительно
+   сверен коммит `914e8402`, поднявший потолок: его `git show --stat`
+   трогает ровно эти два файла.
+7. Пересчитаны относительные лимиты `modelReadyMs` (формула
+   `demo/performance/evaluate.mjs:73-118`, `min(hardMaxMs, max(base*(1+ratio),
+   base+noiseAllowanceMs))`) для профилей `large-house-interaction` **и**
+   `large-house-isometric`, чтобы понять, действительно ли изометрия тоже
+   нарушает свой бюджет (см. находку Medium-3) — числа изометрии из
+   «Симптома» не были проверены автором на предмет реального превышения,
+   я это сделал сам.
+8. Проверено содержимое `test/config-adoption-ownership.test.mjs` (техника
+   — регэксп-скан исходников на предмет прямой записи identity/полей вне
+   владельца) и `test/config-adoption.test.mjs` (host-стаб, юнит без
+   реального Lit-компонента) — чтобы оценить, насколько реалистичны
+   заявленные способы доказательства AC1/AC2.
+9. Проверено, куда пишет диагностика `bootDiag`
+   (`demo/benchmark_large_house.mjs:401,1142,1213-1218`) — только
+   `console.log` строки прогона, не бюджетируемое поле — это и есть
+   основание находки Medium-2.
+10. `git status` — рабочая копия чистая относительно продукта (кроме
+    отсутствующего `node_modules`, не связано с этим ревью).
+
+## Находки
+
+### Medium-1 — обязательный раздел «Риски» отсутствует (§7.1)
+
+**Файл:** тело issue #520, раздел `## ТЗ` (строки 30–67 полученного тела).
+
+`PROCESS.md:535-538` перечисляет обязательные разделы ТЗ, включая
+**риски**; DoR (`PROCESS.md:164`) отдельно требует «риски перечислены»
+как условие перехода в «Готово к разработке». В тексте ТЗ #520 такого
+раздела нет вовсе — ни явного заголовка, ни даже строки вида «риски:
+минимальны, потому что …». Для сравнения — прямой прецедент того же
+класса задачи, `docs/specs/506-startup-performance.md:149-161`
+(«Техническая гипотеза, риски и откат»), перечисляет риски явным
+списком («двойное подключение при overlapping promises…», «исправление
+только glow без устранения isometric regression» и т.д.) и привязывает
+каждый к AC, который его закрывает.
+
+**Сценарий, который упущен без раздела риска.** Контракт снимает
+`_serverCfg`/`_layout` с объявленных реактивных свойств Lit. Реальный
+риск есть и не назван: любой код, читающий
+`houseplan-card.elementProperties` — Lit devtools, будущий рефлектор
+атрибутов, сторонний код, полагающийся на `hasOwnProperty` в
+`options`/`reflect` — перестанет видеть эти два поля в перечне
+реактивных свойств. Ничего в контракте не говорит, проверено ли
+отсутствие такого потребителя (или почему он невозможен) — риск
+следовало либо назвать и снять, либо принять с обоснованием.
+
+**Чем чинится:** одним абзацем в теле issue — не блокирует исполнимость
+контракта, но обязателен для DoR.
+
+### Medium-2 — AC3 не имеет дешёвого воспроизводимого способа доказательства
+
+**Файл:** тело issue #520, строка 56 (`AC3` в таблице AC).
+
+DoR (`PROCESS.md:154-155`) требует, чтобы у каждого AC был назван способ
+доказательства из набора `unit`/`backend`/`smoke`/`golden`/«ревью кода».
+AC1 (source-scan тест по образцу уже существующего
+`test/config-adoption-ownership.test.mjs`), AC2 (юнит), AC4/AC5 (уже
+существующие budgeted-гейты Validate) — все аккуратно попадают в этот
+набор. **AC3 — нет.** Его «чем доказан» — «диагностика `bootDiag` в
+полном сравнении против `a44fbd37`, числа в комментарии issue»; «чем
+краснеет» — «лишняя эпоха вернётся — числа разойдутся с базой».
+
+Проверено чтением `demo/benchmark_large_house.mjs:1213-1218`: `bootDiag`
+только печатается в лог прогона (`console.log`) и явно исключён из
+бюджетируемой записи — сам текст ТЗ подтверждает это разделом «Принято
+предположительно» («печатается в лог прогона, в бюджетируемую запись не
+попадает»). Значит AC3 нельзя перепроверить иначе, чем вручную читая лог
+дорогого прогона `Full Performance` (семь образцов, сравнение с базой) —
+ровно того класса гейтов, который по прямому указанию в задании этого
+ревью «предрелизный, а не гейт ревью». Автор и на этом ревью, и на
+будущем код-ревью вынужден либо доверять уже вставленным в issue числам
+без способа их независимо переснять дёшево, либо вручную гонять полное
+сравнение при каждом цикле.
+
+По существу AC3 не даёт ничего, чего не даёт уже AC4 (тот же корень
+регрессии закрывает и хронометраж, и счётчик эпох) — это не лишний
+критерий, а лишний **недоказуемый** критерий. Не блокирует контракт:
+вариантов чинения минимум два и оба дёшевы — либо явно понизить AC3 до
+«проверено чтением кода при код-ревью (AC1+AC2 логически влекут AC3) +
+однократная сверка `bootDiag` на конкретном SHA перед публикацией беты,
+не гейт CI», либо убрать AC3 из таблицы как поглощённый AC1/AC2/AC4 и
+оставить числа диагностики просто иллюстрацией в тексте причины.
+
+### Medium-3 — регрессия профиля изометрии показана в «Симптоме», но не отражена в контракте/AC
+
+**Файл:** тело issue #520, строка 3 («Профиль изометрии — то же:
+`modelReadyMs` 2165.6 → 2626.3») против раздела «Контракт»/«AC и
+доказательства» (строки 38–58), где изометрия не упомянута вовсе.
+
+Автор сам приводит эти числа как «то же» (то есть тот же механизм
+регрессии) для профиля `large-house-isometric`, но ни один AC, ни
+скоуп/не-скоуп не называют этот профиль. Проверил сам, чтобы понять,
+пустая это формальность или реальный пропуск: у профиля
+`large-house-isometric` (`demo/performance/budgets-large-house-isometric.json:6`)
+`maxRegressionRatio` для `modelReadyMs` — 0.2, `noiseAllowanceMs` — 200.
+По формуле гейта (`demo/performance/evaluate.mjs:73-118`,
+`relativeLimit = max(base*(1+ratio), base+allowance)`) эффективный потолок
+— `max(2165.6*1.2, 2165.6+200) = 2598.72`. Кандидат **2626.3 > 2598.72** —
+изометрия **тоже** нарушает свой относительный бюджет (на ~1 %, у
+`large-house-interaction` — на ~7 %), просто это не абсолютный гейт
+Validate и потому не горит красным в CI сейчас, точно как и обсуждаемый
+в AC4 `modelReadyMs` профиля interaction.
+
+Раз ТЗ уже включает в AC4 покрытие «мягкого» (не абсолютного, а
+относительного) регресса `modelReadyMs` для одного профиля, тот же
+стандарт по симметрии должен применяться и ко второму профилю, который
+сам автор привёл как доказательство единого механизма. Сейчас неясно:
+это фиксится тем же изменением автоматически и просто не нуждается в
+отдельном AC (правдоподобно, раз причина общая для любого первого
+обновления карточки), или это осознанно оставлено вне скоупа задачи —
+но текст ТЗ не говорит ни того, ни другого явно. Это техническая
+неопределённость (не продуктовая — какая часть исправления какой профиль
+чинит, решают агенты между собой, не владелец), поэтому вопрос не выносится
+владельцу, а возвращается автору как находка: либо расширить AC4/AC5
+явным упоминанием `large-house-isometric` (и его budgets-файлов, если они
+регрессировали похожим образом по `cleanFloor`), либо одной фразой
+зафиксировать «то же изменение чинит и профиль изометрии, отдельного
+гейта не заводим, потому что…».
+
+## Что проверено и корректно
+
+- **Причина — измерена, не догадка.** Все три опорных технических
+  утверждения о поведении Lit (`wrapped`, форсированная запись в
+  `changedProperties` на первом апдейте, работа `requestUpdate` с
+  необъявленным именем) подтверждены чтением фактического исходника
+  `@lit/reactive-element`, а не только словами автора. Все цитаты по
+  `houseplan-card.ts` (номера строк) сверены с текущим файлом и точны.
+- **AC1** доказуем дёшево и детерминированно: тот же приём (regex-скан
+  исходников), что уже применяется в этом же тестовом файле для
+  соседнего инварианта — реалистичный, недорогой способ доказательства с
+  чётким «чем краснеет» (мутант возвращает `_serverCfg: { state: true }`).
+- **AC2** корректно опирается на уже проверенное (мной, чтением исходника
+  Lit) поведение — сам механизм гарантирован фреймворком, тесту остаётся
+  подтвердить, что `onBodyReplaced` вызывается на замену и не вызывается
+  на идентичном значении; это не новая инфраструктура.
+- **AC4/AC5** привязаны к точным существующим файлам бюджетов; значения
+  (потолок 180 → 100, `hardMaxMs`/`maxRegressionRatio` для
+  `modelReadyMs`) подтверждены построчно и соответствуют тому, что
+  реально сейчас нарушено (и по абсолютному, и по относительному гейту).
+- **Откат** — простой и однозначный (revert двух строк `static
+  properties` + потолка 180), данные и контракты не затронуты.
+- **Release-артефакты** корректно называют `User-Visible: no` и
+  отсутствие правки changelog, с указанием, какой бета-кандидат
+  разблокирован.
+- **Продуктовая рамка** называет персону и поверхность (`docs/SCOPE.md`
+  J1, «Home admin», «large flat… 20–200 devices») и совпадает с фикстурой
+  (60 комнат/200 устройств); «что человек увидит» сформулировано одной
+  фразой без раздутия.
+- **Выбор полного трека корректен**: perf-влияние безоговорочно нарушает
+  критерий лёгкого трека §5, и автор сам это называет.
+- **Ни одного вопроса владельцу не задано** — и не нужно: весь материал
+  технический, продуктовых развилок в задаче нет.
+- **«Принято предположительно»** оформлено ровно по формату §7.1
+  (сохранение диагностики `bootDiag` в дереве помечено как решение,
+  которое ревьюер вправе оспорить) — не поглощено находками выше.
+
+## Чего не проверял
+
+- **Гейты кода** (`tsc --noEmit`, `npm test`, `npm run build`,
+  `check-docs.mjs`) — не гонял: на этом SHA нет ни строки продуктового
+  кода (`src/**`), только диагностика бенчмарка; предмет этапа spec —
+  текст ТЗ, а не код. Это будет предметом код-ревью после `S5-ready`.
+- **`npm run invariants`** — не гонял по той же причине: геометрия и
+  ссылки на неё не тронуты.
+- **Реальный прогон `Full Performance`/`Validate`** на этом SHA — не
+  запускал: продуктовой правки, которую можно было бы измерить, ещё нет.
+- **Регистрацию мутанта `adoption-bodies-declared-reactive`** в
+  `scripts/mutation-gate.mjs` — не проверял; на этапе ТЗ его ещё не
+  существует по определению (мутант тестирует ещё не написанный код),
+  это предмет код-ревью.
+- **Уровень шума** в уже приведённых числах `Full Performance` (7
+  образцов) — не переоценивал статистически; принял как измеренные, со
+  ссылками на конкретные прогоны CI, приведённые автором.
+
+## Унаследовано / закрытие предыдущего раунда
+
+Не применимо — первый раунд (r1) ревью ТЗ #520.
+
+## Вердикт
+
+Жёлтый. High: 0, Medium: 3 (все в скоупе задачи — чинятся автором в том
+же тексте ТЗ, без нового issue). Возврат в «ТЗ в работе» (`S3-spec`).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/520-first-frame`, коммит `27ea23c1ec25` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `f80bcdd58b72ba2e7b38cfaac3e80e978cbe67c9`
+  ```
+  git log --all --format='%H %T' | grep f80bcdd58b72
+  ```
+- Тело issue: `d370780d07b6217cfaa48aa21213828fc3a6a70ec16dbea7fee15dbf00e22871`
+- Вердикт конвейера: `yellow` · High 0