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