From 97f38ecaae930bc2f35073e9a24d775c1750826d Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:20:14 +0000 Subject: [PATCH] docs: review document for #772 Issue: #772 User-Visible: no --- docs/reviews/CODE-REVIEW-772-r1.md | 157 +++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-772-r1.md diff --git a/docs/reviews/CODE-REVIEW-772-r1.md b/docs/reviews/CODE-REVIEW-772-r1.md new file mode 100644 index 00000000..3c261fe4 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-772-r1.md @@ -0,0 +1,157 @@ +# CODE-REVIEW-772-r1 + +Issue: #772 «smoke-select и риск по участкам: оставшиеся слепые места» +Трек: `track:show`, инфраструктурный маршрут (`S7-code-review` без `S`-метки до первого ревью) +Заход: r1 · блокирующих циклов израсходовано 0 из 2 (лимит `show` — 2) +Материал: `50b163c60cfa8be8cc602ca37c41bc4eecd733cd` (ровно он, рабочая копия на нём) +База: `1f15ee143ed7a35c2e96bbfb1c272ae74a82263e` (= `origin/dev`, merge-base подтверждён) +Validate на материале: success — https://github.com/Matysh/houseplan-card/actions/runs/36889939473 + +## Скоуп + +Один коммит, только класс B (`scripts/`, `test/`, `docs/TESTING.md`); `src/` и +Python не тронуты. Четыре пункта из тела issue: + +1. `smoke-select` не связывал CSS-правку (`.room` в `plan.styles.ts`) со + смоком `smoke_room_fill_transitions`. +2. Вложенный литерал внутри аргумента (`key: { … }`) обрывал поиск вызывающей + функции на `:`, уводя правку в «неопределённость». +3. Сохраняемые декларации конфига вне `types.ts` (`interface StairCommon` и + соседи в `src/stairs.ts`) не давали класс риска `migration`. +4. Не было зарегистрированного мутанта на пропуск К1 #755 (строки импорта и + локальных типов не должны давать риск). + +Правка меняет только инструменты ревью-конвейера (выбор смоков и +классификация риска участков), не продукт — отсюда `User-Visible: no`, +подтверждено диффом (нет правок `docs/CHANGELOG*`, что и требуется при `no`). + +## Как проверялось + +| Гейт | Результат | Основание | +|---|---|---| +| `typecheck`, `npm test`, `npm run build` + `bundle-policy --verify` | не перегонялись | зелёный Validate на точном SHA материала (#343) — ссылка выше | +| `node --test test/smoke-select.test.mjs test/process-track.test.mjs test/task-packet.test.mjs` | **зелёный**, 81/81 | прогнал сам, для точечной проверки новых тестов сверх базового Validate | +| Три новые регрессии (CSS-связь, вложенный литерал, сохраняемый тип лестницы) **умеют падать** | подтверждено | временно откатил `scripts/{smoke-select,smoke-links,change-risk,task-packet}.mjs` до версии `origin/dev`, оставив новые тесты из HEAD — все три упали (`not ok 17`, `not ok 51`, `not ok 52`), затем восстановил файлы до HEAD (`git checkout HEAD -- …`), `git status` снова чист | +| `node scripts/mutation-gate.mjs --check` | **exit 0**, все 4 новых мутанта (`risk-counts-module-and-local-type-rows`, `risk-ignores-persisted-stair-types`, `smoke-select-ignores-style-file-links`, `smoke-select-stops-at-nested-property`) — `ok` (статическая проверка якорей; исполнение мутантов — ночной конвейер, #709) | прогнал сам | +| `git diff origin/dev...HEAD --name-only` | только `docs/TESTING.md`, 5×`scripts/*.mjs`, 3×`test/*.test.mjs` | подтверждает отсутствие `src/**`-диффа | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` (самопроверка инструмента на собственном диффе) | «Исполняемого frontend-диффа нет» | согласуется с заявлением автора: browser-smoke/golden не нужны, т.к. нет `src/**`-диффа | +| Трейлеры коммита | `Issue: #772`, `User-Visible: no` — оба корректны | `git log -1 --format=%H%n%s%n%n%b HEAD` | + +### Чего не проверял и почему + +- `npx tsc --noEmit` / `npm run build` / сверку трёх копий бандла — Validate на + этом SHA зелёный (#343), повторный прогон не добавляет информации для + инфраструктурной правки без `src/**`. +- Browser-smoke, `golden:verify`, `pytest tests_backend`, `npm run invariants` + — диффа в `src/**`, стилях плана или Python нет; сам `smoke-select` + подтверждает «Исполняемого frontend-диффа нет». AC задачи тоже не называют + ни один из этих гейтов. +- Исполнение зарегистрированных мутантов — не гоняется ни на одном треке в + разработке (#709); проверил только то, что от ревьюера требуется — что + мутант статически анкерится в текущем коде (`mutation-gate.mjs --check`). +- `node scripts/process-gate.mjs --issues` — автор заявил зелёный прогон в + комментарии; не перегонял, это не гейт этой задачи, а служебная проверка + самого хэндоффа. + +## AC · чем доказан · чем краснеет + +| AC (пункт issue) | Чем доказан | Чем краснеет | +|---|---|---| +| 1. CSS `.room` → `smoke_room_fill_transitions`, старый визуальный минимум не отменяется | `test/smoke-select.test.mjs`: «#772: CSS комнаты выбирает room-fill smoke…» (проверяет и структуру `selection`, и CLI-вывод, и что соседняя таблица стилей связь не получает) | тест падает на коде `origin/dev` (проверено откатом, `not ok 52`); зарегистрирован мутант `smoke-select-ignores-style-file-links` | +| 2. Вложенный литерал внутри аргумента не обрывает поиск вызова; границы callback/метода/присваивания/завершённого вызова сохранены | `test/smoke-select.test.mjs`: «#772: вложенные свойства аргумента…» (4 позитивных + 4 негативных варианта плюс интеграционный кейс на реальном `resolveIsoOverlayFitEnvelope`) | тест падает на коде `origin/dev` (`not ok 51`); мутант `smoke-select-stops-at-nested-property` | +| 3. Сохраняемый тип вне `types.ts` (лестницы) даёт `migration`, не даёт его на импорт/другой модуль/несохраняемый тип в том же файле, и итоговый трек `show` | `test/process-track.test.mjs`: «#772: сохраняемые типы лестниц…» — 6 позитивных мутаций (все поля/направления/объединение) × полный `decideTrack`, плюс 5 негативных (импорт именованный/деструктурированный, визуальный тип, кеш, другой файл) | тест падает на коде `origin/dev` (`not ok 17`); мутанты `risk-ignores-persisted-stair-types` (игнор контракта) и `risk-counts-module-and-local-type-rows` (общий guard К1 #755/#772 — ложное срабатывание на импортах/несохраняемых типах) | +| 4. Зарегистрирован мутант на пропуск К1 #755 | Строка реестра `risk-counts-module-and-local-type-rows` в `scripts/mutation-registry.mjs`, guard `--test-name-pattern="#755 AC1|#772"` | якорь проходит статическую проверку (`mutation-gate.mjs --check`, exit 0); исполнение — ночной прогон (#709), не гейт ревью | + +## Что проверено и корректно + +- `enclosingCallee` (`scripts/smoke-select.mjs:128–159`): добавление `:` в + список «прозрачных» предыдущих символов корректно пропускает вложенные + свойства объекта/массива внутри аргумента, но не ломает существующие + границы — колбэк (`() => {`), метод (`method() {`), отдельное присваивание + (`const options = {`) и завершённый вызов (`;` на нулевой глубине) + по-прежнему останавливают поиск. Проверил вручную граничный случай, который + не покрыт тестами явно — `switch`/`case N: { … }` внутри диффа: `case` не + даёт ложного символа, потому что (а) если перед ним стоит реальный + завершённый вызов, он останавливает поиск по `;` раньше, чем алгоритм + дойдёт до `case`; (б) `switch` как statement не может быть аргументом + вызова в валидном JS/TS, так что сценария «colon от case уводит поиск в + чужой реально открытый вызов» эмпирически не возникает — проверено прогоном + `parseDiff` на синтетическом ханке с `switch`/`case` внутри завершённого + `outerCall(x);`: `symbols: [], callees: []`. Не нахожу здесь дефекта. +- `registeredSmokes(parsed.symbols, parsed.executable)` и новое поле + `entry.files` (`scripts/smoke-links.mjs`): связь по файлу дополняет + `registered`, но **не** обнуляет `unproven`/`visualMinimum` — в `selectSmokes` + условие `!registered.some((entry) => entry.symbols.length)` специально не + учитывает файловые совпадения без символов, так что старый визуальный + минимум остаётся строго по комментарию «связь по файлу дополняет проверки, + но не доказывает покрытие изменённого контракта» — проверено и чтением, и + тестом (`assert.deepEqual(selection.visualMinimum, [...VISUAL_MINIMUM])`). +- `PERSISTED_TYPES` (`scripts/change-risk.mjs`): ключ — точный путь файла, + значение — множество имён деклараций, извлечённых `TYPE_NAME` из открывающей + строки блока. Разбор `moduleOrTypeDeclarations` (бывший `moduleOrTypeRows`) + корректно сохраняет обратную совместимость — старый экспорт + `moduleOrTypeRows` теперь тонкая обёртка (`new Set(...keys())`), поведение + для существующих потребителей не меняется; grep по репозиторию подтвердил, + что внешних потребителей, кроме самого `classifyRisk`, нет. Добавление + `migration` для персистентного типа не подавляет остальные классы для той же + строки — `add()` дедуплицирует по `${side}${path}:${line}` и копит правила + списком, то есть строка может одновременно нести и `migration` (персистентный + тип), и `geometry` (участок `stairs`) — ожидаемо и подтверждено тестом, + который проверяет именно `risk.classes.includes('migration')` без проверки, + что другие классы исчезли. +- `scripts/mutation-registry.mjs`: четыре новых мутанта используют `find`, + дословно совпадающий со строкой текущего кода (проверил построчно), и + `guard` с `--test-name-pattern`, который реально матчит имя соответствующего + нового теста (сверил руками и `mutation-gate.mjs --check`, который это же + и проверяет статически). +- `docs/TESTING.md`: добавленный абзац точно описывает новое поведение + (`files` в реестре, не отменяет визуальный минимум; `PERSISTED_TYPES`; + мутанты — только якоря в задаче, исполнение ночью) без завышения — нигде не + заявлено, что мутанты были прогнаны. +- `scripts/task-packet.mjs`: `collectInputs` теперь прокидывает `files` из + `selection.registered` в пакет задачи, `requiredChecks` печатает их с + пометкой «(файл)» — проверено тестом `#772: пакет объясняет связь без + символов` (`test/task-packet.test.mjs`) и совпадает с форматом `report()` в + `smoke-select.mjs`. +- Трейлеры коммита верны (`Issue: #772`, `User-Visible: no`); при `no` + changelog трогать не требуется, и диф его не трогает. +- Нет дублированных пользовательских чисел (§8 «одно число — один источник») + — правка не меняет ничего, видимого конечному пользователю карточки; все + изменённые величины (пороги, имена мутантов) — внутренние артефакты + ревью-инструментов, не часть продукта. + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Соответствие критериям `track:show` (§5) + +Задача проходит все пункты подсказки: сложность и риск малы (чистая логика +над уже существующими эвристиками, без продукта), одна поверхность +(инструментарий выбора смоков/риска участков), нет миграции конфига, нет +нового UX-контракта, нет влияния на perf/touch, ожидаемое поведение +зафиксировано в самом issue и теперь в `docs/TESTING.md`. Переклассификация +не требуется. + +## Вердикт + +Зелёный. Все четыре пункта issue закрыты исполнимыми регрессиями, которые +проверены на умение падать; мутанты анкерятся; гейты, применимые к этому +диффу, зелёные (Validate на SHA + точечный `node --test` + статическая +проверка мутаций); трейлеры корректны; документация синхронизирована с +кодом. + +--- + + + +## Материал раунда + +- Ветка: `issue/772-smoke-risk-blind-spots`, коммит `50b163c60cfa` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `acc9a9f067ea0078df4cdcf02beeb382f380fd5e` + ``` + git log --all --format='%H %T' | grep acc9a9f067ea + ``` +- Тело issue: `54a69596f02356934b58e0768f47d05dcffc78eea3f82954cebac8d75bf94405` +- Вердикт конвейера: `green` · High 0 · маршрут `fix` +