diff --git a/docs/reviews/CODE-REVIEW-635-r3.md b/docs/reviews/CODE-REVIEW-635-r3.md new file mode 100644 index 00000000..daec0d48 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-635-r3.md @@ -0,0 +1,103 @@ +# CODE-REVIEW-635-r3 + +- **Issue:** #635 «Индекс документов ревью, уроки, правило хранения раундов» +- **Этап:** code (PROCESS.md §2.7) +- **Заход:** r3 · блокирующих циклов израсходовано 2 из 4 (лимит 4) +- **Материал:** `b38bd0c8b5dcce2e17673ee70e0df2c28903f163` (рабочая копия репозитория была на нём; `git fetch`/`checkout` не выполнялись). + +## Скоуп + +r2 (`CODE-REVIEW-635-r2.md`, материал `3e96b37656b9ce49cd56e1aa2f65ebf0808f113b`) закрыл вердикт жёлтым с H1 (`docs/reviews/INDEX.md`, зафиксированный в материале, устарел на собственном `--check` — ребейз ветки принёс документы #625, снимок индекса их не знал) и M1 (`parseFindings`/`firstParagraph`-фолбэк подставлял хвост перенесённой строки буллета вместо текста находки, минимум 11 документов). + +В этом репозитории точный SHA `3e96b376…` из комментариев issue отсутствует (окружение ревьюера перезатирает историю раунда при перезаходах конвейера — ожидаемо, см. хендофф r2 про `3719a06d`→`22dea36c`), но по дереву и сообщениям коммитов делта видна однозначно: коммит `d9bd5a44` — тот самый код r2 (совпадает по содержимому и описанию с материалом r2-вердикта), следующий за ним `f855cee8` — публикация документа `CODE-REVIEW-635-r2.md`, и далее три коммита ровно этого раунда: + +- `49bae62e` — `--commit-if-stale` в `reviews-index.mjs` + вызовы из `process.yml`/`merge-candidate.mjs` (H1); `firstParagraph` вместо хвоста строки (M1); мутант `reviews-index-paragraph-tail`; `PROCESS.md` §2.10. +- `c9f8b50c` — перенос гейта свежести из юнит-теста в предполётный шаг Validate (`push` в `dev` only), т.к. юнит-тест «байтовой свежести» красился бы на любом раунде, пока `process.yml` не отзеркалирован — это следствие механики самого конвейера, не дефект решения H1. +- `b38bd0c8` — шестой сигнал предполётного вердикта в `test/validate-workflow.test.mjs` (свидетель на список шагов #336). + +`git diff d9bd5a44..HEAD` (за вычетом самого `CODE-REVIEW-635-r2.md`) — 10 файлов, 322/-101 строк: `scripts/reviews-index.mjs`, `test/reviews-index.test.mjs`, `scripts/merge-candidate.mjs`, `test/merge-candidate.test.mjs`, `scripts/mutation-registry.mjs`, `PROCESS.md`, `.github/workflows/process.yml`, `.github/workflows/validate.yml`, `test/validate-workflow.test.mjs`, `docs/reviews/INDEX.md` (пересборка). Это ровно код, который переписывался для закрытия H1/M1, — разбор в этом раунде полный по нему. `docs/LESSONS.md`, правило хранения раундов в PROCESS.md §2.10 (кроме дополненного абзаца про индекс/гейт), `parseDocName`, `parseVerdict`/`parseCounts` (кроме затронутой `numberedItems`) делтой не задеты — унаследованы из r1/r2 (раздел ниже). + +## Как проверялось + +1. Прочитан полный `git diff d9bd5a44..HEAD` по каждому файлу дельты (`reviews-index.mjs`, оба workflow, `merge-candidate.mjs`, `mutation-registry.mjs`, `PROCESS.md`, оба тестовых файла делты). +2. `node --test test/reviews-index.test.mjs` — 12/12 pass (9 унаследованных + 3 новых: свидетель на проводку шага Validate, `firstParagraph`-фикстура, `commitIfStale`). +3. `node --test test/merge-candidate.test.mjs test/validate-workflow.test.mjs` — 38/38 pass, включая расширенный `#516 AC1`, который гоняет реальный сценарий «кандидат после ребейза несёт свежий INDEX.md с обоими документами, коммит индекса — вершина `dev`». +4. `node scripts/reviews-index.mjs --dir=docs/reviews --check` на самом материале ревью (без модификаций рабочей копии) — «свеж». Это прямое опровержение H1 r2 на текущем SHA: та же команда, которая на `3e96b376` печатала «устарел», здесь молчит. +5. Пересчитан заголовок `INDEX.md` независимо от скрипта: `ls docs/reviews/*.md | grep -v INDEX.md | wc -l` → 1000, шапка файла заявляет «Документов: 1000» — совпадает буквально, не только по `--check`. +6. Мутант `reviews-index-paragraph-tail` проверен вручную: патч `current = [line]` вместо накопления абзаца → целевой тест `#635 r2: первый абзац` падает с ожидаемой ошибкой (обрывок `'пусто). Не эскалирую…'` вместо целого абзаца), файл восстановлен (`git status` — чисто после). Тест умеет падать. +7. Прогнал `parseFindings` на всех 11 документах, названных в M1 r2 (`CODE-REVIEW-485-r4/r1/r2`, `258-r1`, `514-r3`, `476-r1`, `304-r1`, `SPEC-REVIEW-199-r1`, `309-r1`, `152-r1`, `419-r1`) через `node --input-type=module -e`: во всех текст теперь начинается с начала предложения, ни одного обрывка «пусто)» или висячей кавычки. +8. Скан по всему каталогу (987 находок во всех документах, не только в 11 названных) — не нашёл искажённых обрывков (единственное совпадение моей грубой эвристики оказалось ложным: `--variants=60` — легитимный флаг, начинающийся с `-`, не буллет). +9. Для новой находки Low (ниже) — прямое сравнение поведения регэкспа с удалённой веткой `нет\b`: пересчитал `parseFindings` по всему каталогу с патчем и без — оба раза 987 находок, 12/12 тестов проходят в обоих случаях; файл восстановлен. + +## Находки + +### Low — `firstParagraph`: ветка `нет\b` в фильтре мёртвая из-за ASCII-only `\b` в JS-регэкспах, расходится с собственным doc-комментарием + +**Файл:** `scripts/reviews-index.mjs:135` (комментарий), `:149` (код). + +Комментарий к `firstParagraph` утверждает: «абзац, начинающийся с «не найдено»/«нет», — не находка». Код: + +```js +if (NOTHING_RE.test(text) || /^(?:\**(?:High|Medium|Low)\**\s*)?(?:не найдено|не обнаружено|нет находок|нет\b|отсутству)/i.test(text)) return null; +``` + +`\b` в JS без флага `/u` — граница между `\w` (ASCII-класс) и не-`\w`; кириллические буквы в `\w` не входят, поэтому у «нет» **все три буквы** не-словесные, и `\b` после «т» никогда не совпадает, если дальше идёт пробел, точка, запятая или любая другая кириллица — то есть в реальном тексте почти всегда: + +``` +$ node -e "console.log(/нет\b/i.test('нет проблем'), /нет\b/i.test('нет.'), /нет\b/i.test('нет'))" +false false false +``` + +Ветка `нет\b` совпадает только если сразу после «нет» идёт ASCII-буква/цифра (`/нет\b/i.test('нетx')` → `true`) — случай, которого в живых документах нет. Проверено на всём каталоге: с веткой и без неё `parseFindings` даёт одинаковые 987 находок на 999 документах, 12/12 тестов проходят в обоих вариантах — ветка не меняет ни одного результата, то есть мертва не гипотетически, а по факту на всём материале. + +Важно: это не регресс и не риск для AC. Будь `\b` кириллически-осведомлённым (как задумано комментарием), фильтр стал бы агрессивнее и мог задеть настоящие находки, начинающиеся со слова «Нет» как обычного русского слова, а не маркера «не найдено» — собственный тест этого раунда содержит именно такой случай (`'Нет golden-сцены для радара (…)'`, должен остаться находкой) и по счастливой случайности не ловится сломанной веткой. Так что текущее поведение практически безопасно, но комментарий вводит в заблуждение о том, что код делает, а мёртвая альтернатива в регэкспе — балласт. + +**Серьёзность:** Low. Не блокирует, AC не затрагивает, на материале ревью не имеет наблюдаемого эффекта (подтверждено пересчётом всего каталога). Снимается с записью: либо убрать `нет\b` из регэкспа как недостижимую ветку, либо поправить комментарий, чтобы не заявлять то, чего код не делает. Оставляю на усмотрение следующего касания этого файла — блокировать раунд из-за неё нет оснований. + +## Закрытие раунда r2 + +| Находка r2 | Чем закрыта | Где это видно | +|---|---|---| +| H1 — `docs/reviews/INDEX.md`, зафиксированный в материале ревью, устарел на собственном `--check` (ребейз принёс документы #625, снимок не пересобран) | `commitIfStale()` в `scripts/reviews-index.mjs` (новый экспорт, `--commit-if-stale --issue=NN`): пересобирает индекс и коммитит его коммитом конвейера, если он разошёлся с каталогом; вызывается в `process.yml` сразу после успешного `git rebase origin/dev`, до push, и в `scripts/merge-candidate.mjs` `rebaseOnto()` после ребейза кандидата слияния. Гейт свежести перенесён с несуществующего CI-шага на предполётную проверку `validate.yml` (`id: reviews_index`, только `push` в `dev`, где свежесть держит конвейер) | `scripts/reviews-index.mjs:316-349` (`commitIfStale`), `.github/workflows/process.yml:428` (вызов после rebase), `.github/workflows/validate.yml:95-108` (шаг + вердикт предполёта); тесты `test/reviews-index.test.mjs` (свидетель на проводку в обоих местах, `--commit-if-stale` на временном git-репо — коммитит только при расхождении, рабочая копия чистая), `test/merge-candidate.test.mjs` `#516 AC1` (реальный сценарий: `dev` после слияния несёт индекс с обоими документами, коммит индекса — вершина); на материале ревью `node scripts/reviews-index.mjs --dir=docs/reviews --check` → «свеж» (проверено лично, п. 4 выше), заголовок индекса «Документов: 1000» совпадает с фактическим числом файлов (п. 5) | +| M1 — фолбэк «первая строка тела блока» вырезал буллет-маркер и подставлял хвост перенесённой строки вместо текста находки (11+ документов, включая с ненулевым Medium) | `firstParagraph()` — тело секции разбивается на абзацы (перенесённые строки склеиваются до пустой строки), берётся первый непустой, не служебный (не начинается с `(`) абзац целиком; маркер буллета и код `**M1.**` снимаются; мета-фразы «не найдено»/«не обнаружено»/«нет находок»/«отсутству» — не находка. `numberedItems` тоже забирает перенесённые строки того же пункта | `scripts/reviews-index.mjs:127-155` (`firstParagraph`), `:157-170` (`numberedItems` с продолжением строк); мутант `reviews-index-paragraph-tail` (`scripts/mutation-registry.mjs`) — поймано вручную в этом раунде (п. 6); все 11 документов, названных в r2, перепроверены лично — текст находок теперь начинается с начала предложения (п. 7); полный скан каталога — 987 находок, без искажённых обрывков (п. 8) | + +## Унаследовано из r1/r2 (без повторного разбора — дельта r3 этого не касается) + +- **AC3 / PROCESS.md §2.10, правило хранения раундов** — записано и принято r1 (`CODE-REVIEW-635-r1.md`); в дельте r3 PROCESS.md правится только в части, описывающей индекс/гейт свежести (см. `git diff d9bd5a44..HEAD -- PROCESS.md`), сам параграф про перенос в `legacy/` при стабильном релизе не тронут. +- **`docs/LESSONS.md`** (12 датированных уроков) — не в дельте r3, принято по r1. +- **`parseDocName`, `parseVerdict`, `parseCounts`, старые форматы заголовков находок, `parseFiles`** — не менялись в дельте r3 (diff `scripts/reviews-index.mjs` этого раунда — только новые `firstParagraph`, `commitIfStale`, правка `numberedItems` под перенос строк); приняты по r1/r2, включая ручную проверку мутанта `reviews-index-counts-first-match` в r2. +- **Мутанты `reviews-index-skips-self-check`, `reviews-index-verdict-substring`, `reviews-index-counts-first-match`** — не менялись в дельте r3, приняты по r1/r2 (там же поймано вручную). +- **`.github/workflows/process.yml`, интеграция публикации документа ревью тем же коммитом** — не изменена в этой части дельты (изменение делты — добавленный вызов `--commit-if-stale` до push, отдельная строка), принято по r1. + +## Гейты — что прогнал, что нет + +| Гейт | Статус | Почему | +|---|---|---| +| `node --test test/reviews-index.test.mjs` | прогнал, 12/12 | дешёвый, прямое покрытие дельты (H1 и M1) | +| `node --test test/merge-candidate.test.mjs test/validate-workflow.test.mjs` | прогнал, 38/38 | оба файла в дельте r3 (новые/изменённые тесты `#516 AC1`, свидетель шестого сигнала) | +| `node scripts/reviews-index.mjs --dir=docs/reviews --check` | прогнал сам, «свеж» | это и есть проверка закрытия H1; на материале r2 та же команда падала | +| Мутант `reviews-index-paragraph-tail` | прогнал вручную (патч + тест) | ловится, файл восстановлен | +| Скан `parseFindings` по всем 999 документам каталога | прогнал сам (дважды: с/без ветки `нет\b`) | обнаружил Low-находку; подтвердил отсутствие других обрывков после фикса M1 | +| `npx tsc --noEmit`, `npm test` (полный), `npm run build` | не гонял | зелёный Validate на этом SHA (run упомянут в задании ревью); diff не касается `.ts`/`src/**` (проверено: `git diff --name-only d9bd5a44..HEAD` — пусто по `^src/` и `\.ts$`) | +| `node scripts/check-docs.mjs` | не гонял | diff не заходит в `src/**` | +| `npm run invariants`, `pytest tests_backend` | не нужны | геометрия и Python не затронуты | +| `demo/smoke_*.mjs`, `smoke-select.mjs` | не гонял | фронтенд-диффа нет | +| `npm run golden:verify` | не нужен | рендер не затронут | + +## Вердикт + +Зелёный. High: 0, Medium: 0. Оба High/Medium r2 закрыты и перепроверены на этом материале не только тестами, но и прямым воспроизведением (`--check` свеж, 11 документов из M1 перечитаны, счётчик документов сходится с файловой системой, мутант `reviews-index-paragraph-tail` ловится). Единственная новая находка — Low (мёртвая ветка регэкспа `нет\b`, инертна на всём каталоге) — снимается с записью, не блокирует. + +--- + + + +## Материал раунда + +- Ветка: `issue/635-reviews-index`, коммит `b38bd0c8b5dc` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `090de0dbb1ce4879630530052d644fb071608de1` + ``` + git log --all --format='%H %T' | grep 090de0dbb1ce + ``` +- Тело issue: `6ae105e700c9b3bc91974f4e25052237ef671f31d5e59ee5a0841cbaa1bb26a6` +- Вердикт конвейера: `green` · High 0