diff --git a/docs/reviews/CODE-REVIEW-635-r2.md b/docs/reviews/CODE-REVIEW-635-r2.md new file mode 100644 index 00000000..a82b70f0 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-635-r2.md @@ -0,0 +1,249 @@ +# CODE-REVIEW-635-r2 + +- **Issue:** #635 «Индекс документов ревью, уроки, правило хранения раундов» +- **Этап:** code (PROCESS.md §2.7) +- **Заход:** r2 · блокирующих циклов израсходовано 1 из 4 (лимит 4) +- **Материал:** `3e96b37656b9ce49cd56e1aa2f65ebf0808f113b` (рабочая копия репозитория была на нём; `git fetch`/`checkout` не выполнялись). Диапазон `origin/dev..HEAD` — три коммита: `898642f8` (исходная реализация #635), `4d9754a1` (публикация документа r1), `3e96b376` (правка r2). + +## Скоуп + +r1 (`CODE-REVIEW-635-r1.md`, SHA `db0f5b9b1be025fca86736f70d6aedd71d057487`) закрыл вердикт жёлтым с одним High: `parseCounts` брал первое совпадение `High:`/`Medium:` по всему тексту документа (часто — цитата чужого раунда), а `parseFindings` не знал живых форматов заголовков находок (`### Medium (в скоупе…) — …`), из-за чего у 108/990 документов колонки H/M были ненулевые, а «Находки» — пустые, и `grep form-kit docs/reviews/INDEX.md` не находил ничего вопреки собственному примеру автора. + +Дельта r2 — ровно один коммит `3e96b376` поверх `4d9754a1`, 4 файла: `scripts/reviews-index.mjs` (+182/-х строк), `test/reviews-index.test.mjs` (+63), `scripts/mutation-registry.mjs` (+11), `docs/reviews/INDEX.md` (пересборка). Это переписывает `parseVerdict`/`parseCounts`/`parseFindings` почти целиком и добавляет `parseFiles` — объём дельты (182 строки правки в файле, который в исходной реализации весил 159 строк) сопоставим с исходной задачей, поэтому разбор в этом раунде — полный по изменённому коду (`scripts/reviews-index.mjs`, тесты, мутант), а не только «проверка, что H1 закрыт». `PROCESS.md`, `docs/LESSONS.md`, `.github/workflows/process.yml`, `parseDocName` дельтой не задеты — унаследованы из r1 (раздел ниже). + +## Как проверялось + +1. Прочитан полный `git diff 4d9754a1..3e96b376` для всех четырёх файлов дельты. +2. Прочитан целиком текущий `scripts/reviews-index.mjs` (298 строк) и `test/reviews-index.test.mjs` (161 строка). +3. `node --test test/reviews-index.test.mjs` — 9/9 pass (включая тест живого каталога, который читает реальный `docs/reviews/`). +4. Мутант `reviews-index-counts-first-match` проверен вручную: патч `for (const scope of [text]) {` вместо цепочки приоритетов → целевой тест падает (`node --test --test-name-pattern="#635 r2: счётчик" test/reviews-index.test.mjs`), файл восстановлен. Тест умеет падать. +5. Дословно воспроизведены оба заявления автора против живого `docs/reviews/INDEX.md`: `grep -c form-kit` → 6 (совпадает); строка `#635 | CODE-REVIEW-635-r1.md` теперь показывает 🟡 1 0 с текстом находки (совпадает). +6. Прогнан `node scripts/reviews-index.mjs --dir=docs/reviews --check` на самом материале ревью (без модификаций рабочей копии, только чтение) — обнаружил H1 ниже. +7. Для H2 (см. ниже) — прямой вызов экспортированных `parseFindings`/`parseCounts` на реальных файлах `docs/reviews/CODE-REVIEW-485-r4.md` и `docs/reviews/CODE-REVIEW-258-r1.md` через `node -e`, плюс скрипт-сканер по всему `docs/reviews/`, воспроизводящий внутреннюю логику `severityBlocks`, чтобы оценить масштаб (11 документов пойманы узкой эвристикой, вероятно больше). + +## Находки + +### H1 — `docs/reviews/INDEX.md`, зафиксированный в материале ревью, устарел на собственном SHA — не проходит `--check`, которым сам же себя проверяет + +**Файл:** `docs/reviews/INDEX.md` (весь материал ревью, `3e96b376`). + +Воспроизведение — три команды, ничего в рабочей копии не менялось: + +``` +$ git rev-parse HEAD +3e96b37656b9ce49cd56e1aa2f65ebf0808f113b +$ git status --short +(пусто) +$ node scripts/reviews-index.mjs --dir=docs/reviews --check +docs/reviews/INDEX.md устарел — пересобрать: node scripts/reviews-index.mjs +``` + +Регенерация в `/tmp` и построчный `diff` с закоммиченным файлом показывают ровно +4 отсутствующие строки — весь issue `#625` (`SPEC-REVIEW-625-r1.md`, +`SPEC-REVIEW-625-r2.md`, `CODE-REVIEW-625-r1.md`, `CODE-REVIEW-625-r2.md`) — и +изменившуюся шапку: «Документов: 992, issue: 345» вместо фактических 996/346. +Эти четыре файла реально лежат в дереве материала (`git ls-files docs/reviews | grep 625` +их находит, `git log --oneline -1 -- docs/reviews/CODE-REVIEW-625-r1.md` → `f10dd1eb`, +предок `3e96b376`), т.е. не артефакт окружения ревьюера, а часть проверяемого коммита. + +Причина, по всей видимости, в многократных ребейзах ветки (`Ревью не запускалось` +×3 в истории issue): `docs/reviews/INDEX.md` — статичный blob, который +перегенерируется только явным запуском скрипта; когда ветку перевозили на новый +`dev`, база успевала получить новые документы ревью (`#625`), а уже закоммиченный +снимок индекса — нет. `--check` — это именно тот гейт, который должен был поймать +расхождение, но он существует только как локальная CLI-опция; ни в одном workflow +(`grep -rn reviews-index .github/workflows/*.yml` — единственное вхождение +в `process.yml`, шаг публикации, не проверка) он не запускается на PR/ветке как +блокирующий шаг. `npm test` тоже не ловит: тест «живой каталог» (`test/reviews-index.test.mjs:87-93`) +проверяет свойства `docs/reviews/` напрямую (покрытие, доля распознанных +вердиктов), а не совпадение с уже закоммиченным `INDEX.md` — так что зелёный +Validate на этом SHA (о котором сказано в задании ревью) действительно не покрывает +этот дефект, это не пропуск в моей проверке гейтов. + +Последствие: ровно то, ради чего заведён #635, — «агент, берущий задачу по +диалогам, не найдёт [решение или урок], кроме как перечитав документ» — +воспроизводится внутри собственного индекса: обе находки `#625` невидимы через +`INDEX.md`, хотя документы существуют. AC1 («индекс покрывает 100 % документов +каталога») не выполнено на материале ревью. + +**Серьёзность:** High, в скоупе. Фикс: перед коммитом дельты (или в отдельном +шаге пайплайна) запускать `node scripts/reviews-index.mjs --dir=docs/reviews` +против **финального** дерева ветки (после последнего ребейза), не против +локального кеша разработчика; либо жёстко привязать регенерацию к +пред-мержевому гейту, а не только к шагу публикации документа ревью. + +### M1 — `parseFindings`/`parseFiles`: фолбэк «первая строка тела блока» вырезает начало буллета и подставляет обрывок продолжения + +**Файл:** `scripts/reviews-index.mjs:190-196` (ветка `severityBlocks` → `first`). + +Когда у заголовка находки (`### Low`, `### Medium` и т. п.) нет собственного +текста после разделителя и нет пронумерованных пунктов (`**M1. …**`), код берёт +первую строку тела, которая не начинается с `|#<-`: + +```js +const first = block.body.map((l) => l.trim()).find((l) => l && !/^[|#<-]/.test(l)); +``` + +Символ `-` в этом исключении означает «пропустить маркер буллета», но +эффект — пропустить **весь буллет**, если он в один физический абзац не +уместился и перенесён на следующую строку (обычный стиль этой кодовой базы — +переносы строк внутри длинного предложения). Тогда «первой подходящей +строкой» становится не начало пункта, а его хвост — бессвязный вне контекста. + +Воспроизведение (реальный файл материала, без изменений): + +``` +$ node -e " +import('./scripts/reviews-index.mjs').then(({parseFindings}) => { + console.log(parseFindings(require('fs').readFileSync('docs/reviews/CODE-REVIEW-485-r4.md','utf8'))); +});" +[ 'пусто). Не эскалирую третий раунд подряд по той же логике r2/r3: защитные' ] +``` + +Настоящий текст пункта в документе (`docs/reviews/CODE-REVIEW-485-r4.md:110-113`): +«Нет golden-сцены для радара (`find demo/golden -iname "*radar*"` — по-прежнему +пусто). Не эскалирую третий раунд подряд по той же логике r2/r3…» — индекс +показывает обрывок с несогласованным «пусто)» в начале, будто предложение +начинается с закрывающей скобки. + +Это не единичный случай. Точечный скан по `docs/reviews/` (репродуцирует +эвристику `severityBlocks` без изменения кода) находит минимум 11 документов с +этим паттерном, включая документы с реальным ненулевым Medium — не только +инертные «Low, унаследовано»: + +- `CODE-REVIEW-258-r1.md` (🟡 M:1) — второй элемент «Находки» в индексе: + `'Записи толщины, которые не найдутся по ключу' теперь мёртв — wall_key` + (начинается с висячей одиночной кавычки — обрывок цитаты). +- `CODE-REVIEW-514-r3.md` (🟡 M:1) — второй элемент: `High не найдено. Medium + вне скоупа не найдено — единственный Medium (M1) целиком внутри…` — это не + находка, а мета-фраза «находок нет», которую `NOTHING_RE` должна была + отсеять, но отсеивает только когда это **всё** тело строки, а не когда это + «первая непустая строка» внутри более длинного абзаца. +- `CODE-REVIEW-476-r1.md`, `SPEC-REVIEW-199-r1.md`, `SPEC-REVIEW-309-r1.md`, + `CODE-REVIEW-304-r1.md`, `CODE-REVIEW-485-r1.md`, `CODE-REVIEW-485-r2.md`, + `SPEC-REVIEW-152-r1.md`, `SPEC-REVIEW-419-r1.md` — тот же паттерн (см. лог + проверки ниже). + +Числа High/Medium (`parseCounts`) этим багом не затронуты — они не используют +эту ветку. Ломается именно то, ради чего добавлена колонка «Находки»/«Файлы» в +этом же раунде: агент, читающий строку индекса вместо документа, получает +синтаксически рваный, не соответствующий по смыслу текст вместо реальной сути +находки — то есть тот же класс дефекта, что и закрытый H1 r1 («индекс молчаливо +[искажает] находки»), только не пустотой, а произвольным обрывком. + +**Серьёзность:** Medium, в скоупе (код и тесты дельты r2, `parseFindings` — то +самое, что в этом раунде переписывалось). Фикс в рамках задачи: при выборе +«первой строки тела» либо склеивать физически перенесённые строки одного +буллета перед поиском первой подходящей, либо не отфильтровывать строки, +начинающиеся с `-`, а прицельно вырезать только буллет-маркер (`^[-*]\s+`) и +использовать очищенную первую непустую строку целиком; плюс усилить `NOTHING_RE` +проверкой не только всей строки, но и вхождения фраз «не найдено»/«целиком +внутри» в начале извлечённого текста. Нужен тест-фикстура с многострочным +буллетом (аналогично уже добавленным фикстурам на реальных документах в этом же +раунде). + +## Что проверено и корректно + +- **AC2 (пример автора).** `grep form-kit docs/reviews/INDEX.md` — 6 строк + (было 0 в r1). Совпадает с заявлением. +- **Приоритет источника вердикта/счётчика.** Пересказ чужого раунда строчными + буквами в шапке документа (`CODE-REVIEW-152-r2.md:9` — «вердикт красный, + High: 1») больше не перебивает собственный зелёный вердикт документа + (строка 142: «**Вердикт: зелёный … High: 0 · Medium: 0**») — проверено и по + живому файлу, и по регэкспу (`VERDICT_OWN_LINE_RE` требует заглавную + «Вердикт» с начала строки). +- **Регресс-мутант.** `reviews-index-counts-first-match` действительно ловится + целевым тестом (проверено ручным патчем, не только доверием к отчёту автора). +- **Новые форматы заголовков находок.** `### Medium (в скоупе…) — …`, + `## Находка N (High…) — …`, `### [High] …`, `**M1. …**`-пункты внутри секции + — все разбираются тестами `test/reviews-index.test.mjs:104-150`, и я + подтвердил на реальных `CODE-REVIEW-639-r1.md`/`637-r1.md`, что + соответствующие строки `docs/reviews/INDEX.md` не пустые. +- **Собственный документ r1.** `CODE-REVIEW-635-r1.md` в живом индексе (строка + 12) теперь 🟡 1 0 с корректным текстом находки — ретроспективная проверка, + что H1 закрыт не только на синтетических фикстурах. +- **9/9 юнит-тестов** `test/reviews-index.test.mjs`, включая тест живого + каталога (100 % покрытие имён, >90 % распознанных вердиктов) — прогнан лично, + не только со слов автора. + +## Чего не проверял + +- `npx tsc --noEmit`, полный `npm test`, `npm run build` — зачтено по зелёному + Validate на этом SHA (run `35859217407`, ссылка дана в задании ревью); диф не + трогает `.ts`/`src/**`, риск расхождения низкий. +- `node scripts/check-docs.mjs` — не запускал: diff не заходит в `src/**` + (только `scripts/`, `test/`, `docs/reviews/INDEX.md`), отпечаток скриншотов + документации не мог устареть от этой правки. +- `npm run invariants`, backend pytest, golden, performance-профили — не + относятся: геометрия, Python и рендер не затронуты. +- `demo/smoke_*.mjs` — фронтенд-дифф отсутствует, `smoke-select.mjs` не + запускал (аналогично r1, диф той же природы). +- Полный аудит остальных ~980 документов на предмет ещё не найденных + вариаций M1 — сканировал узкой эвристикой (одна конкретная форма фолбэка), + реальный масштаб может быть шире 11 найденных; не пытался перечислить все. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где видно | +|---|---|---| +| H1 — `parseCounts` брал первое `High:`/`Medium:` по всему тексту (цитата чужого раунда, пример `CODE-REVIEW-594-r1.md`); `parseVerdict` так же терял приоритет собственной строки (пример `CODE-REVIEW-152-r2.md`); `parseFindings` не знал живых форматов заголовков (`### Medium (в скоупе…) — …`, `## Находка N (…)`, `**M1. …**`); колонки «Файлы» не было, `grep form-kit` находил 0 строк | Три правки в `scripts/reviews-index.mjs`: (1) `parseCounts`/`parseVerdict` — цепочка приоритета «своя строка `Вердикт` → секция `## Вердикт` → единственное значение по тексту → подсчёт по заголовкам» (`verdictLine`, `verdictSection`, `countsIn`); (2) `parseFindings` — `severityBlocks`/`numberedItems`/`FINDING_HEADING_RE` покрывают перечисленные живые форматы; (3) новая `parseFiles` + колонка «Файлы» | `scripts/reviews-index.mjs:50-220` (`git diff 4d9754a1..3e96b376`); тесты-регрессы на реальных документах `test/reviews-index.test.mjs:104-160` (594-r1, 152-r2, 639/637/162/141-фикстуры); в живом `docs/reviews/INDEX.md` — строка `#635 | CODE-REVIEW-635-r1.md` теперь 🟡 1 0 с текстом находки (было бы `0 0 —` до фикса), `grep form-kit` — 6 строк вместо 0; мутант `reviews-index-counts-first-match` (`scripts/mutation-registry.mjs`) — поймано вручную в этом раунде | + +## Унаследовано из r1 (без повторного разбора — дельта r2 этого не касается) + +Документ: `docs/reviews/CODE-REVIEW-635-r1.md`, SHA материала r1 +`db0f5b9b1be025fca86736f70d6aedd71d057487`. + +- **AC3 / PROCESS.md §2.10** — правило хранения раундов (все раунды текущей + линии в `docs/reviews/`, перенос в `legacy/` при стабильном релизе) записано + и принято r1; `PROCESS.md` не входит в diff `4d9754a1..3e96b376`. +- **`docs/LESSONS.md`** (12 датированных уроков) — не изменялся в дельте r2, + принят по содержанию r1. +- **`.github/workflows/process.yml`** — интеграция публикации (индекс + пересобирается и коммитится тем же коммитом, что документ ревью; + `review-doc-guard` пропускает `docs/reviews/`) — не изменялся в дельте r2, + принято по r1. (Отдельно от этого унаследованного факта — H1 этого раунда + показывает, что сама пересборка не гарантирует свежесть после последующих + ребейзов; это новый дефект процесса, не отменяющий корректность самой + интеграции шага.) +- **`parseDocName`** и разбор старых форматов заголовков находок (`### 1. …`, + строки таблиц `| H1 | … |`) — байт-в-байт не менялись в дельте r2 (только + обёрнуты в `forEach` без изменения самих регэкспов) — приняты по r1. +- **Мутанты `reviews-index-skips-self-check`, `reviews-index-verdict-substring`** + — не менялись в дельте, приняты по r1 (там же вручную проверено «поймано 1 + из 1»). + +## Гейты — что прогнал, что нет + +| Гейт | Статус | Почему | +|---|---|---| +| `npx tsc --noEmit`, `npm test`, `npm run build` | не гонял | зелёный Validate на `3e96b376` (run 35859217407); diff не касается `.ts`/`src/**` | +| `node --test test/reviews-index.test.mjs` | прогнал, 9/9 | дешёвый, прямое покрытие дельты | +| `node scripts/reviews-index.mjs --dir=docs/reviews --check` | прогнал, **упал** | не покрыт ни Validate, ни `npm test`; это и есть H1 | +| Мутант `reviews-index-counts-first-match` | прогнал вручную (патч + тест) | ловится | +| `node scripts/check-docs.mjs` | не гонял | diff не в `src/**` | +| `npm run invariants` | не нужен | геометрия не затронута | +| `demo/smoke_*.mjs`, `smoke-select.mjs` | не гонял | фронтенд-диффа нет | +| `npm run golden:verify` | не нужен | рендер не затронут | +| `pytest tests_backend` | не нужен | Python не затронут | + +## Вердикт + +Жёлтый. High: 1 (`INDEX.md` материала ревью устарел, не проходит собственный +`--check`, AC1 не выполнено на этом SHA), Medium: 1 (в скоупе — `parseFindings` +подставляет обрывки многострочных буллетов вместо текста находки, минимум 11 +документов). Оба — по коду, изменённому в этой же дельте r2, чинятся в этом же +issue. + +--- + + + +## Материал раунда + +- Ветка: `issue/635-reviews-index`, коммит `3e96b37656b9` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `1cca2716ecd7f5004a0a235b1ff47da5ffb43019` + ``` + git log --all --format='%H %T' | grep 1cca2716ecd7 + ``` +- Тело issue: `6ae105e700c9b3bc91974f4e25052237ef671f31d5e59ee5a0841cbaa1bb26a6` +- Вердикт конвейера: `yellow` · High 1