mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-06 22:49:16 +00:00
@@ -1,11 +1,12 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 213, issue: 105. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 214, issue: 106. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| бета v1.79.0-beta.1 | [SHIP-REVIEW-v1.79.0-beta.1.md](SHIP-REVIEW-v1.79.0-beta.1.md) | пакетное ревью ship · — | ⚪ — | 0 | 0 | — | — |
|
||||
| #732 | [CODE-REVIEW-732-r1.md](CODE-REVIEW-732-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #725 | [SPEC-REVIEW-725-r1.md](SPEC-REVIEW-725-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | устаревший номер строки в «Проблема» п.3 / «Не-скоуп» | `src/iso-scene-render.ts` `src/houseplan-card.ts` `houseplan-card.ts` `header-menu.ts` `iso-scene-render.ts` |
|
||||
| #724 | [CODE-REVIEW-724-r1.md](CODE-REVIEW-724-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #723 | [CODE-REVIEW-723-r1.md](CODE-REVIEW-723-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #718 | [SPEC-REVIEW-718-r1.md](SPEC-REVIEW-718-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
|
||||
@@ -0,0 +1,198 @@
|
||||
# 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 уже после точки
|
||||
сверки ТЗ) снята ревьюером без возврата автору.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `40607aa37f13` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `901e6cbd1964ef2dcd9a0e08e8ec01a5c47dcac7`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 901e6cbd1964
|
||||
```
|
||||
- Тело issue: `2465e13356e16b5d9f3830fba550dc795e73ad81fbe8b023596ce38a943f7203`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user