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

CODE-REVIEW-737-r1

Материал ревью: ветка issue/737-review-usage @ 10a1c0013395e2720242dec70baab27e149e5684 (рабочая копия уже на нём; git diff origin/dev...HEAD — 13 файлов, 809 вставок / 28 удалений). Трек: ask (критерии §5 complexity, surfaces). Заход r1, блокирующих циклов израсходовано 0 из 4 (предыдущий красный Validate на d1954183 ревью не запускал — цикл не потрачен, #510).

Скоуп

Инфраструктурная задача без файлов класса A: конвейер снимает расход модели (execution_file шага Review) и пишет его одной машинной строкой в документы ревью (CODE-REVIEW/SPEC-REVIEW через блок якорей, SHIP-REVIEW — после машинного блока), а «Метрики процесса» читают эту строку вместо прежнего ненадёжного регэкспа по прозе документа. Три поверхности по ТЗ: К1–К2 (scripts/model-usage.mjs, новый), К3–К5 (_process.yml, _ship-review.yml, review-doc-guard.mjs, ship-review.mjs), К6 (process-metrics.mjs), К7 (PROCESS.md §10.4). Второй коммит — точечный фикс пайплайна (красный Validate на d1954183, причина названа и устранена без ревью, см. ниже).

Эта задача не обслуживает ни один Job из docs/SCOPE.md напрямую — она процессная (измерение самого конвейера ревью), что соответствует её инфраструктурному классу и треку ask, поднятому по complexity/surfaces, а не по продуктовому критерию.

Как проверялось

Полный разбор (не дельта): это первый код-ревью задачи, предыдущая попытка была остановлена гейтом Validate до начала ревью модели и ревью не потребляла.

• Прочитан диф целиком (git diff origin/dev...HEAD) — оба workflow-файла, PROCESS.md, новый scripts/model-usage.mjs, правки process-metrics.mjs, review-doc-guard.mjs, ship-review.mjs, все изменённые тесты.
• Прочитано ТЗ (тело issue #737) целиком, сверены все 7 AC с кодом и тестами построчно, а не на веру по комментарию автора.
• Прочитаны оба коммита (ada91e31, 10a1c001) — трейлеры, сообщение, причина фикса во втором коммите.
• Проверено, что диффу не нужны: src/** не тронут (node scripts/smoke-select.mjs --base origin/dev --head HEAD → «Исполняемого frontend-диффа нет… браузер-смоки не выбираются»), Python не тронут, геометрия/инварианты не тронуты, ci:golden не назначен, performance в AC не назван.
• Дешёвые гейты (tsc, npm test, npm run build + сверка бандла) на этом SHA подтверждены зелёным Validate (run 36859565821) — не перегонялись заново, по инструкции раунда.

Разбор по AC

AC	Проверено	Вывод
AC1 (К1: снятие)	Код usageFromExecutionFile/usageFromMessages прочитан построчно; сверен с фикстурой test/model-usage.test.mjs (сумма по modelUsage двух моделей, откат на result.usage, no-result/no-execution-file/unreadable, секрет из tool_result не печатается, код выхода CLI 0 во всех ветках). Негативные случаи (дробь, отрицательное, пропущенное поле модели, 13 цифр) ведут к unreadable, а не к неверной сумме — проверено чтением и соответствием тесту.	Доказан, тест умеет падать (конкретные мутации входа дают разные reason)
AC2 (К2: формат)	DATA_RE/NONE_RE построены с якорями ^…$, COUNT исключает ведущий ноль и ограничивает 12 цифрами. Таблица отказов в тесте (лишний/пропущенный ключ, другой порядок, знак, дробь, 13 цифр, 40 hex, --> внутри, перевод строки) — все дают null/invalid. Совместимость с регэкспом #728, вписанным в тест буквально, подтверждает, что hp:usage-none им не ловится (пробел после hp:usage — осознанный разделитель).	Доказан
AC3 (К4: документ ТЗ/кода)	materialAnchorBlock дописывает строку последней, withMaterialAnchors не дублирует её при повторной публикации (ребейз + второй push — реальный bash/git в publish-push-refusal.test.mjs, строка одна). CLI --usage= отличает «флага нет» (вызов до #737) от «флаг пуст» (missing) через argv.some(...) — проверено чтением и отдельным тестом с anchor() без флагов. Прежние функции чтения блока (anchorVerdictFrom и др.) не задеты — тест сравнивает их результат до/после добавления строки.	Доказан
AC4 (К5: документ ship)	anchorBlock кладёт строку сразу после закрывающего ``` блока, содержимое блока (which читают гейт беты и покрытие) не меняется — тест сравнивает fenced(withUsage) === fenced(plain) и shipCoverage на обоих вариантах документа. parseAnchorBlock берёт lastUsageIn после последнего маркера — строка в прозе выше блока не в счёт (отдельная проверка).	Доказан
AC5 (К3: проводка)	Прочитаны оба workflow — шаг «Снять расход модели» идёт сразу за id: review, if: always(), continue-on-error: true, несекретный EXEC, выход line. execution_file нигде не выгружается и не печатается (grep по обоим файлам без строк-комментариев — только один EXEC: … на файл). REQUIRED_FILES (review-result-gate.mjs) не менялся — сверено импортом в тесте. Контрактный тест разбирает реальный YAML (срез job/шага текстом) плюс исполняет извлечённое тело на настоящем bash (bash -n и реальный прогон с GITHUB_OUTPUT/GITHUB_STEP_SUMMARY) — не заявление, а исполнение.	Доказан, в т.ч. исполнением
AC6 (К6: читатель)	reviewDocUsage берёт строку только после ANCHOR_MARKER (ревью ТЗ/кода) или последнего SHIP_REVIEW_ANCHOR (ship) — прочитан код и тест, где документ r2 цитирует r1 в прозе и сумма r1 не задваивается. hp:usage-none увеличивает missing, не входит в totals, не печатается нулём (`doesNotMatch(/input_tokens	\b0\b/)). Старый USAGE_LINE_RE` (ловил любую строку в прозе) полностью удалён — копии формата у читателя больше нет.
AC7 (К7: канон)	Абзац «Расход модели» в PROCESS.md §10.4 прочитан и сверен с кодом: источник, формат, 6 причин (no-execution-file, unreadable, no-result, no-usage + missing, invalid у публикации) в том же порядке, что USAGE_REASONS в коде; «выход job, а не artifact» и причина — совпадают с К3. docs/process/* не тронуты — соответствует «ни автор, ни ревьюер ничего нового не делают».	Доказан чтением

Точки, проверенные отдельно (не только по заявлению автора)

• Кавычки и границы данных. --usage="$USAGE" в _process.yml и process.env.USAGE в _ship-review.yml — оба способа передачи переживают строку с пробелами внутри (формат К2 — пять ключей через пробел) без разбиения на слова: первое — потому что значение в кавычках как один аргумент shell, второе — потому что это не shell, а process.env.
• Разбор флага --usage=. value(name) в review-doc-guard.mjs вырезает фиксированный префикс длиной name.length+3, а не до первого =, поэтому значение, само содержащее = (оно содержит, формат — key=N), не обрезается на первом вхождении. Проверено чтением реализации и тестом с реальной строкой данных.
• Два отдельных трейлера Issue: #737/User-Visible: no на обоих коммитах — на месте; оба changelog не тронуты, что и требуется при no. Видимого пользователю числа в этом диффе нет вообще (только внутренний машинный формат и число в еженедельном отчёте владельца) — повторного источника для одного и того же числа не возникает: tokenUsage — один читатель одного формата, который пишет один модуль (model-usage.mjs).
• Второй коммит (фикс Validate). Причина (pre-push-gate.test.mjs: HOOK_FILES не содержал новый scripts/model-usage.mjs, импортируемый review-doc-guard.mjs через цепочку process-gate) подтверждена чтением test/pre-push-gate.test.mjs:144-147 — правка (одна строка в списке) соразмерна причине, лишнего не задела.
• Риск подмены рабочей копии моделью перед шагом снятия расхода (признан автором в ТЗ, «Граница доверия»). Шаг node scripts/model-usage.mjs выполняется в том же job, что и предыдущий шаг review-result-gate.mjs — это не новый класс риска, а то же самое уже принятое архитектурное решение (#556): недоверенная стадия модели выполняет локальные скрипты в своей же рабочей копии до перехода на integrate/publish, где нет Bash/Write у модели. Новый шаг ничего не меняет в границе доверия, только добавляет источник отчётной (не решающей) величины.

Находки

Высоких и средних находок нет.

Низкое, не моё: автор сам отметил в комментарии (не относится к этому диффу, предсуществующий косметический дефект) — withMaterialAnchors копит --- при повторном вызове, и обещал завести отдельный issue. Эта задача его не трогает и не обязана чинить.

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

• Все 7 AC — см. таблицу выше, каждый либо доказан исполняемым тестом с содержательными негативными случаями, либо (контракт по YAML) частично исполнением извлечённого тела на bash.
• Граница доверия #556 не нарушена: REQUIRED_FILES не изменился, расход — выход job, а не запечатанный artifact; публикация разбирает строку строго (invalid/missing вместо падения или мусора в документе).
• Старый небезопасный регэкс читателя (USAGE_LINE_RE, ловил строку в прозе и задваивал цитаты) полностью убран, новый формат имеет одного автора и одного читателя (model-usage.mjs).
• Трейлеры корректны, User-Visible: no обоснован (видимого пользователю поведения нет, только процессная телеметрия для еженедельного отчёта).
• Второй коммит — точечный, по делу, причина явно названа и совпадает с кодом.
• PROCESS.md §10.4 (К7) синхронизирован с кодом по перечню причин и формату.

Чего не проверял

• Не перегонял npx tsc --noEmit, npm test, npm run build + сверку трёх копий бандла — Validate на этом же SHA (10a1c001, run 36859565821) уже зелёный, бюджет раунда не тратится на повтор.
• Браузерные смоки не прогонял — smoke-select.mjs --base origin/dev --head HEAD подтвердил, что src/** не тронут и выбирать нечего; в AC браузер прямо назван ненужным («Браузер не нужен, мутанты не гоняются»).
• python -m pytest tests_backend не прогонял — диф не касается custom_components/**/*.py.
• npm run invariants не прогонял — геометрия и ссылки на неё не затронуты.
• npm run golden:verify не прогонял — метки ci:golden нет, рендер карточки не изменён.
• Performance-профили не прогонял — в AC не названы.
• Мутанты по диффу не запрашивались и не применялись (трек ask, но мутанты в разработке не гоняются ни на одном треке — #709, ночной реестр отдельно).
• Не выполнял вживую gate:small, mutation-gate --check, entry-cost --check, reviews-index --check, process-digests — заявлены автором как зелёные и совпадают по косвенным признакам с прочитанным кодом (списки причин, число слов промпта не менялось — правки вне текста промпта модели), но не перепрогонялись отдельно, так как входят в состав уже подтверждённого Validate/gate:small.

Вердикт

Зелёный. Все AC доказаны исполняемыми тестами с содержательными негативными случаями (кроме части AC5, где контракт YAML частично доказывается чтением и частично — исполнением извлечённого тела на bash, что отдельно отмечено). Находок High/Medium нет.

---

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

• Ветка: issue/737-review-usage, коммит 10a1c0013395 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 9682f3784bc6a18bc6327a0b0752e58042a2cc9b

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

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