Files
2026-09-23 13:52:23 +00:00

24 KiB
Raw Permalink Blame History

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. …**), код берёт первую строку тела, которая не начинается с |#<-:

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

Унаследовано из 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