mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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`.
|
||||
Reference in New Issue
Block a user