mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-28 19:01:34 +00:00
claude[bot]
@@ -1,9 +1,10 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1013, issue: 355. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1014, issue: 356. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| #642 | [SPEC-REVIEW-642-r1.md](SPEC-REVIEW-642-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #641 | [CODE-REVIEW-641-r1.md](CODE-REVIEW-641-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | нет исполняемого автотеста на ключевой guard в accept.mjs | `accept.mjs` `demo/golden/accept.mjs` `test/golden-wsl-artifact.test.mjs` `wsl-attestation.json` `test/golden-capture-provenance.test.mjs` `scripts/mutation-registry.mjs` `scripts/golden-wsl-artifact.mjs` |
|
||||
| #641 | [CODE-REVIEW-641-r2.md](CODE-REVIEW-641-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #640 | [SPEC-REVIEW-640-r1.md](SPEC-REVIEW-640-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | AC3 называет browser-smoke «unit»-тестом | `demo/smoke_furniture_lazy_art.mjs` |
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
# SPEC-REVIEW-642-r1
|
||||
|
||||
Issue: [#642](https://github.com/Matysh/houseplan-card/issues/642) — «Первый вынос из монолита по новому образцу: узкий порт, без делегатов»
|
||||
Этап: ТЗ на ревью (PROCESS.md §2.4), полный трек (сложность 5 > 3, аналитик подтвердил)
|
||||
Заход: r1 · блокирующих циклов израсходовано 0 из 4
|
||||
Ревьюер ТЗ не является автором ТЗ; изучены только тело issue #642, его единственный комментарий (аналитика) и связанные issue #624 (родитель, закрыт), #592 (упомянутый антиобразец), а также фактическое состояние `src/**`, `demo/**`, `scripts/**` на момент ревью (рабочая копия на `adb1727c`).
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
ТЗ выносит диалог «Оптимизировать планы» (`_renderAlignDialog` + связанные методы `HouseplanEditorRuntime`) в новый модуль `src/optimize-plans-dialog.ts` с портом `OptimizePlansDialogPort` (18 членов), устраняя 5 методов-делегатов и 2 стрелки-заглушки в карточке, перенося харнесс 13 смоков + 2 вспомогательных файла и 11 текстовых утверждений `i18n.test.mjs` на исполнение через юнит. Продукт не должен измениться байт-в-байт; связность монолита (`scripts/monolith-baseline.json`) должна снизиться по четырём числам, бандл — не вырасти.
|
||||
|
||||
Задача — прямое продолжение AC3 закрытого #624 (там же зафиксирован метод оценки и решение отложить первый вынос в отдельный issue), поэтому вопрос «служит ли это job из `docs/SCOPE.md`» уже решён на уровне родителя: ни одна из трёх персон изменения не видит («Сценарий»/«Что человек увидит» прямо это утверждают), это инженерная работа против роста связности монолита, ранее одобренная владельцем как `tech-debt`. Открытых продуктовых вопросов к владельцу в ТЗ нет, и это корректно: весь текст ТЗ описывает исключительно техническое решение, наблюдаемое поведение не меняется ни в одной ветке.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Так как задача — рефакторинг с контрактом «байт-в-байт», основной риск ТЗ — не пробелы в структуре (обязательные разделы §7.1 все на месте), а **фактические неточности в описании существующего кода**, которые могли бы выдать догадку за факт или сделать AC невыполнимым. Поэтому вместо формальной проверки одних заголовков разделов каждое количественное и структурное утверждение ТЗ сверено с реальным исходником:
|
||||
|
||||
- Полный текст `## ТЗ` в теле issue #642 и `## Аналитика` в единственном комментарии — прочитаны целиком.
|
||||
- Родительский issue #624 (закрыт) — прочитан целиком, чтобы подтвердить: (а) метрика `monolith-baseline.json` и гейт уже приняты и лежат в `dev` (зависимость issue выполнена — `scripts/monolith-baseline.json` существует, содержит те же первые пять чисел, что в замере ТЗ); (б) AC3 #624 действительно отложило «первый вынос» в отдельную задачу, то есть #642 не самовольно расширяет скоуп.
|
||||
- Issue #592 — прочитан, чтобы подтвердить корректность ссылки «антиобразец»: #592 действительно переносит разметку через `.call(this)`/сохранение исходника в `test/houseplan-source.mjs`, а не через типизированный порт — ТЗ #642 верно называет его тем, чего делать не надо.
|
||||
- Построчная сверка с `src/houseplan-card.ts` и `src/houseplan-editor-runtime.ts` (grep + чтение диапазонов): существование и точные тела всех перечисленных в скоупе методов (`_reportPreflightFailure`, `_copyPreflightDiagnostics`, `_previewAlignDialog`, `_openAlignDialog`, `_toggleOptimizeLivePositions`, `_runAlignToGrid`, `_renderAlignDialog`, `_checkOptimizeGeometry`/`_checkOptimizeGeometryImpl`, `_undoPlanOptimization`), обеих копий литерала `OptimizePlansDialogState` (`houseplan-card.ts:2029`, `houseplan-editor-runtime.ts:443`), полей `_preflightClipboardFallback`/`_reportedPreflightFingerprint` на карточке и в порте, строк сброса `2792`/`7396`, проводки `saveSpaceCopy` → `reportPreflightFailure` (`houseplan-editor-runtime.ts:8337`, `src/space-copy-runtime.ts:19,176`).
|
||||
- Сверка определения метрик в `scripts/monolith-metrics.mjs` (`isDelegateBody`, `portMemberNames`) с проекцией AC1: подтверждено, что метрика `delegates` считает только `MethodDeclaration`/аксессоры с телом-делегатом, поэтому 2 стрелочные заглушки (`_openAlignDialog`, `_toggleOptimizeLivePositions`, объявленные как `PropertyDeclaration` с `ArrowFunction`) в счётчик `delegates` не попадают и не попадали — проекция «154 (−5)» точно соответствует пяти методам-делегатам, названным в скоупе, без двойного счёта.
|
||||
- Сверка проекции `portMembers`/`portPrivates` (−2/−2): `_alignDialog` остаётся членом `HouseplanEditorHostPort`, потому что адаптер `OptimizePlansDialogPort.dialog()/setDialog()` строится рантаймом в конструкторе и продолжает читать/писать `this.host._alignDialog` — а вот `_preflightClipboardFallback` и `_reportedPreflightFingerprint` перестают быть нужны порту рантайма целиком (переезжают в `WeakMap`/приватное поле нового класса), что и даёт ровно −2/−2, а не −3 (как выглядело бы при поверхностном подсчёте всех трёх «портовых приватных» полей).
|
||||
- Существование всех 8 названных мутантов реестра (`grep` по `scripts/mutation-registry.mjs` — все 8 id найдены) и всех 13 названных смоков + `demo/wall-draw-click-harness.mjs` + `demo/golden/harness.mjs` (файлы существуют и действительно ссылаются на переносимые имена).
|
||||
- Точные номера строк `test/i18n.test.mjs`, названные в AC3 (247–254, 296–297, 309–311) — прочитаны: в диапазоне ровно 11 обращений к `cardSource` по разметке/тосту диалога, как заявлено; файл содержит и другие обращения к `cardSource` (итого 19), поэтому корректно остаётся в замороженном списке `test/monolith-text-anchors.test.mjs` после удаления этих 11.
|
||||
- Ссылка на `CODE-REVIEW-295-r1` (M2, устаревание фолбэка) сверена с реальным документом `docs/reviews/CODE-REVIEW-295-r1.md` — раздел M2 существует и описывает именно ту проблему, для решения которой ТЗ предлагает `WeakMap`.
|
||||
- Прецеденты `RadarSetupController` (`src/radar-setup.ts:100`, собирается в `houseplan-editor-runtime.ts:827`) и `live-*`/`config-reload-authority` (`src/live-*.ts`, `src/config-reload-authority.ts`) — существуют, форма конструктора-адаптера подтверждена.
|
||||
- Наличие механизма `test-build`/`tsconfig.test.json` (`include`), которым уже пользуются соседние модули (`radar-setup.ts` и др.) — подтверждено; план добавить туда новый модуль технически исполним без нового инструмента.
|
||||
|
||||
Гейты (typecheck/test/build) не гонялись: это этап ревью ТЗ, кода ещё нет — раздел «Для этапа code» промпта к этому заходу не относится.
|
||||
|
||||
## Находки
|
||||
|
||||
**High: 0. Medium: 0.**
|
||||
|
||||
- **Low (не блокирует, снимается с записью).** Раздел «Состояние» ТЗ утверждает: «Карточка читает его [`_alignDialog`] в четырёх местах: Escape, два блокирующих условия, сброс при переподключении». Фактически в `houseplan-card.ts` не менее пяти самостоятельных чтений/записи поля из кода карточки: Escape (`:2792`), `_warmDialogState` (`:3423`), `_editorSecondaryDialogBlocked` (`:8638`), гейт ленивой загрузки `_editorRuntime` в `editorRuntimeRequested` (`:10665`) и сброс при переподключении (`:7395-7396`); плюс рендер-чтение (`:11308`), которое ТЗ адресует отдельным пунктом скоупа. Это не меняет ни границы скоупа, ни доказуемость AC: план явно оставляет `_alignDialog` полем карточки, и все перечисленные (и не перечисленные) строки, читающие его, по формулировке скоупа не переносятся и не меняются — недосчитанное место (`:10665`) уже сегодня не тронуто и не должно быть тронуто впредь. Риска для реализации нет; отмечаю как неточность в обосновании, а не как пробел в границах задачи. Автору переписывать ТЗ не требуется.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все семь обязательных предметных разделов §7.1 присутствуют: сценарий, что человек увидит до/после, проблема, скоуп и не-скоуп, контракт поведения, UX/модель данных/миграция/i18n (единообразно «не затрагиваются» — обоснованно, диалог не меняет ни то, ни другое), план автотестов, риски, откат, release-артефакты.
|
||||
- Продуктовые разделы («Сценарий», «Что человек увидит») отвечают на оба обязательных вопроса — какая персона/поверхность/момент и что видно до/после — одной фразой без терминов реализации; ответ «никто ничего не увидит» обоснован и совпадает с прецедентом принятого #624.
|
||||
- Каждый AC (1–6) сопровождён явным «чем доказан» и «чем краснеет»; AC1 честно помечен как незащищённое измерение (§2.7), остальные опираются на называемые юнит-тесты и переведённые мутанты реестра — ни один AC не голословен.
|
||||
- Ни одного места, где догадка о продуктовом поведении подана как факт без пометки: единственный содержательный домысел — «эквивалентность фолбэка на WeakMap» — доказывается разбором путей выполнения тут же в тексте, а не постулируется.
|
||||
- Раздел «Принято предположительно, поменять свободно» содержит исключительно технические решения (имя модуля/класса, судьба `_alignDialog`, путь `checkGeometry`, метод подсчёта `hostRefs`) — ни один пункт не является замаскированным продуктовым вопросом, который следовало бы адресовать владельцу.
|
||||
- Не-скоуп корректно исключает соседние риски: `_onKey`/Escape-лестница (обоснованно — вынос поднял бы `hostRefs`), `_saveMarker`/`_applyWallFaceBatch` (следующие кандидаты, отдельные issue), перенос самого `_alignDialog` с карточки (сознательно отклонён — цена/выгода не в пользу переноса).
|
||||
- Откат — один `git revert`, без данных пользователя и миграций; release-артефакты корректно называют `User-Visible: no` и пересборку бандла в этом же коммите, так как меняется `src/**`.
|
||||
- Количественные проекции AC1 (`delegates`, `portMembers`, `hostRefs`, `portPrivates`, `harnessPrivates`, `bundleBytes`) внутренне согласованы с определениями метрик в `scripts/monolith-metrics.mjs`, а не подогнаны на глаз (см. «Как проверялось»).
|
||||
- Мутанты, смоки, номера строк текстовых утверждений и ссылка на прошлый ревью (#295) проверены по факту существования и содержания, а не приняты на веру.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не выполнялась реализация и не гонялись `tsc`/`npm test`/`npm run build`/`lint:unused` — на этапе ревью ТЗ кода ещё нет, это станет предметом код-ревью.
|
||||
- Не пересчитывались вручную все 350 членов `HouseplanEditorHostPort` и все ≈93 вхождения `host.` в затронутых методах — проверена методология подсчёта (метрика) и точечно ключевые поля (`_alignDialog`, `_preflightClipboardFallback`, `_reportedPreflightFingerprint`), а не полный построчный пересчёт каждого числа в таблице AC1: там сама ТЗ честно называет эти числа измерением без защитной логики, ошибка в проекции не блокирует AC.
|
||||
- Не проверялось содержимое остальных ~40 строк харнесса, пишущих `_alignDialog` напрямую, построчно — подтверждено существование затронутых 13 смоков и 2 вспомогательных файлов через grep, не построчная сверка каждого вызова.
|
||||
- Golden-кадры (`optimize-preflight-dialog-*`, `optimize-orphan-references-*`) не сверялись визуально — на этом этапе эталонов ещё нет, это часть AC6 будущего код-ревью.
|
||||
|
||||
## Вывод
|
||||
|
||||
Структура ТЗ полна, каждый AC проверяем и привязан к называемому доказательству, технические утверждения о существующем коде проверены построчно и оказались точными (кроме одной необязывающей неточности в подсчёте мест чтения `_alignDialog`, не влияющей на скоуп или AC). Открытых продуктовых вопросов нет и не требуется — задача не меняет наблюдаемое поведение ни для одной персоны. Blocking-находок нет.
|
||||
|
||||
**Вердикт: зелёный.**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `adb1727c50b0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `c5f33ffb2d92b3b2675ee1f23f67ac61ef572365`
|
||||
```
|
||||
git log --all --format='%H %T' | grep c5f33ffb2d92
|
||||
```
|
||||
- Тело issue: `11d5073fbd1128d0f49f8b082936f26d677bd425b78e516d070b9a8d992d8651`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user