From 0dac3488fb014e466d0109822972bf115586db63 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 27 Sep 2026 22:30:07 +0000 Subject: [PATCH] docs: review document for #682 Issue: #682 User-Visible: no --- docs/reviews/CODE-REVIEW-682-r2.md | 187 +++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-682-r2.md diff --git a/docs/reviews/CODE-REVIEW-682-r2.md b/docs/reviews/CODE-REVIEW-682-r2.md new file mode 100644 index 00000000..a2f7f3ef --- /dev/null +++ b/docs/reviews/CODE-REVIEW-682-r2.md @@ -0,0 +1,187 @@ +# CODE-REVIEW-682-r2 + +- Issue: #682 (эпик #674, волна 5 гигиены) +- Этап: code (PROCESS.md §2.7) +- Заход: r2 · блокирующих циклов израсходовано 1 из 4 +- Материал: диапазон `origin/dev..HEAD`, ровно `0991c45374adc3e9ba79a4d0f3cb08869f2310a9` + (пять коммитов поверх текущего `origin/dev` @ `c64a1ab7`, где #683 уже слит: + `d4a672c7` инструмент, `df46fd1c` перенос ТЗ, `cca9bc85` перенос документов + ревью, `65b3cb25` публикация документа ревью r1, `0991c453` фикс находки r1) +- Трек: инфраструктурная задача — входит сразу в `S7-code-review`, спецификация + в теле issue + +## Скоуп + +Ровно та же волна 5 архива выпущенного, что и в r1: инструмент +`scripts/reviews-archive.mjs` (класс B) + два переноса класса C. Между r1 и r2 +ветка не переписывалась содержательно — она была ребейзнута конвейером на +ушедший вперёд `dev` (после слияния #683: пять исходных коммитов волны стали +пятью новыми SHA с тем же деревом плюс новый `dev`-фундамент), затем добавлен +один новый коммит `0991c453`, закрывающий единственную находку r1. Дельта r1→r2 +локальна: изменения ограничены `scripts/reviews-archive.mjs` (новые функции +`repairLinks`/`repairTreeLinks`/`renamesSince`/`brokenLinks` и CLI-флаги +`--repair-links=`/`--check-links`), его тестом, записью в +`scripts/mutation-registry.mjs`, `PROCESS.md` §2.10 и `docs/DEVELOPMENT.md`, и +собственно перезаписанными ссылками в 46 архивных файлах. Разбор ниже — по +дельте (PROCESS.md §2.10): полностью проверена находка r1 и её закрытие; +остальное (членство по трейлерам, числа переноса, PROCESS.md/DEVELOPMENT.md +кроме добавленного абзаца) унаследовано из r1 — ниже отдельным разделом, с +подтверждением, что рёбейз его не задел по содержанию. + +## Как проверялось + +Validate на `0991c453` зелёный (run 36354378581) — дешёвые гейты (`typecheck`, +`npm test`, `npm run build` + bundle-policy) этим прогоном подтверждены, +повторно не гонял. `src/**`, `dist/**`, `custom_components/**`, `demo/**` в +диапазоне `origin/dev..HEAD` не тронуты (`git diff --stat` по этим путям — +пусто), поэтому golden/smoke/инварианты/pytest/performance неприменимы. + +| Гейт | Прогнан | Результат | +|---|---|---| +| `git diff --stat 97d192687f19..cca9bc85` вне `docs/reviews/INDEX.md` и новых доков #683 | да | 0 — архивное содержимое волны 5, проверенное в r1, идентично после рёбейза; отличия только от параллельно слитого #683 | +| `node --test test/reviews-archive.test.mjs` | да | 8/8 зелёные, включая 4 новых теста находки r1 | +| `node --test test/reviews-archive.test.mjs test/reviews-index.test.mjs test/process-gate.test.mjs test/process-metrics.test.mjs test/review-doc-guard.test.mjs test/task-packet.test.mjs test/check-inputs.test.mjs` | да | 161/161 зелёные — то же число, что заявил автор | +| `node scripts/mutation-gate.mjs --id=reviews-archive-links-from-new-place-only` | да | поймано 1/1 (guard: чистый прогон целевого теста; мутант красит его) | +| `node scripts/reviews-archive.mjs --check-links` | да | 7 битых: 5 — буквальные цитаты примеров находки r1 внутри самого `docs/reviews/CODE-REVIEW-682-r1.md` (документ описывает историю, не навигирует), 2 — «...»-заглушки в `legacy/reviews/v1.72.0/CODE-REVIEW-448-r2.md`; проверено отдельно (см. ниже), что оба существуют уже на `origin/dev` | +| `git show origin/dev:docs/reviews/CODE-REVIEW-448-r2.md \| grep '](\.\.\.'` | да | те же две строки `[#152](...)`/`[#447](...)` — не ссылки, а буквальный текст таблицы, существовавший до этой задачи | +| `git show 0991c453 --numstat` по `legacy/**`/`docs/reviews/**` | да | 46 файлов, 52+52 строк — совпадает по числу файлов с заявленными 46; общее число переписанных ссылок (53) подтверждено инструментом (`ссылок переписано …`), а не только подсчётом строк диффа (одна строка может нести два `](...)` ) | +| `node scripts/process-gate.mjs --range origin/dev..HEAD --issues` | да | гейт пройден, 1 предупреждение (класса A нет — статусная метка не требуется до S7, ожидаемо для инфраструктурного диапазона) | +| `node scripts/check-docs.mjs --external --screenshots=warn` | да | passed (7 файлов, 12 внешних ссылок) — не покрывает `docs/reviews/`, `docs/specs/`, `legacy/**`, как и в r1 | +| `node scripts/reviews-index.mjs --check` | да | «устарел» — **не находка**: по `test/reviews-index.test.mjs:135` («…конвейер пересобирает индекс после своих ребейзов») и `:144` («индекс пересобирается только коммитами, идущими в dev… с #657 ветка задачи индекс не несёт вовсе») свежесть индекса на ветке задачи умышленно не гарантируется — её восстанавливает `merge-candidate.mjs --commit-if-stale` при слиянии в `dev`. Проверил рабочее дерево до и после регенерации и вернул его в committed-состояние (`git reset --hard HEAD`) | +| `node scripts/check-inputs.mjs --coverage` | да | чисто | +| `node --test test/process-digests.test.mjs` | да | 5/5 — цитаты `REVIEWER.md`/`AUTHOR.md` из `PROCESS.md` синхронны после правки §2.10 | +| трейлеры всех 5 коммитов (`git log -1 --format='%(trailers)'`) | да | `Issue: #682` и `User-Visible: no` на каждом; корректно — изменение не пользовательское | +| `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` в диапазоне | да | не тронуты — согласовано с `User-Visible: no` на всех коммитах | +| `npx tsc --noEmit`, `npm run build`, полный `npm test`, bundle-сверка | нет | Validate на этом SHA зелёный, диф не трогает `src/**`/`dist/**` | +| `npm run golden:verify`, смоки, инварианты модели, `pytest tests_backend`, performance | нет | диф не трогает рендер, геометрию, Python, `demo/**` — неприменимо | +| `node scripts/smoke-select.mjs --base --head` | нет | нет исполняемого frontend-диффа — тот же вывод, что и в r1 | + +## Находки + +Нет. Единственная находка r1 (Medium, в скоупе) закрыта; новых находок в +дельте r1→r2 не обнаружено. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **Medium**: перенос не переписывал относительные ссылки — на финальном SHA `97d19268` 53 битые ссылки в 46 файлах трёх категорий (ссылки архивных ревью на ТЗ, самоссылки на предыдущий раунд внутри линии, ссылка вперёд из `legacy/specs/262-…`, ссылка из живого документа на только что архивированный) | Коммит `0991c453`: новые `repairLinks`/`repairTreeLinks` в `scripts/reviews-archive.mjs` пересчитывают ссылку, если она не резолвится от нового места, но резолвится от старого или нового через карту переносов; не трогают ссылку, битую и до переноса. `--apply` вызывает это автоматически (следующая волна не унаследует проблему), `--repair-links=` разово починил уже перенесённое дерево, `--check-links` — постоянная проверка | `git show 0991c453 --numstat` — ровно 46 файлов в `legacy/reviews/**`, `legacy/specs/262-…` и `docs/reviews/CODE-REVIEW-635-r1.md`; `node scripts/reviews-archive.mjs --check-links` — 0 неожиданных битых (7 оставшихся объяснены: 5 — буквальные цитаты в самом отчёте r1, 2 — заглушки, битые ещё на `origin/dev`); 4 новых юнит-теста (`#682 r1 ссылки: …`) с точными ожидаемыми путями, включая тест на реальном дереве `legacy/reviews`+`legacy/specs`; мутант `reviews-archive-links-from-new-place-only` ловится 1/1 | +| Заявление «все 26 [ссылок] резолвятся» в `7feb6177` было верным только на промежуточном дереве | закрыто тем же исправлением; историю не переписывали (не force-push), а зафиксировали правильное состояние новым коммитом | `0991c453` — коммит-сообщение прямо ссылается на находку r1 и объясняет разрыв между «26 резолвятся» (после переноса ТЗ) и «53 битых» (после переноса ревью) | +| Low (уже снят самим ревьюером в r1, без обязательства чинить): `docs/specs/329-junction-limits.md`, `docs/specs/403-area-relocation-safety.md` без живой ссылки по критерию задачи | не требовалось закрывать — но автор дал объяснение в комментарии «Сделано»: `329` имеет живую ссылку из `custom_components/houseplan/junction_limits.py:17`; `403` остаётся кандидатом на следующую волну, файл не мешает ни одному тесту | issue-комментарий «Сделано: исправлена находка r1» (2026-09-27); сам файл `docs/specs/403-area-relocation-safety.md` по-прежнему в `docs/specs/`, что соответствует «кандидат на следующую волну», а не обязательству этой задачи | + +## Унаследовано из r1 + +Ниже — то, что r1 уже проверил мутационно/независимым пересчётом и что дельта +r1→r2 не касается по содержанию. Проверено, что рёбейз не изменил эти файлы: +`git diff 97d192687f1990bfab12ee919d5818cf0d9f6f8e cca9bc85 -- 'legacy/**' +'docs/specs/**' scripts/reviews-archive.mjs scripts/process-gate.mjs +scripts/process-metrics.mjs scripts/review-doc-guard.mjs scripts/task-packet.mjs +PROCESS.md docs/DEVELOPMENT.md test/reviews-archive.test.mjs` — пусто (только +`docs/reviews/INDEX.md` и новые доки #683 отличаются, что ожидаемо от рёбейза +на ушедший вперёд `dev`). + +- **`archivePlan` — правила членства по трейлерам**, мутационно проверенные + (`reviews-archive-first-line-wins`, `reviews-archive-moves-open-line-issue`, + оба 1/1 в r1). Документ и материал: CODE-REVIEW-682-r1.md, SHA + `97d192687f1990bfab12ee919d5818cf0d9f6f8e`. +- **Числа переноса**: 965 файлов / 332 задачи в `legacy/reviews/` (14 + каталогов v1.63.0…v1.77.0), 154 задачи + `INDEX.md` остались в + `docs/reviews/`, 219 ТЗ ушли в `legacy/specs/`, 21 остались в `docs/specs/`, + 694 пары `reviewRounds` до/после — воспроизведены независимо в r1 прямым + подсчётом по дереву. Не пересчитывал заново: перенесённые множества файлов + не менялись между r1 и r2 (см. пустой диф выше). +- **`legacy/` — класс C в `process-gate.mjs`**, порог `test/reviews-index.test.mjs` + снижен до `> 0`, `process-metrics.mjs` считает раунды по обоим каталогам, + `review-doc-guard.mjs`/`task-packet.mjs` исключают `legacy/reviews` из + сравнения деревьев — все эти правки, кроме нового абзаца про ссылки в + `PROCESS.md` §2.10, идентичны r1 (диф выше). +- **git grep по перенесённым `docs/specs/*` вне `legacy/`/`docs/reviews`** — + пусто, проверено в r1; перенесённые множества файлов не менялись. +- **Low, снятый ревьюером в r1** (329/403 в `docs/specs/`) — принят без + повторной проверки методологии снятия; фактическое состояние файлов на r2 + свежее (см. таблицу закрытия выше). + +## Что проверено и корректно + +- **Фикс находки r1 воспроизведён независимо, не только прочитан**: прогнал + `--check-links` сам и получил тот же список из двух категорий остаточных + «битых» ссылок, что и в описании коммита; сверил, что обе заглушки + `CODE-REVIEW-448-r2.md` существовали до этой задачи (`git show origin/dev:…`). +- **Мутационный тест новой логики реален**: `reviews-archive-links-from-new-place-only` + прогнан лично (`node scripts/mutation-gate.mjs --id=…`) — гвард зелёный, + мутант красит именно заявленный тест, не общий прогон. +- **`repairLinks` не «чинит наугад»**: отдельный тест проверяет, что ссылка, + битая и до переноса (`legacy/reviews/v1.72.0/CODE-REVIEW-448-r2.md` → + `nowhere.md`), остаётся нетронутой — предотвращает риск нового класса + битых ссылок вместо старого. +- **Регрессия на реальном дереве, а не только на синтетике**: тест + «`#682 r1 архив legacy/: относительные ссылки резолвятся»`` вызывает + `brokenLinks()` на настоящих `legacy/reviews`/`legacy/specs` — это защитит + и следующую волну (v1.78.0), если её `--apply` пропустит `repairTreeLinks`. +- **Трейлеры всех 5 коммитов диапазона** — `Issue: #682`, `User-Visible: no` + на каждом, включая автогенерированный коммит публикации документа r1. +- **`docs/reviews/INDEX.md`, «устаревший» на этой ветке — не регресс**: + нашёл и перепроверил по `test/reviews-index.test.mjs` (#635 r3, #657), что + это спроектированное поведение: индекс ветки задачи не обязан включать + собственные новые документы ревью, это чинит `merge-candidate.mjs + --commit-if-stale` при слиянии в `dev`. Без этой проверки я бы по ошибке + завёл находку — стоит явно смотреть в тест, чем в интуицию. +- **`PROCESS.md`/`docs/DEVELOPMENT.md` синхронны с `REVIEWER.md`/`AUTHOR.md`** + после правки §2.10 — `process-digests.test.mjs` зелёный. + +## Чего не проверял + +- Полные `npx tsc --noEmit`, `npm test`, `npm run build` со сверкой бандла — + не гонял повторно; засчитан зелёный Validate на этом SHA (диф не трогает + `src/**`/`dist/**`, подтверждено `git diff --stat` по этим путям). +- `golden:verify`, `smoke-select`, инварианты модели, `pytest tests_backend`, + performance — неприменимо: диф не содержит рендера, геометрии, Python, + `demo/**`. +- **Интеграция `repairTreeLinks` внутри `applyPlan` (путь `--apply` для + следующей волны v1.78.0) проверена чтением, не исполнением.** В этой задаче + правки ссылок сделаны через `--repair-links=origin/dev` (разовый ремонт уже + перенесённого дерева), а не через `--apply`, поэтому вызов + `repairTreeLinks(...)` внутри `applyPlan` (scripts/reviews-archive.mjs:262) + ни разу не исполнялся сквозным тестом или реальным прогоном в этой задаче — + только читал код и убедился, что он использует ту же, уже + протестированную функцию с тривиальной обвязкой (`new Map(moves.map(...))`). + Риск невысокий (простая композиция), но это не automated evidence; если + волна v1.78.0 пойдёт через `--apply` и `repairTreeLinks` там почему-то не + сработает, обнаружится это только тестом «архив без битых ссылок» на + следующем ревью — стоит держать в уме на v1.78.0. +- Точное совпадение «53 ссылки» из коммит-сообщения с числом изменённых строк + диффа (52 строки в 46 файлах) не сверял вручную построчно — доверился + выводу самого инструмента (`ссылок переписано N`) и целостной проверке + `--check-links`/`brokenLinks()`, которая ловит содержательный результат + (сколько ссылок реально ведут в никуда), а не подсчёт строк diff. +- Ссылки на GitHub (issue/PR URL) внутри перенесённых документов — не + резолвил сетевым запросом; они не зависят от структуры репозитория и не + могли пострадать от `git mv` (то же ограничение, что и в r1). +- Полную посимвольную вычитку всех 1202 изменённых файлов диапазона — не + делал; полагался на инвариант «до и после рёбейза архивное содержимое + идентично» (проверено диффом) плюс полнотекстовую проверку ссылок. + +--- + +## Материал раунда + +- Ветка: `issue/682-hygiene-wave5`, коммит `0991c45374adc3e9ba79a4d0f3cb08869f2310a9`. +- Материал r1 (для сверки дельты): коммит `97d192687f1990bfab12ee919d5818cf0d9f6f8e` + (не резолвится в локальном дереве по умолчанию — не находка: получен через + `git fetch origin 97d192687f1990bfab12ee919d5818cf0d9f6f8e`, дерево и блобы + доступны и сверены выше), документ `docs/reviews/CODE-REVIEW-682-r1.md`. +- Тело issue сверено по состоянию на момент ревью r2 (2026-09-27/28). + +--- + + + +## Материал раунда + +- Ветка: `issue/682-hygiene-wave5`, коммит `0991c45374ad` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `134a30cbdd2233258daff77b237ceb041b9caab9` + ``` + git log --all --format='%H %T' | grep 134a30cbdd22 + ``` +- Тело issue: `2e9a988e3aad9028017feba6b8bde3e8f5b31db5aee0749e15eb6b5d205ab1b4` +- Вердикт конвейера: `green` · High 0