From 9293109ba196ecfb083c7dd47ce6cbc3d20b418e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:39:14 +0000 Subject: [PATCH] docs: review document for #516 Issue: #516 User-Visible: no --- docs/reviews/CODE-REVIEW-516-r2.md | 177 +++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-516-r2.md diff --git a/docs/reviews/CODE-REVIEW-516-r2.md b/docs/reviews/CODE-REVIEW-516-r2.md new file mode 100644 index 00000000..d2858cbc --- /dev/null +++ b/docs/reviews/CODE-REVIEW-516-r2.md @@ -0,0 +1,177 @@ +# CODE-REVIEW-516-r2 + +**Issue:** #516 — «merge-candidate: patch-id кандидата включает собственный документ ревью» +**Заход:** r2 · блокирующих циклов израсходовано 0 из 2 (лёгкий трек) +**Материал:** `18eded340c40891115a182276b12e01afb376539` (рабочая копия, зафиксировано пайплайном) +**Вердикт:** зелёный · High: 0 · Medium: 0 + +## Скоуп + +Class B (`scripts/**`, `test/**`) — внутренняя механика конвейера ревью, не +продуктовый код `src/**`. `docs/SCOPE.md` не применим напрямую: задача не +закрывает Core user job, а чинит собственную инфраструктуру ревью (патч-id +кандидата ошибочно включал `docs/reviews/**`, из-за чего каждый зелёный +кандидат после сдвига `dev` возвращался «дифф изменился» — #514, #508). + +ТЗ (лёгкий трек, тело issue), три AC: +- AC1 — сдвиг `dev` только документами ревью не возвращает зелёный кандидат + на повторное ревью (тест на реальном git). +- AC2 — реальное изменение содержимого патча при ребейзе по-прежнему + возвращает в `S7` (существующий тест). +- AC3 — мутант пойман штатным раннером. `User-Visible: no`. + +## Почему разбор снова полный, а не по дельте + +Между материалом r1 (`8c13b5d1`, см. вердикт в issue) и текущим `18eded34` +нет ни единой строки нового кода: `git log --oneline origin/dev..HEAD` +показывает ровно два коммита — `6ee9a461` (тот же код, что читал r1) и +`18eded34` (публикация документа `CODE-REVIEW-516-r1.md`, class C). Диапазон +между ними — чистый ребейз на `dev`, который за это время подвинулся на +`ad2858a8` (документы ревью #508 и #515). Это буквально случай из §7.2 — +«ребейз на ушедший вперёд dev, после ребейза это другой код» — поэтому по +инструкции разбор веду полным, а не как экономию по находкам r1 (которых и +не было: r1 дал 0/0). + +Отдельно: ожидание владельца, что механизм reuse (#499) применит зелёный +вердикт r1 без вызова модели, здесь не сработало — иначе меня бы не вызвали. +Причина в бутстрэпе, не в этом diff'е: `#515` (сосед по истории `dev`, +`5a1cddea`) чинит именно то, что якоря материала в документе ревью снимаются +после ребейза, а не до — но документ r1 для #516 создавался в переходном +окне, когда эта правка либо ещё не была в `dev`, либо якорь всё равно указывал +на осиротевшее дерево. Это не дефект #516: `scripts/merge-candidate.mjs` и +`scripts/review-doc-guard.mjs --reuse` — разные шаги конвейера, и вопрос +принадлежит #515/#499, не этой задаче. Упоминаю как объяснение, почему модель +снова читает тот же код, а не как находку. + +## Как проверялось + +Дешёвые гейты не гонял целиком — Validate с мутантами уже зелёный на точном +материале `18eded34`: https://github.com/Matysh/houseplan-card/actions/runs/34407159664 +(комментарий владельца в issue подтверждает: красный workflow_sync был +устранён зеркалом `aeb0a473`, повторный dispatch зелёный). `tsc`/`build`/полный +`npm test` не перегонял по этой причине. + +Прогнал сам, точечно, поскольку код — предмет этого раунда: + +- `node --test test/merge-candidate.test.mjs` → **13/13**, включая новый + `#516 AC1` на настоящем git. +- `node scripts/mutation-gate.mjs --id=merge-rereviews-own-review-doc` → + `ok чистый прогон` + `ok merge-rereviews-own-review-doc: тест покраснел, как + обязан` → **поймано 1 из 1**. Дисциплина «тест умеет падать» выполнена этим + же прогоном (гейт сам откатывает патч и проверяет красный/зелёный статус). +- `node scripts/mutation-gate.mjs --check` → весь реестр структурно валиден. + Побочно нашёл дублирующийся `id: 'openings-filled-tunnel-dark'` (два + определения с одним id) — этот diff его не касается (не входит в список + изменённых файлов), дублирование существовало и до #516. Вне скоупа этой + задачи, отдельный issue не завожу: находка мелкая, не блокирует и не + доказана как реальный дефект поведения гейта (может быть двумя вариантами + сценария под одним именем) — стоит того, чтобы кто-то посмотрел, не стоит + того, чтобы заводить `S1-new` по неподтверждённому подозрению. +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` → + «Исполняемого frontend-диффа нет» — смоки не выбираются, `src/**` не + тронут. Ничего не прогонял из `demo/smoke_*.mjs`. +- Полное чтение `scripts/merge-candidate.mjs` и всего diff (`git show + 6ee9a461`). + +Не применимо и не прогонялось: `check-docs` (нет `src/**`), инварианты модели +(нет геометрии/`layout`/`marker.space`), `golden:verify` (нет рендера), +`pytest tests_backend` (нет `custom_components/**/*.py`), performance-профили +(не названы в AC). + +## Разбор по AC + +**AC1** — подтверждено `#516 AC1` (`test/merge-candidate.test.mjs:271`) на +настоящем git: `dev` двигается только чужим документом ревью +(`CODE-REVIEW-8-r2.md`), ветка несёт свой (`CODE-REVIEW-9-r1.md`); после +ребейза `patchId(materialBase, material) === patchId(devNow, candidate)` +(оба через новый pathspec `':!docs/reviews'`, `scripts/merge-candidate.mjs:116-120`), +`decideMerge` идёт в `push` через Validate, комментария «дифф изменился» нет. +Прогнал лично — зелёный. + +**AC2** — существующий тест `patch-id изменился при ребейзе — S7-code-review` +(`test/merge-candidate.test.mjs:130`, `fakeOps`) не тронут этим diff'ом и +остаётся зелёным: он проверяет ветку `decideMerge` при `patchIdEqual: false`, +логика которой в этом diff'е не менялась (менялся только состав диффа, +который в неё попадает, не сама ветка решения). Отдельно реальный +git-тест «чистый ребейз с равным patch-id и изменённым поведением идёт через +Validate» (`:172`) подтверждает, что exclusion `docs/reviews` не открыл +дыру для настоящих поведенческих изменений вне `docs/reviews` — там patch-id +равен намеренно (сценарий «20→40»), и это не про AC2, а про соседний +инвариант, который diff не задевал. + +**AC3** — `merge-rereviews-own-review-doc` (`scripts/mutation-gate.mjs`, +find/replace возвращает старый `git diff --full-index from to` без pathspec) +поймано штатным `mutation-gate.mjs` 1 из 1, лично воспроизведено. +`User-Visible: no` — оба коммита (`6ee9a461`, `18eded34`) несут +`Issue: #516` и `User-Visible: no`; changelog не тронут — корректно, никакого +видимого пользователю поведения нет. + +## Симметрия с `reviewedFresh` + +Заявление автора «patch-id обязан считаться так же, как `reviewedFresh`» +проверено чтением: `reviewedFresh` (`scripts/merge-candidate.mjs:174-176`) +уже использовал `ops.diffNames(material, actual, ['.', ':!docs/reviews'])` — +эта строка не входит в diff #516 (существовала раньше), и `patchId` теперь +получил тот же pathspec. Симметрия реальна, не декларативна. + +## Что проверено и корректно + +- Единственная содержательная правка — `scripts/merge-candidate.mjs` patchId + теперь исключает `docs/reviews/**`, ровно по описанной причине в issue. +- Тест и мутант независимо подтверждают обе стороны: patch-id равен, когда + разница — только документы ревью (AC1); patch-id по-прежнему различается + при настоящем изменении контента (AC2, не тронут). +- Трейлеры на месте на обоих коммитах ветки. +- Диапазон изменений не выходит за `scripts/**`, `test/**` — class B, + issue переиспользован корректно (тот же #516). +- Дублирование `id` в реестре мутантов — предсуществующее, не в скоупе, + не блокирует. + +## Чего не проверял + +- `npx tsc --noEmit`, `npm test` (полный), `npm run build` — не гонял: + зелёный Validate с мутантами уже есть на точном SHA (ссылка выше), + включает эти гейты. +- `check-docs`, инварианты модели, `golden:verify`, browser-смоки, + `pytest tests_backend`, performance — не применимы диффом (нет `src/**`, + нет геометрии, нет рендера, нет Python). +- Историю бутстрэпа reuse/#499/#515 (почему модель вызвана снова вместо + переиспользования вердикта r1) не разбирал глубже уровня «это другой шаг + конвейера, не предмет #516» — это не входит в AC этой задачи. + +## Закрытие раунда r1 + +r1 (материал `8c13b5d1`) дал вердикт зелёный, High: 0, Medium: 0 — находок +не было, закрывать нечего. + +| Находка r1 | Чем закрыта | Где видно | +|---|---|---| +| — | — | r1 не содержал находок; таблица пуста по построению | + +## Унаследовано из r1 + +Ничего не принято вслепую — код между материалом r1 (`8c13b5d1`) и текущим +материалом (`18eded34`) идентичен (см. раздел «Почему разбор снова полный»), +поэтому весь разбор AC1–AC3 в этом документе — самостоятельная повторная +проверка, а не перенос заявлений автора. Единственное, что действительно +наследуется без личного перезапуска: зелёный результат полного Validate +(`tsc`/`test`/`build`/mutants) на этом точном SHA `18eded34` +(https://github.com/Matysh/houseplan-card/actions/runs/34407159664) — +беру его как основание не гонять `tsc`/`build`/полный `npm test` самому, +как и разрешает вводная инструкция для этого раунда. Документ r1 +(`docs/reviews/CODE-REVIEW-516-r1.md`) как источник для этого использован +только как ссылка на факт «модель уже читала этот код 09.09», не как замена +собственной проверки. + +--- + + + +## Материал раунда + +- Ветка: `issue/516-patch-id-without-review-docs`, коммит `18eded340c40` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `5f06a75613c26bf52de93b06acecb38930ea7c9f` + ``` + git log --all --format='%H %T' | grep 5f06a75613c2 + ``` +- Вердикт конвейера: `green` · High 0