From 2d7a1c9788262f1172c0cb9f5df498267cc1ba15 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 18:38:19 +0000 Subject: [PATCH] docs: review document for #744 Issue: #744 User-Visible: no --- docs/reviews/CODE-REVIEW-744-r2.md | 203 +++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-744-r2.md diff --git a/docs/reviews/CODE-REVIEW-744-r2.md b/docs/reviews/CODE-REVIEW-744-r2.md new file mode 100644 index 00000000..7f22fdfb --- /dev/null +++ b/docs/reviews/CODE-REVIEW-744-r2.md @@ -0,0 +1,203 @@ +# CODE-REVIEW-744-r2 + +**Issue:** #744 · **Трек:** ask · **Заход:** r2 · блокирующих циклов израсходовано 1 из 4 +**Материал:** `d10285b0794da3f9106f3dd4dd682dc877a2dc52` (ветка `issue/744-floor-geometry-key`, +поверх `dev` `9f695d8a`, три коммита: `d1dd6738` продукт + `5e046264` фикс гонки +теста — оба из r1 — плюс `d10285b0` `docs: review document for #744`, публикующий +шаг конвейера) + +## Скоуп + +Делта r1 → r2 — ровно один коммит `d10285b0`, класс C (добавляет +`docs/reviews/CODE-REVIEW-744-r1.md`, 279 строк). Продуктовый и тестовый код не +менялись: `git diff 5e046264..d10285b0 --stat` показывает только новый +docs-файл (плюс сам смок уже входил в материал r1 как часть коммита `5e046264`). +Единственная содержательная правка раунда — не коммит, а комментарий автора в +issue (2026-10-01T18:27:03Z) с новым прогоном Full Performance на актуальной +базе `dev`, закрывающий единственный Medium r1. + +Между материалом r1 и текущим материалом ветка была дополнительно перебазирована +на ушедший вперёд `origin/dev` (`0606a366` → `9f695d8a`): `git diff +0606a366..9f695d8a --stat` — это ровно один docs-only коммит `9f695d8a` +(спек-ревью #762, `docs/reviews/SPEC-REVIEW-762-r1.md` + `INDEX.md`), `src/**`, +`demo/**`, `scripts/**` не затронуты. Ребейз не меняет код, который рассматривал +r1, и не требует отдельного разбора (§2.10 — ребейз на ушедший вперёд `dev` +разбирается полным объёмом, только если сам что-то меняет; здесь дерево +продукта и тестов побайтово то же, что проверил r1 — см. «Унаследовано»). + +Задача по-прежнему проходит критерий §5 «перф» (разобрано в r1, код не +изменился): правка горячего пути переключения этажа, неверный ключ дал бы не +медленный, а неверный кадр. `route: fix`. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium — AC4 доказан не на актуальном материале: единственный прогон Full Performance (`36869385519`, headSha `d4439aee`) стоял на базе `dev 707cb18f`, **до** того как `dev` landed `fa18ca81` (#747), срезавший `switchCycleMs.hardMaxMs` в 5–7 раз (7000/8000 → 950/1550 мс) именно для профилей AC4; материал `8c639b15` уже стоял на `dev 0606a366` со срезанными бюджетами, и повторного прогона против них не было | Автор перезапустил `performance.yml` с явным `comparison_ref=0606a366` (комментарий 2026-10-01T18:27:03Z) — прогон [`36905760140`](https://github.com/Matysh/houseplan-card/actions/runs/36905760140), `headSha=4dd3ce37`, `conclusion: success`, все 10 job зелёные. В issue приведены медианы кандидата/базы по всем метрикам, которые дословно называет AC4 (`spaceSwitchMs`, `switchCycleMs`, `resizePreviewMs`, вся серия `large-house-interaction-v1`), плюс относительный разбор трёх отклонений прошлого (устаревшего) прогона против свежей базы — все три ушли в минус (`modelReadyMs` +8%→−1.0%, +5%→−0.5%; `spaceSwitchMs` +6.5%→−1.5%) | Я проверил это не со слов, а исполнением: `gh run view 36905760140 --json conclusion,headSha,headBranch,workflowName,event` → `success`/`4dd3ce37`/`issue/744-floor-geometry-key`/`workflow_dispatch`; `gh run view 36905760140 --json jobs` → 10/10 `success`; `gh run view 36905760140 --log` (job «Resolve comparison SHA», large-house) → `env.MANUAL_BASE: 0606a3664a6c819a501316f478840d6c3a5b456c` — `comparison_ref` реально был тем самым `0606a366`, а не пустым/родителем. `git merge-base --is-ancestor 0606a366 9f695d8a` → код 0: база прогона — предок текущего `dev`-мерджбейза, и `git diff 0606a366..9f695d8a --stat` (см. «Скоуп») подтверждает, что между ними нет перф-значимых изменений — свидетельство остаётся валидным для материала `d10285b0`, а не только для `4dd3ce37` | + +Нашёл и прочитал новый прогон напрямую (`gh run view`), а не принял числа из +комментария автора на слово: `headSha`, `conclusion`, список job и фактическое +значение `comparison_ref` в логе джоба сверены по API/логам, а не только +процитированы из хендоффа. + +## Унаследовано из r1 + +Продуктовый и тестовый код не менялся между r1 и r2 (делта — только +docs-коммит), поэтому нижеследующее из `docs/reviews/CODE-REVIEW-744-r1.md` +(материал `8c639b15…`, дерево которого больше не резолвится локально — обычное +дело после ребейза, §2.10) принято без повторной проверки, с подтверждением +байт-в-байт по диффу `origin/dev...HEAD` текущего материала, а не только со +слов документа r1: + +- Архитектура ключа (`src/floor-geometry-key.ts`, `floorGeometryKeyReader`) — + отпечаток содержимого записи этажа (`contentFingerprint`), с кэшем на эпоху и + на пару объектов записи (`model`/`current`); при расхождении переданного и + текущего этажа ключ хэширует оба. Перечитан целиком заново в этом раунде — + содержимое файла идентично тому, что описывает r1. +- `src/clean-floor.ts`: сигнатура `floorKey(spaceId) => string` вместо + `configEpoch: number`; `test/clean-floor.test.mjs` обновлён под неё. +- `src/stairs-editor.ts`: `StairEditorRuntime.write` больше не чистит + `_cleanFloorCache` — подтверждено диффом, комментарий `#744` на месте. +- Resize preview/accept/cancel в `houseplan-editor-runtime.ts` читают ключ в + корректном порядке относительно `session.accepted`/`publish` — разобрано r1 + построчно, код не менялся. +- Контракт К1–К5 ТЗ выполнен; защита против «устаревшего кадра» (главный риск) + доказана независимым oracle в `demo/smoke_floor_geometry_cache.mjs` + (AC1, AC2а/б/в) — карточка-oracle монтируется заново на том же конфиге и + сверяется путями стен и площадями, а не только отсутствием падения. +- Второй коммит (`5e046264`) честно чинит гонку с тостом в смоке + (`ac2cPreviewWallStandsWhereAFreshCardDrawsIt` судила только живой слой; + после закрытия тоста и форсированного host-рендера проверяются оба кадра), + а не маскирует красный CI. +- Сторож #735 (`demo/benchmark_large_house.mjs`, `demo/performance/card-contract.mjs`) + видит `_wallUnionPool`/`_innerContourCache`; старый сравнительный бандл читает + 0 на отсутствующих полях, не падает. +- Два мутанта (`floor-geometry-key-global-epoch`, + `floor-geometry-key-ignores-content`) в `scripts/mutation-registry.mjs` + сверены r1 посимвольно с кодом; категория «lifecycle» в + `docs/testing-notes/mutation-browser-guards.md` (90→92, Total 205→207, + выше guideline 200 — предупреждение, не отказ, по прецеденту) согласована. +- Трейлеры `Issue: #744` на всех коммитах; `User-Visible: yes` на продуктовом + коммите с правкой обоих CHANGELOG в том же коммите, текст дословно совпадает + с ТЗ; `User-Visible: no` на тестовом коммите, правящем только смок — корректно. +- Бюджет строк монолита: `wc -l src/houseplan-card.ts` — запас сохранён, + вычисление ключа вынесено в отдельный модуль (не проверял заново числом в + этом раунде — код файла не менялся с r1). +- «Одно число — один источник»: задача не меняет ни одного значения, видимого + в UI (площади и пути стен совпадают до/после по К5, доказано смоком); + CHANGELOG описывает эффект, а не число. + +## Как проверялось в этом раунде + +- Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md` + (повторный раунд — объём по дельте, §2.10), тело issue #744 целиком и все + комментарии, включая ответ автора на Medium r1 и приведённые в нём медианы. +- `git log --oneline origin/dev..HEAD` и `git diff origin/dev...HEAD --stat` — + три коммита, 14 файлов, 786/24 — совпадает построчно с описанием r1 + («Затронутые файлы»/«Скоуп»). +- `git diff 5e046264..d10285b0 --stat` — делта раунда: ровно + `docs/reviews/CODE-REVIEW-744-r1.md`, только добавления. +- `git diff 0606a366..9f695d8a --stat` — ребейз на новый `dev` добавил только + docs/reviews для #762; `git rev-parse origin/dev` → `9f695d8a`, `git + merge-base origin/dev HEAD` → тот же `9f695d8a` — материал ребейзнут на + актуальную вершину `dev` без конфликтов. +- Перечитан `src/floor-geometry-key.ts` целиком и диффы `stairs-editor.ts`, + `clean-floor.ts` против `origin/dev` — содержимое соответствует тому, что + описывает r1, без расхождений. +- `gh run view 36905760140 --json conclusion,headSha,headBranch,workflowName,event` + и `--json jobs` — `success`, `headSha=4dd3ce37`, ветка задачи, 10/10 job + зелёные; `gh api repos/.../actions/runs/36905760140` — `head_commit.message` + = `docs: review document for #744` (та же логическая правка, что ныне + `d10285b0`, только на предыдущей версии дерева ребейза) и `tree_id` + `8e344175…`. +- `gh run view 36905760140 --log` (job large-house, шаг «Resolve comparison + SHA») — `env.MANUAL_BASE: 0606a3664a6c819a501316f478840d6c3a5b456c`: подтверждён + явный `comparison_ref`, как заявляет автор, а не предположен. +- `git merge-base --is-ancestor 0606a366 9f695d8a` → код 0 — база прогона + является предком текущего merge-base, перф-свидетельство валидно для + материала `d10285b0`, а не только для `4dd3ce37`. +- `gh run view 36907042646 --json conclusion,headSha,headBranch,workflowName,event` + и `--json jobs` — Validate на точном SHA материала `d10285b0794da3f9106f3dd4dd682dc877a2dc52`: + `conclusion: success`; «Фронтенд: типы, юниты, мутанты, синхрон бандла» — + success (typecheck/`npm test`/mutation-gate/bundle); «Переиспользование: это + дерево уже проверено» — success (browser-смоки/golden/перф-смок переиспользуют + прогон на дереве, эквивалентном предыдущему пушу, легитимно — продукт/демо не + менялись со времени того пуша). +- Трейлеры `d10285b0`: `Issue: #744`, `User-Visible: no` — docs-only коммит мог + обойтись без них, но они проставлены корректно. + +## Находки + +Нет. Единственная находка r1 (Medium, AC4 на устаревшей базе) закрыта прогоном +Full Performance на актуальной базе, прочитанным и сверенным напрямую по API и +логам, а не принятым со слов хендоффа. Продуктовый и тестовый код в этом +раунде не менялся. + +## Что проверено и корректно + +- AC4 (Full Performance, все профили зелёные, медианы кандидата/базы по + метрикам, названным AC4) — доказан исполнением на базе, равной актуальному + `dev` на момент прогона (`0606a366`), которая сама является предком текущей + вершины `dev` (`9f695d8a`) без перф-значимых изменений между ними. +- Относительные отклонения, отмеченные в исходном (устаревшем) прогоне r1 + (`modelReadyMs +8%`, `+5%`, `spaceSwitchMs +6.5%`), на свежей базе ушли в + минус — риск, из-за которого AC4 был признан недоказанным, не + материализовался. +- `resizePreviewMs` (метрика, которой ТЗ прямо требует судить цену отпечатка + ключа в превью resize) — в пределах ±1,5% во всех профилях. +- Наибольшее отклонение кандидата выше базы среди всех медиан прогона — + `stateUpdateMs` в `large-house-isometric-v1`, 38.6→43.8 мс (+13.5%), предел + 113.6 мс: метрика не из названных AC4, запас к `hardMaxMs` больше чем в 2.5 + раза, не повод для находки. +- Ребейз на новый `dev` (`0606a366`→`9f695d8a`) докс-only и не требует + повторного разбора кода — подтверждено `git diff --stat`, а не + предположением. +- Всё, что перечислено в «Унаследовано из r1», остаётся в силе — код, который + это описывает, не менялся. + +## Чего не проверял + +- Дешёвые гейты (`npx tsc --noEmit`, `npm test` целиком, `npm run build` + + сверка копий бандла) не перегонял — зелёный Validate на точном SHA материала + подтверждён напрямую (`gh run view 36907042646`), делта раунда (docs-коммит) + их не касается. +- Browser-смоки всех шардов и golden не перегонял сам — Validate на материале + легитимно переиспользовал прогон на дереве, эквивалентном предыдущему пушу + (продукт/демо не менялись с r1); отдельно не проверял golden-кадры глазами. +- `npm run gate:small` не запускал — код, который он проверяет, не менялся с + r1, где он был подтверждён. +- `npm run invariants` — не запускал; задача не меняет геометрические формулы, + только ключ кэша (разобрано в r1, код не менялся). +- `pytest tests_backend` / HA harness — не запускал, Python не затронут ни в + одном коммите материала. +- `smoke-select` самостоятельно не перегонял — автор привёл результат в r1 + (64/65, единственный красный — известная причина окружения), код с тех пор + не менялся. +- Full Performance сам не перезапускал — проверил существующий прогон + `36905760140` исполнением (API, логи, trees), этого достаточно: прогон + целиком закрывает буквальное требование AC4 на материале, эквивалентном + текущему. +- Английскую версию USER-GUIDE не проверял — задача не меняет видимое + поведение (как и в r1). + +## Вердикт + +Зелёный. Единственная находка r1 (Medium, AC4 на устаревшей базе) закрыта +прогоном Full Performance на актуальной базе `dev` (`36905760140`, headSha +`4dd3ce37`, сверено исполнением — API/логи/дерево), новых находок нет, код не +менялся. Бюджет раунда остаётся израсходованным на 1 из 4 (унаследовано из r1). + +--- + + + +## Материал раунда + +- Ветка: `issue/744-floor-geometry-key`, коммит `d10285b0794d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `bbc1bb8a252993e98f2faefbe298483509889f64` + ``` + git log --all --format='%H %T' | grep bbc1bb8a2529 + ``` +- Тело issue: `9dec80eead95ea0cbc014364d7ed09eaef1f7f45b7e2c70f97f1a95c586ac465` +- Вердикт конвейера: `green` · High 0 · маршрут `fix` +