From 65b3cb25f7a5507d67f324a42e252504dac65f7f Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 27 Sep 2026 21:21:58 +0000 Subject: [PATCH] docs: review document for #682 Issue: #682 User-Visible: no --- docs/reviews/CODE-REVIEW-682-r1.md | 235 +++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-682-r1.md diff --git a/docs/reviews/CODE-REVIEW-682-r1.md b/docs/reviews/CODE-REVIEW-682-r1.md new file mode 100644 index 00000000..c7baf74f --- /dev/null +++ b/docs/reviews/CODE-REVIEW-682-r1.md @@ -0,0 +1,235 @@ +# CODE-REVIEW-682-r1 + +- Issue: #682 (эпик #674, волна 5 гигиены) +- Этап: code (PROCESS.md §2.7) +- Заход: r1 · блокирующих циклов израсходовано 0 из 4 (бюджет тратят только жёлтые/красные вердикты) +- Материал: диапазон `origin/dev..HEAD`, ровно `97d192687f1990bfab12ee919d5818cf0d9f6f8e` + (три коммита поверх `dev@dd3a5042`: `e9cfc25e` инструмент, `7feb6177` перенос ТЗ, + `97d19268` перенос документов ревью) +- Трек: инфраструктурная задача (не единственный класс A файл) — входит сразу в + `S7-code-review`, спецификация — тело issue + +## Скоуп + +Волна 5 архива выпущенного (#674): инструмент `scripts/reviews-archive.mjs` +(класс B) + два переноса класса C — `docs/specs/*` без живых ссылок → +`legacy/specs/`, документы ревью задач линий ≤ v1.77.0 → `legacy/reviews/<тег>/`. +Плюс правки инфраструктуры вокруг переноса: `process-gate.mjs` (класс путей), +`reviews-index.mjs` (порог теста), `process-metrics.mjs` (счёт раундов по двум +каталогам), `review-doc-guard.mjs` и `task-packet.mjs` (сравнение деревьев с +исключением обоих каталогов), `PROCESS.md` §2.10 и `docs/DEVELOPMENT.md` +(шаг чеклиста). Работа не product-кода: `src/**` не тронут, User-Visible: no +на всех трёх коммитах — корректно. + +## Как проверялось + +Validate на `97d19268` зелёный (run 36350079008/36348494784) — дешёвые гейты +(`typecheck`, `npm test`, `npm run build` + bundle-policy) этим прогоном +подтверждены, повторно не гонял. + +| Гейт | Прогнан | Результат | +|---|---|---| +| `node --test test/reviews-archive.test.mjs test/process-metrics.test.mjs test/process-gate.test.mjs test/reviews-index.test.mjs` | да | 61/61 зелёные | +| `node --test test/task-packet.test.mjs test/review-doc-guard.test.mjs` | да | 76+/76+ зелёные (косвенно задетые файлы) | +| `node scripts/mutation-gate.mjs --id=reviews-archive-moves-open-line-issue` | да | поймано 1/1 | +| `node scripts/mutation-gate.mjs --id=reviews-archive-first-line-wins` | да | поймано 1/1 | +| `node scripts/process-gate.mjs --range origin/dev..HEAD` | да | 0 нарушений, 3 коммита | +| `node scripts/check-docs.mjs --external --screenshots=warn` | да | passed (7 файлов, 12 внешних ссылок) — **не проверяет `docs/reviews/`, `docs/specs/`, `legacy/**`** | +| `node scripts/reviews-index.mjs --check` | да | индекс свеж | +| `node scripts/check-inputs.mjs --coverage` | да | чисто | +| независимая сверка счёта раундов (`reviewRounds` по `git ls-tree` до/после) | да | 694 пары «этап:задача» и до, и после — воспроизвёл заявленное число сам | +| независимая сверка количества перенесённых файлов/задач | да | `legacy/reviews`: 965 файлов, 14 каталогов v1.63.0…v1.77.0 (заявлено 965/332 — файлы совпали); `docs/reviews`: 155 (154 задачи + INDEX.md); `legacy/specs`: 219; `docs/specs`: 21 — все числа автора подтверждены прямым подсчётом | +| `git grep` перенесённых имён `docs/specs/*` вне `legacy/`, `docs/reviews` | да | пусто — подтверждает заявление автора | +| полнотекстовая проверка markdown-ссылок в `docs/reviews/`, `legacy/reviews/`, `docs/specs/`, `legacy/specs/` (собственный скрипт, ниже) | да | **53 битых из 302 проверенных — находка H1** | +| обратная проверка «зачем каждый из 21 оставшихся ТЗ жив» | да | 19/21 объяснены прямой или транзитивной живой ссылкой; 2 — нет (находка M/L, см. ниже) | +| `npx tsc --noEmit`, `npm run build`, полный `npm test` | нет | Validate на этом SHA уже зелёный, диф не трогает `src/**`/сборку | +| `npm run golden:verify`, смоки, инварианты модели, `pytest tests_backend`, performance | нет | диф не трогает рендер, геометрию, Python — не применимо по AC задачи | +| `node scripts/smoke-select.mjs --base --head` | нет | нет исполняемого frontend-диффа (тот же вывод дал автор в «Сделано») — согласен, `src/**` и `demo/**` не менялись | + +## Находки + +### Medium (в скоупе, блокирует зелёный) — систематически битые относительные ссылки в перенесённых документах + +Перенос добавляет документам ревью один уровень вложенности +(`docs/reviews/X.md` → `legacy/reviews/<тег>/X.md`), но скрипт переписывает +относительные ссылки только внутри `legacy/specs/*` (коммит `7feb6177`); ссылки +*внутри* самих переносимых документов ревью (`legacy/reviews/**`) и ссылки +*на* только что перенесённые документы из ещё живых или уже перенесённых +соседей никто не трогает. Итог на финальном дереве (`97d19268`, а не на +промежуточном состоянии после `7feb6177`, которое проверял автор) — 53 битые +ссылки в 46 файлах трёх разных категорий: + +1. **Ссылки на ТЗ из архивных ревью** (~40 шт.), например + `legacy/reviews/v1.68.0/SPEC-REVIEW-223-r1.md` → + `](../specs/223-optimize-coordinate-canonicalization.md)` — было верно из + `docs/reviews/`, стало `legacy/reviews/specs/…` (не существует; нужно + `../../specs/…` или `../../../docs/specs/…`, в зависимости от того, уехал + ли сам ТЗ). +2. **Самоссылки ревью на предыдущий раунд** (~8 шт., линия v1.77.0), например + `legacy/reviews/v1.77.0/SPEC-REVIEW-593-r3.md` → + `](../../docs/reviews/SPEC-REVIEW-593-r2.md)` — этот путь и раньше был + избыточным, но резолвился в саму `docs/reviews/` (два уровня вверх от + `docs/reviews/` возвращают в корень); после переноса на один уровень глубже + он попадает в несуществующий `legacy/docs/reviews/…`. +3. **Ссылка вперёд, в документ, перенесённый позже в этом же PR**: + `legacy/specs/262-readd-child-entity-after-device-delete.md:6` — + `](../../docs/reviews/SPEC-REVIEW-262-r1.md)`. Коммит `7feb6177` (перенос + ТЗ) переписал эту ссылку и на тот момент она была верна; следующий коммит + `97d19268` (перенос ревью) увёл цель в + `legacy/reviews/v1.68.0/SPEC-REVIEW-262-r1.md`, не обновив ссылку — + итог на финальном SHA битый. +4. **Ссылка из ещё живого документа на только что архивированный**: + `docs/reviews/CODE-REVIEW-635-r1.md:104` — `[CODE-REVIEW-594-r1.md](CODE-REVIEW-594-r1.md)` + (задача #635 ещё открыта и не переносится, но цитирует #594 как пример; #594 + выпущен в v1.77.0 и уехал в `legacy/reviews/v1.77.0/` этим же коммитом). + +**Чем воспроизведено** (полный список 53 путей длиннее, ниже — воспроизводящая +команда и точечные примеры): + +``` +$ python3 - <<'EOF' +import re, os +link_re = re.compile(r'\]\(([^)]+)\)') +for root in ['legacy/reviews', 'legacy/specs', 'docs/reviews', 'docs/specs']: + for dirpath, _, files in os.walk(root): + for fn in files: + if not fn.endswith('.md'): continue + fpath = os.path.join(dirpath, fn) + for m in link_re.finditer(open(fpath, encoding='utf-8', errors='replace').read()): + link = m.group(1).strip() + if link.startswith(('http', '#', 'mailto:')): continue + lp = link.split('#')[0].split(' ')[0] + if not lp or not (lp.endswith('.md') or '/' in lp): continue + target = os.path.normpath(os.path.join(dirpath, lp)) + if not os.path.exists(target): + print(fpath, '->', link) +EOF +# 53 строки, например: +legacy/reviews/v1.68.0/SPEC-REVIEW-223-r1.md -> ../specs/223-optimize-coordinate-canonicalization.md +legacy/reviews/v1.77.0/SPEC-REVIEW-593-r3.md -> ../../docs/reviews/SPEC-REVIEW-592-r2.md +legacy/specs/262-readd-child-entity-after-device-delete.md -> ../../docs/reviews/SPEC-REVIEW-262-r1.md +docs/reviews/CODE-REVIEW-635-r1.md -> CODE-REVIEW-594-r1.md +``` + +Это прямо противоречит явному заявлению в теле коммита `7feb6177`: +«Относительные ссылки перенесённых файлов переписаны … все 26 резолвятся» — +утверждение было верным только на промежуточном дереве между вторым и третьим +коммитом, а не на финальном SHA материала (`97d19268`), который и есть предмет +ревью (§2.7: числа и факты сверяются с `git rev-parse HEAD`). Ни один гейт +этого не ловит: `check-docs.mjs` проверяет фиксированный список из 7 +канонических файлов, `docs/reviews/`, `docs/specs/` и `legacy/**` в него не +входят, а автоматика процесса (`review-doc-guard.mjs`, `process-metrics.mjs`) +читает документы как текст/имена файлов, не как markdown-ссылки, поэтому +поломка не мешает конвейеру — только человеку, идущему по ссылке из архива. + +Функционального или продуктового ущерба нет (SHA-якоря «Материала раунда» — +это литеральный текст, не ссылки, и не пострадали; ни один тест или скрипт +не разыменовывает эти пути), поэтому не High. Но это прямая находка в +скоупе задачи — весь смысл переноса «одним коммитом класса C» и стиля +"история не переписывается" в том, чтобы архив оставался читаемым и +навигируемым, а не только грузом в git-истории; 53 битые ссылки в 46 файлах — +не мелочь, и без исправления она войдёт в `legacy/` уже сломанной. Без +второго High это жёлтый вердикт, возврат автору (§3 п.8). + +Похожий класс проблемы будет повторяться на каждой следующей волне архива +(v1.78.0 и далее), если `reviews-archive.mjs` не научится либо переписывать +относительные ссылки при `git mv` (как это уже сделано для `legacy/specs/*` +вручную), либо хотя бы фейлить `--apply` предупреждением о ссылках, +которые он не может проверить. + +### Low (в скоупе, самостоятельно снимаю с записью) — два архивных ТЗ без документированной причины остаться + +Собственный критерий задачи: в `docs/specs/` остаются только ТЗ, на которые +ссылаются живые код/тесты/документы, и те, на которые ссылаются они сами. +Я проверил все 20 файлов (без README) на прямую и транзитивную ссылку и +подтвердил обоснование для 18 из них (например: `067` ← `LIGHT.md`, `084`/`050` +← транзитивно через `067`, `043` ← `scripts/support-relay/README.md`, `505` ← +`demo/helpers/README-ha-dialog.md`, `485*` ← `RADAR.md`/`ARCHITECTURE.md`, +`089/122/160/471` ← `ISOMETRIC.md`/ADR, `141/229` ← `wall-merge.ts`, `220` ← +`space-order.ts`, `146` ← `SUN.md`, `506` ← `ARCHITECTURE.md`, `039` ← +`DECOR-EDITOR.md`). Для двух — `329-junction-limits.md` и +`403-area-relocation-safety.md` — живой ссылки не нашёл: обе задачи закрыты +без последующих коммитов, единственные совпадения по имени — упоминание +номера issue в заголовке раздела `CONFIG-COMPATIBILITY.md` (не ссылка на файл) +и строковый литерал-пример в `test/review-doc-guard.test.mjs`/ +`review-doc-guard.mjs` (синтетическая фикстура с реальными историческими +числами, не зависящая от существования файла). По собственному критерию +задачи эти два файла — кандидаты в `legacy/specs/`, но не мешают ничему: +`scripts/task-packet.mjs` продолжает работать с любым именем в `docs/specs/`, +и живучесть двух лишних файлов не ломает ни один тест. Снимаю как Low: не +блокирует, but стоит доразобрать в следующей волне (или этой же правкой, +на усмотрение автора). + +## Что проверено и корректно + +- **`archivePlan` — чистая функция, покрыта тестами по каждому правилу**: + задача уходит в последнюю свою линию (не в первую — проверил мутантом + `reviews-archive-first-line-wins`, ловится); задача с трейлером в открытой + линии остаётся целиком (мутант `reviews-archive-moves-open-line-issue`, + ловится); закрытая без выпуска — по линии документа; `RELEASE-REVIEW-*` + уходит в каталог своего тега; чужое имя не трогается. Прогнал оба мутанта + сам (`node scripts/mutation-gate.mjs --id=…`) — 1/1 у каждого, не только + поверил заявлению автора. +- **Числа автора воспроизведены независимо**, не только прочитаны в + комментарии: 965/332 (`legacy/reviews`, 14 каталогов), 154+INDEX.md + (`docs/reviews`), 219 (`legacy/specs`), 21 (`docs/specs`), 694 пары + `reviewRounds` до и после (посчитал по `origin/dev` и по `HEAD` отдельно — + совпало). +- **`git grep` по перенесённым `docs/specs/*` вне `legacy/`/`docs/reviews`** — + пусто, подтверждено самостоятельно; под-перенос (файл должен был уехать, но + остался нужным где-то в живом дереве) не найден. +- **`process-gate.mjs`**: `legacy/` — класс C, не пересекается с классом D + (список `CLASS_D` не содержит `legacy/`); коммиты проходят гейт классификации + и трейлеров (0 нарушений на диапазоне). +- **Трейлеры**: все три коммита несут `Issue: #682`, `User-Visible: no` — + верно для инфраструктурной/документной правки без видимого поведения; + порядок коммитов (инструмент → ТЗ → ревью) совпадает с порядком в issue. +- **`legacy/` действительно только Markdown** — проверил + `find legacy -type f ! -name '*.md'` — пусто, соответствует комментарию в + `process-gate.mjs`. +- **Идемпотентность инструмента**: повторный + `node scripts/reviews-archive.mjs --through=v1.77.0` на уже применённом + дереве возвращает «переносится 0», остальные 154 — «задача есть в открытой + линии»; корректное поведение при повторном запуске. +- **`reviewDocNames`/`reviewRounds`**: перенос части раундов задачи в архив не + меняет счёт пар «этап:задача» — проверено и юнит-тестом автора, и отдельным + прогоном на полном дереве репозитория. +- **PROCESS.md §2.10 и DEVELOPMENT.md**: правки текста соответствуют + фактической реализации (команда, условие пустой очереди S7, порядок теги → + архив после ревью линии); не нашёл противоречия между текстом канона и + кодом. + +## Чего не проверял + +- Полные `npx tsc --noEmit`, `npm test`, `npm run build` со сверкой бандла — + не гонял повторно, засчитан зелёный Validate на этом SHA (диф не трогает + `src/**`, `dist/**`). +- `golden:verify`, `smoke-select`, инварианты модели, `pytest tests_backend`, + performance-профили — не применимы: диф не содержит рендера, геометрии, + Python-кода, frontend-исполняемого кода. +- `process-metrics.mjs` живой `fetchSnapshot` через `gh api` — как и автор, + не выполнял (недоступно из песочницы); опирался на статическую сверку + `reviewRounds` по дереву, что и есть предмет изменения. +- Полную посимвольную ручную вычитку всех 965+219 перенесённых файлов — не + делал; проверял структурно (числа, схема имён, случайные и целевые выборки) + и полнотекстовым скриптом на битые markdown-ссылки (раздел «Как + проверялось»), которым и нашёл находку. Не исключаю битых ссылок вне + синтаксиса `[text](path)` (голые URL, ` * `-текст без скобок) — синтаксис + проверен только в стандартной markdown-форме. +- Ссылки из перенесённых документов на GitHub (issue/PR URL) — не резолвил + каждую сетевым запросом; синтаксически они не зависят от структуры + репозитория и не могли пострадать от `git mv`. + +--- + + + +## Материал раунда + +- Ветка: `issue/682-hygiene-wave5`, коммит `97d192687f19` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `790ebed0bed2718b1da899e2b4c58e3de3f48a6a` + ``` + git log --all --format='%H %T' | grep 790ebed0bed2 + ``` +- Тело issue: `2e9a988e3aad9028017feba6b8bde3e8f5b31db5aee0749e15eb6b5d205ab1b4` +- Вердикт конвейера: `yellow` · High 0