mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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 выполнены и подтверждены либо тестом,
|
||||
либо построчным чтением с указанием, что именно проверено. Вердикт —
|
||||
жёлтый.
|
||||
Reference in New Issue
Block a user