Files
2026-10-01 00:15:51 +00:00

16 KiB
Raw Permalink Blame History

CODE-REVIEW-707-r1

Issue: #707 · Процесс: проверка риска треков, измерение эффективности и реализация параллельно ревью ТЗ Трек: ask (подтверждён явной меткой track:ask, аналитик 29.09) · заход r1 · блокирующих циклов 0/4 (лимит 4) Материал ревью: 1d51beade119ddb38e95172d36e19a678a2caa3b (единственный коммит ветки issue/707-risk-by-hunks поверх origin/dev) Validate на этом SHA: success — https://github.com/Matysh/houseplan-card/actions/runs/36794241466

Скоуп

Одна инфраструктурная правка (ни одного файла класса A): единое правило риска по изменённым участкам (scripts/change-risk.mjs), единый источник трека и его основания/лимита/ребейза (process-track.mjs), один вызов этого правила на шаге S7 конвейера (_process.yml), новые разделы пакета задачи (task-packet.mjs), строка риска подтверждённого ship в пакетном ревью (ship-review.mjs), перенос якорей реестра мутантов и правка канона (PROCESS.md, AUTHOR.md, REVIEWER.md, AGENTS.md). ТЗ закрыто зелёным ревью r2 (docs/reviews/SPEC-REVIEW-707-r2.md); единственная находка r1 (strings.json ошибочно числился источником ux) исправлена до кода и проверена в реализации (см. ниже).

Работа обслуживает техдолг конвейера (не строку docs/SCOPE.md — задача не продуктовая, сам SCOPE её не ограничивает); видимого пользователем поведения нет, User-Visible: no в трейлере коммита корректен, changelog не требуется.

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

