mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 04:38:55 +00:00
claude[bot]
@@ -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` + статическая
|
||||
проверка мутаций); трейлеры корректны; документация синхронизирована с
|
||||
кодом.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/772-smoke-risk-blind-spots`, коммит `50b163c60cfa` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `acc9a9f067ea0078df4cdcf02beeb382f380fd5e`
|
||||
```
|
||||
git log --all --format='%H %T' | grep acc9a9f067ea
|
||||
```
|
||||
- Тело issue: `54a69596f02356934b58e0768f47d05dcffc78eea3f82954cebac8d75bf94405`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
<!-- hp:usage input_tokens=4389 output_tokens=25069 cache_creation_input_tokens=97828 cache_read_input_tokens=3012943 num_turns=47 -->
|
||||
Reference in New Issue
Block a user