docs: review document for #62

Issue: #62
User-Visible: no

docs/reviews/SPEC-REVIEW-62-r1.md

@@ -0,0 +1,197 @@
+# SPEC-REVIEW-62-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/62
+- **ТЗ:** `docs/specs/062-i18n-registry.md`
+- **Материал ревью:** ветка `issue/62-i18n-registry`, SHA `21d47c52`
+  (совпадает с SHA, названным автором в хендоффе на ревью)
+- **Заход:** r1 (первый формальный проход; issue шёл `S3-spec` с 2026-08-15,
+  ТЗ актуализировано и отправлено на `S4-spec-review` 2026-08-27)
+- **Трек:** полный. Аналитика и сам документ явно называют нарушенный критерий
+  `small` («одна поверхность») — задета class A i18n сразу в нескольких
+  поверхностях (registry, resolver, editor selector, frontend/backend gate,
+  CONTRIBUTING). Соответствует текущему умолчанию AGENTS.md (#338): отказ от
+  лёгкого трека обосновывается названным критерием, что и сделано.
+
+## Скоуп
+
+Диапазон `origin/dev...HEAD` содержит только два документных коммита класса C:
+`docs/specs/062-i18n-registry.md` (новый файл, 292 строки) и добавление одной
+строки-ссылки в `docs/specs/README.md`. Продуктового кода (class A) в диапазоне
+нет — это ожидаемо для этапа ТЗ, реализация ещё не начиналась.
+
+Задача: убрать дублирование списка языков (`src/i18n.ts` вручную задаёт `Lang`/
+`DICTS`, `src/editor.ts` вручную перечисляет `en`/`ru` в selector,
+`test/i18n.test.mjs` вручную проверяет только эту пару, frontend/backend
+translation-каталоги никак не сверяются) через единый `src/i18n/registry.ts`,
+без изменения видимого поведения English/Russian.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком.
+2. Прочитано тело issue #62 и все три комментария (аналитика 2026-08-14,
+   заведение ТЗ 2026-08-15, актуализация и отправка на ревью 2026-08-27) —
+   через `gh issue view` (MCP `get_issue`/`get_issue_comments` были недоступны
+   без выданного разрешения).
+3. Прочитан ТЗ `docs/specs/062-i18n-registry.md` целиком, сверен на разделы
+   §7.1 PROCESS.md.
+4. Каждое фактическое утверждение ТЗ о текущем коде сверено с реальным
+   деревом, а не принято на слово:
+   - `src/i18n.ts` — подтверждён ручной `Lang`, ручные imports, ручной `DICTS`,
+     докстрока действительно обещает добавление языка без правок TypeScript,
+     что не соответствует коду (`langOf`/`t`/`DICTS` требуют правки);
+   - `src/editor.ts` — подтверждён ручной перечень `en`/`ru` в
+     `selector.select.options` (строки 100–104), без обработки неизвестного
+     сохранённого значения;
+   - `test/i18n.test.mjs` — подтверждено прямое чтение `en.json`/`ru.json`,
+     без registry, без backend file-set проверки;
+   - `custom_components/houseplan/translations/` — подтверждён набор
+     `en.json`/`ru.json`, структурно отличный от frontend-словарей (вложенный
+     HA config_flow формат против плоского key→string) — ТЗ говорит о паритете
+     **набора файлов**, а не содержимого, поэтому структурное различие не
+     противоречит контракту;
+   - `tsconfig.test.json` — подтверждено отсутствие `src/i18n.ts` в `include`,
+     то есть заявленный в §6.4 шаг «включить в tsconfig.test.json» реален и
+     нужен; подтверждён существующий паттерн импорта скомпилированных модулей
+     тестами из `../test-build/*.js` (`npm test` = `tsc -p tsconfig.test.json`
+     → `node --test`), на который ТЗ опирается для registry-driven gate —
+     это не техническая догадка, а перенос уже работающего паттерна;
+   - `src/types.ts:276` — подтверждён комментарий
+     `language?: string; // 'en' | 'ru' | '' (auto — HA profile)`, который ТЗ
+     обещает исправить (цель 6);
+   - `CONTRIBUTING.md` — прочитан целиком (см. находку ниже);
+   - `docs/USER-GUIDE.ru.md` — единственное упоминание `language` (строка 129)
+     ограничено таблицей `auto`/`ru`/`en`; ТЗ не меняет эту строку, что
+     согласуется с заявлением «пользователь en/ru изменений не увидит»;
+   - `docs/CONFIG-COMPATIBILITY.md` — прочитан целиком; поле `language` не
+     геометрическое и не относится к категориям, которые реестр обязан
+     покрывать (модель стен/layout/marker.space), поэтому отсутствие записи в
+     этом реестре не является пробелом ТЗ.
+5. Грепом по `src/**` подтверждено, что знание о конкретных кодах `en`/`ru`
+   вне `src/i18n.ts`/`src/editor.ts`/`src/types.ts` больше нигде не зашито —
+   заявленный ТЗ список затронутых файлов полон.
+6. Продуктового кода в диапазоне нет, поэтому `typecheck`/`test`/`build` не
+   применимы к этому ревью — гонять их не на чем: диапазон не содержит ни
+   одного файла `src/**`/`custom_components/**/*.py`. `check-docs.mjs`
+   аналогично не запускался: скриншотный отпечаток зависит от `src/**`, а этот
+   диапазон его не трогает.
+
+## Находки
+
+### Medium (в скоупе задачи — чинится в этом же ТЗ)
+
+**M1. `CONTRIBUTING.md` останется противоречить новому contribution flow —
+AC9 не покрывает существующую строку.**
+
+- **Файл:** `docs/specs/062-i18n-registry.md`, §9 (и AC9, §10)
+- **Воспроизведение:** текущий `CONTRIBUTING.md` уже содержит утверждение
+  (раздел «Ground rules»):
+  > Adding a language = adding one JSON file + registering it in `src/i18n.ts`.
+
+  После реализации ТЗ добавление языка = frontend JSON + backend JSON + одна
+  запись в `src/i18n/registry.ts`; `src/i18n.ts` при добавлении языка больше не
+  редактируется (§6.1: «`src/editor.ts`, `langOf()` и список локалей в тестах
+  больше не редактируются» — но и сам `src/i18n.ts` тоже не редактируется,
+  поскольку registry выносится в отдельный модуль). §9 ТЗ описывает **новый**
+  раздел «Translations» в `CONTRIBUTING.md`, но нигде не говорит, что
+  существующую фразу в «Ground rules» нужно убрать или привести в соответствие.
+  Реализация, буквально следующая ТЗ, добавит верный раздел «Translations» и
+  оставит в том же файле, несколькими экранами выше, ложную инструкцию,
+  указывающую редактировать `src/i18n.ts` — то есть ровно тот файл, который
+  задача выводит из процесса добавления языка.
+- **Почему это находка, а не придирка:** AC9 формулируется как «CONTRIBUTING и
+  комментарии описывают фактический contribution flow» — с этим пробелом AC9
+  по букве может быть выполнен (новый раздел добавлен), а по смыслу нет:
+  документ будет противоречить сам себе, и следующий contributor, дочитавший
+  до «Ground rules» раньше «Translations», получит неверную инструкцию.
+- **Как закрыть:** добавить в §9 явное указание убрать/переписать эту строку
+  «Ground rules» так, чтобы она либо ссылалась на новый раздел «Translations»,
+  либо была удалена как дублирующая его.
+- **Серьёзность:** Medium, в скоупе (документация — часть DoD этой же задачи,
+  правка тривиальна, отдельный issue не заводится согласно #202).
+
+### Low (снимается с записью)
+
+**L1. AC2 не имеет явного маркера «Доказательство:» в отличие от остальных
+восьми AC.** Формулировка «покрыты матрицей unit-тестов» по смыслу эквивалентна
+`Доказательство: unit`, метод проверки не теряется. Косметика, не блокирует.
+
+**L2. AC5 и AC8 подмешивают в доказательство слово «inspection» (`inspection
+compiled schema`, `production build inspection») — не входит в канонический
+словарь `unit`/`backend`/`smoke`/`golden`/«ревью кода».** По содержанию это и
+есть «проверено чтением кода/бандла, не исполнением» — допустимая категория
+для code-review, но на этапе ТЗ стоило явно назвать её этим термином, а не
+свободным словом. Обе AC при этом дополнительно подкреплены конкретным
+автотестом (unit) или диффом, так что решение проверяемо в любом случае.
+Снимается без правки текста ТЗ.
+
+Оба Low не блокируют переход и не создают риска молчаливого пропуска: метод
+проверки в обоих случаях восстановим по тексту без домысливания.
+
+## Что проверено и признано корректным
+
+- Обязательные разделы §7.1 присутствуют все: сценарий, что человек увидит,
+  проблема, скоуп/не-скоуп, контракт поведения, UX, модель данных и миграция,
+  i18n, AC1–AC9 с доказательствами, план автотестов, риски, откат,
+  release-артефакты — плюс два необязательных (план реализации, «технические
+  предположения, можно менять»).
+- **Продуктовые вопросы отсутствуют по делу, а не по недосмотру.** Единственный
+  пограничный случай с пользовательской видимостью — что видит редактор
+  карточки при неизвестном сохранённом `language` (§6.3, §7, AC6) — решён
+  автором явно и обоснованно (защита от тихой потери значения при
+  редактировании несвязанного поля, в духе уже принятого в `docs/SCOPE.md`
+  принципа «не терять данные пользователя на догадке»), а не спрятан как
+  открытый вопрос. Формулировка достаточно точна для реализации и
+  автотеста без дополнительных уточнений: временная option появляется только
+  для незарегистрированного кода, исчезает после явного выбора, не переживает
+  несвязанное изменение формы в смысле «взаимодействие не должно её стереть».
+  Эскалации владельцу это решение не требовало — оно не меняет поведение
+  English/Russian и не вводит новый пользовательский сценарий, только
+  расширяет обработку уже существующего (произвольная строка в
+  `CardConfig.language`, доступная через YAML) случая.
+- Резолюция языка (§6.2: exact tag → primary subtag → English fallback,
+  регистронезависимо, `_`→`-`) при двух текущих локалях **эквивалентна**
+  нынешнему `l.startsWith('ru') ? 'ru' : 'en'` для всех обычных значений HA
+  locale (`ru`, `ru-RU`, `ru_RU`, `en`, `en-US`, что угодно ещё → `en`) —
+  проверено разбором обеих реализаций построчно. AC7 («видимый selector и
+  значения словарей не меняются») этим не нарушается.
+- Заявление «`en`/`ru` пользователь изменений не увидит» подтверждено:
+  единственная новая видимая ветвь (временная option) активируется только для
+  кода, отсутствующего в registry, то есть никогда для нынешних инсталляций.
+- Тестовая стратегия (§11) реалистична: паттерн «скомпилированный TS-модуль
+  импортируется тестом из `test-build/`» уже используется в проекте (проверено
+  на пяти существующих тестовых файлах), а не изобретается заново.
+- Non-scope (§5) закрывает именно те пункты, которые в issue были спорными —
+  явно снят lazy loading (ранее упомянутый в заголовке issue и вычеркнутый
+  самим автором из формулировки), явно исключены RTL, plural rules, Weblate/
+  Crowdin, привязка backend runtime к frontend registry.
+- Откат (§13) и release-артефакты (§14) заполнены осмысленно, не шаблонной
+  фразой; `User-Visible: no` обоснован отсутствием видимого изменения.
+- Трек и его обоснование (полный, а не `small`) соответствуют текущему
+  умолчанию AGENTS.md/#338 — критерий назван прямо.
+
+## Чего не проверял и почему
+
+- **Гейты `typecheck`/`npm test`/`npm run build`/`check-docs.mjs`/browser
+  smoke/golden/backend pytest** — не запускались. Диапазон `origin/dev...HEAD`
+  не содержит ни одного файла class A/B (`src/**`, `test/**`,
+  `custom_components/**`), только два файла `docs/**`. Гонять их не на чем:
+  это этап ТЗ, продуктовый код ещё не написан. Они будут первым осмысленным
+  гейтом на этапе код-ревью этой же задачи.
+- **Инварианты модели / `smoke-select.mjs`** — не применимо: диапазон не
+  трогает геометрию, `layout`, `marker.space`, `open_spans` и не содержит
+  browser-исполняемого кода.
+- **Реализуемость `Lang`, выведенного из `as const`-массива registry, без
+  отдельного union** — не проверялась компиляцией (кода ещё нет), только
+  разбором формулировки на внутреннюю непротиворечивость; это TypeScript-паттерн
+  без выявленных противоречий, но окончательное слово — у код-ревью, когда
+  появится реальный `.d.ts`.
+
+## Вердикт
+
+Один Medium-дефект в скоупе задачи (документация, AC9), без High. Дефект
+конкретный, воспроизводимый цитатой существующего файла и правится без
+пересмотра контракта — не требует нового цикла продуктового мышления, только
+дополнения §9 одной фразой про существующую строку `CONTRIBUTING.md`.
+
+**Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 →
+в задаче**