Гейт Статус Как
typecheck, npm test, npm run build + bundle-policy --verify подтверждено Validate на точном SHA материала ссылка на прогон выше (#343) — не перегонял
Целевой перегон новых/изменённых тестовых файлов зелёный, прогнал сам node --test test/process-track.test.mjs test/task-packet.test.mjs test/ship-review.test.mjs test/review-doc-guard.test.mjs test/process-digests.test.mjs → 127/127 pass, 0 fail, 0 skipped
node scripts/entry-cost.mjs --check зелёный, прогнал сам author: reviewer 4592/9000 слов — бюджет не задет
node scripts/mutation-gate.mjs --check зелёный, прогнал сам все якоря ok, включая перенесённые guard-infra-keeps-ask-limit, packet-infra-track-ignores-show-default, pipeline-ship-ignores-limits; предупреждений 3 (как на dev, не добавилось)
node scripts/smoke-select.mjs --base origin/dev --head HEAD прогнал, применимости нет «Исполняемого frontend-диффа нет (src/**/*.ts не тронут)» — браузерные смоки этим диффом не выбираются, это не пропуск проверки, а нечего выбирать
«Тест умеет падать» — выборочная проверка подтвердил вручную на двух защитных местах (1) отключил проверку customElements.define( в change-risk.mjs → упал #707 AC1: каждая строка таблицы риска… (not ok, 1 fail); (2) убрал сверку автора строки владельца в ownerTrackLine → упал #707 AC2: происхождение трека… (not ok, 1 fail). Оба отката подтверждены git diff --stat = пусто
npm run invariants, python -m pytest tests_backend, junction parity, golden:verify, performance не прогонял — не применимо диффом не тронут ни один файл src/**, custom_components/**/*.py, зеркало junction limits; меток ci:golden/ci:full нет
npm run gate:small целиком, actionlint не перегонял author заявил зелёным в комментарии; Validate уже подтверждает typecheck/test/build на этом SHA, а YAML-синтаксис _process.yml косвенно подтверждён тем, что именно эта версия workflow сейчас исполняет данный прогон ревью (guard → prepare → review дошли до этого шага)

Находки

Нет. Ни одной High, ни одной Medium.

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

  • AC1 (классификатор риска). Прочитал scripts/change-risk.mjs целиком построчно. Проверил по актуальному dev, что каждый путь из таблицы К1 (areas geometry/touch/migration/devices/perf/visual:render/ visual:ui) существует в репозитории буквально — ни одной опечатки в 38 именах файлов src/ и 15 именах py. Прогнал тест AC1: каждая строка таблицы риска — положительный и отрицательный случай и убедился, что он умеет падать (см. таблицу гейтов). Отдельно проверил регэксп-детали вручную: snap(?:to|pt) не матчит snapshot/snapshotOf (geometry-негатив), pointerUp без регистронезависимости не матчит pointerUpdate (touch-негатив, намеренная эвристика), backdrop-filter: даёт один токен, а не два пересекающихся совпадения. Находка r1 ТЗ (strings.json) закрыта: TOKENS/AREAS не содержат strings.json, classify('custom_components/ houseplan/strings.json') даёт '?', тест AC1(к) и его негатив — на месте.
  • AC2 (происхождение трека). trackOrigin/ownerTrackLine: подтверждение только по автору строки = владельцу репозитория, только для текущего трека, только по самой поздней по времени строке (не по порядку массива). Тест различает цитату (> Трек: …), текст не в начале строки, чужого автора, трек не тот — все четыре дают «предложение», не подтверждение. Отдельно проверил, что комментарий самого конвейера (повышающий ship→show) не может сам себя подтвердить — тест на это есть.
  • AC3 (решение по ship на S7). decideTrack: рамки и риск повышают show одним комментарием; подтверждённый ship с риском только visual остаётся ship; этап spec/без ветки/инфраструктура дают пустой риск. Прогнал тест AC3 (подмножество выше) зелёным.
  • AC4 (шаг конвейера). Контрактные тесты разбирают реальный run: | текст _process.yml (шаг трека, guard, «Решение по вердикту») через bash -n и через настоящее исполнение в песочнице (bare origin, поддельный gh, настоящий git). Проверил глазами: шаг трека делает ровно один вызов process-track.mjs, метки меняются один раз и только по raise=true, risk_note/ship_risk доходят до промпта Review и до hp:ship-merge соответственно, маркер hp:ship-merge не менялся.
  • AC5 (заметка ревьюеру). Текст riskNote для show-неподтверждённого, show-подтверждённого и ask отличается ровно так, как того требует ТЗ и REVIEWER.md (эти формулировки я, как ревьюер, увижу в следующем show-заходе — они совпадают с конспектом буква в букву). Лимит 25 строк проверен тестом с «насыщенным» диффом по всем классам сразу.
  • AC6 (единый источник трека/лимита). Таблица сопоставления с воссозданным прежним bash-правилом (oldGuardLimit) гоняется по полному декартову произведению меток × сценариев файлов, включая 300+ файлов, отказ compare API и отсутствие ветки — совпадает везде, кроме намеренного расхождения (несколько трековых меток → строжайшая, а не старое поведение). Отдельно проверил, что guard в _process.yml не содержит SMALL, TRIVIAL, limit=2, classify(, files.length < 300 — своей логики трека не осталось.
  • AC7–AC11 (пакет). Разделы «Трек», «Следующий шаг», «Риск по участкам», «Обязательные проверки», «Changelog и визуальное свидетельство» — тесты бьют каждую комбинацию (четыре основания трека, чистое/конфликтное/ непроверяемое слияние через настоящий git merge-tree во временном репозитории, смоки трёх видов связи, ci:golden только при visual/render и не при правке только тестов/комментариев, отсутствующий changelog при User-Visible: yes). Пустые разделы действительно не печатаются (проверено тестом и сопоставлено с renderPacket).
  • AC12 (пакетное ревью ship). shipRiskFrom/renderShipBrief: строка риска печатается только если в комментарии hp:ship-merge есть маркер hp:ship-risk; комментарии до #707 (без маркера) дают null, старый SHIP_MERGE_MARKER_RE по-прежнему находит маркер слияния.
  • AC13 (канон). PROCESS.md §5/§5.1/§10.4/§11.7, AUTHOR.md, REVIEWER.md, AGENTS.md — сверил текст диффа с формулировками ТЗ построчно, расхождений не нашёл. entry-cost --check и mutation-gate --check зелёные (см. таблицу гейтов).
  • Трейлеры. Issue: #707, User-Visible: no — корректно: продукт (src/**, custom_components/**) диффом не тронут вовсе, видимого пользователем изменения нет.
  • Одно число — один источник (§8). 300 (потолок compare API) — только COMPARE_FILES_CAP в process-track.mjs, бывший хардкод в bash guard убран. 5 (лимит доказательств) — только RISK_EVIDENCE_LIMIT. 25 (лимит строк заметки) — только RISK_NOTE_LINE_LIMIT, тест сверяет значение через импортированную константу, а не повторяет число.
  • Якоря реестра мутантов. Три якоря, указанные в плане тестов ТЗ (guard-infra-keeps-ask-limit, packet-infra-track-ignores-show-default, pipeline-ship-ignores-limits), перенесены на новый код, а не удалены — проверил и диффом, и прогоном mutation-gate --check (все три — ok).
  • Контракт по монолиту. Тесты используют пути CARD_FILE/RUNTIME_FILE как данные классификатора, не как текст монолита: test/process-track.test.mjs не матчится регэкспом заморозки MONOLITH_ANCHOR_RE, список FROZEN_TEXT_ANCHOR_TESTS не вырос — прогнал monolith-text-anchors.test.mjs отдельно, зелёный.
  • Безопасность шага трека. Шаг «Трек задачи и рамки ship» в prepare по-прежнему тянет scripts/process-track.mjs архивом origin/dev, а не из ветки задачи — задача не может переопределить собственный классификатор риска и обмануть решение по ship. guard тоже чекаутится на ref: dev (вне диффа, не менялось).

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

  • Полный npm run gate:small и actionlint на материале — не перегонял локально; опираюсь на заявление автора и на подтверждённый Validate тем же SHA (включает typecheck/test/build — пересекается с частью gate:small).
  • Живой прогон шага трека на настоящем GitHub Actions runner (а не в песочнице с подменённым gh) — не наблюдался в рамках этого ревью; автор сам называет это риском («первый живой прогон ship и show после слияния — наблюдение, не AC»), и это корректно зафиксировано в ТЗ как остаточный риск, а не как неисполненный AC.
  • Пункты 3/5/6/7 исходного объёма (#727/#726/#728/#729) — вне скоупа #707, не разбирались.
  • Мутанты реестра не гонялись (трек ask, но мутанты по диффу не гоняются ни на одном треке в разработке — ночь #709); защиту трёх новых/перенесённых якорей проверил чтением патчей и прогоном mutation-gate --check, а не полной мутацией.

Вердикт

Зелёный. Все 14 AC доказаны тестами, которые я проверил на способность падать (выборочно — вручную два мутанта; остальные — чтением патчей реестра и сверкой с кодом). Находок нет.


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

  • Ветка: issue/707-risk-by-hunks, коммит 1d51beade119 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 558e5bdf66e297a7b56a94388dfd1ababf63252f
    git log --all --format='%H %T' | grep 558e5bdf66e2
    
  • Тело issue: 33bb0430c3c3ef213d1d69d5993e0c5e4d180fa56c5e46cd5e7849d6dab6677d
  • Вердикт конвейера: green · High 0