Files
2026-10-01 16:20:14 +00:00

16 KiB
Raw Permalink Blame History

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"`

Что проверено и корректно

  • 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