From cc71918126c9fee1c15782df71cb294ab0e7398e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:14:07 +0000 Subject: [PATCH] docs: review document for #659 Issue: #659 User-Visible: no --- docs/reviews/CODE-REVIEW-659-r1.md | 184 +++++++++++++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-659-r1.md diff --git a/docs/reviews/CODE-REVIEW-659-r1.md b/docs/reviews/CODE-REVIEW-659-r1.md new file mode 100644 index 00000000..e04afa96 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-659-r1.md @@ -0,0 +1,184 @@ +# CODE-REVIEW-659-r1 + +Issue: #659 — «Реестр мутантов: смок-гардов стало больше, а не меньше (206 → 262 +из 979) — перевести проверяемые без браузера на `node --test`, ограничить рост +правилом». + +Материал: `6f8c2929afe46a39ab91bedb0051c8a491ccbe8b` (рабочая копия проверялась +на этом SHA; `git rev-parse HEAD` сверен непосредственно перед выводом). +Автор в передаче на ревью назвал `d10f36204231473f0c37501fd2432e8a58931770` — +такого объекта в дереве нет. Это не находка: коммит `6f8c2929` — один squash- +коммит с родителем `9a7020ce` (текущая вершина `origin/dev`), то есть материал +уже перебазирован конвейером на актуальный `dev` (нормальный путь по AGENTS.md +«On a green code review the pipeline rebases…» — здесь ребейз случился ещё до +начала ревью). Содержимое диффа идентично описанному автором изменению. + +`git log --oneline origin/dev..HEAD` = один коммит; `git diff origin/dev...HEAD` += 12 файлов, 810/56. Класс B (`scripts/**`, `test/**`) + C (`PROCESS.md`, +`docs/testing-notes/**`) — файлов класса A нет, инфраструктурный вход в +`S7-code-review` обоснован. + +## Скоуп + +Задача сокращает число browser-smoke-гардов в реестре мутантов +(`scripts/mutation-registry.mjs`) переводом части свидетелей на `node --test`, +фиксирует потолок в `mutation-gate --check`/`npm run inventory`, добавляет +правило в `PROCESS.md` §2.7 и одну сборку бандла на шард там, где патч мутанта +не трогает бандлируемые входы. Инфраструктурная работа — прямого AC на +`docs/SCOPE.md` не закрывает, а обслуживает машинное время CI (§8 гейтов), +что и заявлено в issue. + +## Как проверялось + +| Гейт | Результат | Источник | +|---|---|---| +| `typecheck`, `npm test`, `npm run build`, bundle-policy verify | не перегонялись — зелёный Validate на этом SHA уже подтверждён | https://github.com/Matysh/houseplan-card/actions/runs/36317254856 (success) | +| `node --test test/process-digests.test.mjs` | 5/5 зелёных | выполнено лично | +| `node --test test/mutation-gate.test.mjs test/mutation-browser-offload.test.mjs test/monolith-text-anchors.test.mjs test/testing-notes-index.test.mjs` | 83/83 зелёных | выполнено лично, воспроизводит заявление автора | +| `node scripts/check-docs.mjs` | «Documentation checks passed (7 files, 12 external links)» | выполнено лично (не обязателен — diff не трогает `src/**`, но чисто) | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не выбираются» | выполнено лично — корректный ответ «нечего выбирать», диф не трогает `src/**` | +| `npm run inventory` | строка `mutation browser guards 200/200` присутствует и совпадает с заявленным пределом | выполнено лично | +| `npm run lint:unused` | «мёртвого кода нет… все числа равны базе» (без свежей сборки `bundleBytes` не судится) | выполнено лично; расхождение `bundleBytes` в `inventory.mjs` (2569393 vs база 2567812) — пред­существующий артефакт, диф не трогает `dist/**`/`src/**`, см. «Не проверял» | +| `python -m pytest tests_backend` | не прогонялся | диф не трогает `custom_components/**/*.py` | +| golden/perf/E2E | не прогонялись | нет видимого рендера (`User-Visible: no`, `src/**` не тронут) | + +### Прицельные негативные пробы (мутация/защита) — таблица «чем краснеет» + +| AC | Чем доказан | Чем краснеет | +|---|---|---| +| AC2: 84 переведённых свидетеля реально ловят мутацию (не по исключению) | `node scripts/mutation-gate.mjs --id=` лично прогнан на 7 из 84 id, взятых из разных `UNIT_GUARD_GROUPS` (`stairs-fixed-floor-still-navigates`, `device-hit-scroll-observer-disabled`, `summary-hide-unmounts-before-animation`, `form-kit-segment-breaks-words`, `room-gear-drag-reopens-settings`, `M-615-tile`, `marker-badge-position-forgets-touch`) | все 7: `: заявленный тест покраснел на мутанте`, `поймано 1 из 1` — воспроизведено лично | +| AC2/AC3: лимит 200 краснеет при росте, предупреждает без разметки, молчит когда всё на месте | `browserGuardPolicy()` (`scripts/mutation-browser-policy.mjs`) вызвана лично с синтетическими данными | `overLimit` при 201/200 → `true`; `missingReasons` при недокументированном id → 1 запись; `staleReasons` при устаревшей записи в markdown → 1 запись; на реальном реестре (200/200, документация полная) все три — пусто/`false`. Проверено исполнением функции с тремя сценариями (см. лог сессии) | +| AC4: одна сборка бандла на шард для немутируемых-в-бандле патчей | `test/mutation-gate.test.mjs` «#659: browser-only mutations reuse one clean bundle unless their patch is bundled» — `mutantBundleStrategy`/`mutantPatchesNeedBundle` на синтетическом корпусе | зелёный юнит различает `'seed'`/`'build'`/`'none'` для патча в бандле, патча вне бандла и небраузерного гварда; интеграция `captureBundleSeed`/`restoreBundleSeed` внутри `runCleanGuards`/`runMutant` — **проверено чтением, не исполнением** (сквозной прогон нескольких мутантов одного шарда с измерением количества сборок не проводился: только 4 из 200 browser guards сейчас попадают в ветку `'seed'` — `smoke-guard-blind-to-tail`, `smoke-guard-forgets-to-register-pages`, `report-page-errors-skips-round-trip`, `benchmark-page-verdict-unwatched` — посчитано лично по реестру; риск малой площади посчитан приемлемым для этого рефакторинга) | + +## Числовые факты дифа (сверка «одно число — один источник») + +- **284 → 200** browser guards. Проверено лично: `origin/dev` (`git worktree add + --detach /tmp/dev-check origin/dev`) даёт `dev browser guards: 284`; HEAD даёт + `mutation browser guards 200/200`. Разница 84 совпадает с числом id в + `UNIT_GUARD_GROUPS`. Число «262» из тела issue — снимок недельной давности; + реестр успел вырасти дальше до 284 к моменту реализации, это не расхождение + фактов, а движение времени (issue сам об этом предупреждает: «стало хуже»). + Источник числа один — `browserGuardPolicy()`, и `npm run inventory` и + `mutation-gate --check` читают его же. +- Общее число мутантов (1028) не изменилось диффом — перевод меняет `guard` + существующих id, не добавляет/убирает мутанты. Проверено лично. + +## Находки + +### Medium (в скоупе задачи) — AC5 «замер до/после» не закрыт + +Issue, пункт 5 ожидаемого результата: «Замер до/после на полном реестре +(wall-time ночного `mutation-gate`) и на одном кандидате ревью — в issue». +В единственном комментарии передачи на ревью: + +- «До» дано только для полного реестра: nightly run `36298676826` — wall-time + **54:18**, сумма шагов мутантов шести шардов **4:17:19**. +- «После» на полном реестре явно не завершено автором: «полный ручной run + `36316263355` на этой ветке запущен; ссылку и точные числа добавлю после + завершения». Проверено лично в момент ревью: `gh run view 36316263355` — + `status: in_progress`, ~32 минуты с запуска из ожидаемых часов. Числа в + issue так и не появились (комментариев по-прежнему один). +- Пары «до/после на одном кандидате ревью» нет вовсе — ни до, ни после. Автор + привёл только ссылку на Validate push (`36316250835`) без extракции + wall-time мутационного шага; отдельный workflow_dispatch Validate + (`36317254856`, тот же SHA, тот из системного промпта) показывает шесть + «Мутанты по диффу» job'ов по 1m15s–1m52s (сумма ≈ 9m22s) — но это моя + собственная реконструкция по логам CI, а не число, названное автором в + issue, и без «до»-пары для сравнения она не доказывает «на кандидате + ревью стало быстрее». + +Задача явно требует эти цифры как часть Definition of Done — это единственный +способ убедиться, что оптимизация действительно даёт заявленный эффект +(«≈ −3–4 ч на полный реестр, −10–15 мин на каждый кандидат»), а не только +формально закрывает счётчик 200/200. High-находок нет, поэтому вердикт — +жёлтый: автору нужно дождаться завершения ручного полного прогона, вписать +итоговые числа «до/после» для нереестра и добавить пару «до/после» для одного +кандидата ревью в issue, прежде чем возвращаться на `S7-code-review`. + +## Что проверено и корректно + +- **Классификация (AC1).** `docs/testing-notes/mutation-browser-guards.md` + перечисляет все 200 оставшихся browser guard id по шести категориям с + обоснованием на уровне категории; суммы по категориям (4+4+26+45+36+85) + совпадают с итоговой строкой таблицы и с фактическим числом булитов, + посчитанным построчно (`awk` по файлу) — проверено лично. `documentedBrowserGuards()` + на реальном файле даёт ровно 200 уникальных id — совпадает. + Точечно перепроверил обоснование 5 id из самой крупной категории + («Custom-element and HA browser lifecycle», 85 шт: `config-updated-event-ignored`, + `device-echo-keeps-local-noncanonical`, `namespace-loader-returns-english`, + `locale-failure-toast-dropped`, `discovery-reset-writes-a-copy`) — каждый + `because` называет конкретную браузерную зависимость (динамический импорт + чанка локали, полный цикл custom-element события, DOM-видимость тоста), и + категория совпадает с обоснованием. Дешёвого `node --test` эквивалента не + вижу ни для одного из пяти. +- **Перевод (AC2).** 84 id получили явный `node --test`-гвард + (`UNIT_GUARD_GROUPS`/`UNIT_GUARD_OVERRIDES`, `scripts/mutation-registry.mjs`). + Мутант не исключён и не удалён — `mutant.guard` заменяется, id и патч + остаются те же, значит принцип «убивается проверкой, не исключением» (#558) + соблюдён по конструкции. Лично прогнал 7 случайно выбранных id из разных + групп через `node scripts/mutation-gate.mjs --id=` — все 7 поймали + мутацию («заявленный тест покраснел на мутанте», 1 из 1). +- **Правило и гейт (AC3).** Текст `PROCESS.md` §2.7 точно описывает поведение + кода: рост выше 200 красит (`overLimit` → `stale++` → код возврата 2), id без + разметки — предупреждение, а не отказ (`missingReasons` → `warned++`, + не влияет на код возврата) — оба пути воспроизведены лично на синтетических + данных через `browserGuardPolicy()`. +- **Одна сборка на шард (AC4).** Логика выбора стратегии (`'seed'`/`'build'`/`'none'`) + доказана юнитом автора и лично воспроизведённой синтетической пробой; + откат к прежнему поведению при `bundleSeed === null` сохранён построчно + (`mutantBundleStrategy` возвращает `'build'`, когда `seedAvailable` ложно — + то же самое, что старое `if (guardNeedsBundle) buildBundle(dir)`). + Очистка временной директории (`dropBundleSeed`) обёрнута в `finally` вокруг + всего пути выполнения `main()`, включая ранний `return 2` из + `runCleanGuards` — утечки временных директорий не будет ни на одном пути + выхода (прочитано и прослежено по всем `return`). +- **Трейлеры.** `Issue: #659`, `User-Visible: no` — корректно для + инфраструктурного изменения без видимого поведения; изменений в `CHANGELOG*` + нет, что и требуется при `no`. +- **Один источник числа.** «200» и «284» видны в нескольких местах + (`PROCESS.md`, markdown-таблица, `npm run inventory`, `mutation-gate --check`), + но все читают один и тот же `browserGuardPolicy()`/`MUTANTS` — источник один, + расхождения исключены структурно (проверено чтением). +- Диф не трогает `src/**`, `custom_components/**/*.py`, `dist/**` — верно + заявленный класс B+C, и по-инфраструктурному верно, что смоки/голден/бэкенд + не выбраны (`smoke-select.mjs` подтвердил это лично). + +## Чего не проверял + +- Полный `node scripts/mutation-gate.mjs --check` по всем 1028 мутантам (со + сверкой якорей патчей) — не прогонял: дорого и уже покрыто зелёным Validate + на этом SHA (job «Фронтенд: типы, юниты, мутанты, синхрон бандла» + шесть + шардов «Мутанты по диффу»). Положился на CI. +- Сквозной (end-to-end) прогон нескольких мутантов одного шарда с реальным + переиспользованием `bundleSeed` — не воспроизводил (см. таблицу «чем + краснеет», AC4): только 4 id вообще проходят по этой ветке, площадь риска + малая, логика выбора стратегии проверена изолированно. +- Нативный Windows `gate:small` — не мой инструмент, положился на отчёт автора; + расхождение (`dev-build.test.mjs`) названо автором как известный + platform-only артефакт, не связанный с этим диффом. +- `python -m pytest tests_backend`, golden, browser smokes, performance — + не запускал: диф не задевает их входы (`custom_components/**/*.py` не + тронут, `src/**` не тронут), `smoke-select.mjs` и `check-docs.mjs` + подтверждают это лично, а не только по заявлению автора. +- Итоговый результат ручного полного прогона мутантов на этой ветке + (`36316263355`) — дождаться не мог, он ещё выполняется на момент вывода + вердикта; это и есть предмет находки Medium выше. + +## Вердикт + +Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче +Документ: (публикуется шагом конвейера в docs/reviews/) + +--- + + + +## Материал раунда + +- Ветка: `issue/659-mutation-smoke-guards`, коммит `6f8c2929afe4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `4450383a731fca02ac12313f041a25edf68b0a44` + ``` + git log --all --format='%H %T' | grep 4450383a731f + ``` +- Тело issue: `04adecd63ada54d09bd302fb922e9982e7c99ad71ba8be8cbcbd796fc644dce7` +- Вердикт конвейера: `yellow` · High 0