houseplan-card/docs/reviews/CODE-REVIEW-755-r1.md

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.

---

Материал раунда

• Ветка: issue/755-risk-false-positives, коммит d6ec1ed73632 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: beb9014ba3d795a87f97cf0488eaf687bcd74d37

  git log --all --format='%H %T' | grep beb9014ba3d7
  

• Тело issue: cb57107454cf9e74cee5e0b6d058e648ee6fe5fbf3c2986277afc7aa4cb62d69
• Вердикт конвейера: green · High 0 · маршрут fix