Files
2026-10-02 00:56:04 +03:00

25 KiB
Raw Permalink Blame History

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) подтверждает их по правилам промпта.
  • Browser-смоки (все шарды), golden. Validate на 8c639b15 свернул эти джобы в skipped через «Переиспользование: это дерево уже проверено» — фактически они выполнились на предыдущем пуше 6ff0255d (run 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, 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