15 KiB
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до/после диффа): 26device_inbox.*+ 6marker.*+ 1gs.preflight_reason_*— все убраны; ни один каст той же строки за пределами трёх заявленных семейств не тронут (проверено:tap.*,fill.*,decor.*,junction.*,vac.*,run.*и genericmessage/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:syncgit 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 выполнены и подтверждены либо тестом,
либо построчным чтением с указанием, что именно проверено. Вердикт —
жёлтый.