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

SPEC-REVIEW-725-r1

Issue: #725 «Производительность: принудительные layout и пересборка отпечатка конфигурации на каждом рендере» Этап: spec (PROCESS.md §2.4) · Заход: r1 · блокирующих циклов 0/4 Трек: ask (критерий §5 «перф», обоснован в теле ТЗ) Материал: тело issue #725, раздел ## ТЗ (снимок на момент ревью, issue открыт, лейблы S4-spec-review, track:ask) Проверка кода велась против рабочей копии на 40607aa37f13819c9db37a392a1d99f30382b3c0 (origin/dev на момент ревью) — не как материал ревью (ТЗ ещё не код), а чтобы проверить, что факты и номера строк ТЗ отражают текущий код, а не устаревший снимок.

Скоуп

К1–К3 — три независимых устранения принудительных layout/style reflow и избыточной пересборки _cfgFingerprint на горячем пути переключения этажа (сводная панель, геттер _model, _isoScene); К4 — расширение AST-гейта render-layout-read.mjs, защищающее К1 и К3 от возврата. Обслуживает J1 docs/SCOPE.md («домочадцы на планшете/телефоне переключают этажи» — частое действие). Заявленное отсутствие видимого эффекта (User-Visible: no, «экран не меняется ни на пиксель») проверяется тем же AC4, что доказывает неизменность поведения.

Как проверялось

Ревью ТЗ на этапе spec не гоняет тестовые гейты (кода ещё нет) — вместо этого каждое фактическое утверждение ТЗ (номер строки, имя метода, поведение существующей функции) сверено с текущим деревом, поскольку «догадка, записанная как факт» — находка (§7.1), а неточный номер строки в конкретно этом ТЗ (ниже) оказался именно таким случаем.

Прочитано и сверено построчно:

