From 2eb9fdbf68e423e1e7f7ba851cae4b3362dbe9a6 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 05:42:36 +0000 Subject: [PATCH] docs: review document for #627 Issue: #627 User-Visible: no --- docs/reviews/CODE-REVIEW-627-r1.md | 98 ++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-627-r1.md diff --git a/docs/reviews/CODE-REVIEW-627-r1.md b/docs/reviews/CODE-REVIEW-627-r1.md new file mode 100644 index 00000000..1a01232e --- /dev/null +++ b/docs/reviews/CODE-REVIEW-627-r1.md @@ -0,0 +1,98 @@ +# CODE-REVIEW-627-r1 + +Материал: `40959feb1d44e51d653f8c6e9cdd5caa7f9ad9e8` (`origin/dev..HEAD` — один коммит), дерево `5210484960ba796270c76eaf2125b8ff037af8a5`. Рабочая копия уже на этом SHA; `git fetch`/`checkout` не выполнялись. Заход r1, блокирующих циклов израсходовано 0 из 4 (полный трек: лимит 4). + +## Скоуп + +ТЗ #627 (полный трек): английский слой словарей `settings`/`support`/`topology` остаётся статическим, ru/de/fr выносятся в девять ленивых чанков (пространство × язык); загрузчики онбординга, редактора и Zigbee-оверлея ждут «оседания» своего словаря перед установкой поверхности; составной гейт хоста (`composeLanguageRuntimes`) учитывает словари загруженной на хосте поверхности при смене языка на лету; `bundle-manifest.mjs`/`bundle-budget.mjs` получают новый граф `lazyNamespaceLocaleFiles` и потолок `LAZY_ONBOARDING_GZIP_CEILING`, редакторский потолок пересчитан. Диапазон: класс A (`src/i18n/**`, `houseplan-card.ts`, рантаймы) + B (скрипты бандла, тесты, смок, реестр мутантов) + D (пересобранный бандл, три копии). + +Материал ревью совпадает по содержанию с описанием в хендоффе автора (комментарий от 2026-09-24T04:23:43Z) и итоговым возвратом на `S7-code-review` после ребейза на ушедший вперёд `dev` (комментарий 05:04:50Z, материал переподписан на `a176c4b2` → затем ещё раз перебазирован на актуальный `dev` до `40959feb`, что и дало другой хэш дерева/части файлов вроде `scripts/mutation-registry.mjs` — содержимое диффа `origin/dev...HEAD`, которое я разбираю ниже, при этом полностью совпадает с тем, что описано в хендоффе). + +## Как проверялось + +Дешёвые гейты подтверждены зелёным Validate на этом самом SHA (run [35958661751](https://github.com/Matysh/houseplan-card/actions/runs/35958661751), success): typecheck, `npm test` (юниты), `npm run build`+`bundle:sync`, мутанты по диффу (6/6 шардов) — все success; `smoke`/`golden`/`performance_smoke`/`backend`/`hacs`/`hassfest`/`geometry_parity` в этом прогоне **skipped** (не `heavy`), поэтому браузерные смоки и golden я обязан проверить сам. + +Прогнано лично на этом SHA: + +| Гейт | Команда | Результат | +|---|---|---| +| Typecheck + build | `npm run build` | ok, byte-identical dist (после `bundle-sync` — `git status` чистый) | +| Bundle sync + budget | `node scripts/bundle-sync.mjs && node scripts/bundle-budget.mjs` | initial View 289603 (потолок 290400±2000), lazy editor 227869 (потолок 228900±2000), lazy onboarding 27470 (потолок 28500±2000), lazy namespace locales 20867 (9 чанков) — совпадает с числами хендоффа | +| check-docs | `node scripts/check-docs.mjs --screenshots=warn` | ok (7 файлов, 12 внешних ссылок); фингерпринт скриншотов помечен `WARN` (стало ещё до этой задачи, это предбета-долг #479, не находка) | +| check-inputs | `node scripts/check-inputs.mjs --coverage` | пусто (0), как заявлено | +| Юнит-тесты изменённых файлов | `node --test test/i18n-lazy-namespaces.test.mjs test/i18n-runtime.test.mjs test/zigbee-topology.test.mjs test/bundle-assets.test.mjs` | 4+9+25+36 = 74 pass, 0 fail | +| Мутанты (все 5 новых + 2 с обновлёнными якорями) | `node scripts/mutation-gate.mjs --id=` ×7 | все 7 «поймано 1 из 1»: `onboarding-loader-skips-namespace-ensure`, `bundle-budget-onboarding-ceiling-dropped`, `i18n-namespace-eager-locale`, `locale-gate-ignores-namespaces`, `namespace-loader-returns-english`, `danger-confirm-uses-last-rendered-language-gate`, (топология — см. ниже) | +| Смоки: прямые совпадения `smoke-select` (8) | `node demo/smoke_{danger_confirmation,help_affordance,houseplan_panel,lazy_admin_locale,lazy_editor_chunk,pdf_export,post_write_adoption,support_feedback}.mjs` | 7 OK, **1 упал** — см. находку M1 | +| Смоки: зарегистрированные связи `smoke-select` (3) | `node demo/smoke_{entry_stale,french_locale,zigbee_topology_hover}.mjs` | все 3 OK | +| Смоки, названные в AC регресс-списке, не входящие в прямые/связи | `smoke_general_settings_form`, `smoke_room_settings_form`, `smoke_device_settings_form`, `smoke_space_settings_form`, `smoke_version_recovery`, `smoke_german_locale` | все 6 OK | +| Новый смок AC4–AC7 | `node demo/smoke_lazy_admin_locale.mjs` | OK, все 38 полей true | + +`node scripts/smoke-select.mjs --base origin/dev --head HEAD`: 8 прямых совпадений, 3 зарегистрированные связи, 45 слабых (одно распространённое имя `_config`/`_editorRuntime`) — по правилу промпта слабые не обязательны к прогону; я прогнал дополнительно те AC-именованные смоки из плана автотестов, что не попали в прямые/связи (список выше), итого разобраны все 14 смоков, названных в плане автотестов ТЗ, плюс новый. + +## Находки + +### M1 (Medium, в скоупе задачи) — `demo/smoke_danger_confirmation.mjs` не переведён на ожидание составного гейта; диалог опасного действия может быть незаметно потерян при переключении языка + +**Файлы:** `demo/smoke_danger_confirmation.mjs` (не тронут этим коммитом — единственный из смоков, синхронно переключающих язык поверх открытого редактора/онбординга, который автор пропустил в списке из девяти переведённых на ожидание гейта); причина в `src/houseplan-card.ts:1968-1973` (`_syncDangerConfirmLocaleGate`) и `src/houseplan-card.ts:4033-4036` (авто-отмена на `warm`, #417). + +**Дефект:** `_syncDangerConfirmLocaleGate` теперь берёт составной рантайм загруженной поверхности (`_editorRuntime?.languageRuntime ?? _onboardingRuntime?.languageRuntime ?? LANGUAGE_RUNTIME`), а не только основной каталог. Существующий с #417 механизм «отменить `_dangerConfirm`, если гейт вернул `warm`» из-за этого расширения начинает срабатывать и в момент, когда основной каталог уже осел (тест ждёт именно это условие: `card._t('btn.cancel') !== expectedCancel`), а словарь пространства (`settings`) на этом же хосте — ещё нет: составной гейт остаётся `pending` → рендер-цикл видит `warm` (хост уже был `committed` на английском) → `_cancelDangerConfirm()` срабатывает синхронно, до того как диалог успевает отрисоваться. Вызывающий код получает `false` без единого кадра диалога и без какой-либо видимой подсказки — не «английская вспышка», которую ТЗ прямо предвидело в разделе «Риски», а тихая потеря запроса подтверждения. + +**Воспроизведение:** +1. `npm run build && node scripts/bundle-sync.mjs`. +2. `node demo/smoke_danger_confirmation.mjs` — падает необработанным исключением внутри `localeFits('de', …)`: `TypeError: Cannot read properties of undefined (reading 'click')` (воспроизвелось на первом прогоне в чистой сессии; из ~8 последующих прогонов подряд — 0 падений, гонка зависит от относительной скорости загрузки чанков `de.ts` и `settings-de-*.js`). +3. Детерминированное воспроизведение (не закоммичено, только для диагностики): в копии смока добавлена задержка 40 мс на ответ `**/settings-de-*.js*` через `page.route` перед вызовом `localeFits('de', 'Abbrechen')` — падение стабильно повторяется на каждом прогоне. +4. Контрольный прогон на `origin/dev` (`22c7d634`, до #627) в отдельном воркчасти: тот же смок проходит зелёным («OK», все поля true) — регрессия точно привязана к этому диффу, а не к окружению. + +**Чем краснеет** (защитное свойство «диалог опасного действия не теряется при переключении языка поверхности»): задержка ответа любого из девяти namespace-чанков (`settings`/`support`/`topology` × ru/de/fr) относительно основного каталога — продемонстрировано выше для `settings`-`de`. + +**Почему Medium, а не High:** прямой угрозы безопасности нет — потерянный запрос разрешается в `false` («не подтверждено»), ни одно деструктивное действие не выполняется без подтверждения; в реальном UI хост становится `inert` на время `warm`, что обычно блокирует пользовательский клик в этом же окне. Но это реальная, воспроизводимая регрессия существующего регресс-смока (он попал в «прямое совпадение» инструмента `smoke-select` и напрямую относится к вводимому этой задачей контракту), она интермиттентна и не поймана ни одним мутантом (гвард `danger-confirm-uses-last-rendered-language-gate` — другой файл, `smoke_danger_confirm_branches.mjs`, который эту гонку не задевает) и ни одним юнит-тестом; оставленная как есть, она будет время от времени красить полный смок-прогон перед бетой в зависимости от таймингов сети/чанков — именно то, что процесс просит ловить на этапе код-ревью, а не на релизном гейте. + +**Почему в скоупе, а не отдельным issue:** ломающий смок и вызвавшая его причина — прямое следствие ядра этой задачи (составной гейт по словарям пространств), тот же риск, что ТЗ называет в разделе «Риски» («смоки… нужно перевести на ожидание гейта, а не ослаблять проверки»); чинится он тем же приёмом, что уже применён к девяти другим смокам в этом коммите. + +**Вердикт по AC:** не относится ни к одному конкретному AC1–AC7 напрямую (это не то поведение, которое AC описывают), но затрагивает общий контракт задачи «поверхность не мигает английским / не виснет» — в данном случае «не молчит вместо диалога». + +Low/иных находок нет. + +## Что проверено и корректно + +| AC | Проверено | Итог | +|---|---|---| +| AC1 | `bundle-budget.mjs` печатает и гейтит `lazy onboarding` тем же `lazyGraphCeilingViolation`; реальный манифест и синтетика (`test/bundle-assets.test.mjs`, тесты «#627 AC1», «#593») — прогнано лично, зазор >500 Б с обеих сторон подтверждён числами (28500−27470=1030 сверху, 27470−(28500−2000)=970 снизу); мутант `bundle-budget-onboarding-ceiling-dropped` красит целевой тест | Доказан | +| AC2 | Числа воспроизведены лично (`bundle-budget.mjs`): онбординг 27470 ≤ 28000 и 34526−27470=7056 ≥ 6500; редактор 244433−227869=16564 ≥ 15000 (запас к новому потолку 228900±2000 = 1031/969, оба причины записаны в комментариях к константам); initial View 289603−289449(ориг. измерение из issue)=154 ≤ 300, в полосе потолка (290400±2000, запас 797/1203) | Доказан | +| AC3 | Девять чанков ровно, ни один не пересекается с initial/editor/onboarding — проверено чтением `bundle-budget.mjs`/`bundle-manifest.mjs` + прогоном `test/bundle-assets.test.mjs` (владение по содержимому, отказы графов) и нового `test/i18n-lazy-namespaces.test.mjs` (нет статических импортов ru/de/fr, у каждого загрузчик с двумя попытками, ровно один retry-токен) — оба файла прогнаны лично, 100% pass; мутант `i18n-namespace-eager-locale` красит | Доказан | +| AC4 | Холодное открытие онбординга (ru), «Общих настроек» (de), диалога устройства (fr): первый кадр диалога на языке пользователя, задержка чанка 250 мс скрыта под существующим индикатором, ни английской строки, ни сырого ключа ни в одном кадре — прогнан `smoke_lazy_admin_locale.mjs` лично, все соответствующие поля true; мутант `onboarding-loader-skips-namespace-ensure` красит именно этот смок (проверено запуском) | Доказан | +| AC5 | Live-переключение ru→de при открытых «Общих настройках»: прежний кадр удерживается (`inert`+`aria-busy`), финальный кадр — целиком немецкий, чанк запрошен один раз, вторая карточка не запрашивает повторно — тот же смок + `test/i18n-runtime.test.mjs` «#627 AC5» (прогнан лично, включает случай отказа только пространства без падения `lang` в `en`); мутант `locale-gate-ignores-namespaces` красит | Доказан. **См. M1** — соседнее поведение (danger-confirm) той же самой compound-гейт-логики страдает в узком гоночном окне, которое AC5 не описывает и не покрывает | +| AC6 | Отказ обеих попыток чанка → английский только в словаре этого пространства, один тост, один `console.warn`, диалог не виснет — прогнан смок лично, поля true | Доказан | +| AC7 | Холодный View (en/ru) не запрашивает ни одного namespace-чанка; en-админ в редакторе — тоже; Zigbee-слой на ru запрашивает только `topology`-ru один раз — прогнан смок лично (журнал запросов), плюс `smoke_zigbee_topology_hover.mjs` на ru отдельно — оба зелёные | Доказан | +| Владение графов (`assertSupportBundleOwnership`/`assertNamespaceLocaleOwnership`) | Проверено чтением + прогоном `bundle-budget.mjs` как CLI на реальном `dist/houseplan-assets.json` — не бросает | Корректно | +| Трейлеры | `Issue: #627`, `User-Visible: no` — на единственном коммите; видимое поведение действительно не меняется (сами тексты/ключи не тронуты, изменился только механизм доставки), changelog обоснованно не редактировался | Корректно | +| Одно число — один источник | `LAZY_EDITOR_GZIP_CEILING`/`LAZY_ONBOARDING_GZIP_CEILING` объявлены один раз в `scripts/bundle-budget.mjs`, тесты импортируют константы, а не дублируют литералы; `NAMESPACE_LOCALE_CHUNKS` (9) — тоже единственный источник, импортируется тестами и смоком | Корректно | +| `monolith-baseline.json` `bundleBytes` 2 500 387 → 2 508 676 | Причина записана (десять новых чанков × rollup intro/export), пересчитано и сошлось с фактической сборкой | Корректно | +| tsconfig.test.json / CONTRIBUTING.md | Три новых модуля включены в тестовую компиляцию; абзац про namespace-словари добавлен по существу | Корректно | + +## Чего не проверял + +- `golden:verify` и перф-смок взаимодействия — оба требуют Linux CI/предбета-гейт; эталоны не менялись, автор обоснованно ожидает отсутствие изменений кадров (рантаймы ждут словарь до установки, а не меняют разметку) — не проверял сам, доверяю рассуждению, но это не заменяет обязательный прогон на кандидате. +- Полный реестр мутантов (только диффовые 6/6 шардов подтверждены Validate; ночной `mutation-gate.yml` не запускал). +- ~250 смоков вне прямых совпадений/связей/AC-плана — не прогонял (широкий охват — предрелизная обязанность, не гейт ревью). +- `pytest tests_backend` — Python не менялся, не запускал. +- Живой стенд Home Assistant — не поднимал. +- Perf-смок взаимодействия по диффу `houseplan-card.ts` — по правилам гейтится `heavy`-профилем CI, не гейт код-ревью; не запускал. + +## Материал для повторного раунда + +Если находка M1 будет закрыта правкой `demo/smoke_danger_confirmation.mjs` (по образцу уже переведённых девяти смоков — ждать составной гейт/`aria-busy` перед вызовом `_confirmDanger`, а не только `card._t('btn.cancel')`), разбор r2 может ограничиться: диффом этого файла + одним прогоном (в идеале с искусственной задержкой namespace-чанка, как в этом отчёте) для доказательства, что гонка закрыта, а не просто перестала проявляться на быстром CI-раннере. + +--- + + + +## Материал раунда + +- Ветка: `issue/627-lazy-admin-locales`, коммит `40959feb1d44` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `5210484960ba796270c76eaf2125b8ff037af8a5` + ``` + git log --all --format='%H %T' | grep 5210484960ba + ``` +- Тело issue: `11ffae080ad26691a4b8dbd32f7736895c0c25a3f0cabe1b09da0e21666e6562` +- Вердикт конвейера: `yellow` · High 0