diff --git a/docs/reviews/CODE-REVIEW-553-r2.md b/docs/reviews/CODE-REVIEW-553-r2.md new file mode 100644 index 00000000..c6fe4e89 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-553-r2.md @@ -0,0 +1,197 @@ +# CODE-REVIEW-553-r2 + +Issue: #553 · заход r2 · блокирующих циклов израсходовано 1 из 4 +Материал: `55db3d49866adcc35a231fde5a91692806980c97` (ветка `issue/553-process-doc-canonicalization`, приведена к `dev` конвейером — сверху легло 5 коммитов dev, включая `#551`; предыдущий заход рецензировал `39ce42eb`, который в этом дереве больше не разрешается — это ожидаемое следствие ребейза, не потеря материала) + +## Почему разбор полный, а не по дельте + +Ветка ребейзнута поверх ушедшего вперёд `dev` (39ce42eb → 55db3d49, 5 коммитов dev, +включая правки конвейера ревью #551). По §7.2 после ребейза это другой код — +разбор ведётся полностью, а не по дельте относительно r1. + +## Закрытие раунда r1 + +Вердикт r1: жёлтый · High 0 · Medium 1 (в скоупе). Материал r1: `39ce42eb`. + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Регрессионный тест `#553` в `test/review-doc-guard.test.mjs` (`assert.doesNotMatch(process, /ревью[^\n]{0,80}заменяет тестирование/i)`) не ловит возврат фразы «код-ревью … заменяет тестирование» в `PROCESS.md`, если она реинтродуцируется с тем же переносом строки, что был в исходном тексте — класс `[^\n]` не пересекает `\n`, а «Код-ревью не пропускается» и «заменяет тестирование» в оригинале стояли на разных строках | **Не закрыта.** Содержимое `test/review-doc-guard.test.mjs` в `55db3d49` идентично содержимому, которое рецензировалось на `39ce42eb` (переезд — чистый ребейз, конфликтов в файле не было) | `git show 55db3d49 -- test/review-doc-guard.test.mjs`: регэкс на строке с `#553`-тестом дословно тот же, что был на r1. Экспериментально воспроизвёл проверку r1 на текущем SHA: подменил абзац §5 `PROCESS.md` дословно на оригинальную (дореформенную) формулировку с тем же переносом строки — «Код-ревью не пропускается\nникогда — именно оно в этом процессе заменяет тестирование.» — и перезапустил `node --test test/review-doc-guard.test.mjs`: тест `#553` остался зелёным (`ok 56`, `1..58 / pass 58`). Рабочая копия восстановлена из `git diff`/`git status` (чисто) сразу после эксперимента | + +Находка воспроизводится один в один на новом SHA — рабочая формулировка после +правки (`Код-ревью не пропускается:\nоно проверяет скоуп, риски и качество +доказательств, но не заменяет исполнение\nтестов.`) действительно не содержит +больше исходной фразы, поэтому *сейчас* документ корректен. Но проверка не +защищает от отката: `git revert` этого коммита или ручной откат абзаца к +дореформенной редакции пройдёт мимо теста незамеченным — то есть ровно тот +сценарий регресса, ради которого тест и заводился (issue прямо требует +«ясный checklist», а не декларацию). Это тот же Medium, не новый: правка не +задевала эту строку теста между r1 и r2, только рёбра ребейза. + +## Унаследовано из r1 + +Ничего, кроме списка выше, не наследуется без проверки — разбор в этом заходе +полный (см. «Почему разбор полный» выше), все AC и находки перепроверены на +`55db3d49` заново, а не по ссылке на CODE-REVIEW-553-r1. + +## Скоуп + +Issue #553 — инфраструктурно-документационная задача (класс C: `PROCESS.md`, +`AGENTS.md`, `docs/STATUS.md`; класс B: `test/review-doc-guard.test.mjs`). +Продуктовый код (класс A, `src/**`) не тронут — весь diff это `git diff +origin/dev...HEAD` = один коммит `55db3d49`: + +``` +AGENTS.md | 10 ++++++---- +PROCESS.md | 43 ++++++++++++++++++++++++++++++------------ +docs/STATUS.md | 4 ++-- +test/review-doc-guard.test.mjs | 22 +++++++++++++++++++++ +4 files changed, 61 insertions(+), 18 deletions(-) +``` + +Трейлеры коммита: `Issue: #553`, `User-Visible: no` — корректно (документация +процесса, продукт не меняется; изменения changelog не требуются и не сделаны). + +## Как проверялось + +Материал закреплён на `55db3d49866adcc35a231fde5a91692806980c97` — рабочая +копия уже на нём, `git fetch`/`git pull`/`checkout` на другой коммит не +выполнялись. + +1. Прочитан текст issue #553 и все три комментария (постановка, вердикт r1, + сообщение о несостоявшейся публикации r1 — метка не переставлялась, + вердикт r1 не был применён автоматически, что и объясняет, почему это r2, а + не r1-исправление другого r1). +2. Построчно сверен весь diff (`git show 55db3d49` по каждому изменённому + файлу) с четырьмя противоречиями, перечисленными в теле issue, и с пятью + пунктами приёмки. +3. Экспериментально воспроизведена находка r1 на новом SHA (см. таблицу выше) — + рабочая копия патчилась и восстанавливалась, `git status`/`git diff` после + эксперимента чисты. +4. Прогнан целевой тест `node --test test/review-doc-guard.test.mjs` — 58/58, + включая эксперимент выше. +5. Проверено отсутствие маркеров незакрытого конфликта после ребейза: + `grep -rn '<<<<<<<|=======|>>>>>>>'` по репозиту — совпадений с реальными + конфликт-маркерами нет (только декоративные `===`-разделители в + комментариях смоков и упоминания в исторических ревью-документах). +6. Сверено, что Validate на точном SHA `55db3d49866adcc35a231fde5a91692806980c97` + зелёный: `gh run view 34755639163` → `headSha` совпадает, `conclusion: + success`. Это подтверждает `tsc --noEmit`, `npm test` (полностью, включая + файл из п.4) и `npm run build` на этом же SHA — не перегонял их отдельно, + ссылка привязана к точному SHA и подтверждена явно, а не принята на слово. + +## Проверка AC issue + +| AC (из тела issue) | Статус | Доказательство | +|---|---|---| +| Перечисленные 4 противоречия устранены | **Да**, кроме регрессозащиты одного из них (см. Medium ниже) | PROCESS.md: приоритет источников (repo/AGENTS/CODEX-RUNBOOK) в преамбуле; §2.6 добавляет риск-матрицу; §5 выход из `small` → «полное ТЗ в теле issue по §7.1»; docs/STATUS.md и AGENTS.md убрали точные pins (`Python 3.13`, `Node 22`/`Python 3.14`) в пользу отсылки на `toolchain:check` | +| Новые задачи не требуют archived spec-file | Да | §5 текст заменён, тест `#553` проверяет `doesNotMatch(process, /получает\s+нормальный файл ТЗ/i)` и `match(process, /полное ТЗ в теле issue по §7\.1/)` — оба проходят на `55db3d49` | +| Runtime/pins не дублируются в расходящихся справках; приоритет источников понятен | Да | AGENTS.md/STATUS.md переведены на «repository-pinned»/`toolchain:check`; преамбула PROCESS.md явно объявляет иерархию repo → AGENTS.md → CODEX-RUNBOOK | +| Формулировка о замене testing ревью удалена/исправлена; ясный checklist проверяет результат пользователя | Частично — **формулировка исправлена**, но регрессозащищающий тест не проверяет то, что заявляет (см. Medium) | PROCESS.md §2.7 и §5 текст изменён корректно (проверено чтением); риск-матрица §2.6 — новая, addresses «checklist проверяет результат пользователя»; тест-гвард на откат неполон | +| Independent reviewer, Rule №1, owner arbitration, authorization сохранены | Да | diff не трогает разделы §1, §9–§11, §12; grep по репо не находит новых упоминаний, отменяющих эти гарантии | +| История specs/reviews не переписана, скрытые фичи/changelog не менялись | Да | diff не касается `docs/specs/**`, `docs/reviews/**`, `docs/CHANGELOG*`; `git diff --stat` подтверждает это выше | + +## Находки + +### Medium (в скоупе задачи — жёлтый вердикт, возврат автору, не отдельный issue) + +**Регрессионный тест `#553` не защищает от отката формулировки, ради которого +заведён.** + +- Файл: `test/review-doc-guard.test.mjs`, тест `#553` (строка с + `assert.doesNotMatch(process, /ревью[^\n]{0,80}заменяет тестирование/i);`). +- Сценарий поломки: любой будущий `git revert` этого коммита, либо ручной откат + абзаца §5 `PROCESS.md` к дореформенной формулировке той же разбивкой строк — + «Код-ревью не пропускается\nникогда — именно оно в этом процессе заменяет + тестирование.» — молча пройдёт зелёным тестом `#553` (`node --test + test/review-doc-guard.test.mjs` останется 58/58), потому что `[^\n]{0,80}` не + пересекает перенос строки, а именно на этом переносе стоят слова «пропускается» + и «никогда» в исходной редакции. Воспроизведено экспериментально на текущем + SHA (см. «Закрытие раунда r1» выше). +- Контрольный факт: аналогичная защита для английского текста (`AGENTS.md`, + `assert.doesNotMatch(agents, /review[^\n]{0,80}stands in for testing/i)`) + работает правильно — потому что оригинальная английская фраза «review is + never skipped on either track; it is what stands in for testing.» умещалась в + одну строку. Проблема локальна для русского ассерта и его конкретной + формулировки, а не системная ошибка `doesNotMatch`. +- Почему это Medium, а не Low: это ровно тот регресс, который issue поручает + предотвратить постоянно действующим тестом («ясный checklist проверяет + результат пользователя»); тест сейчас доказывает состояние документа только + на момент написания, а не инвариант на будущее — то есть его защитная роль + фиктивна для этого конкретного абзаца. +- Что нужно для закрытия (не мой выбор реализации, просто фиксирую критерий): + ассерт должен ловить откат независимо от переноса строк — например, заменить + `[^\n]{0,80}` на `[\s\S]{0,80}` (или свернуть пробельные символы перед + матчингом) в этой одной строке теста. + +### Low + +**Стале-цитата правила в §11.4 после правки формулировки §5.** + +- Файл: `PROCESS.md`, строка ~1154: «Это исключение из правила «код-ревью не + пропускается никогда» (§5, §7.1)». +- До этой правки §5 действительно содержал именно эту фразу целиком («Код-ревью + не пропускается\nникогда — именно оно…»), поэтому кавычка в §11.4 была точной + цитатой. После правки §5 звучит как «Код-ревью не пропускается: оно проверяет + скоуп, риски и качество доказательств, но не заменяет исполнение тестов» — + слова «никогда» рядом с «не пропускается» там больше нет. Смысл (код-ревью не + пропускается, кроме единственного исключения §11.4) не нарушен, но кавычка в + §11.4 больше не является дословной цитатой действующего текста §5 — не + блокирует, самостоятельно не чинится этим ревью (не в перечне названных + противоречий issue), можно поправить в рамках этой же задачи одной строкой + или снять как несущественное — на усмотрение автора. + +## Что проверено и корректно + +- Все 4 названных в issue противоречия (замена тестирования, архивный + spec-file при выходе из `small`, `Python 3.13` в STATUS, точные pins в + AGENTS) устранены текстуально — построчно сверено с формулировками issue и + со ссылками на конкретный SHA `66a6485` в теле issue. +- Риск-матрица (§2.6: async/данные и права/геометрия/визуал/объём и + performance/host-input) размещена в разделе «В разработке» и корректно + переиспользована ссылкой в §2.7 («Ревьюер отдельно сверяет применимые + классы риска из §2.6») — не дублирует, а связывает разделы. +- Rule №1, WIP-лимиты, независимость ревьюера, арбитраж владельца, лимит + циклов, авторизация релиза — ни один раздел вне diff не задет. +- Продукт (`src/**`, `custom_components/**/*.py`), changelog, + `docs/specs/**`, `docs/reviews/**` не изменены — соответствует ограничению + «не переписывать историю». +- Ребейз чист: конфликт-маркеров нет, целевой тест 58/58 после ребейза, + Validate зелёный на точном SHA (подтверждено `gh run view`, `headSha` + совпадает). +- Трейлеры коммита корректны (`Issue: #553`, `User-Visible: no`); `changelog` + не требовался и не тронут. + +## Гейты — что прогнал, что нет и почему + +| Гейт | Прогнан? | Результат / причина | +|---|---|---| +| `npx tsc --noEmit` | Не перегонял отдельно | Покрыт зелёным Validate на точном SHA `55db3d49` (`gh run view 34755639163` → `headSha` совпадает, `success`); diff не содержит TS | +| `npm test` (полный) | Не перегонял отдельно | Покрыт тем же Validate; целевой файл из diff (`test/review-doc-guard.test.mjs`) прогнан отдельно, см. ниже | +| `node --test test/review-doc-guard.test.mjs` | Да | 58/58, включая воспроизведение находки r1 (эксперимент с откатом абзаца, копия восстановлена) | +| `npm run build` + сверка 3 копий бандла | Не перегонял | Покрыт Validate на точном SHA; diff не касается `src/**`/бандла | +| `node scripts/check-docs.mjs` | Не применим | diff не трогает `src/**` | +| `npm run invariants -- --config …` | Не применим | diff не трогает геометрию/`layout`/`marker.space`/`open_spans` | +| `python -m pytest tests_backend -q` | Не применим | diff не трогает `custom_components/**/*.py` | +| Браузерные смоки (`demo/smoke_*.mjs`) | Не применимы | diff не трогает `src/**`; `scripts/smoke-select.mjs` не запускал — нет исполняемой поверхности карточки в дельте | +| `npm run golden:verify` | Не применим | diff не меняет рендер/геометрию/стили | +| `git grep` конфликт-маркеров после ребейза | Да | 0 реальных совпадений (только декоративные `===` в комментариях и цитаты в старых ревью-доках) | + +## Вывод + +Медиан-находка r1 не закрыта — тот же дефект теста-гварда воспроизводится один +в один на новом SHA. Задача остаётся в скоупе Medium (не блокирует по High), +поэтому вердикт — жёлтый, возврат автору, отдельный issue не заводится. + +--- + + + +## Материал раунда + +- Ветка: `issue/553-process-doc-canonicalization`, коммит `55db3d49866a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `4f252f3b62eec7f0ba28f6c1bcfe1701719d9d0d` + ``` + git log --all --format='%H %T' | grep 4f252f3b62ee + ``` +- Тело issue: `df64e92bada59856e24d29c886d58c24ba0907508dc476bd206b0e3b1466034d` +- Вердикт конвейера: `yellow` · High 0