mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 12:49:56 +00:00
@@ -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/**` в списке файлов).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/744-floor-geometry-key`, коммит `8c639b15989d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `f96c4db7cb54cd11c52dc0f4efd2476816b2223f`
|
||||
```
|
||||
git log --all --format='%H %T' | grep f96c4db7cb54
|
||||
```
|
||||
- Тело issue: `9dec80eead95ea0cbc014364d7ed09eaef1f7f45b7e2c70f97f1a95c586ac465`
|
||||
- Вердикт конвейера: `yellow` · High 0 · маршрут `fix`
|
||||
<!-- hp:usage input_tokens=4132 output_tokens=57489 cache_creation_input_tokens=161109 cache_read_input_tokens=7472943 num_turns=79 -->
|
||||
Reference in New Issue
Block a user