From bb9aaefe2ac72488ddfcb87aa429d5a2a730bd98 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 17:39:07 +0000 Subject: [PATCH] docs: review document for #744 Issue: #744 User-Visible: no --- docs/reviews/CODE-REVIEW-744-r1.md | 279 +++++++++++++++++++++++++++++ 1 file changed, 279 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-744-r1.md diff --git a/docs/reviews/CODE-REVIEW-744-r1.md b/docs/reviews/CODE-REVIEW-744-r1.md new file mode 100644 index 00000000..010e5c20 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-744-r1.md @@ -0,0 +1,279 @@ +# CODE-REVIEW-744-r1 + +Issue: #744 · Трек: ask · Заход: r1 · блокирующих циклов 0/4 +Материал: `8c639b15989d2c79cf696577c67f5f989f5ff704` (`origin/dev` → `2cced12b` → `8c639b15`) + +## Скоуп + +Четыре кэша структурной геометрии этажа (физические тела, оболочка стен, +внутренние контуры, чистый пол) были ключены глобальной `_cfgEpoch`: правка +любого этажа делала холодными все остальные. Задача меняет ключ на отпечаток +содержимого записи самого этажа (`src/floor-geometry-key.ts`), убирает +полный сброс `_cleanFloorCache` из `StairsEditor.write`, переносит три места +в `houseplan-editor-runtime.ts` (resize preview/accept/cancel) на тот же +ключ, и расширяет сторож #735 (`demo/benchmark_large_house.mjs`, +`demo/performance/card-contract.mjs`) видимостью пула оболочек и кэша +внутренних контуров. Два коммита: продуктовый (`2cced12b`, `User-Visible: +yes`, CHANGELOG RU/EN) и тестовый фикс гонки в `smoke_floor_geometry_cache` +(`8c639b15`, `User-Visible: no`, меняет только сам смок). + +Работа закрывает техдолг горячего пути переключения этажей (J1/J6 через +перф-критерий §5 track:ask): не новая видимая функциональность, а устранение +ложного «подвисания» при переходе на непривеченный этаж. + +## Как проверялось + +Чтение кода и истории без исполнения тяжёлых гейтов (обоснование — ниже), +плюс самостоятельная трассировка по `git`/`gh`, так как AC4 опирается на +перф-бюджеты, которые параллельно менялись в `dev`: + +1. Построчно сверил `git diff origin/dev...HEAD` против ТЗ (контракт К1–К5, + «Затронутые файлы», «Принято предположительно»). +2. Прочитал `src/floor-geometry-key.ts` целиком и проследил каждую точку + чтения `_cfgEpoch` / `_renderCfg` / `_curSpaceCfg`, включая порядок + инициализации полей класса (`_floorKey` — field initializer, конструктор + ещё не завершён, но геттеры читаются лениво при первом вызове — безопасно). +3. Нашёл все вызовы `_wallUnionGeometry`, `_physicalBodiesR`, + `_innerRoomContour`, `_cleanFloor` и проверил, что ни один не передаёт + «чужой» этаж без того единственного оправданного случая (PDF/сводная + панель — не задействуют эти кэши, как заявлено в ТЗ «Не текущий этаж»). +4. Прочитал три точки `houseplan-editor-runtime.ts` (`_rszAcceptPreview`, + `_rszEdgeDown`, `_rszCancelDrag`) и порядок `session.accepted = accepted` + → `input.publish(...)` в `resize-controller.ts`, чтобы убедиться, что + `_floorKey` на этих путях читает корректный (уже опубликованный/уже + восстановленный) кадр, а не кадр на шаг позади. +5. Прочитал `demo/smoke_floor_geometry_cache.mjs` целиком (383 строки): + независимый oracle-карточка, AC1–AC2c, фикс гонки второго коммита (ожидание + исчезновения тоста, два кадра: live и settled). +6. Сверил мутанты в `scripts/mutation-registry.mjs` посимвольно с текущим + `floor-geometry-key.ts` (строки `find` совпадают с деревом) и категорию в + `docs/testing-notes/mutation-browser-guards.md` — по прецеденту + (`barrier-cache-never-invalidated`, `junction-limit-baseline-cache-stale`) + категория «lifecycle» для кэш-инвалидации на полном HA-card сценарии + корректна. +7. Для AC4 (перф) — отдельное расследование, см. находку ниже: сверил через + `gh run view` / `gh run list` и `git log`/`git merge-base`, когда именно + бюджеты `switchCycleMs` были ужесточены (#747) относительно базы, на + которой стоял единственный прогон Full Performance задачи, и какая версия + дерева фактически прогнала более дешёвый перф-смок Validate после этого. +8. Проверил трейлеры обоих коммитов, CHANGELOG RU/EN (текст совпадает с ТЗ + дословно), бюджет строк монолита (`wc -l` 12889 < потолка 12896 в + `test/core-file-budget.test.mjs`), обновление `test/clean-floor.test.mjs` + под новую сигнатуру `floorKey(spaceId)`. + +## Объём гейтов — что прогнано и почему + +- **Дешёвые (typecheck, `npm test`, `npm run build`+bundle-policy).** + Не перегонял: зелёный Validate на точном SHA `8c639b15` (run + [36898830297](https://github.com/Matysh/houseplan-card/actions/runs/36898830297)) + подтверждает их по правилам промпта. +- **Browser-смоки (все шарды), golden.** Validate на `8c639b15` свернул эти + джобы в `skipped` через «Переиспользование: это дерево уже проверено» — + фактически они выполнились на предыдущем пуше `6ff0255d` + (run [36894249224](https://github.com/Matysh/houseplan-card/actions/runs/36894249224), + все шарды и golden — success). Я проверил, что это легитимно: `git diff + --stat dd86bf95 0606a366 -- src/ demo/ custom_components/` пуст — дерево, + на котором стоял `6ff0255d`, и финальная база `dev` 0606a366 не расходятся + ни в одном файле, который могли бы задеть смоки или golden. Эквивалентность + дерева подтверждена, не принята на слово. +- **Мутанты (`mutation-gate --check`, реестр).** Тот же вывод: «Фронтенд: + типы, юниты, мутанты, синхрон бандла» зелёный на `8c639b15` напрямую (не + через reuse), покрывает проверку реестра. Сами два новых мутанта я сверил + построчно с кодом (находки нет). +- **`npm run invariants`.** Не прогонял. Задача не меняет формулы геометрии + (союз стен, контуры, зачистка пола) — только их кэш-ключ; значения, + которые кэши отдают, не меняются логикой этой задачи (кроме + предусмотренных AC2 случаев, доказанных смоком против независимого + oracle). Инварианты модели реагируют на геометрию, а не на то, как она + кэшируется. +- **`pytest tests_backend`.** Не прогонял — задача не трогает + `custom_components/`. +- **`ci:golden` как отдельная метка/прогон на точном SHA.** См. выше — golden + прошёл на доказанно эквивалентном дереве, не на `8c639b15` напрямую. +- **Full Performance (AC4).** Не перегонял сам (дорогой гейт, не входит в + обязательный объём ревью при наличии отчёта автора), но результат автора + оказался недостаточным по изложенной ниже причине — см. находку. +- **`smoke-select` самостоятельно.** Не перегонял; автор привёл результат + (64/65, единственный красный — известная причина окружения + `smoke_infinite_canvas`), и browser-смоки Validate (п. выше) независимо + это покрывают для всех реально изменённых файлов. + +## Находки + +### Medium (в скоупе) — единственное свидетельство AC4 устарело относительно текущих бюджетов `switchCycleMs` + +**Файл:** issue #744, раздел AC4 / комментарий автора 2026-10-01T14:21:02Z. + +AC4 требует зелёный прогон `performance.yml` (Full Performance, 7 сэмплов, +9 профилей) с медианами кандидата/базы. Единственный такой прогон — run +[36869385519](https://github.com/Matysh/houseplan-card/actions/runs/36869385519), +`headSha=d4439aee` (первый, ещё не ребейзнутый коммит задачи, поверх `dev +707cb18f`), `conclusion: success`. + +Проблема: между `dev 707cb18f` (база этого прогона) и `dev 0606a366` (база +финального материала `8c639b15`) в `dev` landed `fa18ca81` («test(perf): pull +the switchCycleMs ceilings down to the warmed level», issue #747, +`707cb18f..0606a366` — подтверждено `git merge-base --is-ancestor +fa18ca81 0606a366`). Этот коммит срезал `switchCycleMs.hardMaxMs` сразу в +шести бюджетных файлах: `budgets.json`, `budgets-interaction-smoke.json`, +`budgets-isometric-smoke.json`, `budgets-isometric-stage3-dense.json`, +`budgets-large-house-interaction.json`, `budgets-large-house-isometric.json` +— с 7000/8000 мс до 950/1550 мс (в 7.4/5.2 раза строже). Причина ужесточения +— дословно из коммита: старый абсолютный потолок «несколько раз» превышал +разумный уровень и не ловил реальные регрессии именно `switchCycleMs`, +именно той метрики, которую #744 в первую очередь меняет (ключ кэшей на +горячем пути переключения этажа). + +Прогон `36869385519`, которым AC4 закрыт в issue, судил кандидата против +бюджетов **до** этого ужесточения (7000/8000 мс) — тривиально проходимых. +После него бюджеты в материале (`8c639b15`) уже содержат срезанные +950/1550 мс, и ни одного Full Performance прогона против них для этой ветки +нет (`gh run list --workflow performance.yml` для +`issue/744-floor-geometry-key` — единственная запись, та же самая). Формально +AC4 в его собственной формулировке («Не зелёный — AC не выполнен») не +доказан на материале. + +**Проверено чтением и трассировкой, не вслепую.** Я не оставляю это чистым +подозрением: отдельная, более дешёвая перф-проверка (`node +demo/benchmark_large_house.mjs` через Validate-джоб «Перф-смок: бюджет +времени кадра», профили `large-house-interaction-v1` и +`large-house-isometric-v1`, те же, что названы в AC4, 3 сэмпла, +абсолютный потолок = тот же `hardMaxMs`) **выполнилась и прошла** на +`6ff0255d`, который стоит на `dev dd86bf95` — а `dd86bf95` уже содержит +`fa18ca81` (подтверждено `git merge-base --is-ancestor fa18ca81 dd86bf95`, +exit 0). Продуктовый код между `6ff0255d` и `8c639b15` не менялся (второй +коммит правит только смок-файл), так что этот зелёный перф-смок — тоже +доказательство на фактически том же коде, под уже срезанным потолком, и оно +не красное. Это сильно снижает вероятность, что полный прогон упадёт — но +не заменяет его: перф-смок не считает относительную деградацию кандидата +против свежей базы `dev` (только абсолютный потолок, 3 сэмпла), а именно +относительный разбор (`+8 % modelReadyMs`, `+6.5 % spaceSwitchMs` — уже +отмеченные автором отклонения в исходном прогоне) — это то, что обязан +закрыть именно Full Performance, и как раз эти проценты посчитаны автором +относительно уже устаревшей базы `707cb18f`, а не текущей `0606a366`, +которая успела вобрать #745, #746, #743, #756 и другие правки горячего пути. + +**Воспроизведение:** `gh run view 36869385519 --json headSha` → +`d4439aee...`; `git merge-base --is-ancestor fa18ca81 707cb18f` → код выхода +1 (ещё не предок); `git merge-base --is-ancestor 707cb18f fa18ca81` → код +выхода 0 (предок, т.е. `fa18ca81` landed уже после базы прогона); текущие +значения в дереве — `grep hardMaxMs demo/performance/budgets-large-house-interaction.json` +→ `950`. + +**Чем закрывается:** перезапустить `gh workflow run performance.yml --ref +issue/744-floor-geometry-key` (или текущий push) против актуального `dev` и +привести в issue медианы кандидата/базы, как и требует AC4 дословно. Это +не правка кода — задача технически уже должна проходить (перф-смок это +показывает), но отчёт по AC4 должен ссылаться на прогон, поставленный +корректно против материала, который реально сливается. + +## Что проверено и корректно + +- **Контракт ключа (К1–К3).** `floorGeometryKeyReader` — отпечаток + содержимого записи этажа (`contentFingerprint`), с кэшем по ссылке на + объект записи внутри эпохи (micro-optimisation, не источник корректности: + источник корректности — стабильность самой строки отпечатка при + неизменных данных, от неё зависит тёплое попадание в `_wallUnionPool` / + `_innerContourCache` / `_cleanFloorCache`, которые ключуются этой строкой + напрямую). Эпоха инкрементируется на каждой мутации (уже существующий + инвариант, не новый), так что reference-equality внутри эпохи безопасна. +- **К2 / «не текущий этаж».** `_innerRoomContour` читает `this._spaceWalls` + (текущий этаж), а не стены переданного `space` — ровно так, как описано в + ТЗ как риск; ключ `floorGeometryKeyReader` корректно хэширует ОБА отпечатка + (модель переданного этажа + `_curSpaceCfg` текущего), когда они различаются, + так что такое значение никогда не разделит ключ с «родной» геометрией + этажа. Сегодня этот путь не вызывается (PDF и сводная панель всегда берут + `this._spaceModel()` — проверено по всем вызовам `_wallUnionGeometry` / + `_physicalBodiesR` / `_cleanFloor` в файле), но конструкция не завязана на + это везение. +- **Resize preview/accept/cancel (`houseplan-editor-runtime.ts`).** + `_rszAcceptPreview` читает `_floorKey` ПОСЛЕ `session.accepted = accepted` + в `resize-controller.ts:219` (до вызова `publish`), то есть видит уже + актуальный превью-кадр, не кадр на шаг позади. `_rszCancelDrag` вызывает + `this.host._resize.cancel(...)` раньше восстановления ключа — к этому + моменту `_resize.preview` уже очищен контроллером, so `_floorKey` читает + обычную (не-превью) запись, корректно совпадающую с до-драговым + состоянием, под которым был сохранён `restoreWallUnion`. +- **Лестницы (К4).** `stairs-editor.ts` больше не чистит + `_cleanFloorCache` — подтверждено диффом; площадь перекрытия лестницей + пересчитается по новому ключу (эпоха растёт, как и раньше). +- **Физические тела — одна запись, не пул.** Сохранён прежний дизайн + (единственный слот `_physicalBodiesCache`, не LRU): AC1 проверяет только + стабильность СТРОКИ ключа после визита на другой этаж и обратно, не факт + переиспользования слота — согласовано со смоком + (`ac1GardenKeepsItsPhysicalBodiesKey`). +- **Сторож #735.** `_wallUnionPool`/`_innerContourCache` добавлены в + `cacheSnapshot` бенчмарка и в опциональные поля `card-contract.mjs` + (старый сравнительный бандл читает `0`, не падает). Категория нового мутанта + в реестре мутаций согласована с прецедентом (`barrier-cache-never-invalidated`, + аналогичная кэш-инвалидация на полном сценарии карточки). +- **Смок `demo/smoke_floor_geometry_cache.mjs`.** Прочитан целиком. AC1 + (переименование комнаты на `f1`, визит `garden` без роста размеров + кэшей), AC2a (серверный пуш меняющий общие стены обоих этажей, сверка с + независимой карточкой-oracle на том же конфиге), AC2b (лестница через + реальный инструмент, `garden` сохраняет свои записи чистого пола), AC2c + (удержанный resize-драг: судятся ОБА возможных кадра — живой превью-слой + и осевший кадр после форсированного host-рендера от смены состояния + света, плюс отмена и возврат к хранимой записи) — каждый сверяется с + независимой свежемонтированной карточкой на том же конфиге, а не просто + «не упал». Второй коммит чинит гонку с тостом (ожидание исчезновения + тоста перед стартом драга) и добавляет два новых чека + (`ac2cSettledFrameEqualsAFreshCard`, `ac2cStateChangeKeepsThePreview`) — + устраняет именно причину красного на CI run 36875756451, не маскирует её. +- **Мутанты.** `floor-geometry-key-global-epoch` и + `floor-geometry-key-ignores-content` в `scripts/mutation-registry.mjs` + сверены посимвольно: их `find`-паттерны совпадают с текущим + `src/floor-geometry-key.ts` (строка с `contentFingerprint(model === current + ? model : [model, current])`), `guard` указывает на новый смок. +- **Трейлеры и changelog.** Оба коммита несут `Issue: #744`. Продуктовый + коммит — `User-Visible: yes` с правкой `docs/CHANGELOG.md` и + `docs/CHANGELOG.ru.md` в том же коммите, текст дословно совпадает с текстом + из ТЗ («After a plan edit, switching to other floors no longer stalls...» / + «После правки плана переход на другие этажи больше не подвисает...»). + Тестовый коммит — `User-Visible: no`, правит только смок — корректно. +- **Один источник числа.** Единственное видимое пользователю число в этом + диффе — нет: задача не меняет ни одного значения, видимого в UI (площади, + пути стен идентичны до/после по К5 и доказаны смоком); изменился только + путь вычисления и кэш-инвалидация. Строка CHANGELOG — описание эффекта + («больше не подвисает»), не число. +- **Бюджет строк монолита.** `wc -l src/houseplan-card.ts` → 12889 строк, + потолок в `test/core-file-budget.test.mjs` — 12896. Запас сохранён, новый + код вычисления ключа вынесен в отдельный модуль, как и планировалось в ТЗ. +- **`test/clean-floor.test.mjs`.** Обновлён под новую сигнатуру + `floorKey(spaceId) => string` вместо `configEpoch: number` — тест даёт + `` `${spaceId}|3` ``, что сохраняет прежнее поведение теста (константный + ключ на этаж). + +## Чего не проверял + +- Не перегонял `npx tsc --noEmit`, `npm test`, `npm run build` + + `bundle-policy --verify` — подтверждены зелёным Validate на точном SHA. +- Не перегонял `npm run invariants` — задача не меняет геометрические + формулы, только ключ кэша; см. обоснование выше. +- Не перегонял `pytest tests_backend` / HA harness — Python не затронут. +- Не запускал Full Performance сам — дорогой гейт; вместо повторного запуска + провёл трассировку причин, почему единственный существующий прогон не + закрывает AC4 буквально (находка выше), и привёл независимое (хоть и не + эквивалентное по строгости) свидетельство через уже выполнившийся + перф-смок на доказанно идентичном коде. +- Не проверял английскую версию USER-GUIDE — задача не меняет видимое в UI + поведение, самой USER-GUIDE ТЗ прямо отмечает «не описывает». +- Не проверял golden-кадры глазами — доверился зелёному джобу Validate на + доказанно эквивалентном дереве (см. «Объём гейтов»), новых визуальных + изменений AC5/К5 не заявляет, и эталоны в диффе не менялись (нет + `demo/golden/baselines/**` в списке файлов). + +--- + + + +## Материал раунда + +- Ветка: `issue/744-floor-geometry-key`, коммит `8c639b15989d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `f96c4db7cb54cd11c52dc0f4efd2476816b2223f` + ``` + git log --all --format='%H %T' | grep f96c4db7cb54 + ``` +- Тело issue: `9dec80eead95ea0cbc014364d7ed09eaef1f7f45b7e2c70f97f1a95c586ac465` +- Вердикт конвейера: `yellow` · High 0 · маршрут `fix` +