From eff786fafaa5b605e678ec2480536a6e3227ea61 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 00:02:11 +0000 Subject: [PATCH] docs: review document for #62 Issue: #62 User-Visible: no --- docs/reviews/CODE-REVIEW-62-r2.md | 189 ++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-62-r2.md diff --git a/docs/reviews/CODE-REVIEW-62-r2.md b/docs/reviews/CODE-REVIEW-62-r2.md new file mode 100644 index 00000000..dd5d624b --- /dev/null +++ b/docs/reviews/CODE-REVIEW-62-r2.md @@ -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(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, дельта локальна и не +требует полного повторного разбора.