• src/summary-panel-runtime-loaded.ts — measureLayout() (:869), safeInsets() (:888), updated() → measureLayout() (:151→:869), resized() (:173), импорт и использование resolveSummaryLayout (:12, :849).
• src/houseplan-card.ts — _summarySlot.connect() (:2537), _cfgFingerprint() (:3796), геттер _model (:3822), _cfgEpoch++ в willUpdate (:4033), requestAnimationFrame(() => this._summary?.resized()) (:4069), _isoScene целиком (:5994–6020, включая спорную строку :6010–6011), _effectiveProjection() (:6027, :6044), stage.clientWidth на фокусе комнаты (:6192, :6206), _stageAspect()-подобный clientWidth/clientHeight (:6277), _renderBody (:10683), renderControls/menuItems (:10841, :10844), renderPanel()/кнопки киоска (:11191, :11192), this._summary?.updated(); … this._headerMenu.revealActiveTab(); (:4055).
• src/header-menu.ts:130 — revealActiveTab(), nav.clientWidth.
• src/iso-scene-render.ts — resolveIsoOverlayFitEnvelope и IsoOverlayFitEnvelopeInput.stageSize (используется только как поле, не читается телом функции — подтверждает «stageSize функция не читает»), IsoOverlaySceneInput.structure (JSDoc #724).
• scripts/render-layout-read.mjs — подтверждён текущий охват (render, _renderBody, willUpdate в houseplan-card.ts, весь src/iso-*.ts; только вызовы getComputedStyle/getBoundingClientRect, без чтения свойств) — совпадает с описанием в разделе «Проблема», п.4.
• scripts/mutation-registry.mjs:13568 — существующий мутант iso-first-frame-reads-paper-during-render с guard: 'node scripts/render-layout-read.mjs', на который ссылается AC5 как на образец.
• demo/smoke_render_perf.mjs — файл уже существует, уже считает _buildModel-вызовы (modelBuildsPer10Renders, clockTickModelBuilds, invalidatesOnEdit) — подтверждает план автотестов «рядом с существующими счётчиками _buildModel».
• test/summary-panel-runtime.test.mjs — hostFixture() уже даёт renderRoot.querySelector, ownerDocument (defaultView: undefined), _stageEl: null; подтверждает «остаётся подставить defaultView и элементы со счётчиками».
• test/core-file-budget.test.mjs — CORE_BAND = 50, потолок 'src/houseplan-card.ts': 12896; текущий факт wc -l = 12889 (в ТЗ указано 12891 — расхождение на 2 строки, не влияет на вывод «полоса не упирается»).
• demo/benchmark_large_house.mjs (цикл switchCycle, ~:1124) и demo/fixtures/large-house.mjs:8 (FLOOR_COUNT = 3) — подтверждают методологическое наблюдение «Не-скоупа»: третий шаг 12-переключенческого цикла — первый заход на этаж 3 ((index % 3) + 1, после стартового этажа 1 и spaceSwitch на этаж 2, :477).
• gh issue view 694/713/654/720 — статусы связанных задач: #694 S6-in-progress (не смержена — прямое подтверждение риска «Параллель с #694» и условия AC6 «после его S8»), #713 и #654 CLOSED (их контракты, на которые ссылается ТЗ, уже действуют), #720 S8-merged.
• node scripts/mutation-gate.mjs --check — прогнан для сверки, что реестр сейчас непротиворечив (не гейт этого ревью, факультативная проверка инструмента, на который ссылается AC5/AC7).

Находки

Low — устаревший номер строки в «Проблема» п.3 / «Не-скоуп»

src/iso-scene-render.ts:691 не указывает на resolveIsoOverlayFitEnvelope (она сейчас на :662–673): после коммита 5f8e8ca7 (#724, «drop the 2.5D overlay data nothing reads»), который лёг после заявленной точки сверки origin/dev 108427dc и который правил именно src/iso-scene-render.ts и src/houseplan-card.ts (см. git log --oneline bbcc88ca..HEAD -- src/), в файл добавлен блок JSDoc к IsoOverlaySceneInput.structure, и все номера строк ниже него сместились. Строка :691 сейчас — cellCm: number; внутри IsoOverlaySceneInput, к делу не относящаяся.

Шапка ТЗ («Факты сверены с origin/dev 108427dc. src/** не менялся с bbcc88ca») была верна в момент написания, но dev с тех пор ушёл вперёд именно в тех двух файлах, которые правит эта задача. На практике пострадала только эта одна ссылка — все остальные проверенные номера строк в houseplan-card.ts (включая самые «хрупкие», :4055 и :6010–6011) совпали след-в-след, и все поведенческие утверждения (bounds не зависит от aspect/ stageSize, stageSize не читается телом функции) подтвердились чтением текущего кода независимо от номера строки.

Почему Low, а не Medium: утверждение о поведении верно и проверяемо по имени функции (resolveIsoOverlayFitEnvelope), АС3/АС5 ссылаются на функцию по имени и тесту, а не на номер строки — реализатор не будет введён в заблуждение при работе с кодом. Снимаю находку сам, без возврата автору; формулировку строки при реализации поправит автор попутно (контекст уже записан здесь для CODE-REVIEW).

Других расхождений факт/код не найдено: полная выборка из ~20 процитированных номеров строк по трём файлам (houseplan-card.ts, summary-panel-runtime- loaded.ts, header-menu.ts, iso-scene-render.ts) совпала, включая несамоочевидные ссылки вроде :4055 и :6206.

Что проверено и корректно

• Обязательные разделы §7.1 — все на месте: сценарий, что увидит человек до/после, проблема, скоуп/не-скоуп, контракт поведения (К1–К4), UX·данные· i18n·touch, граничные случаи (§2.6, все шесть классов риска явно пройдены — async/данные/геометрия/визуал/объём/host-input), AC1–AC7 с доказательством и oracle, план автотестов (включая «чем краснеет» по каждому AC), риски, откат, release-артефакты.
• Каждый AC однозначен и имеет названный способ доказательства (unit / smoke / gate / Full Performance), с конкретными числовыми ожиданиями (0 вызовов, ровно 1 измерение, не больше 3 getBoundingClientRect и т. д.) — не «примерно», а проверяемое число.
• AC1, AC2 реалистичны относительно существующей инфраструктуры: фикстура hostFixture() и smoke smoke_render_perf.mjs уже содержат нужные точки расширения (подтверждено чтением).
• AC3/AC5 корректно используют уже существующий инвариант (bounds в resolveIsoOverlayFitEnvelope не зависит от aspect/stageSize уже сегодня, только view зависит) — «Принято предположительно» п.4 сформулирован верно и доказуемо.
• AC5 ссылается на реальный, а не придуманный образец мутанта (iso-first-frame-reads-paper-during-render, тот же guard).
• AC6 корректно признаёт зависимость от #694 (S6-in-progress на момент ревью, не S8) как риск, а не замалчивает её; условие «прогон на ветке, приведённой к dev после #694» технически исполнимо и явно сформулировано.
• Защитные AC (AC1 «0 чтений», AC5 «гейт ловит возврат») имеют заполненную строку «чем краснеет» — счётчики > 0 на текущем коде, мутанты в реестре. Не пустой третий столбец ни у одного защитного критерия.
• Не-скоуп обоснован фактурой, а не декларацией: наблюдение про холодный заход на этаж 3 внутри switchCycle подтверждено чтением фикстуры (FLOOR_COUNT=3, цикл (index % 3) + 1) — не домысел.
• DoR (§2.5) закрыт полностью: файлы и модули перечислены; i18n — «нет» (обоснованно, никакого нового текста); миграция/compatibility — «нет» (никаких изменений конфига); touch — адресован через docs/TOUCH-SUPPORT.md, киоск как блокирующая поверхность покрыт AC4; перф-бюджеты названы явно («не меняются», бандл — полоса 2000 Б, #699); release-артефакты по каждому пункту явно «да» или «нет»; откат описан и тривиален (revert, ни данных, ни миграций, ни флагов); открытых продуктовых вопросов нет — и правомерно: пять пунктов «Принято предположительно» все технические (не наблюдаемы пользователем), продуктовых вопросов, ошибочно не заданных владельцу, не нашёл.
• Трек ask обоснован корректно: критерий «перф» из §5 применим, файлы класса A есть, задача не инфраструктурная.

Чего не проверял

• Не гонял typecheck/npm test/npm run build — на этапе spec нет кода для проверки, кода ещё не существует (это обязанность реализатора и следующего код-ревью).
• Не запускал Full Performance (AC6) и не оценивал правдоподобность цифр 20–45/85/42 мс — они помечены в ТЗ как «оценка, а не обещание», судит AC6 на реальном прогоне после реализации.
• Не проверял обоснованность значений допуска AC1 («не больше 3 getBoundingClientRect») числовым моделированием — принял как разумную инженерную оценку с открытой проверкой в тесте.
• Не рассматривал техническую реализуемость «экспортируемого модуля» К2 за пределами прочтения существующих аналогов в test-build/ (plan-geometry- preflight.js и т. п.) — подтверждает паттерн, не гарантирует конкретный API.
• Не проверял состояние #694 глубже статуса/меток — не моя обязанность на этом этапе; факт зафиксирован как риск, а не как блокер DoR (АС6 явно откладывает свою проверку до после S8 #694).

Вердикт

Зелёный. ТЗ полное, каждый AC однозначен и проверяем, защитные AC доказаны таблицей «чем краснеет», открытых продуктовых вопросов нет, единственная находка (Low, устаревшая ссылка на строку из-за коммита #724 уже после точки сверки ТЗ) снята ревьюером без возврата автору.

---

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

• Ветка: dev, коммит 40607aa37f13 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 901e6cbd1964ef2dcd9a0e08e8ec01a5c47dcac7

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

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