diff --git a/docs/reviews/SPEC-REVIEW-371-r1.md b/docs/reviews/SPEC-REVIEW-371-r1.md new file mode 100644 index 00000000..c3f193e4 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-371-r1.md @@ -0,0 +1,222 @@ +# SPEC-REVIEW-371-r1 + +Issue: [#371 — Add French localization files](https://github.com/Matysh/houseplan-card/issues/371) +Трек: `small` (лёгкий), заход r1, лимит циклов лёгкого трека — 2, израсходовано 0/2 (жёлтый вердикт первого захода расходует цикл; см. §4 PROCESS.md). +Ревьюер: Claude (роль «ревьюер ТЗ»), автор ТЗ: Codex (issue-комментарий от 2026-08-29). +Материал: тело issue #371 на момент разбора (комментарий "S2→S3" от Matysh, 2026-08-29T09:59:53Z), без файла в `docs/specs/` — соответствует метке `small`. + +## Скоуп + +Добавление французского как четвёртого полноценного UI-языка (после en/ru/de), +на инфраструктуре реестра локалей #62 и лениво-загружаемого немецкого +прецедента #348. Контракт (К1–К4 в теле issue): файлы словарей +(`src/i18n/fr.json`, `src/i18n/fr.ts`, `custom_components/houseplan/translations/fr.json`), +запись в `registry.ts` + правки `bundle-manifest.mjs` (retry-токен, `_role`, +regex), авто-выбор по HA-локали, правки USER-GUIDE и changelog. AC1–AC5 с +указанием доказательства (unit/smoke/gate). User-Visible: yes. + +## Как проверялось + +1. Прочитан `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком — критерии + лёгкого трека (§5), обязательные разделы ТЗ (§7.1) и упрощённый шаблон для + `small` (проблема · контракт · AC · откат). +2. Прочитано тело issue #371 и оба комментария владельца (принятие вклада, + S2→S3 с обоснованием трека). +3. Сверены технические утверждения ТЗ с текущим кодом на `dev` (`914bb4ed`): + `src/i18n/registry.ts`, `src/i18n/de.ts`, `scripts/bundle-manifest.mjs`, + `test/i18n.test.mjs`, `docs/USER-GUIDE.md` / `.ru.md`, `docs/CHANGELOG.ru.md`, + история коммитов `12cd8a1a` (German locale), `78c60207`/`4a241cef` (#353, + entry-fallback). +4. Арифметически перепроверена «оценка вклада»: `en.json` на `dev` — + 1126 ключей; 1026 (в присланном словаре) + 121 (недостающих) − 21 + (устаревших) = 1126 — сходится, догадкой не является. +5. Проверено предыдущее полноформатное ТЗ немецкой локализации (#348, issue + + `docs/specs/348-german-localization.md`) как прецедент трека — German шёл + **полным** треком (первое использование инфраструктуры #62 для реального + языка), French заявлен `small`, ссылаясь на то, что German эту + инфраструктуру уже обкатал. Обоснование трека признано состоятельным (см. + ниже). +6. Прочитан `demo/smoke_german_locale.mjs` — файл существует, ссылка АС3 не + висит в воздухе (`ls demo/smoke_*.mjs` → 202 файла). + +Гейты (typecheck/test/build/scripts) на этом этапе не запускались — этап +`spec`, продуктового кода ещё нет, запускать нечего. + +## Находки + +### M1 (Medium, в скоупе) — К2 не покрывает второй файл в bundle-manifest.mjs, который тоже хардкодит список локалей + +`scripts/bundle-manifest.mjs` содержит **два** независимых места, где список +UI-языков зашит по именам, а не выводится из `LANGUAGE_REGISTRY`: + +- `bundleManifestPlugin`/`editorRuntimeRetryUrlPlugin` — то, что перечисляет + К2 (роль `'locale'` по `/src/i18n/de.ts`, `DE_RETRY_ASSET_TOKEN`, regex + `de-`). Это ТЗ покрывает: явно расписано, что для `fr` заводится параллельный + токен, парная роль и обобщённый regex `(de|fr)-`. +- `entryFallbackPlugin` (строки 192–224, конкретно тернарник строк 208–212): + локализованный текст «страница устарела, перезагрузите» выбирается по + `navigator.language`, но веткой обёрнуты только `ru` и `de` — любой другой + язык, включая ещё не существующий на момент написания этого кода, молча + получает английский текст. Функция добавлена **после** German (#348) — + коммитом `78c60207`/`4a241cef` (issue #353, "lazy delivery survives flaky + networks and stale caches"), и её ветки `ru`/`de` — прямое следствие того, + что на тот момент это были все три UI-языка. Иными словами: у проекта уже + есть прецедент "третий язык получает свою ветку в этом файле", и French + станет четвёртым языком без такой ветки, если К2 останется как есть. + +ТЗ формулирует К2 как исчерпывающий список правок именно в +`bundle-manifest.mjs`: "FR-токен в retry-плагине..., `_role: 'locale'`..., +regex localeRoots обобщается...". Это выглядит как полный список изменений +файла, но пропускает второй хардкод в том же файле. Ни один AC (1–5) не +покрывает entry-fallback текст — при код-ревью эта ветка просто не будет +исполнена ни одним тестом, и дыра проедет в релиз незамеченной, ровно как +исчезновение записи толщины или несинхронного числа: не сбой теста, а тихое +несоответствие тому, что #353 уже установил как контракт «полноценный язык — +значит, локализован transaction-fallback тоже». + +**Что нужно решить, не владельцу.** Это техническое, не продуктовое: добавить +`l.startsWith("fr") ? "House Plan a été mis à jour — veuillez recharger la +page (Ctrl+F5)."` веткой рядом с `ru`/`de`, обновить троекратную проверку +`editorReplacements`/`onboardingReplacements`/`germanReplacements` на четвёртый +счётчик — либо явно записать в ТЗ решение «оставляем английский текст для +French в этой ветке» с причиной (например: сама ветка emergency-only и не +входит в обещание «полноценный язык»). Молчание — не то и не другое. + +### M2 (Medium, в скоупе) — AC1 не решает судьбу двух German-специфичных тестов качества, добавленных как часть оригинального AC #348 + +`test/i18n.test.mjs` содержит **шесть** паритетных тестов, зациклённых по +`LANGUAGE_REGISTRY` (строки 26–133) — они действительно "расширяются на fr" +простым фактом попадания `fr` в реестр, как обещает AC1. Но там же есть **два** +теста, написанных именно под немецкий и не читающих реестр вообще: + +- `i18n: German catalog keeps the product glossary and has no translation + sentinels` (строки 327–338) — точечные assert'ы конкретных немецких строк + (`de['btn.save'] === 'Speichern'` и т.п.) плюс скан на маркеры машинного + перевода (`ZXQPH`, `QXZ`, `⟦HP`) и на кириллицу; +- `i18n: German values equal to English are explicitly reviewed` (строки + 340–370) — сверяет множество ключей, где `de[key] === en[key]`, с + жёстко прописанным allow-list; появление нового совпадения красит тест — + это ловит именно тот класс дефекта, который «0 расхождений плейсхолдеров, 0 + подозрительного контента» из оценки вклада не проверяет: пропущенный, + скопированный из английского ключ, который синтаксически валиден (не пуст, + плейсхолдеры совпадают), но семантически не переведён. + +`git log -S` подтверждает: оба теста появились **в одном коммите** с самой +немецкой локализацией (`12cd8a1a`), то есть были частью изначального AC +German-задачи, а не более поздней надстройкой поверх неё. Раз ТЗ AC1 заявляет +«существующий тестовый механизм расширяется на fr» как единый факт, а по +факту половина механизма (родовые тесты) расширяется бесплатно, а другая +половина (эти два теста) не расширяется вообще — это ровно тот случай +«утверждение о поведении, которого нет ни в одном документе, выдано за +решение» из инструкции ревью, только с обратным знаком: тут не придумано +поведение, а не поставлен вопрос о нужном. + +**Что нужно решить.** Либо АС1 явно включает french-аналоги этих двух тестов +(глоссарий ключевых терминов + скан на артефакты машинного перевода/кириллицу ++ allow-list явных совпадений с английским для `fr`), либо ТЗ прямо +проговаривает, что для French эта дополнительная проверка не пишется сейчас и +почему (например: контрибьютор уже прогнал скрипт с эквивалентной +эвристикой, а нативная проверка обещана постфактум — согласно комментарию +владельца в issue). Оба варианта закрывают находку; отсутствие любого из них — +нет. + +### L1 (Low, снимается ревьюером с записью) — формулировка AC5 избыточна и путает мутанта + +"мутант «fr-словарь исключён из паритета»... формулировка проще: мутант +«запись fr удалена из LANGUAGE_REGISTRY»" — два предложения описывают, по +всей видимости, один и тот же мутант разными словами, оставляя +неопределённость, сколько мутантов реально проверяется. Смысл читаем (мутация +реестра должна покраснить AC2 и AC3), содержательной проблемы нет — это +стилистическая шероховатость, не блокирует. Снимаю без правки: при +реализации это разрешится тем, что действительно будет один тест на +исчезновение записи `fr`. + +### L2 (Low, снимается ревьюером с записью) — DoR-пункты touch/perf не проговорены явным предложением + +DoR (§2.5 PROCESS.md) требует явно назвать влияние на perf и touch (или явное +«нет»). ТЗ покрывает perf через AC4 (`bundle:budget`), но нигде не пишет +фразой «touch не затронут». Для чисто словарной правки без нового UI это +самоочевидно и уже подтверждается критериями лёгкого трека («нет влияния на +touch-контракт» — один из пяти одновременных критериев `small`, названных в +самом ТЗ). Не блокирует, добавить одну строку не помешает, но не обязательно. + +## Что проверено и корректно + +- **Классификация трека `small`.** Все пять критериев §5 проверены по + отдельности: сложность/риск (единственная точка расширения — реестр, + спроектированная под это самим автором инфраструктуры, судя по комментарию + в `registry.ts`: «Adding a locale means adding its frontend/backend JSON + files and one static entry»); одна поверхность (locale pipeline); миграции + конфига нет (язык — уже существующее строковое поле, `languageOptions` + сохраняет неизвестные сырые значения); нового UX-контракта нет (селектор + языка уже существует, просто получает четвёртый пункт); влияния на + perf/touch не заявлено и не просматривается в диффе, который описывает ТЗ. + Прецедент German (#348) шёл полным треком именно потому, что обкатывал + инфраструктуру впервые — довод, что второй проход по уже проверенному + пути дешевле, содержателен, а не удобная отговорка. +- **К1 (файлы).** Арифметика вклада проверяется: 1026 (снимок контрибьютора) + + 121 (новые ключи) − 21 (устаревшие) = 1126 = точное число ключей в + `src/i18n/en.json` на `dev` сейчас. Это не бездоказательная цифра. + Backend-паритет «6/6» тоже сходится: `custom_components/houseplan/translations/en.json` + действительно содержит 6 листовых ключей. +- **К2 (registry.ts).** Описанный `loadFrench`/токен/запись в + `LANGUAGE_REGISTRY` — точная копия существующего `loadGerman` + (`src/i18n/registry.ts:20-25,33-37`) и `src/i18n/de.ts` (клон тривиален: + импорт JSON + экспорт fingerprint-токена). Технически выполнимо без + дополнительных решений. +- **К2 (bundle-manifest.mjs, заявленная часть).** Проверено построчно: + `_role: 'locale'` определяется по `endsWith('/src/i18n/de.ts')` + (`scripts/bundle-manifest.mjs:32`), `localeRoots` — по этой роли или + regex `de-` (`:61-62`), `DE_RETRY_ASSET_TOKEN` и проверка счётчика замен + 1/1/1 в `editorRuntimeRetryUrlPlugin` (`:9,141-172`). Всё, что ТЗ обещает + здесь исправить, действительно требует исправления и действительно + ограничивается перечисленными тремя точками (не считая M1). +- **К3 (авто-выбор).** `resolveLanguageCode` (`src/i18n/registry.ts:119-136`) + уже обобщён — точное совпадение, затем primary-код (`locale.split('-')[0]`). + Существующий юнит-тест `i18n.test.mjs:135-152` доказывает это для + `de-DE`/`de-AT`/`de-CH` без отдельных записей на каждый диалект в реестре — + то же самое автоматически сработает для `fr-FR`/`fr-CA`/`fr-BE`/`fr-CH` без + дополнительного кода. Утверждение ТЗ «существующая нормализация... юнит + фиксирует» не голословно. +- **К4 (доки).** Абзац auto-языка в `docs/USER-GUIDE.ru.md:153-157` и + зеркальный в `docs/USER-GUIDE.md` действительно построен по шаблону + «`auto` учитывает... `de`, `de-DE`, `de-AT` и `de-CH`» — French по той же + форме добавляется без изобретения новой терминологии. Прецедента + благодарности контрибьютору по нику в changelog нет, но и противоречия + стилю нет — это первый внешний вклад такого рода, и тон уже задан + комментарием владельца в issue. +- **АС3 (смок).** `demo/smoke_german_locale.mjs` существует — ссылка ТЗ не + на пустое место. +- **Продуктовая рамка.** Локализация интерфейса не входит буквальной строкой + в J1–J7 `docs/SCOPE.md`, но прецедент #62/#348 уже прошёл этот вопрос: #62 + специально спроектирован как обобщённая инфраструктура под будущие языки, а + German — первый язык, принятый на нём с P1. Открывать этот вопрос заново на + четвёртом языке для того же самого пайплайна избыточно. +- **Догадки, выданные за факт.** Не найдено ни одного утверждения о + поведении, которого нет в коде/документах и которое не помечено как + предположение. Оба технических пробела (M1, M2) — это не придуманное + поведение, а нерешённые точки контракта; это ровно та категория, которую + инструкция просит решать самому ревьюеру, а не выносить владельцу. + +## Чего не проверял + +- Не открывал приложенный контрибьютором zip-архив (`houseplan-fr-translations-only.zip`) + — на этапе ТЗ файлов French в репозитории ещё нет (`ls src/i18n/` подтверждает: + только `de.json/de.ts/en.json/ru.json/registry.ts/language-runtime.ts`), проверка + реального содержимого словаря — предмет код-ревью, не ТЗ. +- Не запускал `npm run typecheck`/`test`/`build` — класс A ещё не тронут, + фиксировать зелёный прогон не на чем. Это будет обязательным гейтом + код-ревью (#371, следующий этап), не этого. +- Не проверял `npm run bundle:budget` вживую (нет собранного бандла с + French) — числовой бюджет AC4 по своей природе проверяется только после + реализации. +- Не оценивал лингвистическое качество французского перевода — вне + компетенции ревью ТЗ; process явно оставляет финальную проверку носителем + языка на постфактум-ревью после беты (см. комментарий владельца в issue). + +## Вердикт + +Жёлтый: 0 High, 2 Medium в скоупе (M1, M2), оба чинятся правкой текста ТЗ в +теле issue — не отдельным issue, без блокировки владельцем. Автор либо +дополняет К2/AC1 решением по двум названным пробелам, либо явно фиксирует, +почему они не нужны сейчас, и повторно выставляет `S4-spec-review`.