diff --git a/docs/reviews/CODE-REVIEW-502-r1.md b/docs/reviews/CODE-REVIEW-502-r1.md new file mode 100644 index 00000000..afe2892a --- /dev/null +++ b/docs/reviews/CODE-REVIEW-502-r1.md @@ -0,0 +1,74 @@ +# CODE-REVIEW — issue #502, заход r1 + +**Материал:** `441d3cc3de3dcf76446e4e914b72c16671494f83` (единственный коммит поверх `origin/dev`; ветка приведена к `dev` конвейером — до ребейза было `1d581fb4`, после — другой код, разбор ниже полный, не по дельте, как и предписано при таком переносе). + +**Класс изменения:** B (гейты/тестовая инфраструктура) — `test/i18n-dead-keys.test.mjs` (правка) + `test/helpers/i18n-consumers.mjs` (новый файл). Продуктовый код (`src/**`) не тронут. `User-Visible: no`. + +## Скоуп + +Задача сужает критерий «динамический потребитель i18n-ключа» в гейте `test/i18n-dead-keys.test.mjs`: конкатенация/шаблонная строка признаётся потребителем семьи ключей только если её статическая часть содержит `.` (разделитель `namespace.key`) и хотя бы одну букву. До правки `'r' + Date.now().toString(36)` превращался в `^r.+$` и маскировал реально мёртвые ключи на «r» (обнаружено на `radar.bad_references`, #485). Пользователь ничего не наблюдает — гейт защищает переводчика/владельца от накопления неиспользуемых переводов. + +## Как проверялось + +- Прочитан диф целиком (`git diff origin/dev...HEAD`, 2 файла, +279/−68) и оба файла построчно. +- Прочитано тело issue #502 целиком (проблема, ТЗ, контракт п.1–5, «принято предположительно», зависимости, таблица AC, откат) и все комментарии (S2 → S4-spec-review → зелёный вердикт спек-ревью r1 → кандидат реализации → красный Validate по внешней причине → возврат в S7 с объяснением). +- Дешёвые гейты **не перегонялись повторно** — Validate на точном материале ревью (`441d3cc3`) зелёный: https://github.com/Matysh/houseplan-card/actions/runs/34445317690 (`headSha` подтверждён `gh run view`, `conclusion: success`). Job «Фронтенд: типы, юниты, мутанты, синхрон бандла» — success, три job'а «Мутанты по диффу» — success (мутант `i18n-dead-key-returns` пойман). HACS/hassfest/backend/smoke/golden/perf — `skipped`, что корректно: диф не затрагивает пути, от которых они зависят. +- Тем не менее прогнан локально сам изменённый тестовый файл — это дешево и напрямую доказывает AC1–AC4: + `node --test test/i18n-dead-keys.test.mjs` → 9/9 green, диагностика: `discarded 821 dynamic patterns; keys covered only by discarded patterns: none`. +- `npx tsc --noEmit -p tsconfig.test.json` — чисто (новый `.mjs`-helper не ломает тестовый typecheck). +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` → «Исполняемого frontend-диффа нет (src/**/*.ts не тронут). Browser-smoke этим диффом не выбираются». Подтверждает, что смоки/golden/check-docs здесь не нужны — диф не трогает `src/**`. +- Проверено чтением: `radar.bad_references` действительно используется как строковый литерал в `src/radar-setup.ts:334` (зависимость из раздела «Зависимости» ТЗ уже закрыта, #485 закрыт) — значит на реальном дереве список `DYNAMIC_KEY_FAMILIES` пуст обоснованно, а не по недосмотру. +- Трейлеры коммита проверены: `Issue: #502`, `User-Visible: no` — корректно, changelog не тронут (`User-Visible: no` не требует правки обоих changelog), проверено `git diff --stat` по путям `docs/CHANGELOG*`. + +## Проверка контракта и AC по коду + +| # | Контракт/AC | Где выполнено | Проверено | +|---|---|---|---| +| п.1/AC1 | Литерал без точки + выражение — не потребитель | `isKeyShapedPattern` (`i18n-consumers.mjs:66-68`) требует `staticText.includes('.')` и `/[A-Za-z]/` | юнит-тест `AC1` зелёный; вручную прослежена трассировка для `'r' + Date.now()...`, `` `${a}b` ``, `'x' + id` — везде `staticText` не содержит `.` | +| п.2 | Паттерн вида `^r.+$` невозможен по построению | требование точки в `isKeyShapedPattern` устраняет этот класс паттернов целиком | прочитано, логически подтверждено: без `.` в статике `isKeyShapedPattern` всегда false | +| AC2 | `` `radar.${code}` ``, `prefix + '.title'`, `` `${ns}.aria` `` остаются потребителями | тест `key-shaped joins keep consuming their families (#502 AC2)` | зелёный, плюс негативные проверки `doesNotMatch('room.name', …)`, `doesNotMatch('x.titles', …)` — паттерн не расползается | +| AC3 | Мёртвый ключ ловится даже когда id-генератор делит первую букву | тест `a dead key is reported even when an id generator shares its first letter (#502 AC3)` на синтетическом AST/словаре | зелёный: `unusedKeys` → `['probe.dead']`, `discarded.length === 1` | +| п.3/AC4 | `DYNAMIC_KEY_FAMILIES` — по записи на семью, пустая причина/непокрытый паттерн — красный тест | `familyProblems` (`i18n-consumers.mjs:150-165`) + тест `every declared dynamic key family...` + `an explicit family rescues a data-driven key... (#502 AC4)` (4 варианта дефектов) | зелёный на всех 4 синтетических дефектах; на реальном дереве `DYNAMIC_KEY_FAMILIES = []` — подтверждено `node --test`: основной тест «every i18n key has a … consumer» зелёный, т.е. 0 непокрытых ключей и семьи не нужны | +| п.4 | Реально мёртвые ключи не маскируются, не подсовываются в семьи ради зелёного гейта | `radar.bad_references` теперь потребляется литералом в `src/radar-setup.ts:334`; список семей пуст, а не дополнен фиктивной записью под этот ключ | подтверждено чтением `src/radar-setup.ts:334` и всех 4 словарей | +| п.5 | Сообщение об ошибке не изменено по форме | `test/i18n-dead-keys.test.mjs`, строка с `Unused i18n keys: ...` — текст идентичен версии на `dev` | сверено дифом — да, не тронуто | +| AC5 | Число отброшенных паттернов и список ключей «только на отброшенном» — записаны | `narrowingReport` печатает через `t.diagnostic`; автор привёл в issue-комментарии реализации точный вывод (`discarded 821 …, none`) | воспроизведено локально: тот же вывод — `discarded 821 dynamic patterns; keys covered only by discarded patterns: none` | + +Отдельно проверено «принятое предположительно» (`` `${a}.${b}` ``, `a + '.' + b` — не потребители, т.к. статика — одна точка без буквы, что дало бы `^.+\..+$`, совпадающий с любым ключом): прослежена ручная трассировка построения `staticText` для обоих случаев — действительно даёт `'.'` без буквы, что и требуется отбросить по контракту п.2 (иначе паттерн вида «пустой хвост + точка + пустой хвост» был бы новой версией того же дефекта `^r.+$`, только в другой форме). Решение корректно и в скоупе задачи. + +## Что проверено и корректно + +- Диф ограничен `test/**`, что и требовал контракт (класс B, один файл + опциональный helper — использован). +- Чистые функции (`expressionPattern`, `isKeyShapedPattern`, `collectConsumers`, `unusedKeys`, `familyProblems`, `narrowingReport`) действительно чистые, без чтения `src/**`/словарей — юнит-тесты гоняют их на синтетическом AST, как и требовало «принято предположительно» ТЗ. +- Мутационный гейт (Validate, «Мутанты по диффу», 3/3 success) подтверждает, что тесты не декоративны: дисциплина «тест должен уметь падать» удовлетворена независимым инструментом, а не только моим прочтением. +- Зависимость на #485 (`radar.bad_references`) действительно разрешена до реализации, в правильном порядке, как и требовал раздел «Зависимости». +- Трейлеры, класс изменения, отсутствие правок в `dist/**`/changelog — всё соответствует `User-Visible: no`. + +## Чего не проверял и почему + +- **Полный `npm test`, `npx tsc --noEmit` (весь проект), `npm run build` со сверкой бандла** — не перегонял: Validate уже зелёный на точном SHA `441d3cc3` (ссылка выше), и диф не может задеть сборку (тестовый файл не входит в бандл). +- **Смоки, golden, performance, pytest** — не прогонял: `smoke-select.mjs` прямо говорит «Browser-smoke этим диффом не выбираются», диф не трогает `src/**`, `custom_components/**/*.py`, стили или рендер. Golden/check-docs также не нужны по той же причине (нет изменений видимого результата). +- **Инварианты модели (`npm run invariants`)** — не запускал: диф не касается геометрии, `layout`, `marker.space`, `open_spans`, толщины стен. +- **Ручной прогон полного `node --test` по всему `test/**`** — не делал отдельно (кроме изменённого файла): это покрыто job'ом «Фронтенд» в уже зелёном Validate. + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Вывод + +Контракт (п.1–5), все пять AC и «принятое предположительно» выполнены и подтверждены как автотестом (девять юнитов в изменённом файле, включая мутационную проверку в CI), так и чтением продуктового кода в точке, где решается судьба конкретного ранее мёртвого ключа (`src/radar-setup.ts:334`). Диф строго ограничен заявленным классом B, трейлеры корректны, откат тривиален (задача сама это утверждает, и это подтверждается: изменения лежат в двух тестовых файлах без побочных эффектов на продукт). + +Вердикт: зелёный. + +--- + + + +## Материал раунда + +- Ветка: `issue/502-i18n-dead-keys-narrow`, коммит `441d3cc3de3d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `5d0db6767239964f0c804f9212d5b038fca0dd3d` + ``` + git log --all --format='%H %T' | grep 5d0db6767239 + ``` +- Вердикт конвейера: `green` · High 0