mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,69 @@
|
||||
# CODE-REVIEW-516-r1
|
||||
|
||||
**Issue:** #516 — merge-candidate: patch-id кандидата включает собственный документ ревью
|
||||
**Материал:** `8c13b5d16de41db7adfc3ecd00710fb9837ed432` (`origin/dev..HEAD` — один коммит: `ci: merge-candidate compares patch-ids without the review documents`)
|
||||
**Этап:** code · заход r1 · блокирующих циклов израсходовано 0 из 2
|
||||
**Класс изменения:** B (гейты/инструменты) — `scripts/merge-candidate.mjs`, `scripts/mutation-gate.mjs`, `test/merge-candidate.test.mjs`. Ни одного файла класса A — задача не трогает продукт.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Симптом (описан в issue #514/#508): `merge-candidate.mjs` считал `patchId(materialBase, material)` и `patchId(devNow, candidate)` по полному диффу, включая `docs/reviews/**`. Кандидат (вершина ветки) уже несёт собственный `CODE-REVIEW-N-rKmd`, материал (SHA, прочитанный ревьюером) — нет. Поэтому patch-id расходились всегда, когда `dev` двигался хотя бы одним паблиш-коммитом документа другой задачи, и зелёный кандидат уходил на повторное ревью без единой содержательной причины.
|
||||
|
||||
Правка: `realOps.patchId` считает `git diff --full-index from to -- . ':!docs/reviews'` — то же исключение, которое `reviewedFresh` рядом уже применяет через `diffNames`. Добавлен мутант `merge-rereviews-own-review-doc` (снимает pathspec) и тест на настоящем git, воспроизводящий ровно сценарий #514/#508.
|
||||
|
||||
Это ТЗ лёгкого трека, живёт в теле issue; отдельного файла в `docs/specs/` нет и не должно быть — соответствует.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты на этом SHA уже зелёные (Validate `8c13b5d1`, https://github.com/Matysh/houseplan-card/actions/runs/34405162422) — `npx tsc --noEmit`, `npm test`, `npm run build` со сверкой бандла и `check-docs` не перегонял: diff не трогает `src/**`, класс A отсутствует, геометрия/`layout`/толщина стен не затронуты — инварианты модели, golden, смоки и backend-тесты к этой задаче не относятся.
|
||||
|
||||
Прогнал точечно, поскольку diff их непосредственно меняет:
|
||||
|
||||
1. `node --test test/merge-candidate.test.mjs` — 13/13 зелёных, включая новый `#516 AC1`.
|
||||
2. Дисциплина «тест умеет падать» — вручную откатил pathspec в `patchId` (убрал `'.', ':!docs/reviews'`), прогнал тот же файл: тест `#516 AC1` красный (`'rereview' !== 'push'`), остальные 12 держатся. Откатил правку файла обратно, `git status`/`git diff` чистые.
|
||||
3. `node scripts/mutation-gate.mjs --check` — весь реестр валиден, патчи применяются к текущему коду.
|
||||
4. `node scripts/mutation-gate.mjs --id=merge-rereviews-own-review-doc` — «поймано 1 из 1», совпадает с заявленным в handoff.
|
||||
5. `node --test test/mutation-gate.test.mjs` — 42/42, реестр (guard-файлы, применимость патчей) не сломан добавлением записи.
|
||||
6. Прочитал `decideMerge`/`mergeCandidate` целиком, сверил, что `patchId` — единственное затронутое место, а `reviewedFresh` (уже использующий то же исключение) не тронут и логически согласован с новым `patchId` — **проверено чтением, не исполнением** отдельно от юнит-тестов.
|
||||
|
||||
## Проверка AC
|
||||
|
||||
- **AC1** («сдвиг dev только документами ревью не возвращает зелёный кандидат на повторное ревью») — доказан автотестом `#516 AC1` на настоящем git (materialBase без документа, кандидат с публикацией `CODE-REVIEW-9-r1.md`, `dev` двинут чужим `CODE-REVIEW-8-r2.md`): `action === 'push'`, комментария с «patch-id» нет, оба документа и код доехали до `origin/dev`. Тест умеет падать (см. п.2 выше). **Выполнен.**
|
||||
- **AC2** («реальное изменение содержимого патча при ребейзе по-прежнему возвращает в S7») — покрыт существующим unit-тестом `patch-id изменился при ребейзе — S7-code-review...` (test/merge-candidate.test.mjs:130, мокнутые patchId). Этот тест не тронут правкой и остаётся зелёным (входит в 13/13). Логика неравенства patch-id в `mergeCandidate` не менялась — менялась только сама функция `patchId`, а не место её использования, поэтому мок на уровне `decideMerge`/`mergeCandidate` достаточен: содержательное изменение патча (не в `docs/reviews`) по-прежнему учитывается pathspec `.` наравне с остальным деревом. **Выполнен.**
|
||||
- **AC3** («мутант пойман штатным раннером», `User-Visible: no`) — `merge-rereviews-own-review-doc` зарегистрирован с guard `node --test test/merge-candidate.test.mjs` (согласовано со стилем соседних мутантов этого файла — той же группы, не требующей пересборки бандла), `--id=` прогон даёт «поймано 1 из 1». Трейлеры коммита: `Issue: #516`, `User-Visible: no` — присутствуют; changelog (`docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`) не тронут, что корректно при `User-Visible: no`. **Выполнен.**
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Диапазон правки минимален и точно соответствует пункту ТЗ 1–3, скоуп не расширен (только заявленные 3 файла).
|
||||
- `patchId(materialBase, material)` и `patchId(devNow, candidate)` теперь считаются симметрично одному и тому же правилу исключения, что и `reviewedFresh` — нет риска, что одна проверка учитывает `docs/reviews`, а другая нет.
|
||||
- Мутант зарегистрирован в том же стиле (`because`, `guard`, `patches.find/replace` на точную строку кода) — реестр не разъезжается со стилем соседних записей.
|
||||
- Пуш кандидата в ветку и последующий Validate по-прежнему выполняются на кандидате, несущем документ ревью — исключение из patch-id не означает исключение файла из самого коммита/пуша, документ едет в `dev` вместе с кодом (проверено тестом AC1).
|
||||
- Единственное число, видимое пользователю, здесь не применимо — задача не выводит величин пользователю (`User-Visible: no`), правило «одно число — один источник» не задействовано.
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- Полный `npm test` / `npx tsc --noEmit` / `npm run build` со сверкой бандла — не перегонял, засчитан зелёный Validate на этом же SHA (`8c13b5d1`, ссылка выше); прогнал только файлы, которые эта задача непосредственно меняет или из которых прямо следует поведение (`merge-candidate.test.mjs`, `mutation-gate.test.mjs`, `mutation-gate.mjs --check`/`--id`).
|
||||
- `node scripts/check-docs.mjs` — не запускал: diff не трогает `src/**`, отпечаток документации не мог устареть от этой правки.
|
||||
- `npm run invariants`, golden, backend pytest, performance-профили, `demo/smoke_*.mjs` — не относятся: diff не трогает геометрию, рендер, `custom_components/**/*.py` или чувствительные к перфу пути; в AC ни один смок не назван.
|
||||
- Реальное поведение GitHub Actions (`gh workflow run`, `gh run list`, `--force-with-lease` против настоящего GitHub API) — не воспроизводилось, только через `realOps` на локальном bare-репозитории в тесте (сеть/API не поднимались); это тот же уровень доказательства, что и у остальных тестов файла, включая уже принятые ранее раунды.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет. High: 0, Medium: 0.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. AC1–AC3 выполнены и доказаны (автотест + доказанная «умение падать» + прогон мутанта), скоуп не расширен, трейлеры корректны, изменение точечное и симметрично уже принятому решению по `reviewedFresh`.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/516-patch-id-without-review-docs`, коммит `8c13b5d16de4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `26ba415b57cc52fb8c084ed654452b2fce3f0099`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 26ba415b57cc
|
||||
```
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user