docs: review document for #624

Issue: #624
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-23 14:29:13 +00:00
parent fe790ca01f
commit 37879e0c82
+90
View File
@@ -0,0 +1,90 @@
# SPEC-REVIEW-624-r1
**Issue:** [#624](https://github.com/Matysh/houseplan-card/issues/624) — «Монолит после выноса рантайма: 36 мёртвых дублей объявлений, 14 дублей типов, порт из 343 членов и 297 делегатов — метрика связности вместо «декомпозиции»»
**Этап:** ТЗ на ревью (S4-spec-review) · заход r1 · трек: полный (аналитик явно назвал нарушенные критерии лёгкого трека: сложность 5, два модуля)
**Материал:** тело issue #624, раздел `## ТЗ`; комментарий `S2-analysis` от 2026-09-23 (автор — владелец репозитория, `Matysh`)
## Скоуп ревью
Проверялось ТЗ в теле issue #624 на:
1. наличие всех обязательных разделов §7.1 PROCESS.md;
2. однозначность и доказуемость каждого AC (включая непротиворечивость столбца «чем краредеет» его собственной формулировке);
3. отсутствие догадки, выданной за решённый факт (продуктовые и технические утверждения проверены отдельно);
4. соответствие `docs/SCOPE.md` — какую строку Core user jobs закрывает задача и не нарушает ли `docs/SCOPE.md`/PROCESS.md;
5. фактическую точность количественных утверждений ТЗ на текущем материале (не только правдоподобие).
Продуктовых разделов (USER-GUIDE.ru.md, канонические документы подсистем SUN/LIGHT/CANVAS/…) это ТЗ не касается по существу: задача явно объявляет «Ни одна персона ничего не увидит» — сверено ниже, утверждение подтверждается.
## Как проверялось
- Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1–§10.2), тело issue #624 целиком, комментарий аналитика.
- Факты ТЗ сверены с текущим деревом на SHA материала (`f234855e66ee7baba528b7c856c5ab79e9491156`):
- `wc -l src/houseplan-card.ts src/houseplan-editor-runtime.ts` → 13 731 + 12 755 = **26 486** — совпадает с числом в S2-analysis и в ТЗ буквально;
- `grep -c "_editorRuntimeOrThrow()" src/houseplan-card.ts` → **307** — совпадает;
- `grep -c "^\s*private " src/houseplan-card.ts` → **1 107** — совпадает;
- `interface HouseplanEditorHostPort` найден в `src/houseplan-editor-runtime.ts:830` — поверхность, о которой говорит ТЗ, существует;
- `npx tsc --noEmit --noUnusedLocals -p tsconfig.json` на живом дереве → **768** ошибок TS6133/TS6192 (в S2-analysis — «757» на другом SHA `dev`-ветки; расхождение на 11 ошибок между разными коммитами не искажает вывод ТЗ, порядок величины и структура факта — верны, не выдуманы).
- Проверено существование инфраструктуры, на которую ссылается ТЗ: `scripts/gate-small.mjs` (подтверждена связка build+typecheck / no-new-any / smoke-select → юниты → bundle-tree → bundle-budget, как описано в скоупе п.2), `scripts/check-inputs.mjs`, `scripts/inventory.mjs` (существует, метрик `delegates`/`port-members`/`host-refs`/`port-privates` там пока нет — задача действительно добавляет их, а не переизобретает существующее), `scripts/bundle-budget.mjs` (подтверждён абсолютный потолок `INITIAL_VIEW_GZIP_BUDGET`, а не сравнение с предыдущим значением).
- Дешёвые гейты не гонялись отдельно — задача ещё не в коде, ревью ТЗ гейтов не требует; `npx tsc --noUnusedLocals` прогнан целенаправленно как факт-чек самого ТЗ, а не как гейт задачи.
## Находки
### Medium (в скоупе, правится в этой же задаче) — AC1-b: «чем краснеет» не соответствует заявленному свидетелю
**Файл:** тело issue #624, раздел «AC — чем доказан — чем краснеет», строка AC1-b.
**Формулировка ТЗ:** *«AC1-b. Бандл не растёт» — доказано `npm run build`; в хендоффе — размеры трёх копий до/после (`stat -c %s`), три копии побайтово равны · чем краснеет: CI `bundle-sync`; budget-тест (`gate:small` → budget)*.
**Проблема:** ни один из названных автоматических свидетелей не проверяет заявленный контракт «рост — отказ» относительно состояния **до** этой задачи.
- `bundle-sync`/`bundle-tree` (подтверждено чтением `scripts/gate-small.mjs`, шаг «копии бандла совпадают») сравнивает три **синхронные копии друг с другом** (dist ↔ frontend ↔ demo/srv/assets) — он красен только при рассинхронизации копий, а не при росте размера как такового.
- `bundle:budget` (подтверждено чтением `scripts/bundle-budget.mjs`) сравнивает итоговый gzip с **абсолютным** потолком `INITIAL_VIEW_GZIP_BUDGET` (301 066 Б на текущем дереве), а не с размером до правки. Бандл может вырасти на любое число байт, оставаясь далеко под потолком, — оба названных гейта останутся зелёными, а заявление AC1-b при этом будет нарушено незамеченным автоматикой.
Фактическое доказательство «не растёт» в этом ТЗ — не автотест, а ручное сравнение двух чисел (`stat -c %s` до/после) в тексте хендоффа, то есть «проверено чтением, не исполнением» в терминах PROCESS §2.7. Это законный способ доказательства AC, но ТЗ подписывает его как «чем краснеет: CI …», создавая у ревьюера кода ложное впечатление автоматической защиты там, где её нет. Ревьюер кода, увидев зелёный `gate:small`, может засчитать AC1-b доказанным гейтом — и не заметить регресс в несколько сотен байт, если хендофф не содержит фактических чисел или числа не сверены.
**Сценарий отказа:** правка на кодовом этапе случайно оставляет один лишний тип/импорт непереименованным (например, тип-дубль перенесён, но старое объявление осталось как реэкспорт «для совместимости»); бандл вырастает на 300–500 Б, остаётся на 50 КБ ниже бюджета и байтово идентичен между тремя копиями. `bundle-sync` и `bundle:budget` зелёные, AC1-b формально «доказан заявленными свидетелями» — хотя контракт «рост — отказ» нарушен.
**Требуется:** привести формулировку в соответствие одному из двух вариантов (решение техническое, не продуктовое — не эскалируется владельцу):
1. либо честно пометить доказательство как «доказано чтением, не гейтом»: хендофф обязан явно печатать оба числа (до/после) и разницу, а ревьюер кода сверяет их сам — без ссылки на `bundle-sync`/`budget` как на «чем краснеет»;
2. либо добавить пятое отслеживаемое число (byte-size dist) в тот же `scripts/monolith-baseline.json`/гейт из п.2 скоупа — тогда «одно число — один источник» и реальный ratchet-гейт действительно закраснеет на росте, симметрично `delegates`/`port-members`/`host-refs`/`port-privates`.
Без High-находок это не блокирует переход — по §2.7/§2.4 вердикт жёлтый, правка выполняется автором в рамках этой же задачи, без нового issue (#202).
## Что проверено и признано корректным
- Обязательные разделы §7.1 присутствуют все: сценарий и «что человек увидит» (первыми, как требует §7.1), проблема, скоуп и не-скоуп, контракт поведения, модель данных/миграция/i18n/UX («не затрагиваются» — корректно для задачи без видимого поведения), критерии приёмки с доказательством, план автотестов, риски, откат, release-артефакты.
- Соответствие `docs/SCOPE.md`: задача не добавляет фичу и не обещает пользователю ничего нового — она инструментальная (`tech-debt`), обслуживает J6 («Keep the plan true as the home evolves») косвенно, снижая риск незаметного роста связности при будущих рефакторингах. Прямого конфликта со SCOPE нет; прецеденты такого рода задач в процессе есть (#512, #425 упомянуты как образец).
- Заявление «ни одна персона ничего не увидит» проверено: скоуп ограничен `src/houseplan-card.ts`, `src/houseplan-editor-runtime.ts` (удаление мёртвого кода и дублей типов, поведение не меняется), гейт-скриптами, `scripts/inventory.mjs`, `scripts/monolith-baseline.json` и абзацем PROCESS.md — ни одного пользовательского контракта, ни UI, ни API конфига не задето. UX/i18n/USER-GUIDE.ru.md обоснованно не привлекались.
- Количественные факты ТЗ (26 486 строк, 307 вызовов `_editorRuntimeOrThrow()`, 1 107 `private`-членов, наличие `HouseplanEditorHostPort`) сверены с деревом на SHA материала и подтверждены точно; факт про ~757/768 ошибок `--noUnusedLocals` подтверждён по порядку величины (расхождение объясняется разными коммитами на `dev` vs. материал ревью, не выдумкой).
- Каждый AC (AC1, AC2, AC4, AC5) — кроме AC1-b (находка выше) — называет проверяемого свидетеля (автотест/фикстура/CI-джоб) и осмысленный мутант, который реально снимает заявленную защиту (проверено чтением формулировок мутантов: `|| true` в фильтре гейта, `>` вместо `>=` в сравнении с базой, подмена определения «делегата» без учёта `async`/неявного `return`, ослабление проверки якорей с «⊆» до «длина ≤» — все четыре осмысленно ломают именно заявляемую защиту, а не соседнюю).
- «Одно число — один источник» для метрик соблюдено явно и намеренно: и гейт (AC1, allow-list `port-privates`), и `npm run inventory` (AC2) читают один и тот же модуль/базу `scripts/monolith-baseline.json` — ТЗ прямо называет этот принцип текстом, не оставляя его на усмотрение реализации.
- Нумерация AC (AC1, AC2, AC1-b, AC4, AC5, без AC3) на первый взгляд выглядит как пропуск, но объяснена в тексте «Не-скоуп»: исходный AC3 (первый вынос по новому образцу) сознательно вынесен в отдельный будущий issue, и номер за ним зарезервирован, чтобы не путать перекрёстные ссылки с аудитом-первоисточником. Не является дефектом, отмечаю для следующего ревьюера, чтобы не поднимал повторно.
- Раздел «Принято предположительно, поменять свободно» содержит три пункта — все технические (расположение гейта, формат базы, грубость подсчёта `host-refs` через regex), продуктовой неоднозначности в задаче нет вообще, что согласуется с «ни одна персона ничего не увидит»: спрашивать владельца здесь действительно нечего, «не бывает сложной задачи без единого открытого вопроса» — для этой задачи открытый вопрос является техническим и явно передан на усмотрение ревьюера кода («ревьюер может предложить AST» — приглашение к техническому спору, а не к молчаливому решению).
- Правка PROCESS.md (новый абзац §2.7 в скоупе, п.4) не требует отдельной эскалации владельцу: автор самого S2-analysis-комментария, фиксирующего этот скоуп, — сам владелец репозитория (`Matysh`), т.е. решение о новом процессном правиле уже принято тем, кто единственный вправе его принимать.
- Откат и release-артефакты описаны однозначно и достаточно для DoR (§2.5): `git revert` одним коммитом, базовые числа в JSON возвращаются автоматически, `User-Visible: no`, бандл пересобирается в том же коммите потому что затронут `src/**` — корректно по D-классу (PROCESS §1).
## Чего не проверял
- Не запускал и не мог запустить `npm run lint:unused`, `test/unused-locals-gate.test.mjs`, `test/monolith-metrics.test.mjs`, `test/monolith-text-anchors.test.mjs`, `scripts/monolith-baseline.json` — их не существует, задача ещё не реализована; это предмет код-ревью, не ревью ТЗ.
- Не проверял тяжёлые/browser-гейты (golden, smoke, performance) — на этапе ТЗ они не требуются и не относятся к скоупу spec-review.
- Не оценивал точное итоговое число «мёртвых» деклараций (`36`/`14`/`~12`) построчно — это оценка масштаба работы для реализации, не критерий приёмки; правильность финального списка удалённого — предмет код-ревью по AC5 и рискам («список удалённого — в хендоффе»).
- Не проверял независимо, действительно ли `\bhost\.`-grep даёт «одинаково до и после» без ложных срабатываний — автор сам пометил это как грубое и предположительное, находка не требуется, ревьюер кода может её оспорить по факту реализации.
## Вердикт
Единственная находка — Medium, в скоупе задачи. High нет. По §2.4/§2.7 PROCESS.md это жёлтый вердикт: ТЗ возвращается автору на правку строки AC1-b (сделать доказательство честным либо добавить реальный ratchet-гейт на размер бандла), без нового issue.
**Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче · Документ: (путь к этому файлу в docs/reviews проставит шаг публикации)**
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `f234855e66ee` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `703233aee85cb3c3cbf101b84e288ec9dba3ce34`
```
git log --all --format='%H %T' | grep 703233aee85c
```
- Тело issue: `d6eced31cfbe6c961c0420b8961747ac902aae4eb680b2744fcd20e6b79f84ed`
- Вердикт конвейера: `yellow` · High 0