mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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`, инертна на всём каталоге) — снимается с записью, не блокирует.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/635-reviews-index`, коммит `b38bd0c8b5dc` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `090de0dbb1ce4879630530052d644fb071608de1`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 090de0dbb1ce
|
||||
```
|
||||
- Тело issue: `6ae105e700c9b3bc91974f4e25052237ef671f31d5e59ee5a0841cbaa1bb26a6`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user