From 7a0702adea7c1639c573e03645f2c14cbe814b70 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 07:25:35 +0000 Subject: [PATCH] docs: review document for #348 Issue: #348 User-Visible: no --- docs/reviews/SPEC-REVIEW-348-r1.md | 205 +++++++++++++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-348-r1.md diff --git a/docs/reviews/SPEC-REVIEW-348-r1.md b/docs/reviews/SPEC-REVIEW-348-r1.md new file mode 100644 index 00000000..a67569c2 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-348-r1.md @@ -0,0 +1,205 @@ +# SPEC-REVIEW-348-r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/348 +- **Этап:** spec (PROCESS.md §2.4) +- **Заход:** r1 (первый и единственный документ ревью на этот issue) +- **Ветка / SHA материала:** `issue/348-german-localization` @ `2e650a02` + (один коммит `docs: specify German localization` поверх `dev`@`918fc9e2`) +- **Трек:** полный (обоснование в S2-analysis корректно: несколько + поверхностей, новый языковой/loading UX-контракт, влияние на perf budget + и touch — критерии §5 не проходят одновременно) +- **Вердикт: зелёный** + +## Скоуп ревью + +Диф `origin/dev..origin/issue/348-german-localization` — только класс C: + +``` +docs/specs/348-german-localization.md | 385 +++++++++++++++++++++++++ +docs/specs/README.md | 1 + +``` + +Продуктового кода нет, гейты code-уровня (`typecheck`/`test`/`build`) к +предмету ревью неприменимы — это ревью текста ТЗ, а не кода. Ниже — +раздел «Гейты и проверки» с тем, что фактически было прогнано для проверки +технических утверждений документа. + +## Как проверялось + +Ревью состязательное: документ прочитан целиком, каждое проверяемое +техническое утверждение сверено с реальным состоянием `origin/dev`, а не +принято на слово автора. + +1. **Зависимость #62.** Проверено `gh issue view 62` — `S8-merged`, то есть + инфраструктура (typed registry, locale resolution, parity gates) уже в + `dev`, а не гипотетическая. Прочитан `src/i18n/registry.ts`: `LANGUAGE_REGISTRY`, + `resolveLanguageCode` (exact → primary → fallback), `languageOptions` — + контракт locale resolution, который ТЗ обещает не менять, действительно + существует в описанном виде. +2. **Бюджет initial View.** Число «255 910 B gzip из 256 000 B» — не + принято на веру: собран бандл с `origin/dev` (`npm run build` + + `node scripts/bundle-budget.mjs`), результат: + `initial View: 255910 B gzip`, `lazy editor: 131779 B gzip` — **точное + совпадение** с ТЗ, включая экстремально узкий запас (90 B). `INITIAL_VIEW_GZIP_BUDGET + = 256_000` подтверждён в `scripts/bundle-budget.mjs`. +3. **Классификация lazy-групп в манифесте.** Прочитан + `scripts/bundle-manifest.mjs`: сейчас ровно два динамических класса + (`lazyOnboardingFiles` по имени чанка, всё остальное — `lazyEditorFiles`). + Требование ТЗ добавить третий класс `lazyLocaleFiles` — реальное изменение + этого файла, и он прямо назван в §12 «Затронутые файлы» — не забыт. +4. **Прецедент lazy-loading + bounded retry.** Найдены + `EDITOR_RETRY_ASSET_TOKEN` / `ONBOARDING_RETRY_ASSET_TOKEN` в + `scripts/bundle-manifest.mjs` и рабочие `await import('./houseplan-editor-runtime')` + / `import('./houseplan-onboarding-runtime')` в `src/houseplan-card.ts`, а + также браузерный тест `demo/smoke_lazy_editor_chunk.mjs` с перехватом + маршрута (`page.route`). План §6 (dedupe, page-lifetime cache, bounded + retry, fail-open English, fingerprint) и план тестов AC4/AC5 (задержанный + импорт, provoked failure) — не фантазия, а расширение уже работающего в + продукте паттерна на новый субъект (locale вместо editor/onboarding). +5. **Терминология и реальность строк.** Проверены по `src/i18n/en.json` + ключи, стоящие за глоссарием ТЗ: `markup.partition` → «Partition», + `markup.column` → «Column», `space.zero_wall_style` → «Zero-thickness + walls», `decor.fill` → «Fill», `space.glow_enabled`/`marker.glow_radius_label` + → «Glow» и т.д. — термины глоссария не изобретены, они соответствуют + реальным строкам продукта. Сверено с `docs/USER-GUIDE.ru.md`: «Space» → + «Пространство» в действующей русской терминологии, немецкое «Bereich» не + противоречит смыслу. +6. **Мёртвые legacy-ключи.** `editor.lang_en` / `editor.lang_ru` в + `src/i18n/en.json` существуют и не встречаются больше нигде в `src/*.ts` + и `test/*.mjs` — заявление §7 «доказанно неиспользуемые» подтверждено + поиском, а не принято на слово. +7. **Backend translations.** `custom_components/houseplan/translations/en.json` + реально содержит только `config`/`options`/`issues` — соответствует §3.1 + («config flow, options flow и repair issue»). +8. **CONFIG-COMPATIBILITY.md** не упоминает `language` вовсе — подтверждает + заявление ТЗ «`language` остаётся строковым полем без изменения schema и + миграции», а не пропуск темы. +9. **390 px как канонический touch-viewport.** Подтверждено по + `demo/smoke_*.mjs` (`card_controls`, `color_picker`, `gear_tabs`, + `plan_picker` и др. используют `width: 390`) — ссылка ТЗ на этот размер + в §8 не выдумана. +10. **Существование release-артефактов.** `docs/USER-GUIDE.md`, + `docs/USER-GUIDE.ru.md`, `docs/TESTING.md`, оба `CHANGELOG*.md` + существуют — §12/§15 указывают на реальные файлы. +11. **Обязательные разделы §7.1 PROCESS.md.** Сценарий (§1), что человек + увидит до/после (§2), скоуп/не-скоуп перевода (§3), контракт поведения + (§4–9), модель данных и миграция (§5, §14 — «миграция не нужна»), + AC1…AC10 с доказательством (§10), план автотестов (§11), риски (§13), + откат (§14), release-артефакты (§15) — присутствуют по существу, не + только по заголовку. + +## Находки + +Ни одной находки уровня High или Medium. Два Low-замечания, оба решением +ревьюера **сняты без правки** (документ остаётся годным для DoR), с записью +причины ниже — как того требует §12 PROCESS.md («Оставили в тексте ревью» +не считается закрытием, поэтому обе оставлены явной записью, а не +формулировкой «доработать»). + +### L1 — de-CH унаследует «ß», хотя стандартная швейцарская орфография его не использует + +`docs/specs/348-german-localization.md`, §4 правило 3 («Используются +настоящие ä, ö, ü, ß…») применяется к единственному словарю `de`, на который +по §5 резолвится и `de-CH`. Реальный швейцарский стандартный немецкий не +использует «ß» вовсе (пишет «ss»), поэтому носитель de-CH увидит +орфографически нетипичный для своего региона текст. + +**Почему не блокирует.** Это решение — прямое следствие уже принятого на +этапе S2-analysis и не оспоренного владельцем "safe default" — «de-DE/de-AT/de-CH +используют общий de» (единый словарь для регионов был явно объявлен и принят +молчанием владельца по §2.2 до написания ТЗ). Требовать от одного словаря +одновременно двух орфографий — уже отдельная, более крупная задача +(региональные варианты внутри одного языка), а не пробел этого ТЗ. Дефект +не функциональный: интерфейс остаётся читаемым, AC2 (locale resolution) +не страдает. + +**Решение ревьюера:** снято с записью. Если в будущем швейцарские +пользователи укажут на несоответствие, это отдельный issue на региональный +вариант орфографии, не возврат текущего ТЗ. + +### L2 — явного блока «принято предположительно, поменять свободно» в конце документа нет + +PROCESS.md §7.1 предписывает собирать не-продуктовые технические решения +в отдельный блок в конце ТЗ, чтобы ревьюер мог их оспорить одним взглядом. +В этом документе такие решения (устройство page-lifetime cache, форма +dedupe, retry policy, разбиение ролей в manifest) разбросаны по §6–§7 как +формулировки контракта, а не собраны отдельным блоком. + +**Почему не блокирует.** По содержанию все технические решения либо прямо +привязаны к проверяемому AC (AC4, AC5, AC6, AC7 — это не «свободно +меняемые предположения», а часть контракта, который тестами и будет +доказываться), либо повторяют уже работающий в проекте паттерн +(editor/onboarding lazy-loading, retry-asset token). Оспаривать в них +нечего — ревью выше по каждому пункту нашло реальное покрытие, а не догадку. +Форматное требование не задевает проверяемость ТЗ. + +**Решение ревьюера:** снято с записью, правка не нужна. + +## Что проверено и признано корректным + +- Классификация трека (полный) и перечисление нарушенных критериев §5 в + S2-analysis — соответствует содержанию задачи. +- Зависимость #62 реально `S8-merged`, её API (`LANGUAGE_REGISTRY`, + `resolveLanguageCode`, `languageOptions`) совпадает с тем, что ТЗ обещает + расширить, а не заменить. +- Locale resolution matrix (§5) не меняет существующую политику exact → + primary → English — подтверждено чтением `resolveLanguageCode`. +- Заявленный текущий bundle budget (255 910 / 256 000 B gzip) — точное, + а не приблизительное число; проверено сборкой. +- План тестов (AC1–AC10) — для каждого назван реалистичный, уже + прецедентный на проекте способ доказательства (unit registry-parity, + Playwright semantic smoke, delayed-import smoke по образцу + `smoke_lazy_editor_chunk.mjs`, bundle-manifest unit). +- Не-скоуп (§3.2) корректно исключает контент, который карточка принципиально + не переводит нигде (имена HA-сущностей, значения, README) — согласовано + со SCOPE.md («мы не редактируем реестр HA», «UI, не документация»). +- Откат (§14) не требует миграции данных — подтверждено отсутствием + `language` в CONFIG-COMPATIBILITY.md. +- Оба changelog, оба User Guide, TESTING.md существуют как файлы — release + artifacts (§15) указывают на реальные, а не гипотетические цели правки. +- Открытых продуктовых вопросов действительно не осталось: все пять + дефолтов, которые ТЗ фиксирует как решённые (native label, единый словарь + для de-*, отсутствие перевода HA-контента, English fallback, отсутствие + регрессии EN/RU), были явно объявлены и приняты по правилу «молчание — + согласие» ещё в S2-analysis, до написания ТЗ — это соответствует + предписанному в PROCESS.md порядку (продуктовые вопросы решаются пачкой, + с дефолтом, не в спеке задним числом). + +## Чего не проверял и почему + +- **Гейты `typecheck`/`test`/`build`/`bundle:budget` на самой ветке + задачи** — diff ветки не содержит кода (только `docs/specs/**`), гейты + этого рода проверяют код, а не текст ТЗ; прогон дал бы тот же результат, + что и на `dev`, и ничего не доказал бы про качество спецификации. Вместо + этого сборка `dev` использована лишь как источник факта (текущий + bundle budget), см. пункт 2 «Как проверялось». +- **`node scripts/check-docs.mjs`** — diff не касается `src/**`, условие + запуска (PROCESS.md §8) не выполнено. +- **Смоки, golden, backend pytest** — нет кода для прогона; они появятся + только на этапе реализации и будут предметом код-ревью. +- **Точный итоговый размер lazy-loader кода** (риск «Loader превышает + initial budget даже без словаря», §13 ТЗ) — не проверяем и не можем + проверить на этапе спецификации: кода ещё нет. Риск в документе назван + явно, с мерой («только доказанная очистка мёртвых i18n keys, budget не + повышается») и с проверкой через AC6 (bundle-manifest/budget unit) — + этого достаточно для DoR; если реализация не уложится, это остановит + код-ревью, а не проходит на этом этапе незамеченным. +- **Ручной аудит немецкого перевода на естественность/грамматику** — + вне полномочий этого ревью (ревьюер ТЗ не носитель и не переводчик); + проверена структура контракта (glossary, правила капитализации, + placeholders, no-English-fallback gate), а не текстовое качество + конкретных строк, которых на этапе ТЗ ещё и не существует (кроме + примеров таблицы §4). + +## Унаследовано из предыдущего раунда + +Неприменимо — это первый и единственный заход (r1), лимит циклов ТЗ на +полном треке (4) не тронут. + +## Итог + +ТЗ полное, проверяемое, все десять AC имеют названный способ доказательства, +технические утверждения документа сверены с реальным состоянием репозитория +и не являются догадками, выданными за факт. Продуктовые вопросы закрыты +корректно на этапе аналитики. Два Low-замечания сняты с записью причины. +Задача готова к переходу в `S5-ready`.