mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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 не заводится.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/553-process-doc-canonicalization`, коммит `55db3d49866a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `4f252f3b62eec7f0ba28f6c1bcfe1701719d9d0d`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 4f252f3b62ee
|
||||
```
|
||||
- Тело issue: `df64e92bada59856e24d29c886d58c24ba0907508dc476bd206b0e3b1466034d`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user