diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 851ccb36..653f3313 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -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 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-725-r1.md b/docs/reviews/SPEC-REVIEW-725-r1.md new file mode 100644 index 00000000..5fec4774 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-725-r1.md @@ -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 уже после точки +сверки ТЗ) снята ревьюером без возврата автору. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `40607aa37f13` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `901e6cbd1964ef2dcd9a0e08e8ec01a5c47dcac7` + ``` + git log --all --format='%H %T' | grep 901e6cbd1964 + ``` +- Тело issue: `2465e13356e16b5d9f3830fba550dc795e73ad81fbe8b023596ce38a943f7203` +- Вердикт конвейера: `green` · High 0