From 007efaee2854aa8b9b4a063db8e037ecfc5d3f98 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 15:39:00 +0000 Subject: [PATCH] docs: review document for #624 Issue: #624 User-Visible: no --- docs/reviews/INDEX.md | 5 +- docs/reviews/SPEC-REVIEW-624-r2.md | 94 ++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-624-r2.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 6107a93b..6bb6ff2d 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -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 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-624-r2.md b/docs/reviews/SPEC-REVIEW-624-r2.md new file mode 100644 index 00000000..83c3e20f --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-624-r2.md @@ -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 проставит шаг публикации)** + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `37879e0c824d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `054f65bc2ea720f55b1a0f3934899e4a1e8a611a` + ``` + git log --all --format='%H %T' | grep 054f65bc2ea7 + ``` +- Тело issue: `177d70b46dd21fd7f6fcd040b8ada864c3fcfacc214730d15d7a58be1b8025a5` +- Вердикт конвейера: `green` · High 0