docs: review document for #454

Issue: #454
User-Visible: no

docs/reviews/CODE-REVIEW-454-r3.md

@@ -0,0 +1,258 @@
+# CODE-REVIEW — issue #454 · заход r3 (см. «Заход vs факт» ниже)
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/454
+- Материал: `git rev-parse HEAD` = `4b443d8c` (ветка `issue/454-review-round-counter`,
+  ранее реализация была на `634478a5`; конвейер перед ревью привёл её к `dev`
+  через ребейз (+12 коммитов `dev`, включая `05ef3181`, `aa51ed2b`, `8935a736`,
+  `169fed01`, `4203a01b`, `2202b207` и др.) — после ребейза это другой код
+  (PROCESS.md §7.2), поэтому разбор ниже **полный**, не по дельте.
+- `git diff origin/dev...HEAD --stat`: 8 файлов, +1226/−3 —
+  `.github/workflows/process.yml`, `scripts/review-doc-guard.mjs`,
+  `scripts/mutation-gate.mjs`, `test/review-doc-guard.test.mjs`,
+  `docs/specs/454-review-round-counter.md`, `docs/specs/README.md`,
+  `docs/reviews/SPEC-REVIEW-454-r1.md`, `-r2.md`.
+- Класс изменения: B (инфраструктура), `User-Visible: no` на всех коммитах —
+  проверено `git show -s --format=%B` на каждом из 7 коммитов диапазона.
+
+## Заход vs факт (важно для чтения этого документа)
+
+Заголовок задачи называет это «заход r3». Проверено по таймлайну меток
+(`gh api repos/.../issues/454/timeline`): метка `S7-code-review` навешена
+на #454 **ровно один раз**, в 16:57:56. Это первый и единственный код-ревью
+раунд #454 — фактически он `r1`. Номер `r3` в имени документа — не ошибка
+приложенного шаблона, а живое проявление ровно того дефекта, который #454
+чинит: см. **M1** ниже, где это воспроизведено вплоть до реального лога guard
+этого самого прогона. Оставляю нумерацию файла как задаст шаг публикации
+(`CODE-REVIEW-454-r3.md`) — переименовывать его вручную значит повторить
+ту же ошибку «не доверять факту конвейера», от которой и уходит #454.
+Раздела «Унаследовано из r<N−1>»/«Закрытие раунда r<N−1>» в этом документе
+нет: предыдущего код-ревью раунда #454 не существовало.
+
+## Скоуп
+
+`process.yml`: шаг `guard`/`decide` больше не считает `attempt`/`spent`
+целиком по прозе комментариев issue — рядом встаёт независимый счёт по
+опубликованным `docs/reviews/${marker}-${NUM}-r*.md` (через новый мелкий
+checkout `ref: dev` + `gh api contents`). Логика вынесена в чистые функции
+`scripts/review-doc-guard.mjs` (`reviewRoundsFromFiles`, `attemptFromRounds`,
+`verdictDeclaration`, `blockingFromDocs`, `reviewCounters`, CLI `--counters`),
+покрыта `test/review-doc-guard.test.mjs`, три новых мутанта в
+`scripts/mutation-gate.mjs`. `docs/specs/README.md` — строка индекса.
+Продуктовый код (`src/**`), i18n, бандл, touch — не задеты.
+
+## Как проверялось — гейты
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Typecheck | `npx tsc --noEmit` | чисто, без вывода |
+| Unit-тесты | `npm test` | 1936 pass / 0 fail / 1 skip |
+| Build | `npm run build` | сборка прошла (`rollup -c`), см. отдельное наблюдение о фингерпринте ниже — не относится к диффу #454 |
+| Mutation gate | `node scripts/mutation-gate.mjs --check` | `EXIT=0`, все мутанты (включая три новых `review-round-*`) поймали своё |
+| check-docs | `node scripts/check-docs.mjs` | passed (7 files, 12 links) — не обязателен для этого диффа (`src/**` не тронут), прогнан для полноты |
+
+Не гоняю: `golden:verify`, `smoke_*.mjs`, `pytest tests_backend`, performance —
+диапазон не касается рендера, геометрии, бэкенда или perf-путей. Инварианты
+модели (`npm run invariants`) не нужны — ни ребро, ни `layout`, ни
+`marker.space` не затронуты.
+
+## Находки
+
+### M1 (Medium, в скоупе задачи) — страховка по комментариям может завысить и `attempt`, и `spent` через посторонний маркер в прозе; заявление «перерасчёт невозможен по построению» не выполняется на реальных данных
+
+**Воспроизведено исполнением на боевых данных #454, не на синтетике.**
+
+Guard этого самого прогона (job `101105199249`, run `33898021033`,
+`gh api repos/Matysh/houseplan-card/actions/jobs/101105199249/logs`)
+напечатал:
+
+```
+этап code, заход 3, блокирующих циклов 0 из 4
+```
+
+при том, что по таймлайну меток это первый код-ревью раунд #454 (должно быть
+`заход 1`). Причина — старая (пока не заменённая, `process.yml` живёт на
+`main`, куда #454 ещё не смёржен) страховка по комментариям:
+
+```sh
+of_stage="[.comments[] | select(.body | test(\"Вердикт:\")) | select(.body | test(\"CODE-REVIEW\"))]"
+```
+
+Прогон дословно этим выражением на реальных комментариях #454
+(`gh issue view 454 --json comments`, `jq`) даёт `of_stage.length = 2`:
+
+- `#issuecomment-5543499255` (16:29:05) — зелёный вердикт **SPEC-REVIEW** r2.
+  Матчит `Вердикт:` (свой) и `CODE-REVIEW` — потому что текст вердикта
+  разбирает историю #449 и содержит фразу «маркер `CODE-REVIEW` в нём есть,
+  но в прозе». Это не отсылка к собственному файлу, а цитата чужого маркера
+  внутри анализа самой этой задачи.
+- `#issuecomment-5543835259` (16:57:54) — комментарий «Реализация готова»,
+  вообще не вердикт. Матчит `Вердикт:` — потому что в тексте есть фраза
+  «строки `Вердикт:` в нём нет» (описание AC2 в обратных кавычках), и
+  `CODE-REVIEW` — потому что в тексте разбираются найденные баги на
+  `CODE-REVIEW-441-r1.md`/`CODE-REVIEW-230-r2.md`/«CODE-REVIEW 439».
+
+Оба совпадения — ложные: ни один из двух комментариев не является
+код-ревью-вердиктом #454. `attempt = 2 + 1 = 3`, `spent = 0` (ни один из двух
+не «жёлтый/красный» в своей строке) — совпадает с логом guard buкв в букву.
+
+**Тот же результат получен НОВЫМ кодом, который и рецензируется**, не только
+старым shell (проверено исполнением на `HEAD`):
+
+```
+$ node -e 'import("./scripts/review-doc-guard.mjs").then(m => console.log(
+    m.reviewCounters({ rounds: [], docs: [], comments: { attempt: 3, spent: 0 } })))'
+{
+  attempt: 3, spent: 0,
+  attemptFiles: 1, spentFiles: 0,
+  attemptComments: 3, spentComments: 0,
+  blocking: [], unread: []
+}
+```
+
+`rounds: []` — потому что ни одного `CODE-REVIEW-454-r*.md` ещё не
+опубликовано (проверено `git ls-tree -r HEAD --name-only | grep 454` — есть
+только два `SPEC-REVIEW-454-*`). Файловая половина эту задачу считает
+абсолютно верно (`attemptFiles=1`). Ломает её `Math.max` со второй, не
+переписываемой этим ТЗ половиной.
+
+**Почему это не «синтетический край», а найденный вживую пример класса #89.**
+ТЗ и AC5 формулируют защиту так: «Вердикт чужого этапа не влияет на счёт
+(#89 не сломан)» — без оговорки. По факту это верно только для
+`reviewRoundsFromFiles`/`attemptFromRounds` (доказано юнитом
+«чужой этап и чужая задача в счёт не идут», `test/review-doc-guard.test.mjs:300`)
+и НЕ верно для итогового `reviewCounters`, потому что `attemptComments`
+считается прежним, недоказанным способом и участвует в `max()` наравне с
+надёжным источником. В параграфе §5 самого ТЗ это фактически признано
+оговоркой «по файлам» («Правило… по файлам оно выполняется строго»), но
+таблица AC5 эту оговорку не несёт, а раздел §3 идёт дальше и заявляет
+«перерасчёт невозможен по построению» — это неверно в общем виде: невозможно
+удвоение счёта (сумма против максимума), но не невозможно завышение одной из
+компонент максимума, и именно это сейчас произошло.
+
+**Чем краснеет сильнее, чем в этом раунде.** В этом прогоне `spentComments`
+совпал с `spentFiles` (оба 0) только потому, что заражающий SPEC-REVIEW-вердикт
+был зелёным. Формула `blocking` смотрит на цвет строки вердикта самого
+комментария, а не на то, кому вердикт принадлежит: будь заражающий комментарий
+жёлтым/красным вердиктом СВОЕГО (не CODE-REVIEW) этапа, `spentComments` вырос
+бы, `max(spentFiles, spentComments)` унаследовал бы это, и код-ревью потерял
+бы цикл бюджета §4 раньше, чем реально стартовал, — тот самый вред, ради
+которого заведено #89 и который AC5 обещает не допустить.
+
+**Практический эффект сегодня** — не потеря артефакта (`docs/reviews/` пуст
+для `CODE-REVIEW-454-*`, коллизии нет) и не потеря бюджета (`spent` верен),
+а искажённое имя файла: шаг публикации назовёт этот документ
+`CODE-REVIEW-454-r3.md`, оставив `r1`/`r2` навсегда пропущенными для этой
+задачи. Не деструктивно, но прямо противоречит зафиксированному в ТЗ «После:
+каждый заход получает собственное имя файла» — здесь заход получает чужое.
+
+**Почему Medium, а не High.** Первичный вред, ради которого заведено #454 —
+уничтожение предыдущего артефакта ревью (#220/#365-класс) — этим случаем не
+демонстрируется: коллизии имени нет, `spent` не искажён. Демонстрируется
+недоказанность одного конкретного защитного заявления (AC5/§3) в комбинации
+источников, а не поломка первичной гарантии. Это чинится в текущей задаче
+(в скоупе — счёт раундов ровно то, чем #454 занимается), не отдельным issue.
+
+**Предлагаемое направление (не мандат, выбор за автором):**
+1. Ужесточить страховку по комментариям: матчить не голый `CODE-REVIEW`, а
+   что-то вроде `CODE-REVIEW-<NUM>-r` — привязка к номеру issue резко снижает
+   шанс случайного совпадения в прозе, оставаясь той же «прозой», от которой
+   ТЗ явно отказывается полагаться как на основной источник, но убирает
+   наиболее грубый класс ложных срабатываний из резервного источника;
+   и/или
+2. Добавить видимость расхождения: `::warning::`, когда
+   `attemptComments > attemptFiles + 1` без объясняющего файла — сегодня
+   расхождение тихое, лог `guard` печатает только итоговое число;
+   и/или
+3. Смягчить формулировку «перерасчёт невозможен по построению» в
+   `docs/specs/454-review-round-counter.md` §3 и в комментарии `process.yml`
+   до того, что реально доказано («не даёт двойного счёта одного раунда»,
+   а не «не может завысить»), и закрепить тестом текущее (несовершенное)
+   поведение вместо документирования отсутствующей гарантии.
+
+Любое из трёх снимает находку; я не настаиваю на конкретной реализации.
+
+## Наблюдения — не находки, не блокируют
+
+- **Бандл `custom_components/houseplan/frontend/houseplan-card.js` уже устарел
+  относительно фингерпринта исходников `dev`** (проверено: свежий
+  `npm run build` даёт другой `__HOUSEPLAN_BUILD_FINGERPRINT__`, чем
+  закоммиченный). Причина найдена: коммит `28a4cb5e` (issue #455, уже в
+  `dev`) поменял `package.json` — входит в `BUILD_INPUTS`
+  `scripts/source-fingerprint.mjs` — без последующего `bundle:sync`. К #454
+  отношения не имеет (диф #454 не трогает `src/**`/`package.json`/`dist/**`),
+  предсуществует на `dev` независимо от этой задачи, и штатный механизм
+  (`rollup.config.mjs`: несовпавший фингерпринт обязан «fail closed» перед
+  golden/perf-съёмкой) рассчитан ровно на такой разрыв. Не завожу отдельный
+  issue: не деструктивно, самоисправится ближайшим `bundle:sync` перед любой
+  задачей, трогающей рендер/perf. Владельцу стоит знать на случай, если
+  ближайшая golden-задача упрётся в неожиданный отказ.
+- `docs/specs/README.md`: строка `#454` вставлена перед `#440`/`#447` —
+  таблица в остальном идёт по возрастанию номера issue. Косметика, не правлю
+  находкой.
+
+## AC — таблица доказательств
+
+| AC | Критерий | Доказано | Комментарий |
+|---|---|---|---|
+| AC1 | `attempt` = max(номер)+1, не count+1 | unit, чтением | `test/review-doc-guard.test.mjs:288` — дыра `[1,3]→4` проверена |
+| AC2 | «#449 как есть» → `attempt=3, spent=1` | unit на слепке реальных данных | перепроверено мной независимым `gh` снимком #449 — числа совпадают |
+| AC2b | Реконструкция r1/r2/r3 → `attempt=4, spent=2` | unit | пройден |
+| AC3 | Имя файла никогда не повторяется | unit + мутант `review-round-counts-files-not-max` | мутант краснеет корректно (проверено `--check`) |
+| AC4 | Зелёный вердикт цикл не тратит | unit | `test/review-doc-guard.test.mjs` «зелёный вердикт цикла не тратит» |
+| AC5 | Вердикт чужого этапа не влияет на счёт (#89) | unit — **только для `reviewRoundsFromFiles`** | **см. M1**: не доказано для `reviewCounters` в целом; комментарийная половина не покрыта тестом на это утверждение и на реальных данных нарушается |
+| AC6 | Отказ публикации не занижает счёт | unit + мутант `review-round-drops-comment-insurance` | пройден |
+| AC7 | Нет ветки/API недоступен → поведение как раньше | unit | пройден, включая «мусор на входе» |
+| AC8 | `process.yml` идентичен в `main` и `dev` | существующий шаг Validate | сейчас идентичны (проверено `diff`); автор откладывает зеркалирование в `main` до после мержа — обоснованно, гейт сравнивает ветки между собой и покраснел бы раньше времени |
+| AC9 | Логика в тестируемом модуле, не в inline-shell | unit + чтение | `test/review-doc-guard.test.mjs` проверяет сам текст шага `decide` на вызов `node scripts/review-doc-guard.mjs --counters` |
+
+## Что проверено и корректно
+
+- Файловая половина счёта (`reviewRoundsFromFiles`, `attemptFromRounds`,
+  `verdictDeclaration`, `blockingFromDocs`) — корректна, включая защиту от
+  цитирования чужого вердикта («описание чужого раунда не объявляет вердикт»,
+  коммит `4b443d8c`, добавлен по итогам находки самим автором на корпусе 629
+  документов) и от блока внутри тройных кавычек не по месту.
+- Три новых мутанта реально ловятся штатным раннером (`--check`, exit 0).
+- Трейлеры всех 7 коммитов диапазона: `Issue: #454`, `User-Visible: no` —
+  корректно для класса B, changelog не требуется.
+- `process.yml` `main`↔`dev` совпадают сегодня; план зеркалирования после
+  мержа обоснован и не является дефектом.
+- Гейты (typecheck/test/build/mutation/check-docs) зелёные на `HEAD`.
+
+## Чего не проверял
+
+- `golden:verify`, `smoke_*.mjs`, `pytest tests_backend`, performance-профили,
+  инварианты модели — diff их не касается.
+- Реальный прогон `guard` С НОВЫМ кодом на живом GitHub Actions (сам workflow
+  ещё не в `main`/`dev`) — логика проверена юнитами и прямым вызовом
+  `reviewCounters`/`--counters` на данных, снятых с реального API, но не в
+  контексте самого `job` целиком (шаг разрешения ветки через
+  `git ls-remote`/`gh api matching-refs`, постраничный лимит 1000 файлов) —
+  эта часть осталась shell-кодом без юнитов по прямому требованию ТЗ
+  («guard остаётся лёгким», §4 ТЗ) и проверена только чтением.
+- Не пересчитывал вручную мутацию `review-round-drops-file-source` построчно
+  за пределами штатного раннера — доверился `--check`.
+
+## Вердикт
+
+Один Medium в скоупе, без High. Возврат автору тем же issue (#454), не
+отдельный issue.
+
+Документ: `docs/reviews/CODE-REVIEW-454-r3.md` (публикуется шагом конвейера;
+см. раздел «Заход vs факт» насчёт номера).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/454-review-round-counter`, коммит `634478a51077` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `f8467cb113cd3c09cf3a10235bb7034ee9121063`
+  ```
+  git log --all --format='%H %T' | grep f8467cb113cd
+  ```
+- ТЗ `docs/specs/454-review-round-counter.md`, блоб `b5af292101a495e4699d839397e837a3a2467c85`
+  ```
+  git log --all --find-object=b5af292101a495e4699d839397e837a3a2467c85 -- docs/specs/454-review-round-counter.md
+  ```