docs: review document for #354

Issue: #354
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-28 14:45:28 +00:00
parent 149b2af7f4
commit 468eacdb3b
+147 -162
View File
@@ -1,188 +1,173 @@
# SPEC-REVIEW-354-r1
# SPEC-REVIEW-354-r2
- **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`, рабочее дерево чистое)
- **Вердикт: жёлтый**
- **Заход:** r2 · блокирующих циклов израсходовано 1 из 2 (лимит ТЗ на лёгком треке — 2; r1 был жёлтым и потратил 1 цикл — §4/#227)
- **ТЗ:** тело issue #354, «Ревизия 2» (файл в `docs/specs/` отсутствует — корректно для `small`)
- **SHA на момент ревью:** `149b2af7` (`dev`, рабочее дерево чистое; с момента r1 (`77302eaf`) в код не внесено ни одного продуктового коммита — единственный новый коммит `149b2af7` кладёт документ ревью r1, продуктового/тестового кода не касается)
- **Вердикт: зелёный**
## Примечание о нумерации захода
Стартовые данные этой сессии указывали «Заход: r1 · блокирующих циклов
израсходовано 0 из 2». Это не соответствует фактическому состоянию: в
`docs/reviews/SPEC-REVIEW-354-r1.md` (закоммичен `149b2af7`) и в комментариях
issue уже есть завершённый раунд r1 — жёлтый вердикт от `claude` (комментарий
`IC_kwDOTOcLQM8AAAABRRPAyw`, 2026-08-28T14:40:01Z, SHA `77302eaf`), за которым
последовала правка автора («Ревизия 2 ТЗ по r1», комментарий
`IC_kwDOTOcLQM8AAAABRRPa0g`) и возврат метки `S4-spec-review`. Раунд, который
разбирает эта сессия, — второй по факту (`r2`), а не первый; бюджет цикла к
этому моменту уже потратил 1 из 2 (жёлтый вердикт r1 расходует бюджет по §4).
Документ поэтому назван и пронумерован как `r2`, вопреки стартовым данным —
иначе он затёр бы уже существующий `SPEC-REVIEW-354-r1.md` или конфликтовал
бы с ним по номеру, ровно тот сценарий, от которого предостерегает сам номер
захода. Расхождение стартовых данных со стоянием репозитория — само по себе
не блокирует эту задачу, но стоит показать тому, кто обслуживает конвейер.
## Скоуп
Задача техдолга: продакшн-объект `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`.
Без изменений относительно r1: собрать продакшн-`LANGUAGE_RUNTIME` в
`src/i18n/registry.ts` как `new LanguageRuntime(...)` вместо рукописного
литерала (`germanDictionary`/`germanPending`/`germanFailed`/`settleGerman`),
чтобы существующий `test/i18n-runtime.test.mjs` действительно проверял код
продакшна, плюс попутный (N7) тост при отказе загрузки словаря локали.
Ревизия 2 не меняет предмет задачи и не расширяет её — правит формулировку
одного контракта (К2) и переименовывает один i18n-ключ. Конфликта со
`docs/SCOPE.md` по-прежнему нет (см. SPEC-REVIEW-354-r1.md, раздел «Скоуп» —
наследуется, ниже).
По `docs/SCOPE.md`: задача — техдолг вокруг J4/J6 (инфраструктура i18n из #62),
пользовательского функционала не добавляет, кроме одного нового тоста (N7).
Отдельного раздела «Job» не требует — это внутренняя правка тестируемости плюс
мелкое UX-улучшение по уже существующему паттерну (`editor.load_failed`).
Конфликта со SCOPE.md нет.
## Дельта r1 → r2
## Как проверялось
`git diff <SHA r1>..HEAD` не показателен: SHA r1 (`77302eaf`) и текущий
(`149b2af7`) отличаются только doc-коммитом ревью r1, продуктового кода нет
вообще (стадия ТЗ). Предмет дельты — правка тела issue #354, зафиксированная
автором как «Ревизия 2 ТЗ по r1» (комментарий `IC_kwDOTOcLQM8AAAABRRPa0g`).
Сверено построчно с текстом, процитированным в `SPEC-REVIEW-354-r1.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` ``,
утверждение верно на сегодняшнем коде.
1. **К2** — было (цит. по r1, находка M1): «Тост показывается ... Карточка
подписывается при подключении... показывает тост» (без указания, какая из
четырёх поверхностей рендер-гейта). Стало: «Тост показывается ТОЛЬКО в
`houseplan-card` (View — единственная поверхность с тост-инфраструктурой
`_showToast`)... Остальные три поверхности рантайма (`space-card`,
GUI-редакторы обеих карточек) остаются при `console.warn`, как сейчас, —
тост-инфраструктура там не заводится.»
2. **Ключ тоста** — было `i18n.load_failed` (находка L1). Стало
`toast.locale_load_failed`, в К2 и в AC4.
3. Остальной текст (К1, К3, AC1–AC3, «Откат», трейлер `User-Visible: yes`)
не изменился — сверено посимвольно с цитатами в `SPEC-REVIEW-354-r1.md`
(разделы «Что проверено и корректно», «Скоуп»).
Гейты не гонялись — стадия ревью ТЗ по коду не работает (кода ещё нет, ветка
не создана); это ожидаемо для S4-spec-review и не является пропуском.
Дельта локальна (правка формулировки одного контракта плюс переименование
одного ключа), не задевает новую подсистему, не меняет контракт поведения
сверх уже согласованного в r1, по объёму несопоставима с исходной задачей —
условие «разбор остаётся полным» (PROCESS.md §2.10) не наступает. Разбор
этого раунда ограничен дельтой: заново проверялись только К2 (N7) и ключ
тоста — единственное, что дельта задевает.
## Как проверялось (r2)
1. Прочитано текущее тело issue #354 целиком, сверено с цитатами r1 —
найдены ровно две правки (см. «Дельта» выше), больше отличий нет.
2. Прочитан комментарий владельца «Ревизия 2 ТЗ по r1» — подтверждает, что
обе правки целенаправленно закрывают M1 и L1, а не что-то другое.
3. Перечитан текст К2 revision 2 на однозначность: названа ровно одна
поверхность (`houseplan-card`/View), явно перечислены три поверхности,
остающиеся при `console.warn`, явно сказано «тост-инфраструктура там не
заводится» — двух прочтений эта формулировка больше не допускает.
4. `grep -n '"toast\.' src/i18n/en.json` — подтверждён неймспейс `toast.*`
(40+ существующих ключей); `grep -rn "locale_load_failed\|i18n.load_failed"
src/ test/ demo/` — пусто: новый ключ не конфликтует с существующим кодом
и ещё не реализован (ожидаемо для стадии ТЗ).
5. `grep -n "6.3\|houseplan-space-card" docs/specs/348-german-localization.md`
— подтверждён раздел «6.3. Render gate» и список из четырёх поверхностей,
на который опирается формулировка К2 (та же ссылка, что использовал r1).
6. `git log 77302eaf..HEAD --oneline` — один коммит (`149b2af7`, doc r1),
без изменений в `src/**`/`test/**`: код и его состояние идентичны
зафиксированным в r1, повторная проверка К1/К3/AC1–AC3 по коду не нужна.
Гейты (`tsc`/`test`/`build`) не гонялись — стадия ревью ТЗ, кода по задаче ещё
нет; как и в r1, это не пропуск, а неприменимость этапа.
## Находки
### Medium (в скоупе, чинится в тексте ТЗ) — 1
Нет. High: 0, Medium: 0, Low: 0.
**M1. Не решено, на каких из четырёх поверхностей рендер-гейта показывается
новый тост N7 (К2).**
Обе находки r1 закрыты правкой текста, новых находок дельта не вносит.
- **Где:** контракт К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`, как до задачи — тост-поверхности там
нет, и заводить её не входит в эту задачу». Это не расширяет объём работы, а
сужает и фиксирует уже существующий фактический скоуп.
## Закрытие раунда r1
### Low (снимается с записью, не блокирует) — 1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| **M1** (Medium, в скоупе) — К2 не называл, на какой из четырёх поверхностей рендер-гейта показывается тост N7; формулировка «Карточка» в единственном числе была совместима с двумя разными объёмами работы. | К2 переписан: «Тост показывается ТОЛЬКО в `houseplan-card` (View...)... Остальные три поверхности рантайма (`space-card`, GUI-редакторы обеих карточек) остаются при `console.warn`, как сейчас, — тост-инфраструктура там не заводится.» Формулировка совпадает с предложенным в r1 вариантом по умолчанию почти дословно. | Тело issue #354, раздел «Контракт», К2, первый и последний абзацы. |
| **L1** (Low, зафиксирована) — ключ `i18n.load_failed` был единственным вне пространства `toast.*`. | Ключ переименован в `toast.locale_load_failed` — и в К2, и в AC4. | Тело issue #354, К2 («новым ключом `toast.locale_load_failed`») и AC4 («текстом `toast.locale_load_failed`»). |
**L1. Ключ `i18n.load_failed` не следует конвенции именования тостов.**
## Унаследовано из r1
- Все существующие ключи, показываемые через `_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.*`-пространство при
реализации ради единообразия со всеми соседними тостами, но это не
критерий приёмки и не повод для повторного цикла.
Без повторной проверки в этом раунде — код и тесты с r1 не менялись
(единственный новый коммит — doc-коммит ревью, класс C), проверка велась
только над текстом ТЗ:
## Что проверено и корректно
- **Скоуп задачи и соответствие `docs/SCOPE.md`** — принято по
`SPEC-REVIEW-354-r1.md`, раздел «Скоуп» (SHA `77302eaf`).
- **К1 (замена литерала на `new LanguageRuntime(...)`) реализуема без
побочных поломок** — все четыре вызывающих места используют только
контракт `state/dictionary/ensure` — принято по `SPEC-REVIEW-354-r1.md`,
раздел «Что проверено и корректно», пп. 1–2 (SHA `77302eaf`, строки кода
`houseplan-card.ts:10713`, `editor.ts:118`, `space-card.ts:766`,
`space-editor.ts:60`).
- **АС1–АС3 однозначны и механически проверяемы** (grep, instanceof-юнит,
мутационный тест на К3) — принято по `SPEC-REVIEW-354-r1.md`, раздел «Что
проверено и корректно» (SHA `77302eaf`); текст АС1–АС3 в ревизии 2 не
менялся (сверено построчно, см. «Дельта» выше).
- **Паритет-тесты (`test/i18n.test.mjs`) действительно стерегут новый ключ
независимо от его имени** — принято по `SPEC-REVIEW-354-r1.md`, пп. 7 и
L1 (SHA `77302eaf`); вывод не зависит от конкретного имени ключа, поэтому
переименование в `toast.locale_load_failed` его не меняет.
- **Существующий смок `demo/smoke_german_locale.mjs` уже содержит сценарий
«оба ретрая упали → English»**, в который AC4 добавляет проверку тоста без
новой инфраструктуры смоков — принято по `SPEC-REVIEW-354-r1.md`, п. 8
(SHA `77302eaf`).
- **Подписка/отписка (К2) соответствует принятой конвенции
`subscribeXxx(...)`** — принято по `SPEC-REVIEW-354-r1.md`, п. 9 (SHA
`77302eaf`).
- **Открытый нефинирующий пункт для код-ревью**: не проверялось, как
`subscribeLanguageLoadFailures`-колбэк получит код локали `de` из
`warn(message, error)` (сигнатура класса передаёт только текст). Не
находка (решаемо замыканием при единственном сегодня ленивом языке), но
стоит явно проговорить на код-ревью, если добавится второй ленивый язык —
перенесено из `SPEC-REVIEW-354-r1.md`, раздел «Чего не проверял».
- **К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 корректен — задача добавляет
видимый тост.
## Что проверено и корректно (r2, по дельте)
- К2 revision 2 однозначен: ровно одна поверхность для тоста
(`houseplan-card`), явно перечислены три поверхности без тоста, явно
сказано, что новая тост-инфраструктура на них не заводится — двух
прочтений формулировка не допускает, находка M1 закрыта по существу, а не
косметически.
- Новый ключ `toast.locale_load_failed` следует существующей конвенции
именования тостов (проверено grep по `src/i18n/en.json`, 40+ ключей в том
же пространстве) и не конфликтует с уже существующим кодом (grep пуст).
- Правка не расширяет скоуп и не вводит новый UX-контракт: критерии лёгкого
трека (§5 PROCESS.md) по-прежнему выполнены — ревизия 2 их не колеблет
(одна поверхность тоста явно зафиксирована как единственная, а не
расширена на четыре).
- АС4 (после ревизии) по-прежнему проверяем механически: юнит на
`subscribeLanguageLoadFailures`, смок `smoke_german_locale` с проверкой
`card._toast`, мутационный тест на «тост выброшен».
## Чего не проверял
- Код по задаче ещё не написан (стадия ТЗ), поэтому гейты `tsc`/`test`/`build`
не запускались — на этом этапе они неприменимы, а не пропущены.
- Не проверял реальный текст немецкого/русского перевода нового ключа — его
ещё нет; при код-ревью нужно свериться с паритет-тестами и glossary из
`docs/specs/348-german-localization.md`.
- Не проверял, действительно ли `subscribeLanguageLoadFailures`-колбэк способен
получить локаль-код `de` из `warn(message, error)` (сигнатура класса передаёт
только текстовое сообщение, не структурированный код). Технически это
решаемо уже сегодня (в реестре ровно один ленивый locale, `de` — код может
быть захвачен замыканием при построении `warnWithNotify`, без парсинга
строки), поэтому не поднимаю как отдельную находку, но это стоит явно
проговорить при код-ревью, если добавится второй ленивый язык.
## Раунд не первый? — не применимо
r1: разделы «Закрытие раунда r0» и «Унаследовано из r0» не пишутся —
предыдущего цикла ревью по этой задаче нет.
- Код по задаче ещё не написан (стадия ТЗ) — гейты `tsc`/`test`/`build` не
запускались, как и в r1; на этой стадии они неприменимы.
- Не проверял заново K1/K3/AC1–AC3 по коду — код с момента r1 не менялся
(см. «Дельта» и «Унаследовано из r1» выше), текст этих пунктов ТЗ тоже не
менялся.
- Не проверял реальные тексты переводов для `toast.locale_load_failed`
(en/ru/de) — их ещё нет; сверка с паритет-тестами и glossary
`docs/specs/348-german-localization.md` — задача код-ревью.
## Итог
Вердикт **жёлтый**: High-находок нет, но одна Medium-находка (M1) — скоуп
тоста N7 по четырём render-gate поверхностям не зафиксирован, хотя факт
асимметрии (только `houseplan-card.ts` имеет тост-инфраструктуру) уже виден в
коде и должен быть явно записан как решение, а не оставлен на усмотрение
исполнителя. Автор правит ТЗ (одно предложение в К2), Low-находка (L1)
зафиксирована и не требует правки для перехода в `S5-ready`.
Вердикт **зелёный**: обе находки r1 (M1 Medium, L1 Low) закрыты точечной
правкой текста, дельта не вносит новых находок и не задевает то, что r1 уже
принял. Issue переходит в `S5-ready`.