german/houseplan-card
1
0
Fork 0
mirror of https://github.com/Matysh/houseplan-card synced 2026-09-29 03:09:36 +00:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity
Files
main
houseplan-card/docs/reviews/CODE-REVIEW-682-r1.md
T
claude[bot] 65b3cb25f7 docs: review document for #682 
Issue: #682
User-Visible: no
2026-09-27 22:10:47 +00:00

21 KiB
Raw Permalink Blame History

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
Reference in New Issue View Git Blame Copy Permalink