mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 20:29:00 +00:00
claude[bot]
@@ -0,0 +1,176 @@
|
||||
# CODE-REVIEW-390-r1
|
||||
|
||||
Issue: #390 · Заход: r1 · Класс изменения: A (`src/**`) · Лёгкий трек (`small`)
|
||||
SHA под ревью: `cbf5cc1baa75df13cd45f6b7ad39c56e36837d42` (HEAD ветки `dev` на момент ревью)
|
||||
Дата: 2026-08-30
|
||||
|
||||
## Скоуп
|
||||
|
||||
#390 — типовой долг, найденный при разборе диапазона #388: гейт `no-new-any`
|
||||
(#342) увидел пять мест с явным `any`, потому что диапазон пуша временно
|
||||
оказался широким (~80 коммитов). Задача: для каждого из пяти мест —
|
||||
настоящий тип либо обоснование `// any-ok: <причина>`. Спецификация лежит в
|
||||
теле issue (лёгкий трек, файла в `docs/specs/` нет — процессный гейт это и
|
||||
предупреждает, и допускает).
|
||||
|
||||
Пять мест:
|
||||
1. `houseplan-card.ts:12080` — `_climateCache: { h: any; r: any; mk: any; ... }`
|
||||
2. `houseplan-editor-runtime.ts:11828` — `Object.values<any>(full?.entities || {})`
|
||||
3. `houseplan-editor-runtime.ts:11831` — `Object.values<any>(full?.devices || {})`
|
||||
4. `houseplan-editor-runtime.ts:12063` — `this.host._t('device_inbox.reason_excluded_integration' as any, ...)`
|
||||
5. `houseplan-editor-runtime.ts:12065` — `` this.host._t(`device_inbox.reason_${row.reason}` as any) ``
|
||||
|
||||
Реализовано одним коммитом `cbf5cc1b`, трейлеры `Issue: #390` ·
|
||||
`User-Visible: no` на месте. `User-Visible: no` корректен: правка стирается
|
||||
при компиляции, эмит бандла байт-в-байт идентичен (см. ниже) — changelog не
|
||||
требуется, и его действительно нет в диффе.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Готового зелёного прогона Validate на этом SHA нет — гейты прогнаны мной,
|
||||
дёшево и на точном SHA `cbf5cc1b`.
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| typecheck | `npx tsc --noEmit` | чисто, без вывода |
|
||||
| unit-тесты | `npm test` | **1635 pass, 0 fail, 1 skip** (совпадает с заявленным автором) |
|
||||
| сборка | `npm run build` | успешно, 15.5s |
|
||||
| сверка бандла (стенд) | `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | совпадают побайтово |
|
||||
| сверка бандла (воспроизводимость) | полная пересборка из исходников → `git status --short` пуст | дерево не отличается от закоммиченного — сборка детерминирована, эмит не изменился |
|
||||
| bundle:budget | `npm run bundle:budget` | 281785 B gzip / бюджет 300000 / запас 18215 — совпадает с заявленным |
|
||||
| no-new-any (#342) | `node scripts/no-new-any.mjs --base HEAD~1 --head HEAD` | «Новых any нет» — 22 добавленные строки в 2 файлах, чисто |
|
||||
| **check-docs** | `node scripts/check-docs.mjs` | **ERROR: screenshot source fingerprint is stale** — см. находку H1 |
|
||||
| smoke-select | `node scripts/smoke-select.mjs --base HEAD~1 --head HEAD` | 1 прямое совпадение: `demo/smoke_cover_tap.mjs` (символ `Marker`) |
|
||||
| smoke прямого совпадения | `node demo/smoke_cover_tap.mjs` | **OK**, все 32 ассерта зелёные (после `echo "127.0.0.1 demo.local" >> /etc/hosts` и `npm run bundle:sync` — песочница без DNS-записи, тот же класс ограничения, что в CODE-REVIEW-39-r1/302-r2) |
|
||||
|
||||
Не прогонял и почему:
|
||||
- `golden:verify` — diff не меняет видимый результат (типы стираются при
|
||||
компиляции, бандл идентичен байт-в-байт), AC этого не требует;
|
||||
- остальные `demo/smoke_*.mjs` — `smoke-select` не назвал других совпадений,
|
||||
AC не называет смоков, диффа в геометрии/UI нет;
|
||||
- `python -m pytest tests_backend` — diff не трогает `custom_components/**/*.py`;
|
||||
- `node scripts/model-invariants.mjs` — diff не трогает геометрию, `layout`,
|
||||
`marker.space`, `open_spans`, ссылки на стены;
|
||||
- performance-профили — не названы в AC, диффа в горячих путях рендера нет.
|
||||
|
||||
«Одно число — один источник»: неприменимо, diff не добавляет и не меняет
|
||||
видимую пользователю величину — это чисто типовая правка, значения времени
|
||||
исполнения не меняются (подтверждено байт-в-байт идентичным бандлом).
|
||||
|
||||
## Находки
|
||||
|
||||
### H1 (High) — `check-docs` красный на этом SHA, гейт не прогнан и не назван автором
|
||||
|
||||
`node scripts/check-docs.mjs` на `cbf5cc1b` завершается ошибкой:
|
||||
|
||||
```
|
||||
ERROR screenshot source fingerprint is stale; run npm run build && node demo/docs/capture.mjs
|
||||
```
|
||||
|
||||
Проверено, что причина — именно этот коммит, а не унаследованный долг:
|
||||
откат только двух правленых файлов (`src/houseplan-card.ts`,
|
||||
`src/houseplan-editor-runtime.ts`) к состоянию `HEAD~1` при прочем дереве на
|
||||
`HEAD` возвращает `check-docs` в зелёное состояние («Documentation checks
|
||||
passed»). Значит непосредственно перед этим коммитом `dev` был зелёным по
|
||||
этому гейту, а сам коммит его сломал.
|
||||
|
||||
`PROCESS.md` §8 называет `check-docs` обязательным **при любом diff по
|
||||
`src/**`** без исключений («не по важности, а по механике: отпечаток
|
||||
считается по всему `src/**`... выборка «по diff и AC» здесь не работает —
|
||||
diff всегда попадает, и решать нечего») и прямо описывает цену пропуска:
|
||||
«скриншоты не пересняли в #230 и #234, и `dev` стоял с красным job `docs`,
|
||||
пока это не нашли при следующей задаче (#237)». Это тот же класс инцидента:
|
||||
коммит уже слит в `dev` (ветка ревью — сам `dev`, отдельного PR нет), гейт не
|
||||
прогнан и не упомянут ни в хендофф-комментарии автора, ни как сознательно
|
||||
пропущенный — то есть сейчас `dev` красный по job `docs`, и без этого ревью
|
||||
находка осталась бы незамеченной до следующей задачи, как в #230/#234.
|
||||
|
||||
Воспроизведение:
|
||||
```
|
||||
git checkout HEAD~1 -- src/houseplan-card.ts src/houseplan-editor-runtime.ts
|
||||
node scripts/check-docs.mjs # → "Documentation checks passed (7 files, 10 external links)."
|
||||
git checkout HEAD -- src/houseplan-card.ts src/houseplan-editor-runtime.ts
|
||||
node scripts/check-docs.mjs # → "ERROR screenshot source fingerprint is stale"
|
||||
```
|
||||
|
||||
Требуемое действие (по `PROCESS.md` §8): пересъёмка через джобу `Docs
|
||||
screenshots` (`workflow_dispatch`) и приёмка `npm run docs:accept --
|
||||
--reviewed --from=<артефакт>`, коммит вместе с задачей — руками, не мной
|
||||
(«коммит делает человек»). Это блокирует зелёный вердикт: без него `dev`
|
||||
остаётся в красном состоянии по обязательному гейту.
|
||||
|
||||
### Информационно, не находка — 26 таких же `as any` вне скоупа
|
||||
|
||||
В `houseplan-editor-runtime.ts` тот же паттерн (`as any` на литеральных и
|
||||
шаблонных ключах `device_inbox.*`) остаётся ещё в 25 строках (26 вхождений,
|
||||
строка 12055 — два каста). Автор сам обнаружил и назвал это в комментарии,
|
||||
явно оставив решение ревьюеру («В скоуп этого issue не тащил — заведу
|
||||
отдельным, если скажете»). Это Medium **вне скоупа** #390 (он был заведён под
|
||||
пять конкретно перечисленных мест) — заведено отдельным issue со ссылкой на
|
||||
#390: **#391** (`tech-debt`, `P3`, `S1-new`). Оставлять это только текстом
|
||||
ревью было бы нарушением §12 («Оставили в тексте ревью» закрытием не
|
||||
считается), поэтому issue заведён явно, а не просто упомянут.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Все пять мест закрыты типом, не заглушкой.** Ни одного `// any-ok`;
|
||||
каждое место получило либо настоящий тип, либо снятие каста там, где он не
|
||||
требовался. Проверено чтением и подтверждено компиляцией (`tsc --noEmit`
|
||||
чист — будь тип неверным относительно фактического использования, сборка
|
||||
бы упала).
|
||||
- `_climateCache.h: unknown` — вычитано по коду (`houseplan-card.ts:12111`):
|
||||
поле сравнивается только `c.h === planHass`, значение никогда не
|
||||
читается по содержимому. `unknown` корректен и строже `any`: он не
|
||||
позволил бы случайно прочитать содержимое, если бы кто-то попытался.
|
||||
- `_climateCache.r: CompiledIconRule[] | undefined`, `mk: Marker[] | undefined` —
|
||||
типы взяты из фактических источников присваивания
|
||||
(`this._iconRules`, `this._serverCfg?.markers`), других присваиваний
|
||||
полю в файле нет.
|
||||
- `_registryIntegrations()` — минимальная структурная форма
|
||||
`{ entities?: Record<string, { platform?: unknown }>; devices?: Record<string, { identifiers?: unknown[] }> }`
|
||||
описывает ровно два поля, которые код читает (`reg?.platform`,
|
||||
`device?.identifiers?.[0]`), оба защищённо через опциональную цепочку —
|
||||
формой сужена только точка входа, старая логика (`String(...)`,
|
||||
`Array.isArray`) не тронута.
|
||||
- `'device_inbox.reason_excluded_integration'` — ключ присутствует в
|
||||
словаре i18n (иначе `tsc` не прошёл бы без каста); снятие `as any`
|
||||
оправдано.
|
||||
- `` `device_inbox.reason_${row.reason}` as I18nKey `` — `row.reason` имеет
|
||||
тип `DeviceInboxReason` (12 литералов, `src/device-inbox.ts:13-18`), т.е.
|
||||
шаблонный литерал — не произвольная строка; `as I18nKey` того же вида уже
|
||||
используется в этом файле (`_rszReasonText`, строка 3703) — стиль
|
||||
последователен.
|
||||
- **Поведение не изменилось** — не заявление, а измерено: полная
|
||||
пересборка из исходников дала дерево, побайтово совпадающее с
|
||||
закоммиченным (`git status --short` пуст после `npm run build`), и
|
||||
`dist/houseplan-card.js` совпадает со стендовой копией
|
||||
(`custom_components/houseplan/frontend/houseplan-card.js`). Аннотации
|
||||
типов действительно стёрты emit-нейтрально.
|
||||
Сообщение автора о том, что `as any` на первом аргументе `_t(...)`
|
||||
«заодно снимал контроль и со второго» — не перепроверял отдельно
|
||||
(в TS проверка второго параметра идёт независимо от типа первого); на
|
||||
корректность правки это не влияет, `tsc` прошёл начисто в любом случае.
|
||||
- **Трейлеры на месте**: `Issue: #390`, `User-Visible: no` — оба
|
||||
корректны, changelog не тронут и не должен быть тронут.
|
||||
- **Гейт #342 (`no-new-any`) отвечает за свою часть корректно** — новых
|
||||
`any` в добавленных строках нет, независимо подтверждено мной, а не
|
||||
принято на слово.
|
||||
- Геометрия/`layout`/`marker.space`/`open_spans` не затронуты —
|
||||
`model-invariants` неприменим, проверено чтением диффа (только сигнатуры
|
||||
типов кэша климата и реестра HA).
|
||||
|
||||
## Унаследовано (для сведения; это первый заход по #390, наследовать не от чего)
|
||||
|
||||
Не применимо — заход r1.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Единственная блокирующая находка (H1) не в правильности типовой правки — она
|
||||
верна, доказана компиляцией и байт-в-байт идентичным бандлом — а в
|
||||
обязательном гейте `check-docs`, который этот же коммит сделал красным на
|
||||
`dev` и который не был прогнан и не назван автором. Это ровно то, что
|
||||
`PROCESS.md` §8 называет ценой пропуска на конкретных примерах (#230, #234,
|
||||
#237): гейт нужно чинить в той же задаче, не переносить на следующую.
|
||||
|
||||
Вердикт: жёлтый · заход r1 · блокирующих циклов 1/2 · High: 1 · Medium: 1 → #391
|
||||
Документ: docs/reviews/CODE-REVIEW-390-r1.md
|
||||
Reference in New Issue
Block a user