mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 13:18:58 +00:00
claude[bot]
@@ -0,0 +1,83 @@
|
||||
# CODE-REVIEW-755-r1
|
||||
|
||||
Материал раунда: `d6ec1ed73632496630b3c01935c3c514d5ad8b63` (`issue/755-risk-false-positives` поверх `dev` `88985d2e`, один коммит). Заход r1. Трек: show. Лимит циклов: 2 (0 потрачено). Issue: #755.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Инфраструктурная задача (класс B: `scripts/change-risk.mjs`, `test/process-track.test.mjs`, `PROCESS.md`). Продуктовый код (`src/**`, `custom_components/**`) не затронут. `User-Visible: no` — изменений в changelog нет, соответствует диффу.
|
||||
|
||||
Правка эвристики риска по изменённым участкам (#707): строки `import`/`export … from`/`export type`/голова `interface`/`type X =` и строки внутри такого блока декларации больше не дают класс риска для `.ts`-файлов класса A (кроме файлов участка `migration`, где типы — контракт конфига). Плюс сужение участков `stairs*` → `stairs`, `stairs-box`, `stairs-editor-model` и `config-*` → `config-adoption`, `config-store`, `config-reload-authority`, `config-write-conflict`; дедупликация заменённой строки в одно доказательство. PROCESS.md §5 получает одно предложение-исключение.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Прочитаны (в порядке из промпта): `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md`, тело issue #755 и три комментария (оценка владельца, взятие в работу, «Сделано»). PROCESS.md §5 (окружение правленого абзаца, таблица треков, рамки `ship`). Канонический документ подсистемы для этой задачи — сам PROCESS.md §5 (она и есть изменяемая подсистема); отдельного файла вроде `SUN.md` не требуется — задача про конвейер, не про продукт.
|
||||
|
||||
Код разобран построчно, не только прочитан: `scripts/change-risk.mjs` целиком (`isCommentOrBlank`, `isModuleOrTypeStatement`, `opensBlock`, `moduleOrTypeRows`, `parseUnifiedDiff`, `classifyRisk`) и весь добавленный блок `test/process-track.test.mjs` (`#755 AC1/AC2/AC3` плюс тест дедупликации заменённой строки). Прослежена работа парсера вручную на всех нетривиальных фикстурах теста (контекст ханка, блок-ключ `${block}${side}`, закрытие блока однострочной декларацией, закрытие по `}`, парный «заменённая строка») — построчная трассировка подтвердила, что код делает ровно то, что заявлено в ТЗ и в комментариях кода, без исполнения, чтением.
|
||||
|
||||
Проверено `ls`, что все файлы в сужённых участках (`stairs`, `stairs-box`, `stairs-editor-model`, `stairs-editor`, `stairs-view`, `config-adoption`, `config-store`, `config-fingerprint-pass`, `config-reload-authority`, `config-write-conflict`) реально существуют в `src/` — сужение не ссылается на несуществующие пути.
|
||||
|
||||
Проверено `grep` по `PROCESS.md` и `docs/process/*.md` на оставшиеся упоминания старых широких участков (`stairs*`, `config-*`) — не найдено, документация не разошлась с кодом.
|
||||
|
||||
### Гейты
|
||||
|
||||
| Гейт | Статус | Как |
|
||||
|---|---|---|
|
||||
| `npx tsc --noEmit`, `npm test`, `npm run build` + сверка бандла | подтверждены | зелёный Validate на `d6ec1ed7`: https://github.com/Matysh/houseplan-card/actions/runs/36859286288 — не перегонялись повторно (§8) |
|
||||
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | прогнан | вывод: «Исполняемого frontend-диффа нет (src/**/*.ts не тронут). Browser-smoke этим диффом не выбираются — выбирать нечего». Браузерные смоки не нужны |
|
||||
| `npm run golden:verify` | не нужен | нет метки `ci:golden`, диффа в пути отрисовки нет |
|
||||
| `python -m pytest tests_backend -q` | не нужен | `custom_components/**/*.py` не тронут |
|
||||
| `npm run invariants -- --config …` | не нужен | геометрия продукта не тронута (только `scripts/`, `test/`, `PROCESS.md`) |
|
||||
| performance-профили | не нужен | не назван в AC |
|
||||
| mutation-gate / мутанты | не гонял | трек `show`: мутанты в разработке не гоняются ни на каком треке (#709); защита каждого AC доказана встроенным отрицательным случаем в самом тесте, не мутантом реестра — это допустимая альтернатива по REVIEWER.md |
|
||||
|
||||
## AC — разбор
|
||||
|
||||
ТЗ в теле issue задаёт AC1–AC3, каждый с заполненной колонкой «чем краснеет» — пустой третий столбец отсутствует.
|
||||
|
||||
**AC1 (К1/К3: строки модулей и типов не дают риск, кроме участка `migration`).** Проверено чтением кода (`isModuleOrTypeStatement`, `moduleOrTypeRows`) и прослежено на всех фикстурах `test/process-track.test.mjs:308–341` (блок `#755 AC1`):
|
||||
- Ханк #741 (удалённый член под заголовком `export interface IsoOverlayFitEnvelopeInput {`) → `classes: []`; то же тело под `export class X {` → `raising: ['perf']`. Проверено трассировкой: `opensBlock` зависит от конца строки-заголовка (`{` открывает, `;` закрывает), что отличает интерфейс от класса корректно только по синтаксису declaration head, а не по имени — совпадает с заявленным правилом.
|
||||
- Член многострочного `import {`, его закрывающая `} from '...'`, реэкспорт (`export * from`, `export type { … } from`) — не дают риска; построчная трассировка подтвердила работу общего `ctx`-состояния на обеих сторонах (`-`/`+`) одного блока.
|
||||
- Импорт с токеном `migrateX` в монолите (`CARD_FILE`) не даёт `migration`, тот же токен в вызове функции — даёт; трассировка подтвердила: `continue` для типо/модульной строки происходит до проверки токенов, что обоснованно (импорт символа — не вызов).
|
||||
- `src/types.ts` (участок `migration`) — член интерфейса и новое поле сохраняют класс `migration`; подтверждено, что `AREAS.migration.some(r => r.re.test(p))` верно резолвится для `types.ts` и исключение К1 для него не срабатывает.
|
||||
- Python-файл с `import`/`type`-подобным текстом не подпадает под исключение (`&& p.endsWith('.ts')`) — проверено отдельным тестом (`auth.py` → `devices`).
|
||||
|
||||
Чем краснеет: без фикса ханк #741 давал `perf` (реальный случай, зафиксированный в #741 как ложное повышение `ship → show`); тест это воспроизводит явно двумя assert — с интерфейсом (ожидание `[]`) и с классом (ожидание `['perf']`), так что тест способен падать в обе стороны, а не только проверяет константу.
|
||||
|
||||
**AC2 (К2: сужение участков `stairs*`/`config-*`).** `test/process-track.test.mjs:386–396`. Прослежено по `AREAS.geometry`/`AREAS.migration` в `scripts/change-risk.mjs:38–56`: `stairs-view.ts` вне `geometry` (но в `visual:render`, т.к. в списке `visual:render` явно есть `stairs-view`), `stairs-editor.ts` нейтральная строка — без класса, указатель — `touch` через токен (не через участок), `stairs.ts`/`stairs-box.ts`/`stairs-editor-model.ts` — `geometry`. `config-fingerprint-pass.ts` вне `migration`, `config-store`/`config-adoption`/`config-reload-authority`/`config-write-conflict` — в `migration`. Все перечисленные файлы существуют в `src/` (проверено `ls`) — сужение не фиктивно. Чем краснеет: обратный revert `stairs*`/`config-*` в таблице участков сразу делает эти тесты красными (каждый путь — отдельный assert, не агрегат).
|
||||
|
||||
**AC3 (сквозное + канон).** `test/process-track.test.mjs:869–895`, тест на настоящем bash-шаге `_process.yml` (`#755 AC3`): диф вида #741 (удалённый член интерфейса) с `track:ship` → `raise=false`, `ship=true`, без комментария повышения; то же тело под `export class` → `raise=true`, `track: show`, комментарий содержит `perf: … (удалена) · участок iso-scene-render`. Это честная сквозная проверка — задействован реальный bash-степ пайплайна, не только чистая функция. PROCESS.md §5 получил предложение-исключение (К4), текст сверен построчно с `git show` — совпадает с тем, что заявлено в ТЗ и в комментарии «Сделано». Существующие тесты `#707`/`#726` в этом же файле не изменены и, по зелёному Validate, остаются зелёными.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Парсер `parseUnifiedDiff` корректно разносит «блок» (группу изменений между контекстными строками) и «сторону» (`-`/`+`), и `moduleOrTypeRows` берёт начальное состояние открытости блока из `ctx`, подражая тому, как git формирует заголовок ханка — проверено трассировкой на фикстуре с контекстом (`withContext`, ожидание одной записи `evidence.perf` на объединённой строке 13).
|
||||
- Дедупликация заменённой строки (новая возможность этого коммита) смешивает правила удалённой и добавленной строки в одну запись только когда они из одного `block`/`at`; разные ханки с одинаковым номером строки не считаются парой — подтверждено отдельным тестом (`test/process-track.test.mjs:~877`, последний assert: `counts.visual` = 2 для разных ханков).
|
||||
- Сужение участков `migration` не тронуло ранее работавшие сигналы: `src/types.ts` (`Marker.display`, `ServerConfig.volumetric_view`, тесты #588/#649) остаётся в `migration` — не только по словам ТЗ, но по прочитанному коду и тесту.
|
||||
- Все критерии §5 трека `show` для route выполняются: сложность низкая (чистая функция + тесты, без продукта), одна поверхность (один скрипт риска + его тест + его абзац в PROCESS.md), миграции конфига нет, нового UX-контракта нет, влияния на перф/touch продукта нет, поведение зафиксировано в самом PROCESS.md §5 и в теле issue. Маршрут вердикта — `fix`.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Мутационный реестр (`mutation-gate.mjs --check`) не перегонял сам — трек `show` их не требует в разработке (#709); доверился утверждению автора «как на dev» без повторного прогона, это осознанное сужение, а не пропуск.
|
||||
- Golden/perf/HA-pytest/invariants не запускал — диффом не задеты (обоснование в таблице гейтов выше).
|
||||
- Не проверял исторический прогон эвристики на 160 задачах с 15.09, который автор привёл в комментарии «Сделано» (таблица #741/#693/#694/#725/#624) — это аргумент автора о регрессии на истории, а не часть AC данной задачи; к вердикту не привязан, но выборочно свёл логику AC2 с реальными путями в репозитории (см. выше), расхождений не нашёл.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет. High: 0, Medium: 0, Low: 0.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. AC1–AC3 доказаны автотестами, каждый тест прослежен построчно и способен падать (проверено трассировкой на позитивном и негативном случае для каждого AC, не только констатацией). Дешёвые гейты подтверждены зелёным Validate на материале; по диффу и AC добавочные гейты не требуются — смоук-селектор подтвердил «выбирать нечего». Документация (PROCESS.md §5) синхронна с кодом. Маршрут — `fix`, задача проходит все критерии §5.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/755-risk-false-positives`, коммит `d6ec1ed73632` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `beb9014ba3d795a87f97cf0488eaf687bcd74d37`
|
||||
```
|
||||
git log --all --format='%H %T' | grep beb9014ba3d7
|
||||
```
|
||||
- Тело issue: `cb57107454cf9e74cee5e0b6d058e648ee6fe5fbf3c2986277afc7aa4cb62d69`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
Reference in New Issue
Block a user