mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 04:38:55 +00:00
claude[bot]
@@ -0,0 +1,201 @@
|
||||
# CODE-REVIEW-726-r1
|
||||
|
||||
Issue: [#726](https://github.com/Matysh/houseplan-card/issues/726) — «Процесс:
|
||||
выход из неудачного show — неверная классификация → ask без нового лимита».
|
||||
Трек `ask` (инфраструктура конвейера, подтверждена владельцем — ревью ТЗ
|
||||
зелёное). Заход r1, блокирующих циклов израсходовано 0 из 4.
|
||||
|
||||
Материал ревью: ветка `issue/726-reclassify-route` @
|
||||
`08d4c8e4554c0fcff0581a1b63e93eea496cd181` поверх `origin/dev` `52dc08a0`,
|
||||
один коммит (`Issue: #726`, `User-Visible: no`). Рабочая копия репозитория на
|
||||
этом SHA, `git fetch`/`checkout` на другой коммит не делался.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Чисто инфраструктурное изменение конвейера ревью (класс B): `src/**` и
|
||||
`custom_components/**` не тронуты, продуктового кода нет, `docs/SCOPE.md` и
|
||||
`docs/USER-GUIDE.ru.md` к этой задаче неприменимы — подтверждено
|
||||
`node scripts/smoke-select.mjs --base origin/dev --head HEAD` («Исполняемого
|
||||
frontend-диффа нет, тронуто файлов: 14», смоки не выбираются — выбирать
|
||||
нечего, а не пропуск проверки).
|
||||
|
||||
Изменённые файлы: `.github/workflows/_process.yml`, `PROCESS.md`,
|
||||
`docs/process/AUTHOR.md`, `docs/process/REVIEWER.md`,
|
||||
`scripts/process-track.mjs`, `scripts/review-doc-guard.mjs`,
|
||||
`scripts/review-result-gate.mjs`, `scripts/wait-verdict.mjs` и пять тестовых
|
||||
файлов. Соответствует заявленному в ТЗ разделу «Затронутые файлы» один в
|
||||
один.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Построчно прочитан весь `git diff origin/dev...HEAD` (не только тестовые
|
||||
файлы) — `scripts/process-track.mjs` (новые `SHOW_CRITERIA`, `routeNote`,
|
||||
`reviewRoute`, `routeComment`, `routeSummary`, CLI-команда `route`),
|
||||
`scripts/review-result-gate.mjs` (`ROUTES`, `verdictRoute`, `verdictProblems`),
|
||||
`scripts/review-doc-guard.mjs` (хвост якоря `materialAnchorBlock`, CLI
|
||||
`--route`/`--criterion`), `scripts/wait-verdict.mjs` (два новых вида
|
||||
`PIPELINE_EVENTS`), `.github/workflows/_process.yml` (схема вердикта, промпт,
|
||||
шаг «Решение по вердикту» целиком, публикация документа), `PROCESS.md` §4/§5/
|
||||
§7.2/§10.4 и оба конспекта.
|
||||
|
||||
Каждый контрактный пункт К1–К6 ТЗ сверен построчно с кодом (таблица AC ниже).
|
||||
Дополнительно:
|
||||
|
||||
- Прогнан весь тестовый набор (`node --test` по затронутым тестовым файлам:
|
||||
`process-track`, `review-result-gate`, `review-doc-guard`, `wait-verdict`,
|
||||
`publish-push-refusal`, `process-digests`) — 145/145 зелёных, 0 упавших.
|
||||
- Выполнены **две ручные мутации** на ключевых защитных путях, в обоих
|
||||
случаях тест покраснел, затем код восстановлен (`git diff` после — пусто):
|
||||
1. Убрана проверка `reclassify при зелёном вердикте` из
|
||||
`verdictProblems` (`scripts/review-result-gate.mjs`) → упал
|
||||
`test/review-result-gate.test.mjs` («#726 AC1», 15/16).
|
||||
2. `spentAfter >= limitAfter` заменено на `spentAfter > limitAfter`
|
||||
(`reviewRoute`, `scripts/process-track.mjs`) → упало 4 теста в
|
||||
`test/process-track.test.mjs` (AC3 и реал-bash сценарии исчерпания).
|
||||
- `node scripts/mutation-gate.mjs --check` — зелёный, 3 предупреждения,
|
||||
идентичные тем, что уже есть на `dev` (не новые).
|
||||
- `node scripts/entry-cost.mjs --check` — exit 0, бюджеты не превышены (author
|
||||
5126/12000, reviewer 4652/9000).
|
||||
- Подсчитан промпт ревьюера программно: ровно 1400 слов — автор заявил «было
|
||||
занято 1397, стало 1400»; тест `process-digests` держит лимит `≤ 1400`
|
||||
(`test/process-digests.test.mjs:192`) — проходит, но без запаса (см. Low
|
||||
ниже).
|
||||
- Проверено поведение `gh issue edit --add-label "a,b"`: официальная справка
|
||||
`gh issue edit --help` этой версии CLI (2.101.0) прямо документирует
|
||||
comma-joined значение (`--add-label "bug,help wanted"`) — подтверждает, что
|
||||
путь «два ярлыка одним вызовом» (ветка `owner-question` + одновременное
|
||||
исчерпание, `addLabels: ['blocked', 'review-4']`) не требует собственного
|
||||
парсинга в bash и работает тем же кодом, что уже покрыт реал-bash тестом на
|
||||
одном ярлыке.
|
||||
- Трейлеры коммита: `Issue: #726`, `User-Visible: no` — на месте;
|
||||
`docs/CHANGELOG.md`/`.ru.md` не тронуты, что с `User-Visible: no`
|
||||
согласуется. Коммит не несёт `demo/golden/baselines/**`, `Release:` не
|
||||
требуется.
|
||||
- Один источник числа: бюджет `spentAfter/limitAfter` вычисляется один раз в
|
||||
`reviewRoute` и далее используется без копий — и в комментарии
|
||||
(`routeComment`), и в сводке прогона (`routeSummary`), и в логе; хвост якоря
|
||||
документа несёт только `route`/`criterion`, не числа. Дублирования с
|
||||
независимым источником не нашёл.
|
||||
|
||||
## Критерии приёмки — доказательство и «чем краснеет»
|
||||
|
||||
| AC | Чем доказан | Чем краснеет |
|
||||
|---|---|---|
|
||||
| AC1 (граница доверия) | `test/review-result-gate.test.mjs` «#726 AC1» | Отрицательные случаи в самом тесте (`route` вне словаря, `green`+`reclassify`) плюс мутация реестра (убрана строка отказа) — красный |
|
||||
| AC2 (таблица маршрутов) | `test/process-track.test.mjs` «#726 AC2» | Таблица случаев включает все отрицательные ветки (ask/spec/без критерия/с посторонним `criterion`, включая `__proto__`, число, объект) — построчно сверено с кодом `reviewRoute` |
|
||||
| AC3 (немедленный `review-4`) | «#726 AC3» + реал-bash «AC5 … исчерпание, вопрос владельцу» | Мутация `>=`→`>` на `exhausted` — красные 4 теста (проверено лично) |
|
||||
| AC4 (бюджет через треки) | «#726 AC4» (`reviewCounters`+`cycleLimit`+`reviewRoute` вместе) | Явные границы r3 (не исчерпан) / r4 (исчерпан) и раздельный счёт `CODE-REVIEW`/`SPEC-REVIEW` в одном дереве документов |
|
||||
| AC5 (проводка `_process.yml`) | «#726 AC5» (3 теста: разбор workflow + два реал-bash прогона настоящим `bash`+фейковым `gh`) | `bash -n` на изменённые `run` в одном из тестов; реал-bash тесты проверяют фактические вызовы `gh` по сценариям reclassify/exhausted/owner-question/fix/green/сбой |
|
||||
| AC6 (якорь документа) | «#726 AC6» (два теста: unit + CLI на настоящем процессе) | Границы «вне словаря», «не по формату criterion», «route отсутствует» — явно проверены как «не попадает в якорь» |
|
||||
| AC7 (`wait-verdict`) | «#726 AC7» (2 теста, включая асинхронный `waitForVerdict` с фейковым `readSnapshot`) | Проверен порядок находки события при совместном `exhausted`+`owner-question` в одном комментарии (`.find` берёт `exhausted` первым — перепроверено чтением `PIPELINE_EVENTS`/`find`) |
|
||||
| AC8 (канон) | `test/process-digests.test.mjs` (новые строки в `KEY_RULES` §4) | Тест на точное вхождение ключевой фразы канона — пропадёт с правкой текста §4 |
|
||||
| AC9 (гейт) | `mutation-gate --check` (прогнан лично, 3 предупреждения = как на dev), `entry-cost --check` (прогнан лично, exit 0), якорь `process-label-step-combined-again` найден (`scripts/mutation-registry.mjs:4085`) и цель патча (`status-label.mjs --repo=...`) не изменена — проверено `grep` | — (учётный AC, не защитный по существу) |
|
||||
|
||||
Все девять AC доказаны автотестами, которые я лично прогнал; для AC1 и AC3
|
||||
проверка «тест умеет падать» сделана прогоном собственной мутации, а не
|
||||
доверием к заявлению автора.
|
||||
|
||||
## Найдено
|
||||
|
||||
### Low — дисмиссед с записью
|
||||
|
||||
**Нет реал-bash теста на совместный ярлык `blocked,review-4` одним вызовом
|
||||
`gh issue edit`.** Единственный путь, где `reviewRoute` реально отдаёт два
|
||||
ярлыка в `addLabels` одновременно (`['blocked', 'review-4']`), — `show`,
|
||||
подтверждён владельцем, `spent` на входе `limit - 1` (т.е. последний
|
||||
разрешённый заход). Юнит-тест `reviewRoute` это проверяет
|
||||
(`test/process-track.test.mjs:1053`), но реал-bash тесты «#726 AC5 …» гоняют
|
||||
`owner-question` только при `SPENT: '0'` (не исчерпывающий заход), так что
|
||||
фактический вызов `gh issue edit --add-label "blocked,review-4"` через
|
||||
настоящий bash не воспроизведён.
|
||||
|
||||
Решение — не возвращать в работу: сам bash-путь к этому моменту уже не
|
||||
содержит специального кода для «нескольких ярлыков» — `$add` подставляется
|
||||
вербатим в `--add-label "$add"` тем же кодом, что покрыт реал-bash тестом на
|
||||
одном ярлыке; разветвления по числу элементов в bash нет. Официальная справка
|
||||
`gh issue edit --help` текущей версии CLI прямо документирует
|
||||
comma-joined-значение как штатный способ задать несколько меток одним
|
||||
флагом. Риск гипотетический, не поведенческий дефект; отдельного issue не
|
||||
завожу — достаточно отметить здесь.
|
||||
|
||||
Больше находок, блокирующих или нет, не обнаружено.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Вся контрактная логика К1–К6 ТЗ #726 реализована и совпадает построчно:
|
||||
поля `route`/`criterion` схемы, граница доверия, `reviewRoute` как
|
||||
единственная точка решения, `routeComment`/`routeSummary`, якорь документа,
|
||||
`wait-verdict`, обновление канона и обоих конспектов.
|
||||
- Все отклонения от ТЗ, перечисленные автором в комментарии «Сделано»
|
||||
(пункты 1–8), я сверил с кодом по отдельности — каждое подтверждено (хвост
|
||||
строки якоря, 1400 слов промпта ровно, заметка риска `show`+confirmed,
|
||||
текущие метки через `gh issue view` с фолбэком на guard, перечень
|
||||
документов+комментариев при исчерпании, отдельный короткий комментарий для
|
||||
«`reclassify` не применён», совмещённый комментарий `exhausted`+
|
||||
`owner-question`, добавленный тест в `publish-push-refusal.test.mjs`).
|
||||
- Инвариант «`reclassify` сам никогда не ставит `review-4`, потому что guard
|
||||
на `show` не пускает третий заход» — перепроверен чтением и совпадает с
|
||||
явным комментарием в коде и тестом AC3.
|
||||
- Безопасность текста: недоверенный `criterion` от модели нигде не протекает
|
||||
в Markdown/HTML конвейера без `safeToken`/регулярки формата — проверено на
|
||||
вредоносном значении (`` q` --> <b> `` → `q??--???b?`) и в
|
||||
`routeComment`, и в `materialAnchorBlock`.
|
||||
- Старые записи, не несущие `route` (вердикты до #726), продолжают читаться
|
||||
как `fix` на границе доверия и как прежняя строка якоря — обратная
|
||||
совместимость подтверждена тестами и собственным чтением кода.
|
||||
- Guard-страховка `spent -ge limit` на входе в `S7`/`S4` не тронута —
|
||||
осталась дублирующей защитой, как и заявлено в ТЗ (не-скоуп).
|
||||
- Трейлеры коммита и отсутствие изменений в changelog корректны для
|
||||
`User-Visible: no`.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- **Полный `npx tsc --noEmit` / `npm run build` + сверка бандла.** Не
|
||||
гонял: Validate на этом точном SHA (`08d4c8e4`) уже зелёный
|
||||
([прогон](https://github.com/Matysh/houseplan-card/actions/runs/36807983560)),
|
||||
диффа в `src/**`/`dist/**` нет, бандл не копия-чувствителен к этой правке.
|
||||
- **`npm test` целиком единым прогоном** — явно не гонял (только
|
||||
затронутые тестовые файлы отдельными `node --test` вызовами, 145/145,
|
||||
плюс один полный прогон `npm test` в процессе проверки, тоже зелёный,
|
||||
3382/3383 passed, 1 skipped, без отношения к #726). Этого достаточно при
|
||||
зелёном Validate на этом SHA.
|
||||
- **Браузерные смоки.** Не прогонял: `smoke-select.mjs` отвечает «нечего
|
||||
выбирать» (frontend-диффа нет) — не относится к «слабой связи», это прямое
|
||||
«неприменимо».
|
||||
- **`npm run golden:verify`, `pytest tests_backend`, `npm run invariants`,
|
||||
performance.** Не прогонял — ни метки `ci:golden`, ни правок Python, ни
|
||||
геометрии/ссылок на неё, ни AC с performance в задаче нет.
|
||||
- **Мутанты по диффу (ночной реестр).** Не запрашивались и не гонялись —
|
||||
по правилу трека на этапе разработки (#709); ревьюер применяет их только
|
||||
проверкой, что защита названа мутантом в реестре (см. AC9) — сделано.
|
||||
- **`actionlint`** на `_process.yml` — не запускал (инструмент не установлен
|
||||
в этой среде); синтаксическая валидность изменённых `run`-блоков проверена
|
||||
тестом `bash -n` (входит в прогнанный `process-track.test.mjs`), что
|
||||
покрывает основной риск опечатки в shell.
|
||||
- **Живое поведение на реальном GitHub-событии** (первый настоящий
|
||||
`reclassify` на проде). Согласно ТЗ, это заявлено как «наблюдение, не AC» —
|
||||
не предмет этого ревью.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Код делает ровно то, что заявлено в ТЗ, каждый AC доказан автотестом с
|
||||
проверенной способностью падать (для двух ключевых защитных путей — лично
|
||||
прогнанной мутацией), обратная совместимость подтверждена, трейлеры и
|
||||
гейты в порядке. Единственная находка — Low, дисмиссед с записью выше и не
|
||||
требует правки.
|
||||
|
||||
**Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/726-reclassify-route`, коммит `08d4c8e4554c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `15b43118d1ebe033d66d2b797a4846c6f8efbada`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 15b43118d1eb
|
||||
```
|
||||
- Тело issue: `daf13164d67eb2612f80437127463957f152876d86adf9f60eda40d55848ef33`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user