From 2518abd961780b21333d47a5b2eb1970c8904812 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 17:00:13 +0000 Subject: [PATCH] docs: review document for #391 Issue: #391 User-Visible: no --- docs/reviews/CODE-REVIEW-391-r1.md | 189 +++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-391-r1.md diff --git a/docs/reviews/CODE-REVIEW-391-r1.md b/docs/reviews/CODE-REVIEW-391-r1.md new file mode 100644 index 00000000..6df4cc98 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-391-r1.md @@ -0,0 +1,189 @@ +# CODE-REVIEW-391-r1 + +Issue: #391 «refactor: type editor i18n keys» +Этап: code (S7-code-review) +Заход: r1 (первый код-ревью раунд; спек-ревью прошло зелёным на r1 отдельно) +Ветка: `issue/391-i18n-key-casts` +SHA под ревью: `81a4278aa4236cec656e473e3ff3953a05c6c5d1` (пайплайн привёл ветку к `dev` +поверх коммита `f7f088fe`; после ребейза это другой код — разбор ниже полный). +Материал: `git diff origin/dev...HEAD` относительно локально закэшированного +`origin/dev` = `eb5aa2a0f8dea5514ee2fee2dbd609457dd0fb4d`. + +## Скоуп + +Класс A: `src/houseplan-editor-runtime.ts` — снятие `as any` с 33 вызовов +`this.host._t(...)` в трёх семействах ключей (`device_inbox.*` — 26, +`marker.*` — 6, `gs.preflight_reason_*` — 1) и замена вычисляемых ключей на +узкий `as I18nKey`, без изменения словарей, текстов, разметки и ветвления. +Класс B: `scripts/mutation-gate.mjs` — перенос mutation-anchor #295 +(`preflight-reason-lost-in-dialog`) на новый файл/сигнатуру после того, как +lazy-load рефакторинг увёл вызов из `houseplan-card.ts` в +`houseplan-editor-runtime.ts` (без изменения `guard`/`because`). +Класс D: `dist/**`, `custom_components/houseplan/frontend/**` — пересборка. +`User-Visible: no`, трейлер `Issue: #391` на месте — оба корректны для +чисто типового рефакторинга без пользовательского эффекта; изменений в +`docs/CHANGELOG*.md` нет и не требуется. + +## Как проверялось + +Зелёного Validate на этом SHA не найдено, поэтому гейты прогнаны вручную на +чистом дереве этого коммита (Node 22, зависимости уже установлены): + +| Гейт | Команда | Результат | +|---|---|---| +| Typecheck | `npx tsc --noEmit` | pass, без вывода | +| Unit/integration | `npm test` | 1644 pass / 0 fail / 1 skipped | +| No-new-any (#342) | `git diff origin/dev...HEAD -- src/houseplan-editor-runtime.ts \| node scripts/no-new-any.mjs --diff -` | «Проверено добавленных строк: 32 в 1 файл(ах). Новых any нет.» | +| Mutation-gate анchor (#295) | `node --test test/mutation-gate.test.mjs` | 10/10 pass | +| Build | `npm run build` | pass | +| Bundle sync | `npm run bundle:sync` | pass; `git status --porcelain` после пересборки — пусто (обе generated-копии побайтово совпадают с закоммиченными) | +| Bundle budget | `npm run bundle:budget` | initial View 283100 B / budget 300000 B; lazy editor 143903 B; lazy locale 46996 B — совпадает с числами автора | +| Inventory | `npm run inventory` | Node 1645 (было 1643 у автора; расхождение — от несвязанного коммита `eb5aa2a0`, легшего поверх после отправки на ревью, не дефект этой задачи) | +| Docs fingerprint | `node scripts/check-docs.mjs` | **FAIL** — см. находку M1 | + +Diff не трогает геометрию/модель (нет правок `wall-thickness.ts`, `layout`, +`marker.space`, `open_spans`, узлов/рёбер) — `npm run invariants` не +запускался, это осознанный пропуск, а не мелочь. +Diff не трогает `custom_components/**/*.py` — `pytest tests_backend` не +запускался. +`golden:verify` не запускался: изменение не может повлиять на рендер — +это доказано не только заявлением автора, а тем, что все правки состоят +из удаления TypeScript-каста (`as any`/`as I18nKey`), который стирается на +этапе компиляции без остатка; визуальных/строковых/ветвящихся изменений в +diff нет. + +### Браузерные смоки + +`node scripts/smoke-select.mjs --base origin/dev --head HEAD`: + +``` +Изменено файлов src/**: 1 · символов проекта на изменённых строках: 4 +Прямое совпадение (7): _showToast → + smoke_help_affordance, smoke_junction_limits, + smoke_optimize_coordinate_canonicalization, smoke_partition_openings, + smoke_room_resize, smoke_tap_ctx, smoke_zero_wall_migration_unblocked +``` + +Символ `_showToast` попал в выборку механически: он встречается на той же +строке, что и переименованный `_t('device_inbox.saved' as any)` → +`_t('device_inbox.saved')`, хотя сам вызов `_showToast` не менялся. Решение: +прогнать все 7 — дёшево, а инструмент прямо предупреждает не полагаться на +тематический отбор (регресс #234 был пойман смоком, чьё название не +подсказывало связь). Все 7 — `OK`. Дополнительно прогнан +`node demo/smoke_preflight_diagnostics.mjs` — это ровно guard-команда +mutation-anchor `preflight-reason-lost-in-dialog`, который правит diff; +`OK`. Итого 8 браузерных смоков, все зелёные. Полный набор (208 смоков) не +запускался — задача не задевает ничего, кроме уже перечисленного, это +предрелизная обязанность, а не гейт код-ревью. + +## Находки + +### M1 (Medium, в скоупе — правится в этой же задаче) — стал протухшим отпечаток скриншотов документации + +`node scripts/check-docs.mjs` на SHA `81a4278a` падает: + +``` +ERROR screenshot source fingerprint is stale; run npm run build && node demo/docs/capture.mjs +``` + +Воспроизведение и причина, а не голословно: `visualFingerprint()` +(`scripts/source-fingerprint.mjs`) хэширует весь `src/**` плюс несколько +build-input файлов. Коммит правит `src/houseplan-editor-runtime.ts`, поэтому +отпечаток обязан был измениться — и изменился (проверено прямым вызовом: +на этом SHA `visualFingerprint('.')` = `74378a15…`, а +`docs/images/screenshots.json.sourceFingerprint`, оставшийся из `dev`, — +`d27cbb20…`; для контроля тот же расчёт на чистом клоне `origin/dev` +(`eb5aa2a0`, без диффа этой задачи) даёт согласованные `d27cbb20…` с обеих +сторон — несовпадение появляется ровно с этим коммитом, а не раньше). +Коммит не содержит правок `docs/images/**`, то есть шаг +`npm run build && node demo/docs/capture.mjs` не выполнялся. + +Это не гипотетическая придирка: `docs` — один из перечисленных gate-джобов +Validate (AGENTS.md), и ровно этот пропуск уже дважды оставлял `dev` с +красным `docs` до отдельной последующей задачи (#230, #234 → почищено в +#237). Смысловых изменений отпечаток не несёт (правки — чистое удаление +типовых кастов, кадры почти наверняка выйдут пиксель-в-пиксель прежними), +но манифест обязан отражать актуальный `sourceFingerprint`, иначе `docs` +red на `dev` сразу после мержа. + +Почему Medium, а не High: не портит пользовательское поведение, не ломает +контракт, чинится механически. Почему в скоупе этой задачи, а не отдельный +issue: находка — прямое следствие ровно этого diff (правка src/** этой +задачи сделала отпечаток невалидным), а не постороннего дефекта соседней +подсистемы — правило «Medium вне скоупа → отдельный issue» сюда не +применяется. + +Что нужно для закрытия: `npm run build && node demo/docs/capture.mjs`, +проверить, что кадры не изменились (`imageSha256` в манифесте), закоммитить +обновлённый `docs/images/screenshots.json` (и PNG, если оптимизатор дал +другие байты) в той же ветке с трейлерами `Issue: #391` / `User-Visible: no` +— это не пользовательская документация, а служебный манифест, видимое +поведение не меняется. + +## Что проверено и корректно + +- Пересчитаны все 33 целевых вызова построчно (`grep` по + `_t(.*as any` до/после диффа): 26 `device_inbox.*` + 6 `marker.*` + 1 + `gs.preflight_reason_*` — все убраны; ни один каст той же строки за + пределами трёх заявленных семейств не тронут (проверено: `tap.*`, + `fill.*`, `decor.*`, `junction.*`, `vac.*`, `run.*` и generic `message`/ + `hintKey`/`labelKey` остаются с `as any` — корректно, они вне скоупа). +- Условный `readd`/`add` (строка ~12054) переписан на литеральный union без + каста на каждой ветке, как требует правило реализации №5 — подтверждено + диффом. +- `I18nKey` был импортирован и `_t` уже строго типизирован до этой правки + (строки 245, 1104 в текущем файле) — задача действительно восстанавливает + существующий контракт, а не создаёт новый. +- Ни `as never`, ни `as unknown as`, ни новых `any`, ни расширения + сигнатуры `_t`/`I18nKey` в diff нет — подтверждено чтением полного diff и + `no-new-any.mjs`. +- Ключи, тексты и ветвление визуально не изменились — единственные строки + diff это удаление/сужение каста; проверено чтением, не исполнением, для + всех 33 мест плюс подтверждено 8 зелёными браузерными смоками и `npm + test`, включая i18n parity/policy-тесты в общем прогоне. +- Mutation-anchor `preflight-reason-lost-in-dialog` (#295) актуализирован + корректно: `find`-строка патча совпадает буквально со строкой 9769 + текущего файла, `guard`/`because` не изменены, `node --test + test/mutation-gate.test.mjs` зелёный (уникальность/применимость мутантов + проверяется этим набором), `node demo/smoke_preflight_diagnostics.mjs` — + `OK`. +- Generated bundle (класс D) синхронизирован и совпадает побайтово: после + `npm run build && npm run bundle:sync` `git status` пуст. +- Трейлеры коммита корректны: `Issue: #391`, `User-Visible: no`; changelog + оправданно не тронут. +- Ремарка спек-ревью про два «нечистых» вычисляемых ключа + (`marker.value_badge_attr_${source.attribute}`, где `attribute: string`, + и `.replace()/.replaceAll()`, теряющие литеральный тип) — не новый риск: + тот же профиль (произвольная строка, приведённая к `I18nKey`) был и до + правки под `as any`; ТЗ сознательно разрешает это как единственно + доступный вариант, когда компилятор сам не выводит ключ. + +## Чего не проверял и почему + +- `npm run invariants` — diff не касается геометрии/модели (нет правок + толщины стен, `layout`, `marker.space`, `open_spans`, рёбер решётки). +- `python -m pytest tests_backend` — diff не касается + `custom_components/**/*.py`. +- `npm run golden:verify` — правка стирается на этапе компиляции + (`as any`/`as I18nKey` не производят рантайм-кода), рендер не может + измениться; смоки подтверждают отсутствие видимых эффектов дешевле, чем + полный golden-прогон. +- Полный набор из 208 браузерных смоков — не оправдано ни AC, ни объёмом + diff (одна поверхность, 33 каста в одном файле); прогнаны все 7 + «прямых совпадений» инструмента подбора плюс guard-смок мутанта — этого + достаточно для типовой правки такого размера. +- Performance-профили — не названы в AC, diff не касается чувствительных к + перфу путей. +- Ручное тестирование в браузере (UI) не проводилось — не требуется: + изменение не может изменить скомпилированную семантику, что подтверждено + выше техническим аргументом (стирание типовых кастов), а не только словом + автора. + +## Вердикт + +Единственная находка (M1) — Medium, в скоупе задачи, чинится в ней же: +`npm run build && node demo/docs/capture.mjs`, проверка, что кадры не +изменились, коммит обновлённого манифеста с теми же трейлерами. High +находок нет. Все AC из тела issue выполнены и подтверждены либо тестом, +либо построчным чтением с указанием, что именно проверено. Вердикт — +жёлтый.