Files
houseplan-card/docs/reviews/CODE-REVIEW-391-r1.md
2026-08-30 17:13:01 +00:00

15 KiB
Raw Permalink Blame History

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 выполнены и подтверждены либо тестом, либо построчным чтением с указанием, что именно проверено. Вердикт — жёлтый.