docs: review document for #354

Issue: #354
User-Visible: no

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

@@ -0,0 +1,188 @@
+# SPEC-REVIEW-354-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/354
+- **Этап:** ревью ТЗ (PROCESS.md §2.4), лёгкий трек (`small`)
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 2 (лимит ТЗ на лёгком треке — 2)
+- **ТЗ:** тело issue #354 (файл в `docs/specs/` отсутствует — корректно для `small`)
+- **SHA на момент ревью:** `77302eaf` (`dev`, рабочее дерево чистое)
+- **Вердикт: жёлтый**
+
+## Скоуп
+
+Задача техдолга: продакшн-объект `LANGUAGE_RUNTIME` в `src/i18n/registry.ts` —
+рукописный литерал (`germanDictionary`/`germanPending`/`germanFailed`/
+`settleGerman`), логически дублирующий уже протестированный класс
+`LanguageRuntime` (`src/i18n/language-runtime.ts`), который покрывает
+`test/i18n-runtime.test.mjs`. Тесты сейчас доказывают свойства не того кода.
+Фикс: собрать `LANGUAGE_RUNTIME` как `new LanguageRuntime(...)`, удалить
+рукописные поля, добавить контроль от возврата дубля (К3) и, попутно (N7, Low),
+тост при отказе загрузки словаря вместо тихого `console.warn`.
+
+По `docs/SCOPE.md`: задача — техдолг вокруг J4/J6 (инфраструктура i18n из #62),
+пользовательского функционала не добавляет, кроме одного нового тоста (N7).
+Отдельного раздела «Job» не требует — это внутренняя правка тестируемости плюс
+мелкое UX-улучшение по уже существующему паттерну (`editor.load_failed`).
+Конфликта со SCOPE.md нет.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `PROCESS.md` целиком (жизненный цикл, §4, §5,
+   §7.1, §2.10).
+2. Прочитаны тело issue #354 и комментарий аналитика (S2→S3, единственный
+   комментарий на issue).
+3. Прочитан текущий код: `src/i18n/language-runtime.ts` (класс `LanguageRuntime`,
+   контракт `LanguageRuntimeContract`), `src/i18n/registry.ts` (текущий рукописный
+   `LANGUAGE_RUNTIME`) — проблема из issue подтверждена буквально: поля
+   `germanDictionary`/`germanPending`/`germanFailed`/`settleGerman` существуют
+   ровно как описано, ни один тест их не импортирует (`grep` по `test/**` —
+   пусто).
+4. Прочитан `test/i18n-runtime.test.mjs` — подтверждает, что класс
+   `LanguageRuntime` уже кроет dedup/retry/fingerprint/render-gate сценарии,
+   которые дублирует рукописный объект.
+5. Проверены все точки использования `LANGUAGE_RUNTIME` (`houseplan-card.ts`,
+   `editor.ts`, `space-card.ts`, `space-editor.ts`) — все обращаются только
+   через контракт `state/dictionary/ensure` (`languageRenderGate(this,
+   LANGUAGE_RUNTIME, …)`), замена литерала на экземпляр класса безопасна для
+   всех четырёх поверхностей без изменения их кода (К1 не ломает вызывающих).
+6. Проверен существующий тост-паттерн: `_showToast`/`_toast` **только** в
+   `src/houseplan-card.ts` (`grep -n "toast" editor.ts space-editor.ts
+   space-card.ts` — пусто). Проверен паттерн `editor.load_failed` +
+   `lazyLoadFailureMessage` как образец для N7.
+7. Проверены существующие паритет-тесты локалей: `test/i18n.test.mjs`
+   («every registered dictionary carries the English key set», «no empty
+   values», «placeholders match between languages», German glossary/sentinel
+   checks) — заявление ТЗ «паритет-тесты уже стерегут» новый ключ
+   подтверждено, это не догадка.
+8. Проверен существующий смок `demo/smoke_german_locale.mjs` (84 строки) —
+   сценарий «both retries failed → English» уже есть
+   (`out.failureFallsBackAndUnblocks`, `out.failureWarnsOnce`), в него
+   действительно можно добавить проверку тоста без новой инфраструктуры смоков.
+9. Проверен паттерн подписки на странице (`subscribeLabs`/
+   `subscribePageVisibility` в `connectedCallback`/`disconnectedCallback`
+   `houseplan-card.ts`) — предложенный `subscribeLanguageLoadFailures`
+   стилистически идентичен уже принятым конвенциям, это не новый паттерн.
+10. Сверены соответствия К1 «warn-текст для de байт-в-байт совпадает с
+    классовым» — оба места используют один и тот же формат
+    `` `[houseplan] unable to load ${entry.code} locale; using English` ``,
+    утверждение верно на сегодняшнем коде.
+
+Гейты не гонялись — стадия ревью ТЗ по коду не работает (кода ещё нет, ветка
+не создана); это ожидаемо для S4-spec-review и не является пропуском.
+
+## Находки
+
+### Medium (в скоупе, чинится в тексте ТЗ) — 1
+
+**M1. Не решено, на каких из четырёх поверхностей рендер-гейта показывается
+новый тост N7 (К2).**
+
+- **Где:** контракт К2 в теле issue #354 — «Карточка подписывается при
+  подключении... показывает тост».
+- **Почему это находка, а не мелочь:** `LANGUAGE_RUNTIME` и
+  `languageRenderGate` используются на **четырёх** самостоятельных корневых
+  поверхностях (зафиксировано в каноническом `docs/specs/348-german-localization.md`
+  §6.3: `houseplan-card`, `houseplan-space-card`, GUI editor `houseplan-card`,
+  GUI editor `houseplan-space-card`) — все четыре подпишутся на один и тот же
+  `LANGUAGE_RUNTIME` и все четыре могут столкнуться с отказом загрузки de.
+  Однако инфраструктура тоста (`_showToast`/`_toast`) существует **только** в
+  `src/houseplan-card.ts`; в `editor.ts`, `space-editor.ts` и `space-card.ts`
+  тоста нет вовсе (проверено `grep -n "toast"` по всем трём файлам — пусто
+  строк). ТЗ говорит «Карточка» в единственном числе и не называет, что
+  происходит на трёх остальных поверхностях: тихий статус-кво
+  (`console.warn`, как сейчас) или им тоже нужен тост — а тост на редакторах
+  означал бы новую UX-поверхность (`_showToast` там ещё не существует) и уже
+  не укладывался бы в критерий лёгкого трека «нет нового UX-контракта» / «одна
+  поверхность».
+- **Как проявится, если не поправить:** разработчик может по-разному
+  трактовать «Карточка» — реализовать только `houseplan-card.ts` (наиболее
+  вероятное и дешёвое прочтение, соответствует единственному существующему
+  смоку `demo/smoke_german_locale.mjs`, который создаёт только
+  `houseplan-card`), либо решить, что нужно тащить тост-инфраструктуру и в три
+  других файла, расширяя скоуп лёгкой задачи без разрешения. AC4 и K2 сейчас
+  одинаково совместимы с обоими прочтениями — цикл код-ревью не сможет
+  предъявить претензию ни к одному варианту, потому что ТЗ ничего не
+  фиксирует.
+- **Требуемая правка:** одно предложение в К2, явно называющее это решённым —
+  предлагаемый вариант по умолчанию (не продуктовый вопрос владельцу, ровно
+  тот случай, который снимается ревьюером/автором самостоятельно): «Тост
+  показывается только в `houseplan-card` (View); `space-card`, оба GUI-редактора
+  продолжают писать только `console.warn`, как до задачи — тост-поверхности там
+  нет, и заводить её не входит в эту задачу». Это не расширяет объём работы, а
+  сужает и фиксирует уже существующий фактический скоуп.
+
+### Low (снимается с записью, не блокирует) — 1
+
+**L1. Ключ `i18n.load_failed` не следует конвенции именования тостов.**
+
+- Все существующие ключи, показываемые через `_showToast`, лежат в
+  пространстве `toast.*` (`toast.cfg_reload_failed`, `toast.space_order_changed`,
+  `toast.conflict`, `toast.pos_save_failed`, ещё 30+ примеров в `en.json`) — ни
+  одного ключа `i18n.*` в репозитории не существует
+  (`grep -n '"i18n\.' src/i18n/en.json` пусто). Предложенный `i18n.load_failed`
+  вводит новое, единственное в своём роде пространство имён вместо очевидного
+  `toast.locale_load_failed` или аналога.
+- Не блокирует: паритет-тесты (`test/i18n.test.mjs`) одинаково защищают любой
+  ключ независимо от префикса, поведенчески это не меняет ничего наблюдаемого
+  пользователем.
+- **Решение ревьюера:** не блокировать, зафиксировать в этом документе.
+  Автору стоит рассмотреть переименование в `toast.*`-пространство при
+  реализации ради единообразия со всеми соседними тостами, но это не
+  критерий приёмки и не повод для повторного цикла.
+
+## Что проверено и корректно
+
+- **К1 реализуем без побочных поломок.** Все четыре вызывающих места
+  (`houseplan-card.ts:10713`, `editor.ts:118`, `space-card.ts:766`,
+  `space-editor.ts:60`) используют `LANGUAGE_RUNTIME` только через контракт
+  `LanguageRuntimeContract` (`state`/`dictionary`/`ensure`), которому
+  `LanguageRuntime`-инстанс соответствует по построению — замена литерала на
+  `new LanguageRuntime(...)` их не касается.
+- **Публичная поверхность действительно не меняется** (раздел «Откат»):
+  `dictionaryFor`/`ensureLanguage`/`languageOptions` — сигнатуры и экспорт
+  сохраняются, откат — один revert.
+- **AC1–AC3 однозначны и механически проверяемы:** grep по
+  `germanPending|germanFailed|settleGerman`, `instanceof`-юнит, мутационный тест
+  с инлайн-литералом вместо `new LanguageRuntime(...)` — все три воспроизводимы
+  без дополнительных решений на этапе кода.
+- **Заявление «паритет-тесты уже стерегут» — не догадка**, а точное описание
+  существующего `test/i18n.test.mjs` (проверено чтением тест-файла).
+- **Заявление «warn-текст байт-в-байт совпадает с классовым» — верно** на
+  сегодняшнем коде обоих файлов.
+- **N7 использует существующий паттерн `_showToast`**, а не изобретает новый
+  UX-примитив (кроме открытого вопроса M1 о том, где именно он вызывается).
+- **Подписка/отписка (К2) соответствует уже принятой конвенции**
+  `subscribeXxx(...)`/`_xxxUnsub` в `connectedCallback`/`disconnectedCallback`,
+  а не является новым архитектурным паттерном.
+- Формат ТЗ для лёгкого трека соблюдён: проблема · контракт · AC1…AC4 с
+  доказательством · откат — все четыре раздела на месте (§5 PROCESS.md).
+- Трейлер `User-Visible: yes` в теле issue корректен — задача добавляет
+  видимый тост.
+
+## Чего не проверял
+
+- Код по задаче ещё не написан (стадия ТЗ), поэтому гейты `tsc`/`test`/`build`
+  не запускались — на этом этапе они неприменимы, а не пропущены.
+- Не проверял реальный текст немецкого/русского перевода нового ключа — его
+  ещё нет; при код-ревью нужно свериться с паритет-тестами и glossary из
+  `docs/specs/348-german-localization.md`.
+- Не проверял, действительно ли `subscribeLanguageLoadFailures`-колбэк способен
+  получить локаль-код `de` из `warn(message, error)` (сигнатура класса передаёт
+  только текстовое сообщение, не структурированный код). Технически это
+  решаемо уже сегодня (в реестре ровно один ленивый locale, `de` — код может
+  быть захвачен замыканием при построении `warnWithNotify`, без парсинга
+  строки), поэтому не поднимаю как отдельную находку, но это стоит явно
+  проговорить при код-ревью, если добавится второй ленивый язык.
+
+## Раунд не первый? — не применимо
+
+r1: разделы «Закрытие раунда r0» и «Унаследовано из r0» не пишутся —
+предыдущего цикла ревью по этой задаче нет.
+
+## Итог
+
+Вердикт **жёлтый**: High-находок нет, но одна Medium-находка (M1) — скоуп
+тоста N7 по четырём render-gate поверхностям не зафиксирован, хотя факт
+асимметрии (только `houseplan-card.ts` имеет тост-инфраструктуру) уже виден в
+коде и должен быть явно записан как решение, а не оставлен на усмотрение
+исполнителя. Автор правит ТЗ (одно предложение в К2), Low-находка (L1)
+зафиксирована и не требует правки для перехода в `S5-ready`.