16 KiB
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 (areasgeometry/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, бывший хардкод в bashguardубран.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— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
558e5bdf66e297a7b56a94388dfd1ababf63252fgit log --all --format='%H %T' | grep 558e5bdf66e2 - Тело issue:
33bb0430c3c3ef213d1d69d5993e0c5e4d180fa56c5e46cd5e7849d6dab6677d - Вердикт конвейера:
green· High 0