docs: review document for #624

Issue: #624
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-23 15:39:00 +00:00
parent 37879e0c82
commit 007efaee28
2 changed files with 98 additions and 1 deletions
+4 -1
View File
@@ -1,6 +1,6 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1000, issue: 347. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1003, issue: 348. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|---|---|---|---|---:|---:|---|---|
@@ -11,10 +11,13 @@
| #636 | [CODE-REVIEW-636-r1.md](CODE-REVIEW-636-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #635 | [CODE-REVIEW-635-r1.md](CODE-REVIEW-635-r1.md) | code · r1 | 🟡 жёлтый | 1 | 0 | индекс молчаливо теряет находки и врёт числами по текущему | `docs/reviews/CODE-REVIEW-639-r1.md` `CODE-REVIEW-637-r1.md` `docs/reviews/CODE-REVIEW-594-r1.md` `docs/LESSONS.md` |
| #635 | [CODE-REVIEW-635-r2.md](CODE-REVIEW-635-r2.md) | code · r2 | 🟡 жёлтый | 1 | 1 | docs/reviews/INDEX.md, зафиксированный в материале ревью, устарел на собственном SHA — …; parseFindings/parseFiles: фолбэк «первая строка тела блока» вырезает начало буллета и п… | `docs/reviews/INDEX.md` `SPEC-REVIEW-625-r1.md` `SPEC-REVIEW-625-r2.md` `CODE-REVIEW-625-r1.md` `CODE-REVIEW-625-r2.md` `process.yml` `test/reviews-index.test.mjs` `INDEX.md` |
| #635 | [CODE-REVIEW-635-r3.md](CODE-REVIEW-635-r3.md) | code · r3 | 🟢 зелёный | 0 | 0 | firstParagraph: ветка нет\b в фильтре мёртвая из-за ASCII-only \b в JS-регэкспах, расхо… | `scripts/reviews-index.mjs` |
| #625 | [SPEC-REVIEW-625-r1.md](SPEC-REVIEW-625-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | новый инвариант markers[].id не описывает исход для уже испорченной хранимой конфигурации; продуктовые формулировки §7.1 неполны; не проговорены явные «нет» по i18n/touch | `validation.py` `__init__.py` |
| #625 | [SPEC-REVIEW-625-r2.md](SPEC-REVIEW-625-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
| #625 | [CODE-REVIEW-625-r1.md](CODE-REVIEW-625-r1.md) | code · r1 | 🟡 жёлтый | 0 | 0 | Три из четырёх точек вызова validate_active_marker_ids не имеют ни одного теста, exerci…; AC5 текстуально обещает «отдельные тесты сохраняют поведение при отсутствующем length» …; store.py:async_save_config_state — controller.async_flush() и последующий controller.re… | `custom_components/houseplan/websocket_api.py` `test_validation.py` |
| #625 | [CODE-REVIEW-625-r2.md](CODE-REVIEW-625-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |
| #624 | [SPEC-REVIEW-624-r1.md](SPEC-REVIEW-624-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | AC1-b: «чем краснеет» не соответствует заявленному свидетелю | `scripts/gate-small.mjs` `scripts/bundle-budget.mjs` `scripts/monolith-baseline.json` |
| #624 | [SPEC-REVIEW-624-r2.md](SPEC-REVIEW-624-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
| #621 | [CODE-REVIEW-621-r1.md](CODE-REVIEW-621-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #619 | [CODE-REVIEW-619-r1.md](CODE-REVIEW-619-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #614 | [SPEC-REVIEW-614-r1.md](SPEC-REVIEW-614-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
+94
View File
@@ -0,0 +1,94 @@
# SPEC-REVIEW-624-r2
**Issue:** [#624](https://github.com/Matysh/houseplan-card/issues/624) — «Монолит после выноса рантайма: 36 мёртвых дублей объявлений, 14 дублей типов, порт из 343 членов и 297 делегатов — метрика связности вместо «декомпозиции»»
**Этап:** ТЗ на ревью (S4-spec-review) · заход r2 · трек: полный (не изменился — сложность 5, два модуля, аналитик уже отверг лёгкий трек в S2-analysis)
**Материал:** тело issue #624, редакция r2 (правка автора `Matysh`, комментарий 2026-09-23T15:33:41Z, «Возвращаю `S4-spec-review`»)
**Предыдущий раунд:** `docs/reviews/SPEC-REVIEW-624-r1.md` — жёлтый, High 0, Medium 1 (AC1-b). Материал r1 зафиксирован блоком «Материал раунда»: ветка `dev`, коммит `f234855e66ee7baba528b7c856c5ab79e9491156`, дерево материала `703233aee85cb3c3cbf101b84e288ec9dba3ce34`, тело issue — блоб `d6eced31cfbe6c961c0420b8961747ac902aae4eb680b2744fcd20e6b79f84ed`.
## Скоуп ревью (r2, по дельте — PROCESS.md §2.10)
Предмет раунда — дельта тела issue между r1 и r2, а не задача целиком. Автор объявил дельту явно в комментарии-ответе: правка коснулась скоупа п.2–3 (пять чисел базы вместо четырёх, место гейта после сборки, единый модуль `scripts/monolith-metrics.mjs`), строки AC1-b таблицы AC, плана автотестов, одного риска (пины toolchain для `bundle-bytes`) и одного допущения («сырой размер, не gzip»).
Дельта признана локальной, разбор сокращён до неё плюс AC, которых она касается:
- не меняет продуктовую рамку — «Сценарий», «Проблема», «Контракт поведения», «Не-скоуп», «Откат», «Release-артефакты» текстуально не редактировались (сверено построчно с текущим текстом issue);
- не меняет контракт поведения: `User-Visible: no` остался, «ни одна персона ничего не увидит» не оспаривается правкой;
- не задевает новую подсистему — правка целиком внутри той же измерительной инфраструктуры (`scripts/monolith-baseline.json`, гейт `lint:unused`), которую вводит эта же задача;
- по объёму сопоставима с находкой r1, которую закрывает (одна строка AC1-b + симметричные правки скоупа/автотестов/рисков под неё), не с исходной задачей.
Повторно проверялись только AC1-b (предмет находки r1) и AC2 (её формулировка тоже перешла с «четырёх» на «пять» чисел и опирается на тот же модуль). AC1, AC4, AC5, продуктовая рамка, модель данных/i18n/UX, откат, release-артефакты — унаследованы без повторной проверки (раздел ниже).
## Как проверялось
- Прочитан вердикт и материал r1 (`docs/reviews/SPEC-REVIEW-624-r1.md`) целиком.
- Прямого `git diff` по телу issue нет — issue не файл в дереве репозитория (материал спек-ревью, PROCESS.md §2.4). Дельта установлена сопоставлением дословных цитат ТЗ из текста r1-находки (раздел «Находки», подраздел «Формулировка ТЗ» и цитаты скоупа) с текущим текстом тела issue #624, полученным через `gh issue view 624 --json body`.
- Построчно перечитаны все разделы, которые автор назвал изменёнными: Скоуп п.2 (текст гейта `unused-locals-gate.mjs`), п.3 (состав пяти метрик), таблица AC — строки AC1-b и AC2, «План автотестов», «Риски» (пункт про пины toolchain #496), «Принято предположительно» (допущение про `bundle-bytes`/gzip).
- Проверена внутренняя непротиворечивость правки по всему телу issue: единственное упоминание `bundle-sync` в тексте — в новой строке AC1-b, и оно теперь помечено honestly как «равенство копий, не рост» (не как свидетель роста); других мест, где `bundle-sync`/`bundle:budget` подписаны как «чем краснеет» для AC1-b, в тексте не осталось.
- Сверена согласованность счётчика «пять чисел» по всем упоминаниям: скоуп п.2 («считает пять чисел... падает, если любое выросло»), п.3 («База пяти чисел»), AC1 («в хендоффе — вывод с пятью числами»), AC2 («печатает пять чисел; ... содержит те же пять»), план автотестов («пять счётчиков на фикстурном исходнике и файле-бандле») — везде одно и то же число и один и тот же источник (`scripts/monolith-metrics.mjs` → гейт и `npm run inventory`), «одно число — один источник» не нарушено.
- Дешёвые гейты не гонялись: задача ещё не в коде (тот же довод, что в r1 — `scripts/unused-locals-gate.mjs`, `scripts/monolith-metrics.mjs`, `scripts/monolith-baseline.json`, все три новых теста не существуют), ревью ТЗ их прогона не требует и не может прогнать несуществующий код.
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| Medium — AC1-b: «чем краснеет» (CI `bundle-sync`; budget-тест) не проверяет заявленный контракт «рост — отказ»: `bundle-sync` красит только рассинхрон трёх копий, `bundle:budget` сравнивает с абсолютным gzip-потолком, а не с состоянием до правки; фактическое доказательство было ручным (`stat -c %s` в хендоффе), но подписано как автоматическое | Выбран вариант 2 из двух предложенных ревьюером r1: `bundle-bytes` (сырой размер `dist/houseplan-card.js` после `npm run build`) стал пятым отслеживаемым числом в `scripts/monolith-baseline.json`; гейт `lint:unused` сравнивает его с базой строгим `>` и падает при любом росте — симметрично `delegates`/`port-members`/`host-refs`/`port-privates`. Упоминание `bundle-sync` в строке AC1-b переформулировано честно: «три копии — `bundle-sync` (равенство копий, не рост)» — больше не выдаётся за свидетеля роста | Тело issue #624, ТЗ: строка `AC1-b` таблицы «AC — чем доказан — чем краснеет» (мутант `monolith-metrics-baseline-strict`: `>` → `>=`/пропуск `bundle-bytes` из сравнения → фикстурный тест «рост на 1 байт — красный» падает); скоуп п.2 («Затем гейт считает пять чисел (п. 3) и падает, если любое выросло»); скоуп п.3 («bundle-bytes (размер dist/houseplan-card.js в байтах после npm run build)», «База пяти чисел»); план автотестов (`test/monolith-metrics.test.mjs` — «пять счётчиков... рост любого числа на 1 — красный»); риски (новый пункт про пины toolchain для воспроизводимости байтового размера, `#496`) |
Находка закрыта полностью: требование ревьюера («добавить пятое отслеживаемое число... тогда реальный ratchet-гейт действительно закраснеет на росте») выполнено буквально, а не просто переформулировано.
## Находки
Нет. High: 0, Medium: 0.
Дельта не вводит новой неоднозначности: строка AC1-b теперь по формату идентична AC2/AC4 (называет мутант, который снимает именно заявленную защиту), число совпадает во всех пяти местах, где оно упоминается, гейт технически размещён там же, где и остальные четыре метрики (`gate:small`, после `npm run build`), допущение «сырой размер, не gzip» — техническое и явно отделено от `bundle:budget` (gzip-бюджет), риск ложноположительного срабатывания от версии toolchain назван и снят ссылкой на существующую инфраструктуру пинов (#496).
## Что проверено и признано корректным (дельта r2)
- AC1-b больше не подписывает `bundle-sync`/`bundle:budget` как «чем краснеет» для роста бандла — единственный оставшийся автоматический свидетель роста — фикстурный тест гейта на мутанте `monolith-metrics-baseline-strict`, что соответствует формату остальных защищённых AC этой задачи.
- AC2 согласована с AC1-b: обе ссылаются на одни и те же «пять чисел» одного модуля (`scripts/monolith-metrics.mjs`) и одну базу (`scripts/monolith-baseline.json`) — «одно число — один источник» не нарушено новой метрикой.
- Мутант `monolith-metrics-baseline-strict`, добавленный в «План автотестов», зарегистрирован в списке мутантов реестра (§2.7) наравне с `unused-gate-allows-everything` и `monolith-anchors-length-only` — не забыт как «висячая» ссылка только в таблице AC.
- Новый риск (пины toolchain для rollup/Node влияют на байтовый размер) снят ссылкой на уже существующую инфраструктуру: единые пины для CI и `gate:small` (issue #496), с явным указанием, что решающий прогон — CI/Validate, а локальное расхождение фиксируется в хендоффе, а не заводит ложный отказ.
- Допущение «`bundle-bytes` — сырой размер, не gzip» в разделе «Принято предположительно» корректно отделяет новую метрику от `bundle:budget` (который остаётся gzip-бюджетом) — не создаёт двух источников истины для одной сущности «размер бандла», они измеряют разное (raw ratchet vs. gzip potolok) и это названо явно.
- Дельта не тронула продуктовую рамку, поэтому вывод r1 «ни одна персона ничего не увидит» и соответствие `docs/SCOPE.md` остаются в силе без повторной проверки (см. «Унаследовано из r1»).
## Унаследовано из r1
Без повторной проверки, по документу `docs/reviews/SPEC-REVIEW-624-r1.md`, материал зафиксирован на SHA `f234855e66ee7baba528b7c856c5ab79e9491156` / дерево `703233aee85cb3c3cbf101b84e288ec9dba3ce34`:
- Наличие всех обязательных разделов §7.1 (сценарий и «что человек увидит» первыми, проблема, скоуп/не-скоуп, контракт поведения, модель данных/миграция/i18n/UX, AC с доказательством, план автотестов, риски, откат, release-артефакты) — дельта r2 не убрала и не добавила разделов.
- Соответствие `docs/SCOPE.md`: задача инструментальная (`tech-debt`), не обещает пользователю ничего нового, косвенно обслуживает J6 («Keep the plan true as the home evolves»), прямого конфликта нет.
- Заявление «ни одна персона ничего не увидит» — проверено и подтверждено в r1, дельта r2 (метрика бандла, тексты гейта) не расширяет видимую поверхность.
- Количественные факты ТЗ (26 486 строк, 307 вызовов `_editorRuntimeOrThrow()`, 1 107 `private`-членов, `HouseplanEditorHostPort`, ~757/768 ошибок `--noUnusedLocals`) — сверены в r1 с деревом материала и подтверждены точно/по порядку величины; дельта r2 их не касается.
- Мутанты AC1 (`unused-gate-allows-everything`), AC2 (подмена определения делегата), AC4 (`monolith-anchors-length-only`) — признаны осмысленными в r1, дельта их не меняла.
- Нумерация AC (без AC3, зарезервирован для отдельного будущего issue про первый вынос) — объяснена и не является дефектом.
- Единственный открытый технический вопрос («host-refs через `\bhost\.`-grep — грубо, ревьюер кода может предложить AST») — технический, явно передан на усмотрение код-ревью, не требует эскалации владельцу; дельта r2 не добавила и не сняла этот вопрос.
- Правка PROCESS.md §2.7 (новый абзац про монолит в скоупе, п.4) не требует отдельной эскалации: скоуп зафиксирован владельцем репозитория в S2-analysis.
- Откат (`git revert` одним коммитом, база чисел в JSON возвращается автоматически) и release-артефакты (`User-Visible: no`, бандл пересобирается тем же коммитом) — достаточны для DoR (§2.5); дельта пятого числа базы не меняет механику отката.
## Чего не проверял
- Не запускал и не мог запустить `npm run lint:unused`, `test/unused-locals-gate.test.mjs`, `test/monolith-metrics.test.mjs`, `test/monolith-text-anchors.test.mjs` — они не существуют, задача не реализована; предмет код-ревью.
- Не проверял тяжёлые/browser-гейты (golden, smoke, performance) — на этапе ТЗ не требуются.
- Не переоценивал точное число «мёртвых» деклараций (36/14/~12) — не критерий приёмки, эта оценка не входила в дельту r2 и не проверялась повторно (наследуется из r1).
- Не проверял независимо огрубление `host-refs` через regex — не входило в дельту r2, открытый технический вопрос унаследован как есть.
- Не проверял техническую реализуемость строгого `>`-сравнения `bundle-bytes` в неопубликованном коде гейта (скрипта не существует) — на этапе ТЗ достаточно, что формулировка однозначна и симметрична остальным четырём метрикам; фактическая работоспособность — предмет код-ревью («тест умеет падать»).
## Вердикт
Находка r1 закрыта буквально выбранным автором вариантом (пятое число базы, честная переформулировка `bundle-sync`). Дельта r2 не вносит новых High/Medium/Low находок: формулировка AC1-b теперь однозначна, доказуема автотестом с осмысленным мутантом, согласована со всеми остальными упоминаниями «пяти чисел» в тексте, «одно число — один источник» не нарушено. Продуктовых вопросов дельта не поднимает («ни одна персона ничего не увидит» не оспорено). ТЗ готово к переходу в DoR.
По §7.2 зелёный вердикт бюджет циклов не тратит и цикла не образует (#227).
**Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0 · Документ: (путь к этому файлу в docs/reviews проставит шаг публикации)**
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `37879e0c824d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `054f65bc2ea720f55b1a0f3934899e4a1e8a611a`
```
git log --all --format='%H %T' | grep 054f65bc2ea7
```
- Тело issue: `177d70b46dd21fd7f6fcd040b8ada864c3fcfacc214730d15d7a58be1b8025a5`
- Вердикт конвейера: `green` · High 0