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

SPEC-REVIEW-520-r1

Якоря

• Issue: #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).

---

Материал раунда

• Ветка: issue/520-first-frame, коммит 27ea23c1ec25 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: f80bcdd58b72ba2e7b38cfaac3e80e978cbe67c9

  git log --all --format='%H %T' | grep f80bcdd58b72
  

• Тело issue: d370780d07b6217cfaa48aa21213828fc3a6a70ec16dbea7fee15dbf00e22871
• Вердикт конвейера: yellow · High 0