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