From 8c61fe79dc38b278f306aa5a1c0822d8d925f8ae Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 09:46:47 +0000 Subject: [PATCH] docs: review document for #635 Issue: #635 User-Visible: no --- docs/reviews/CODE-REVIEW-635-r1.md | 233 +++++++++++++++++++++++++++++ 1 file changed, 233 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-635-r1.md diff --git a/docs/reviews/CODE-REVIEW-635-r1.md b/docs/reviews/CODE-REVIEW-635-r1.md new file mode 100644 index 00000000..96f204d6 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-635-r1.md @@ -0,0 +1,233 @@ +# CODE-REVIEW · issue #635 · заход r1 + +Issue: #635 · Ветка `issue/635-reviews-index` · Материал: `db0f5b9b1be025fca86736f70d6aedd71d057487` +(ребейз на `dev` = `b1f1f6b7`) · Заход r1 · блокирующих циклов до этого раунда: 0/4 + +## Скоуп + +Инфраструктура (S1-new минуя ordинарный вход, #562 — сразу S7), класс B/C: +`scripts/reviews-index.mjs`, `test/reviews-index.test.mjs`, генерируемый +`docs/reviews/INDEX.md`, `docs/LESSONS.md`, шаг публикации в +`.github/workflows/process.yml`, дописка в `PROCESS.md` §2.10, два мутанта в +`scripts/mutation-registry.mjs`. Продукт (`src/**`) не тронут. + +AC из тела issue: +- AC1 — `INDEX.md` покрывает 100 % документов каталога, пересобирается + детерминированно (тест). +- AC2 — «что находили по файлу X» — одно чтение ≤ 2 k строк. +- AC3 — правило хранения раундов записано в PROCESS.md §2.10. + +## Как проверялось + +Рабочая копия уже на материале `db0f5b9b`, `git fetch`/`checkout` не делал. + +- `git diff origin/dev...HEAD --stat` — 7 файлов, +1321/-0, новых файлов нет + за пределами класса B/C/генерируемого. +- Прочитан весь diff (`PROCESS.md`, `process.yml`, `mutation-registry.mjs` + целиком; `docs/LESSONS.md` целиком) и весь `scripts/reviews-index.mjs`. +- **Дешёвые гейты не перегонял** — Validate на этом же SHA зелёный + (https://github.com/Matysh/houseplan-card/actions/runs/35843661626): + `npx tsc --noEmit`, `npm test`, `npm run build` со сверкой копий бандла + засчитаны по этой ссылке. +- `node --test test/reviews-index.test.mjs` — прогнал сам (новый файл, + Validate его гоняет в общем `npm test`, но проверил отдельно для + читаемого вывода): 6/6 pass. +- `node scripts/reviews-index.mjs --dir=docs/reviews --check` — «свеж»: + закоммиченный `INDEX.md` совпадает с тем, что сгенерировал бы конвейер + сейчас. +- Дисциплина «тест умеет падать»: применил оба патча из + `reviews-index-skips-self-check` и `reviews-index-verdict-substring` + вручную поверх рабочей копии, прогнал соответствующий guard — + оба упали с ожидаемым assertion (не setup-failure), файл вернул к + исходному, `git status` после — чисто. +- `node -e "import('./scripts/mutation-registry.mjs')..."` — модуль + грузится, 833 мутанта, дублей `id` нет (конфликт слияния с #637 + разрешён без коллизии якорей). +- Проверил регэксп из теста «конвейер пересобирает индекс тем же коммитом» + напрямую на `.github/workflows/process.yml` — совпадает. +- `allowlist` `scripts/review-doc-guard.mjs` — `docs/reviews/` по префиксу + каталога, `docs/reviews/INDEX.md` проходит; шаг публикации индексирует + ровно `--dir=docs/reviews`, что совпадает с `--output` по умолчанию + (`join(dir, INDEX_FILE)`), значит `git add -- docs/reviews/INDEX.md` + после генерации добавляет тот же файл, который написал скрипт. +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — + «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не + выбираются». `src/**` не тронут → golden, инварианты модели, браузерные + смоки, `pytest tests_backend`, перф-профили нерелевантны диффу; не + гонял. +- Не гонял `node scripts/check-docs.mjs` — фингерпринт скриншотов считается + по `src/**`, diff туда не заходит. +- Проверял главное утверждение AC2 не на словах автора, а на реальном + запросе: `grep 'form-kit' docs/reviews/INDEX.md` (пример из хендоффа) и + на живых строках `#594`, `#639` — см. находку ниже. + +## Находки + +### High — H1: индекс молчаливо теряет находки и врёт числами по текущему, +не устаревшему формату заголовков + +AC2 обещает: «поиск по индексу отвечает на вопрос «что находили по файлу X» +за одно чтение». На практике для трети каталога это не так, и это не +хвост старых документов — это формат, которым пишутся review-документы +**прямо сейчас**. + +**Воспроизведение 1 — findings пустые при ненулевых H/M.** +`docs/reviews/CODE-REVIEW-639-r1.md` (r1 предыдущей задачи, на минуту +раньше #635 в этой же очереди) содержит раздел +`### Medium (в скоупе — чинится в этой же ветке)` с текстом находки и +`**Вердикт: жёлтый.**`. Строка индекса: + +``` +| #639 | [CODE-REVIEW-639-r1.md](CODE-REVIEW-639-r1.md) | code · r1 | 🟡 жёлтый | 0 | 0 | — | +``` + +H=0, M=0, «Находки: —» — как будто находок не было вовсе, хотя в +документе явно оформленный Medium. Тот же результат у +`CODE-REVIEW-637-r1.md` (`### Medium (в скоупе задачи) — ложный «—»…`). +Причина: `parseFindings` ищет заголовки вида `### H1 — …`/`### M2: …` +(severity-код сразу после `###`), а живой формат — `### Medium (…) — текст` +или `### Medium (…)\n\n**M1 …**` — код не сразу после `###`. Посчитано по +всему каталогу: 108 документов имеют `(high+medium) > 0` по собственному +подсчёту генератора, но `findings.length === 0`, хотя у документа есть +заголовок `### High/Medium/Low (...)`; 330 документов из 990 имеют такой +заголовок и пустую колонку «Находки» одновременно. + +**Воспроизведение 2 — счётчики берутся не из своего раздела.** +`docs/reviews/CODE-REVIEW-594-r1.md`, строка 14: «ТЗ прошло ревью зелёным +на r3 (…, High: 0, Medium: 0)» — это цитата счётчика **чужого** документа +(SPEC-REVIEW r3). Строка 106, раздел «## Вердикт» этого же документа: +«Жёлтый. High: 0, Medium: 3». `parseCounts` берёт **первое** совпадение +`/High:\s*(\d+)/` и `/Medium:\s*(\d+)/` по всему тексту файла — попадает на +строку 14, а не 106. Индекс: + +``` +| #594 | [CODE-REVIEW-594-r1.md](CODE-REVIEW-594-r1.md) | code · r1 | 🟡 жёлтый | 0 | 0 | — | +``` + +Реально в этом документе 3 Medium (M1 приёмка эталонов, M2 отпечаток +скриншотов, M3 неполное покрытие AC1) — индекс показывает 0. По каталогу: +17 документов, где встречаются ≥2 разных значения `High:`, и 47 — где ≥2 +разных значения `Medium:`; для каждого такого документа `parseCounts` +детерминированно берёт первое вхождение, а не значение из собственного +раздела вердикта — то есть какое из двух значений попадёт в индекс, не +определяется структурой документа. + +**Почему это High, а не Medium.** Это не деградация для процента старых +файлов, распознаваемых «по остаточному принципу» (для такого случая в коде +уже есть явный сигнал низкой уверенности — бейдж ⚪ для нераспознанного +вердикта). Здесь сигнала нет вообще: строка `H:0 M:0 Находки: —` выглядит +**неотличимо** от документа, где действительно нечего было находить. +Ровно вокруг этого антипаттерна в этой же задаче написан урок в +`docs/LESSONS.md` («Лог без итоговой строки — обрыв, а не «ok»: отчёт +судит по маркеру завершения…», 2026-09-21, #604) — новый код воспроизводит +его в другой форме: пустая колонка выглядит как «ok», а не как «не +извлечено». Ради этого индекс и строился (см. постановку issue: «агент… +не найдёт, что r1 #600 уже нашёл разрыв слов в сегментах, кроме как +перечитав документ») — по факту для трети каталога агент по-прежнему не +найдёт находку через индекс и не узнает, что не нашёл: строка выглядит +завершённой. AC2 в буквальном прочтении не выполнен: пример из +собственного хендоффа автора (`grep 'form-kit' docs/reviews/INDEX.md`) +при реальном запуске **не возвращает ни одной строки**, хотя пять +документов (`CODE-REVIEW-594-r1..r3`, `595-r1`, `597-r1`) содержат +`form-kit` в тексте разбора — индекс, в отличие от заявленного, не +годится для этого запроса. + +Фикс не выходит за скоуп задачи: тестовая фикстура покрывает только один +формат заголовка (`### H1 — a`), нужно расширить `parseFindings` под +формат `### (High|Medium|Low) (...) — …` (и его вариант с телом на +следующей строке), а `parseCounts` — искать `High:`/`Medium:` внутри +секции `## Вердикт` (тот же приём, что уже применён в `parseVerdict` для +поиска цвета — секция вычленяется, а не берётся первое совпадение по +всему файлу), либо явно вернуть «не определено», если однозначного +раздела нет, вместо первого попавшегося числа. + +## Что проверено и корректно + +- AC1 (покрытие и детерминизм) — да: `collectEntries` на живом каталоге + даёт 0 `skipped`, `buildIndex(dir) === buildIndex(dir)`, + `--check` подтверждает свежесть закоммиченного файла; тест на фикстуре + проверяет самоисключение `INDEX.md`, сортировку issue от новых к + старым и ТЗ-перед-кодом внутри issue, посторонние файлы — отдельным + списком. Мутант `reviews-index-skips-self-check` ловится. +- Распознавание вердикта (три источника: явная строка, раздел «Вердикт», + свободная форма хвоста) — корректно по фикстурным кейсам и по границе + слова: `\b` в JS ASCII-only, авторы явно заменили её на негативный + lookahead `(?![а-яёa-z])`, тест `'зелёныйзаголовок вердикта'` → `'—'` + проходит, мутант `reviews-index-verdict-substring` ловится тем же + тестом. На живом каталоге распознано 936/986 по хендоффу; перепроверил + текущим прогоном — свежий `INDEX.md` подтверждает похожую долю (⚪ у 48 + из 990 строк, ~95 % распознано), это отдельная, честно обозначенная + категория неопределённости — в отличие от находки H1. +- Публикация в конвейере: шаг после `git add -- "$doc"` вызывает + `reviews-index.mjs --dir=docs/reviews` и добавляет `docs/reviews/INDEX.md` + **до** проверки `git diff --cached --name-only | review-doc-guard.mjs`, + то есть индекс публикуется тем же коммитом, что документ, и проходит тот + же гейт allowlist (`docs/reviews/` по префиксу). Условие `if [ -f "$doc" ]` + верно ограничивает пересборку случаем, когда документ реально появился в + рабочей копии (не выполняется на «документ уже опубликован ревьюером», + что и требуется — иначе индекс пересобирался бы вхолостую на каждом + прогоне). +- `mutation-registry.mjs`: конфликт слияния с #637 разрешён без потери и + без дублирования якорей (833 мутанта, id уникальны, модуль импортируется + без ошибок). +- PROCESS.md §2.10: текст про индекс и про хранение документов физически + попадает в раздел «2.10 Повторный раунд ревью — объём по дельте», номер + секции совпадает с тем, что называют хендофф-комментарии («AC3 закрыт», + «PROCESS.md §2.10»). AC3 по тексту задачи выполнен: решение владельца + зафиксировано дословно (все раунды остаются; перенос в `legacy/reviews/` + при стабильном релизе — пункт чеклиста). +- Трейлеры коммита: `Issue: #635`, `User-Visible: no` — верно, изменение не + задевает продукт/UI; правка обоих CHANGELOG не требуется и не сделана. +- `docs/LESSONS.md` — 12 строк, дата/урок/источник, ссылки на существующие + issue; не гейтится скриптом (не заявлено AC), формат читаем. + +## Чего не проверял + +- `npx tsc --noEmit`, `npm run build` со сверкой трёх копий бандла — не + гонял лично, зачтено по зелёному Validate на этом же SHA (см. «Как + проверялось»); diff их не касается (нет `src/**`, нет `.ts`). +- `npm run golden:verify`, `npm run invariants`, браузерные смоки, + `pytest tests_backend`, перф-профили — не выбираются диффом + (`smoke-select.mjs` подтвердил «исполняемого frontend-диффа нет»); не + гонял. +- `node scripts/check-docs.mjs` — фингерпринт по `src/**`, diff туда не + заходит; не гонял. +- Полный список всех 990 документов на предмет прочих форматов + заголовков — не вычитывал построчно; масштаб находки H1 оценивал + программным подсчётом (108 и 330 из 990), не выборочно на глаз. +- Не проверял поведение `--check` в CI-контексте (переменные окружения, + права на запись) — только локальный вызов; сама логика тривиальна и + совпадает с уже проверенным `writeFileSync`/`readFileSync`. + +## Вердикт + +Жёлтый · заход r1 · блокирующих циклов 0/4 · High: 1 · Medium: 0 + +AC1 и AC3 выполнены, конвейерная интеграция и хранение раундов корректны, +мутанты подтверждены. Но H1 — индекс для 108–330 из 990 документов +(включая свежие #637/#639, написанные буквально прошлым раундом) либо +теряет находки целиком, либо молча берёт число из чужого раздела текста, +неотличимо от «находок не было». Это прямое нарушение AC2 на +собственном примере автора (`grep 'form-kit'`) и воспроизводимо +детерминированно — не вкусовщина о полноте, а конкретный ложный «0 0 —» +там, где в документе явно оформленный Medium. Возврат автору: расширить +`parseFindings` под формат `### (High|Medium|Low) (...) — …` (с телом на +следующей строке), исправить `parseCounts` на поиск внутри секции +«## Вердикт» вместо первого совпадения по всему файлу, перегенерировать +`INDEX.md` и добавить тест на оба варианта регресса — иначе индекс +воспроизводит тот самый провал памяти, ради устранения которого заведена +#635. + +--- + + + +## Материал раунда + +- Ветка: `issue/635-reviews-index`, коммит `db0f5b9b1be0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `305413bc4104386c6e2c6111800d4976c593ef23` + ``` + git log --all --format='%H %T' | grep 305413bc4104 + ``` +- Тело issue: `6ae105e700c9b3bc91974f4e25052237ef671f31d5e59ee5a0841cbaa1bb26a6` +- Вердикт конвейера: `yellow` · High 1