diff --git a/docs/reviews/CODE-REVIEW-707-r1.md b/docs/reviews/CODE-REVIEW-707-r1.md new file mode 100644 index 00000000..6ab136ff --- /dev/null +++ b/docs/reviews/CODE-REVIEW-707-r1.md @@ -0,0 +1,162 @@ +# 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