mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-28 19:01:34 +00:00
@@ -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=<rev>` разово починил уже перенесённое дерево, `--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).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/682-hygiene-wave5`, коммит `0991c45374ad` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `134a30cbdd2233258daff77b237ceb041b9caab9`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 134a30cbdd22
|
||||
```
|
||||
- Тело issue: `2e9a988e3aad9028017feba6b8bde3e8f5b31db5aee0749e15eb6b5d205ab1b4`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user