mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-07 15:09:30 +00:00
claude[bot]
@@ -0,0 +1,209 @@
|
||||
# CODE-REVIEW-779-r2
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/779
|
||||
- **Этап:** `S7-code-review` (код-ревью, PROCESS.md §2.7)
|
||||
- **Трек:** `track:show` (до 3 AC в теле issue, лёгкое код-ревью)
|
||||
- **Заход:** r2 · блокирующих циклов израсходовано 1 из 2 (бюджет `track:show`)
|
||||
- **Материал:** ветка `issue/779-reviews-index-foreign-verdict`, ровно
|
||||
`1015585578e8fa004f9a74398ee3a804307d5b2b` (рабочая копия на нём;
|
||||
`git log --oneline origin/dev..HEAD` даёт три коммита: `fafc83e0` (фикс r1,
|
||||
материал прошлого раунда) → `85e96b22` (публикация CODE-REVIEW-779-r1.md) →
|
||||
`10155855` (фикс r1-находки, материал этого раунда)). Validate на
|
||||
`10155855` — зелёный:
|
||||
https://github.com/Matysh/houseplan-card/actions/runs/37545580751.
|
||||
Ветка не приводилась к `dev` (трек `show`, #696): `dev` впереди на 16
|
||||
коммитов, слияние без конфликта — нормально для этого трека, не находка.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Предмет r2 — ровно Medium-находка r1 (`CODE-REVIEW-779-r1.md`): функция
|
||||
`summaryParagraph()` брала «последний абзац, начинающийся с `High:`» по всему
|
||||
документу, доверяя допущению «пересказы — в начале, вывод — в конце». Порядок
|
||||
секций в корпусе не зафиксирован (`SPEC-REVIEW-728-r2.md` кладёт `## Вердикт`
|
||||
до `## Унаследовано`, `774-r2.md`/`806-r2.md` — после), поэтому пересказ,
|
||||
процитированный отдельным абзацем `- High: N, Medium: N`, мог обогнать
|
||||
настоящую сводку.
|
||||
|
||||
Коммит `10155855` вводит понятие «собственного текста документа» —
|
||||
`ownText()` (`scripts/reviews-index.mjs:158-182`): заголовки уровня 2+,
|
||||
которые являются пересказом («Закрытие раунда», «Унаследовано», `Inherited`,
|
||||
`Previous round`, либо любой заголовок с чужим номером раунда — раунд
|
||||
документа берётся из имени файла, иначе из `# …-rN`), вместе с их
|
||||
подсекциями заменяются пустыми строками; то же — с цитатами `>` (включая
|
||||
«ленивое» продолжение без `>`). Блоки кода остаются собственным текстом
|
||||
(ревьюеры кладут туда шаблон §7.2), заголовок `#` внутри блока — не
|
||||
заголовок. `parseVerdict`, `parseCounts` и обновлённая `summaryParagraph`
|
||||
(теперь предпочитает абзац из секции собственной строки вердикта, иначе
|
||||
последний в собственном тексте) ищут источник только в `ownText(...)`.
|
||||
|
||||
Один класс изменений (B — `scripts/**`, `test/**`; генерируемый класс D
|
||||
`docs/reviews/INDEX.md` не расходится с кодом коммита). Инфраструктурная
|
||||
задача, `S7-code-review` напрямую — соответствует правилу AGENTS.md.
|
||||
`User-Visible: no` на обоих коммитах корректен: продукт не меняется, второй
|
||||
changelog не требуется. Чисел, видимых пользователю дважды с разными
|
||||
источниками, в диффе нет — это генератор `docs/reviews/INDEX.md`, его выход
|
||||
не дублируется другим путём отображения.
|
||||
|
||||
## Маршрут (route)
|
||||
|
||||
Критерии §5 для `show` не изменились относительно r1 (делта не трогает ни
|
||||
поверхность, ни конфигурацию, ни UX, ни перф/touch): `complexity`,
|
||||
`surfaces`, `migration`, `ux-contract`, `perf-touch`, `undocumented` — все
|
||||
проходят по тем же основаниям, что в `CODE-REVIEW-779-r1.md`. **route: fix**.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
| Гейт | Статус | Результат |
|
||||
|---|---|---|
|
||||
| Validate (tsc/test/build) на `10155855` | не перегонялся | зелёный прогон на этом SHA подтверждён ссылкой выше (#343) — диффа в `src/**` нет, дешёвые гейты приняты без повтора |
|
||||
| `node --test test/reviews-index.test.mjs` | прогнан | 16/16 pass, включая новый `#779 r2: счётчики и вердикт — из собственного текста…` (6 под-сценариев: порядок «после»/«до» пересказа, шаблонный пересказ §7.2, заголовок со своим `rN`, цитата, свой шаблон в блоке кода) |
|
||||
| **Эмпирическая проверка «тест умеет падать»** (не из рабочего дерева: блоб `fafc83e0:scripts/reviews-index.mjs`, код r1 до этого фикса, прогнан локально против трёх сценариев нового теста) | выполнено | `after`: `зелёный {high:2,medium:1}` (должно `0/0`) — КРАСНЫЙ на r1; `before`: `жёлтый {high:0,medium:0}` (должно `0/2`) — КРАСНЫЙ; `templated`: `красный {high:1,medium:0}` (должно `зелёный 0/0`) — КРАСНЫЙ. На `10155855` все три проходят (см. тест). Это прямое, а не декларативное подтверждение таблицы автора «до/после» |
|
||||
| `node scripts/mutation-gate.mjs --check` (структурная проверка реестра) | прогнан | 0 FAIL; новый `reviews-index-retold-sections-own` и соседи (`verdict-substring`, `counts-first-match`, `own-verdict-any-tail`, `paragraph-tail`) — все `ok`, find-строки патчей совпадают по коду ровно один раз |
|
||||
| Реальный прогон мутантов | не прогонялся | `track:show` их не гоняет ни на каком треке (REVIEWER.md «Трек show»); защита названа мутантом в реестре — названа |
|
||||
| `node scripts/reviews-index.mjs --check` | прогнан | `устарел` — ожидаемо: `docs/reviews/CODE-REVIEW-779-r1.md` (добавлен `85e96b22`) ещё не попал в закоммиченный `INDEX.md`; его пересобирает конвейер при слиянии (`--commit-if-stale`), на ветках индекс не судится (подтверждено в `CODE-REVIEW-779-r1.md` и в validate.yml, #635) |
|
||||
| Сравнение `docs/reviews/INDEX.md` в диффе ветки против `dev` | прочитан | меняется ровно одна строка — `#662 SPEC-REVIEW-662-r3: 🟢 0/0 → 🟡 0/2`, как и в r1; новых строк (кроме отсутствующей #779-r1) нет |
|
||||
| `tsc --noEmit`, `npm run build`, bundle-policy | не перегонялись | диффа в `src/**`/бандле нет; покрыто Validate на SHA |
|
||||
| golden/`ci:golden`, invariants, pytest backend, performance, smoke-select | не применимо | нет изменений в рендере/геометрии/Python/перф-коде/`demo/**`/`src/**`; меток `ci:golden` на issue нет |
|
||||
| Трейлеры коммитов | прочитаны | `fafc83e0`, `85e96b22`, `10155855` — у каждого `Issue: #779`, `User-Visible: no`; корректно для класса B/C без видимого пользователю поведения |
|
||||
|
||||
## Находки
|
||||
|
||||
Нет находок, блокирующих вердикт.
|
||||
|
||||
### Отмечено и снято ревьюером (Low) — поздние фолбэки `parseCounts`/`parseVerdict` по-прежнему читают весь текст документа, включая пересказы
|
||||
|
||||
**Файл:** `scripts/reviews-index.mjs:323-339` (ветки «упоминание где угодно»,
|
||||
«единственный счётчик во всём файле», «подсчёт по заголовкам» в `parseCounts`)
|
||||
и `scripts/reviews-index.mjs:102-106` (аналогичные ветки `explicit`/`tail` в
|
||||
`parseVerdict`). Эти ветки используют `text`, а не `ownText(text, {round})`,
|
||||
т.е. в принципе могут взять число/цвет из секции-пересказа, если её
|
||||
содержимое не попадает под более приоритетные проверки (`verdictLine(own,
|
||||
true)`, `verdictSection(own)`, `summaryParagraph(own)`).
|
||||
|
||||
Это ровно тот же класс дефекта, который задача устраняет для приоритетных
|
||||
веток — но автор явно его не трогал и явно объяснил почему в теле коммита
|
||||
`10155855`: «The later fallbacks (verdict mention, unique counter, headings)
|
||||
read the whole text as on dev. Moving them to own text fixes some legacy rows
|
||||
but loses verdict recognition in others (289-r2, 376-r2, 462-r2/r3), which is
|
||||
a separate decision.» Я проверил, что названные документы существуют и
|
||||
относятся к архиву, не к индексируемому корпусу:
|
||||
`legacy/reviews/v1.68.0/{SPEC,CODE}-REVIEW-289-r2.md`,
|
||||
`legacy/reviews/v1.69.0/{SPEC,CODE}-REVIEW-376-r2.md`,
|
||||
`legacy/reviews/v1.72.0/SPEC-REVIEW-462-r{2,3}.md` — т.е. заявление не
|
||||
голословно, это реальные файлы, и правка этих веток была бы регрессией по
|
||||
уже существующим записям, а не чистым выигрышем.
|
||||
|
||||
**Почему Low, не Medium.** В отличие от находки r1 (которая была про функцию,
|
||||
введённую этим же усилием именно для решения данного класса бага, и
|
||||
воспроизводилась на правдоподобной форме документа из текущего
|
||||
индексируемого корпуса), здесь: (1) код — не новый, унаследован от #635,
|
||||
задача `Что сделать` его не называла; (2) предполагаемый побочный документ,
|
||||
на котором ветка срабатывает (`SPEC-REVIEW-403-r2`), лежит в
|
||||
`legacy/reviews/` и не индексируется `docs/reviews/INDEX.md` — проверено
|
||||
(`collectEntries` читает только `dir` = `docs/reviews`); (3) `--check`
|
||||
подтверждает отсутствие регресса по всем ~1000 документам реального корпуса;
|
||||
(4) будущие документы пишутся по шаблону §7.2 этого же промпта («Вердикт:
|
||||
цвет» с двоеточием) и не попадают на эти низкоприоритетные ветки вовсе. Это
|
||||
бухгалтерия незавершённой генерализации старого кода, а не дефект поведения,
|
||||
который увидит пользователь на существующем или новом документе — снимаю без
|
||||
цикла и без отдельного issue (REVIEWER.md «Трек show»: «нечувствительный…
|
||||
— Low и цикла не открывает»; здесь по аналогии — риск есть, но не
|
||||
воспроизводится ни на одном реальном индексируемом документе и не возникнет
|
||||
на новых).
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| Medium: `summaryParagraph()` выбирает «последний абзац на `High:`» по всему тексту, доверяя порядку секций (не гарантирован форматом) | `ownText()` (`scripts/reviews-index.mjs:158-182`) вычленяет собственный текст документа (без секций-пересказов и цитат) до любого разбора; `summaryParagraph` (строки 193-216) ищет абзац только в нём, предпочитая секцию собственной строки вердикта | `scripts/reviews-index.mjs:91-108` (`parseVerdict`), `:315-322` (`parseCounts`) передают `ownText(...)` во все три приоритетные ветки; новый тест `test/reviews-index.test.mjs` `#779 r2: счётчики и вердикт — из собственного текста…` (строки 251-283) покрывает оба порядка секций (728-r2-style и 774/806-r2-style) плюс пересказ в форме шаблона §7.2; я эмпирически прогнал эти три сценария на коде r1 (`fafc83e0`) — все три красные (`2/1`, `0/0`, `красный 1/0` вместо ожидаемых), на `10155855` — зелёные |
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Принято без повторной проверки (документ `docs/reviews/CODE-REVIEW-779-r1.md`,
|
||||
материал `fafc83e05b144ec001efae3c33532e4c036ab39d`):
|
||||
|
||||
- Основной фикс #779 — различение «Вердикт: цвет» (своя строка, двоеточие
|
||||
сразу после слова) от «Вердикт rN — …» (пересказ прошлого раунда), включая
|
||||
машинную строку «Вердикт конвейера:» и legacy-форму без двоеточия.
|
||||
Делта r2 его не трогает (diff не затрагивает `VERDICT_OWN_LINE_RE`,
|
||||
`VERDICT_LEAD_LINE_RE`, `VERDICT_LINE_RE`).
|
||||
- Тестовая фикстура `#779 пересказ «Вердикт rN — цвет»…`, построенная по
|
||||
реальному `SPEC-REVIEW-662-r3.md`, и факт, что `--check` меняет в индексе
|
||||
ровно эту одну строку (`#662` r3).
|
||||
- Реестр мутантов на момент r1: `reviews-index-own-verdict-any-tail` (новый
|
||||
в r1) и анкеры `verdict-substring`/`counts-first-match`/`paragraph-tail` —
|
||||
структурная валидность подтверждена в r1 и переподтверждена здесь после
|
||||
сдвига строк (см. таблицу гейтов выше).
|
||||
- Трейлеры `fafc83e0`: `Issue: #779`, `User-Visible: no` — верно, повторно не
|
||||
перепроверялись.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Новая `ownText()` корректно различает: заголовок-пересказ по ключевым
|
||||
словам («Закрытие раунда», «Унаследовано», `Inherited`, `Previous round`,
|
||||
«Предыдущий/прошлый раунд»); заголовок с чужим номером раунда
|
||||
(`## Дельта r1 → r2` в документе r2); цитату `>` с ленивым продолжением;
|
||||
блок кода (не считается ни заголовком, ни цитатой — проверено тестом на
|
||||
шаблоне §7.2 внутри ``` ``` ```); границы секции снимаются правильно через
|
||||
стек уровней заголовков (подсекции пересказа наследуют `retold`).
|
||||
- Собственный раунд документа берётся из `meta.round` (имя файла) в
|
||||
`indexEntry`, что соответствует всем реальным документам схемы
|
||||
`(CODE|SPEC)-REVIEW-NN-rN.md`; резервный путь (разбор `# …-rN`) нужен только
|
||||
вызовам `parseVerdict`/`parseCounts` напрямую (тесты) — в проде не
|
||||
задействован, т.к. `indexEntry` всегда передаёт `round`.
|
||||
- `summaryParagraph` корректно предпочитает абзац из секции собственной
|
||||
строки вердикта, когда она есть (сценарий «до») и откатывается на последний
|
||||
абзац собственного текста, когда её нет (сценарий «после») — оба пути
|
||||
проверены и эмпирически, и тестом.
|
||||
- Реестр мутантов структурно согласован (`mutation-gate --check`, 0 FAIL);
|
||||
предупреждения в выводе (`browser guards: 243`, несовпадающие
|
||||
`--test-name-pattern` в других файлах) — не относятся к этой задаче,
|
||||
присутствуют и на `dev`.
|
||||
- Трейлеры всех трёх коммитов диапазона корректны; `docs/reviews/INDEX.md` в
|
||||
диффе ветки не содержит никакого расхождения чисел High/Medium против
|
||||
текста документов.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полный `tsc --noEmit`/`npm test`/`npm run build` + сверка бандла целиком —
|
||||
положился на зелёный Validate `10155855` (#343); дифф не трогает `src/**`.
|
||||
- Реальный прогон мутантов (включая новый `reviews-index-retold-sections-own`)
|
||||
— `track:show` их не гоняет; проверена только регистрация и анкеры.
|
||||
Поимка — задача ночного прогона (#709).
|
||||
- Полный ручной обзор всех ~1000 документов `docs/reviews/*.md` на предмет
|
||||
других скрытых форм пересказа — положился на детерминированный `--check`
|
||||
(побайтовое сравнение) как эквивалент для существующего корпуса; для
|
||||
будущих документов эквивалента нет (см. Low-находку выше).
|
||||
- Содержимое `legacy/reviews/*` за пределами выборочной проверки
|
||||
существования трёх названных в коммите файлов (289-r2, 376-r2, 462-r2/r3) —
|
||||
не индексируется, не входит в предмет код-ревью индекса.
|
||||
- `golden:verify`, `npm run invariants`, backend `pytest`, performance,
|
||||
`smoke-select` — не применимы к диффу (нет рендера/геометрии/Python/
|
||||
перф-кода/`demo/**`/`src/**` в изменениях).
|
||||
|
||||
## Вывод
|
||||
|
||||
r1-находка закрыта и эмпирически подтверждена (старый код красный на всех
|
||||
трёх новых сценариях, новый — зелёный). Новых High/Medium нет; один Low —
|
||||
известное, явно раскрытое автором ограничение старых низкоприоритетных веток
|
||||
парсера, не задевающее ни один документ индексируемого корпуса и не
|
||||
возникающее на документах, написанных по текущему шаблону — снят без цикла.
|
||||
|
||||
**Вердикт: зелёный.**
|
||||
|
||||
High: 0 · Medium: 0 · Low: 1 (снята ревьюером).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/779-reviews-index-foreign-verdict`, коммит `1015585578e8` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `4cad1a680a2c4e3971a96ad8ad1760326c662c78`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 4cad1a680a2c
|
||||
```
|
||||
- Тело issue: `11ae425b9b6d40675b2bd539fdf0caf9883f5997c3728bdcc5334d775d2fc5ee`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
<!-- hp:usage input_tokens=4449 output_tokens=36353 cache_creation_input_tokens=113260 cache_read_input_tokens=2135537 num_turns=32 -->
|
||||
Reference in New Issue
Block a user