mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 12:18:51 +00:00
docs: review document for #62
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 2m36s
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 1m13s
Проверка (CI) / Классификация изменённых файлов (push) Successful in 1m43s
Проверка (CI) / HACS: валидация репозитория (push) Skipped
Проверка (CI) / Hassfest: манифест интеграции (push) Skipped
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Skipped
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Skipped
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 2m36s
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 1m13s
Проверка (CI) / Классификация изменённых файлов (push) Successful in 1m43s
Проверка (CI) / HACS: валидация репозитория (push) Skipped
Проверка (CI) / Hassfest: манифест интеграции (push) Skipped
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Skipped
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Skipped
Issue: #62 User-Visible: no
This commit is contained in:
@@ -0,0 +1,189 @@
|
||||
# CODE-REVIEW-62-r2
|
||||
|
||||
Issue: #62 — Инфраструктура i18n: единый реестр языков и автоматические проверки
|
||||
Этап: code (PROCESS.md §2.7)
|
||||
Заход: r2 · блокирующих циклов израсходовано 1 из 4
|
||||
SHA под ревью: `c755a0ef` (диапазон `origin/dev..HEAD`, 7 коммитов)
|
||||
Предыдущий раунд: код-ревью r1, вердикт **красный**, SHA `6540474f`, High: 1, Medium: 0.
|
||||
|
||||
## Скоуп раунда
|
||||
|
||||
Раунд по дельте (PROCESS.md §2.9/§2.10). Дельта `6540474f..c755a0ef` — один
|
||||
commit `c755a0e` "fix: normalize i18n registry lookup keys":
|
||||
|
||||
```
|
||||
.../houseplan/frontend/houseplan-card.js | 4 +-
|
||||
dist/houseplan-card.js | 4 +-
|
||||
docs/images/screenshots.json | 22 +-
|
||||
docs/reviews/CODE-REVIEW-62-r1.md | 270 +++++++++++++
|
||||
src/i18n/registry.ts | 13 +-
|
||||
test/i18n.test.mjs | 12 +
|
||||
6 files changed, 306 insertions(+), 19 deletions(-)
|
||||
```
|
||||
|
||||
Дельта строго локальна: один production-файл (`src/i18n/registry.ts`), один
|
||||
тест-файл, регенерированные bundle-копии и документационный fingerprint
|
||||
(следствие пересборки), плюс коммит предыдущего ревью-документа (артефакт
|
||||
конвейера, не код). Ребейза на ушедший вперёд `dev` не было — `dev` не
|
||||
двигался под веткой между раундами (проверено: коммит `6540474f`, на котором
|
||||
получен r1, лежит в текущей истории `HEAD` без слияний). Новая подсистема не
|
||||
затронута, контракт поведения не меняется — коммит явно чинит единственную
|
||||
находку r1. Разбор по границе «находка r1 плюс всё, до чего дотягивается
|
||||
дельта» — полный прогон AC1–AC9 заново не требуется.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **H1.** `LANGUAGE_BY_CODE` строился как `new Map(LANGUAGE_REGISTRY.map(entry => [entry.code, entry]))` — ключ - «сырой» `entry.code` (canonical spelling, например `pt-BR`), а `languageEntry()` ищет по `normalizeLanguageTag()` (lowercase). Для `en`/`ru` совпадение случайное; для будущего `pt-BR`/`zh-Hant` `languageEntry()`/`t()`/`hasTranslation()` молча откатывались бы на English без ошибки и без падающего теста. | Ключ и поиск теперь проходят через одну и ту же чистую функцию `normalizeLanguageTag`: добавлена `buildLanguageLookup<T extends {code: string}>(entries)`, которая строит `Map` по `normalizeLanguageTag(entry.code)`; `LANGUAGE_BY_CODE = buildLanguageLookup(LANGUAGE_REGISTRY)`; `languageEntry()` не изменился — `LANGUAGE_BY_CODE.get(normalizeLanguageTag(value))`. Поскольку обе стороны сопоставления проходят через одну и ту же функцию, совпадение гарантировано структурно для любого будущего кода, а не только для сегодняшних `en`/`ru`. | `src/i18n/registry.ts:26-45` (diff `6540474f..c755a0ef`). Добавлен тест `test/i18n.test.mjs:43-51` — `buildLanguageLookup([{code:'pt-BR',...}])` резолвит `'pt-br'` и `normalizeLanguageTag('PT_br')`, плюс цикл `languageEntry(entry.code.toUpperCase())` по всей продакшн `LANGUAGE_REGISTRY`. |
|
||||
|
||||
Проверка не ограничилась чтением диффа. Собрал старую (r1, до фикса)
|
||||
реализацию `languageEntry`/`LANGUAGE_BY_CODE` в отдельном scratch-скрипте вне
|
||||
репозитория и прогнал реальный `normalizeLanguageTag` из собранного
|
||||
`test-build/i18n/registry.js` против неё:
|
||||
|
||||
```
|
||||
loop over registry (with synthetic pt-BR) with OLD languageEntry: FAILED -> mismatch for pt-BR
|
||||
+ undefined
|
||||
- { code: 'pt-BR', dictionary: {}, nativeLabel: 'x' }
|
||||
```
|
||||
|
||||
т.е. старая реализация действительно ломалась на смешанном регистре — H1
|
||||
воспроизводим, а не гипотетичен. С исправлением та же проверка (через
|
||||
`buildLanguageLookup`, которым сейчас реально построена `LANGUAGE_BY_CODE`)
|
||||
проходит: `test/i18n.test.mjs` зелен (см. «Гейты» ниже), и логика симметрична
|
||||
по построению — не привязана к тому, что сегодняшние `en`/`ru` уже lowercase.
|
||||
|
||||
**Важный нюанс, который стоит отметить, не как блокирующую находку.**
|
||||
Добавленный тест `i18n: canonical regional codes use normalized lookup keys`
|
||||
доказывает корректность самой функции `buildLanguageLookup` на синтетическом
|
||||
`pt-BR` — это единственный способ вообще проверить чувствительность к
|
||||
регистру, пока в продакшн-реестре только `en`/`ru` (оба уже lowercase, и
|
||||
поэтому цикл по реальному `LANGUAGE_REGISTRY` в этом же тесте не может
|
||||
отличить старое и новое поведение — я это тоже проверил прогоном: старая
|
||||
реализация проходит этот цикл на `en`/`ru` без единой ошибки). Однопроточное
|
||||
связывание `LANGUAGE_BY_CODE = buildLanguageLookup(LANGUAGE_REGISTRY)` —
|
||||
единственное место, которое реально несёт риск отката H1, и оно не покрыто
|
||||
тестом, который заметил бы регресс именно в этой строке при сегодняшних
|
||||
данных; страховка — то, что это буквально одна читаемая строка рядом с
|
||||
протестированным примитивом. Считаю это Low, не заводящим находку: раздвинуть
|
||||
покрытие потребовало бы либо держать в продакшн-реестре синтетическую запись
|
||||
не только в тестах, либо экспортировать внутреннее состояние ради теста —
|
||||
непропорционально риску одной строки композиции. Фиксирую для протокола, не
|
||||
блокирую.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Из `docs/reviews/CODE-REVIEW-62-r1.md` (SHA `6540474f`, вердикт красный,
|
||||
High: 1 — единственная находка закрыта выше) принято без повторной проверки:
|
||||
|
||||
- **AC3** (frontend/backend file-set parity), **AC4** (registry-driven
|
||||
key/placeholder/help-key parity), **AC5** (порядок Auto/English/Русский без
|
||||
ручного списка в `src/editor.ts`), **AC7** (значения словарей и видимый
|
||||
selector не меняются), **AC8** (нет async/dynamic import в production
|
||||
bundle), **AC9** (CONTRIBUTING описывает актуальный flow) — дельта
|
||||
`6540474f..c755a0ef` не касается ни `src/editor.ts`, ни словарей `en.json`/
|
||||
`ru.json`, ни `CONTRIBUTING.md`, ни путей загрузки/бандлинга; доказательства
|
||||
r1 остаются в силе.
|
||||
- Гейты r1 (typecheck/test/build/check-docs/`git diff --check`, сверка SHA-256
|
||||
трёх bundle-копий, ручная проверка единственного изменившегося PNG
|
||||
`06-device-editor.png` через `ImageChops.difference`) — перепрогнаны заново
|
||||
в этом раунде (см. ниже), не просто унаследованы, так как код изменился.
|
||||
- Отсутствие применимости `npm run invariants` и golden/backend/perf гейтов —
|
||||
унаследовано: дельта r2 такая же не геометрическая и не HA-Python, как и
|
||||
весь diff r1.
|
||||
|
||||
Не унаследовано, перепроверено заново в этом раунде: **AC1** (`Lang`,
|
||||
`langOf()`, options — из одного registry) и **AC2** (exact/normalize/fallback
|
||||
матрица разрешения языка) — именно они опираются на `languageEntry()`,
|
||||
затронутый фиксом. **AC6** (неизвестный сохранённый язык не ломает
|
||||
card/editor) косвенно затронут через `languageOptions()` (строка 90 вызывает
|
||||
`languageEntry(raw)` для подписи сохранённого чужого значения) — проверил
|
||||
чтением: ветка `!options.some(...)` выполняется только для значений, не
|
||||
совпадающих ни с одним зарегистрированным кодом буквально, так что для
|
||||
сегодняшних `en`/`ru` изменение не меняет наблюдаемое поведение AC6; для
|
||||
будущих canonical кодов фикс делает эту подпись корректной тем же
|
||||
структурным аргументом, что и AC1/AC2.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Зелёного Validate на SHA `c755a0ef` нет — прогнал гейты сам:
|
||||
|
||||
- `npx tsc --noEmit` — green.
|
||||
- `npm test` — green: `tests 1416 / pass 1415 / fail 0 / skipped 1` (единственный
|
||||
skip — `issue 281 private exact fixture has no enabled zero-range handle`,
|
||||
условный на отсутствующей приватной фикстуре, не связан с этой задачей и
|
||||
не связан с дельтой).
|
||||
- `npm run build` — green; сверил `dist/houseplan-card.js` и
|
||||
`custom_components/houseplan/frontend/houseplan-card.js` по SHA-256 — оба
|
||||
`17901d69...` — совпадают. Третья копия (`demo/srv/assets/houseplan-card.js`)
|
||||
генерируется рантаймом демо-сервера и в `.gitignore`, в бандл-инвариант не
|
||||
входит.
|
||||
- `node scripts/check-docs.mjs` — green: `Documentation checks passed (7
|
||||
files, 10 external links)`. Diff трогает `src/**` (`registry.ts`), поэтому
|
||||
гейт обязателен; изменившийся `docs/images/screenshots.json`
|
||||
(`sourceFingerprint`/`sourceSha256` пересчитаны на новый исходник,
|
||||
`imageSha256` всех сценариев не изменился) — согласован с этим гейтом.
|
||||
- `git diff 6540474f..c755a0ef --check` — green, без пробельных ошибок.
|
||||
- Трейлеры коммита `c755a0e`: `Issue: #62`, `User-Visible: no`. Соответствует
|
||||
факту — фикс не меняет наблюдаемое поведение сегодняшних `en`/`ru`
|
||||
(подтверждено неизменностью `imageSha256` во всех 9 сценариях
|
||||
`screenshots.json`), правок changelog не требуется и не сделано.
|
||||
|
||||
**Не прогонял и почему:**
|
||||
|
||||
- `npm run invariants` — diff не касается геометрии/`layout`/`marker.space`/
|
||||
`open_spans`; тот же вывод, что в r1.
|
||||
- `python -m pytest tests_backend -q` — diff не касается
|
||||
`custom_components/**/*.py` (проверил `git diff --stat` по этому пути —
|
||||
пусто).
|
||||
- `npm run golden:verify` — diff не может изменить видимый рендер: правка
|
||||
живёт внутри `languageEntry()`, применяется только когда сохранённый код
|
||||
языка отличается регистром/разделителем от canonical записи реестра;
|
||||
сегодняшний реестр (`en`, `ru`) уже canonical lowercase, поэтому видимый
|
||||
результат не меняется — независимо подтверждено `imageSha256` в
|
||||
`screenshots.json` (не изменился ни в одном из 9 сценариев).
|
||||
- Браузерные смоки — прогнал `node scripts/smoke-select.mjs --base 6540474f
|
||||
--head c755a0ef`:
|
||||
|
||||
```
|
||||
Изменено файлов src/**: 1 · символов проекта на изменённых строках: 5
|
||||
Матрица: 194 смоков · порог «широкого» символа: больше 38 смоков
|
||||
|
||||
НЕОПРЕДЕЛЁННОСТЬ: дифф исполняемый, но ни один смок не связан доказуемо.
|
||||
Символы, которых нет ни в одном смоке: LANGUAGE_BY_CODE, LANGUAGE_REGISTRY,
|
||||
LanguageEntry, buildLanguageLookup, normalizeLanguageTag
|
||||
```
|
||||
|
||||
То же НЕОПРЕДЕЛЁННОСТЬ, что и в r1 (тогда — по всему PR, сейчас — по этой
|
||||
более узкой дельте, символы те же). ТЗ §11 обоснованно исключает browser
|
||||
smoke для этой задачи; в r1 уже принято решение не прогонять — унаследовано,
|
||||
дельта не меняет этот вывод.
|
||||
- Performance-профили — не названы в AC, диф не трогает чувствительные к
|
||||
перфу пути.
|
||||
- «Одно число — один источник» — не применимо: дельта не добавляет и не
|
||||
меняет пользовательскую видимую величину (нет чисел, только строковый
|
||||
lookup языка).
|
||||
|
||||
## Находки
|
||||
|
||||
Нет находок High или Medium в скоупе. Один Low зафиксирован выше в разделе
|
||||
«Закрытие раунда r1» (тонкое место в покрытии регрессионного теста,
|
||||
непропорциональное риску одной строки композиции) — не блокирует, не
|
||||
заводится отдельным issue (не Medium и не вне скоупа).
|
||||
|
||||
## Вывод по AC
|
||||
|
||||
AC1, AC2 — перепроверены в этом раунде: доказаны чтением
|
||||
(`src/i18n/registry.ts:26-45`) плюс unit `test/i18n.test.mjs:43-51`, тест
|
||||
умеет падать (продемонстрировано прогоном старой реализации против того же
|
||||
входа — см. «Закрытие раунда r1»). AC6 — перепроверен чтением частично
|
||||
(затронутая ветка `languageOptions()`), для сегодняшних данных поведение не
|
||||
меняется. AC3–AC5, AC7–AC9 — унаследованы из r1 без повторной проверки, дельта
|
||||
их не касается.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. H1 из r1 закрыт корректно, фикс структурно общий (не завязан на то,
|
||||
что сегодня `en`/`ru` уже lowercase), гейты green, дельта локальна и не
|
||||
требует полного повторного разбора.
|
||||
Reference in New Issue
Block a user