mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 20:29:00 +00:00
@@ -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 доказаны тестами, которые я проверил на способность падать
|
||||
(выборочно — вручную два мутанта; остальные — чтением патчей реестра и сверкой
|
||||
с кодом). Находок нет.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/707-risk-by-hunks`, коммит `1d51beade119` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `558e5bdf66e297a7b56a94388dfd1ababf63252f`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 558e5bdf66e2
|
||||
```
|
||||
- Тело issue: `33bb0430c3c3ef213d1d69d5993e0c5e4d180fa56c5e46cd5e7849d6dab6677d`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user