From bcfb6e11235594a8c2af95b9897cf859f722073b Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:32:28 +0000 Subject: [PATCH] docs: review document for #779 Issue: #779 User-Visible: no --- docs/reviews/CODE-REVIEW-779-r2.md | 209 +++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-779-r2.md diff --git a/docs/reviews/CODE-REVIEW-779-r2.md b/docs/reviews/CODE-REVIEW-779-r2.md new file mode 100644 index 00000000..b2a7193b --- /dev/null +++ b/docs/reviews/CODE-REVIEW-779-r2.md @@ -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 (снята ревьюером). + +--- + + + +## Материал раунда + +- Ветка: `issue/779-reviews-index-foreign-verdict`, коммит `1015585578e8` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `4cad1a680a2c4e3971a96ad8ad1760326c662c78` + ``` + git log --all --format='%H %T' | grep 4cad1a680a2c + ``` +- Тело issue: `11ae425b9b6d40675b2bd539fdf0caf9883f5997c3728bdcc5334d775d2fc5ee` +- Вердикт конвейера: `green` · High 0 · маршрут `fix` +