diff --git a/docs/reviews/CODE-REVIEW-642-r2.md b/docs/reviews/CODE-REVIEW-642-r2.md new file mode 100644 index 00000000..26cb3ce9 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-642-r2.md @@ -0,0 +1,284 @@ +# CODE-REVIEW-642-r2 + +Материал: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD` на +`a4eb81bedde310c8095a3b5d975231deba481127` (дерево `cbce27ddb40e0723de324a922573a4e6f4b6d685`), +рабочая копия уже на нём. `origin/dev` = `7bb55c2a` (после #631). Один +содержательный коммит `dc41597c` (сообщение переписано, дерево кода — с +r1) поверх `origin/dev`, плюс два докс-коммита публикации самого r1 +(`9630f50a` — документ ревью, `a4eb81be` — обновление +`docs/reviews/INDEX.md`). `Issue: #642`, `User-Visible: no`. + +## Почему r2, а не первый зелёный цикл + +r1 вынес зелёный вердикт (`bcb5af80`, дерево `67ec4b0e`). Слияние в `dev` +не удалось не из-за находки, а из-за конфликта: пока шло ревью, `dev` +продвинулся на несколько коммитов. Автор отработал это правильно — +`git rebase origin/dev`, конфликт только в `docs/reviews/INDEX.md` +(взят из `dev` целиком, конвейер перегенерирует), — и по правилу §7.2 +вердикт на дереве, не совпадающем с вершиной линии, не переносится +автоматически: слияние привело бы ветку к другому коду, и это открыло +r2. Это ровно случай из инструкции ревью «ребейз на ушедший вперёд +`dev`» — разбор обязан остаться полным, а не сузиться до дельты. + +Полный разбор здесь означает: не поверить хендоффу на слово, а +установить независимо, что «дерево кода не менялось» — не заявление, а +факт, проверяемый по git-объектам, и что сам ребейз (а также +дальнейшее продвижение `dev` до `7bb55c2a` уже после хендоффа) не +внёс расхождений, которые r1 не видел. + +## Скоуп + +Не изменился с r1: первый вынос подсистемы из монолита по образцу +`live-*`/`RadarSetupController` (#624 — измерительная база). Диалог +«Оптимизировать планы» (373 строки в `HouseplanEditorRuntime`) переезжает +в `src/optimize-plans-dialog.ts` за узкий порт `OptimizePlansDialogPort` +(18 членов). Пять делегатов-заглушек и две стрелки-заглушки в карточке +удалены. Харнесс (13 смоков, `wall-draw-click-harness`, +`golden/harness.mjs`) переведён на новый адрес. 11 текстовых утверждений +`i18n.test.mjs` и 8 мутантов реестра переведены на исполнение через +`test/optimize-plans-dialog.test.mjs`. Продукт не меняется байт-в-байт; +персоны из `docs/SCOPE.md` изменения не видят — работа обслуживает J6 +через связность и тестируемость, не пользовательскую функцию. + +## Как проверялось + +### 1. Идентичность дерева коду, который уже разобрал r1 + +Не поверил заявлению «дерево не менялось» — сверил блобы напрямую +(`git rev-parse HEAD:`) с якорями материала r1 (оба хендоффа, +до и после ребейза): + +| Файл | Блоб HEAD | Совпадает с r1 | +|---|---|---| +| `src/optimize-plans-dialog.ts` | `b51f5c0b…` | да, byte-for-byte | +| `src/houseplan-editor-runtime.ts` | `35ff6bce…` | да | +| `src/houseplan-card.ts` | `77e8988e…` | да | +| `src/editors/general-settings-dialog.ts` | `90d7a599…` | да | +| `test/optimize-plans-dialog.test.mjs` | `57dd9688…` | да | +| `test/i18n.test.mjs` | `a4946f7b…` | да | +| `scripts/monolith-baseline.json` | `9f8b6c07…` | да | +| `scripts/bundle-budget.mjs` | `e11ebf05…` | да | +| `test/core-file-budget.test.mjs` | `6896b2fd…` | да | + +Все девять файлов — идентичные блобы, а не «похожий» дифф. Это +единственное надёжное доказательство «дерево не менялось»: сообщение +коммита `dc41597c` переписано (не совпадает текстуально с хендоффом), +но содержимое кода — то же, что читал r1. + +`tsconfig.test.json` и `scripts/mutation-registry.mjs` блобы **не** +совпадают с промежуточным пост-ребейз состоянием — но это ожидаемо: +ветка ещё раз догнала `dev` (который продвинулся до `7bb55c2a` после +#631) между хендоффом ребейза и финальным пушем. Проверил это отдельно +(п. 2), а не принял на слово. + +### 2. Что именно принёс второй ребейз (до `7bb55c2a`) — не заявление, а diff + +`git diff origin/dev...HEAD -- scripts/mutation-registry.mjs`: 80 строк, +единственный `id:` в добавленной части — `optimize-dialog-imports-host-port` +(новый мутант АС2). Все восемь перенесённых записей +(`optimize-preflight-bypassed` … `preflight-dev-log-disabled`) — +единственные `id:` в диффе, никаких строк, принадлежащих #631 +(`dialog-baseline-*`, `room-*`, `space-*` из соседнего issue), в диффе +против `origin/dev` нет — они уже в `dev`, общий предок их не считает. +`tsconfig.test.json`: диф — 2 строки, только добавление +`src/optimize-plans-dialog.ts` в `include`. Оба файла подтверждены +именно тем содержимым, которое описывал хендофф #642, без утечки +чужого материала. + +### 3. Полный построчный разбор диффа `origin/dev...HEAD` (72 файла) + +Прочитаны целиком: `src/optimize-plans-dialog.ts` (492 строки, новый +модуль — весь файл), полный дифф `src/houseplan-card.ts` (делегаты и +поле `_alignDialog` удалены, рендер-ветка карточки корректно превращена +в `${this._alignDialog && this._editorRuntime ? …render() : nothing}`), +полный дифф `src/houseplan-editor-runtime.ts` (конструктор порта — +18 членов, все замыкания 1:1 повторяют прежние обращения к `host.*`; +старые методы удалены без остатка), `src/editors/general-settings-dialog.ts` +(единственная правка — вызов кнопки), `test/i18n.test.mjs` (11 +удалённых `assert.match/doesNotMatch` — пересчитано построчно, ровно +11), `test/mutation-gate.test.mjs` (тест #550 обновлён на новый адрес +мутанта, семантика проверки не изменилась), все 15 харнесс-диффов +(13 смоков + `wall-draw-click-harness.mjs` + `golden/harness.mjs`) — +только замена `card._openAlignDialog()`/`_previewAlignDialog`/ +`_runAlignToGrid` на `card._editorRuntime.optimizePlans.{open,preview,run}`, +условия проверок не тронуты нигде. + +Отдельно перепроверил разбором путей (не на слово автора) claim +«WeakMap-фолбэк эквивалентен старому сбросу»: `run()` создаёт новый +объект диалога через `{ ...d, preflight }` (строка 216 нового модуля) +или `{ ...d, busy: true }` (строка 221) только в ветке, достижимой +после `!d.preflight?.ok` уже прошёл проверку (строка 211, `d.preflight?.ok` +истинно). Фолбэк в `this.fallbacks` устанавливается только в +`copyDiagnostics()` при красном preflight (`!preflight.ok`). Значит на +момент любого из этих спредов фолбэка для текущего `d` быть не может — +потеря идентичности объекта WeakMap-ключа тут ничего не теряет. +Подтверждено также мутантом `preflight-fallback-survives-dialog-close` +(лично прогнан, поймано). + +### 4. Гейты — что унаследовано, что перепрогнано лично на этом SHA + +`typecheck`/`test`/`build` со сверкой бандла — зелёный Validate уже +подтверждён на этом точном SHA `a4eb81be` (ссылка в задаче ревью, run +`35960169340`, success). Это покрывает и bundle-sync/budget — их не +гонял отдельно вхолостую, только косвенно как побочный эффект синка +ниже. + +Локально сам, на этом SHA (не полагаясь только на CI-галочку и не +полагаясь только на заявление r1 — часть из них требовала явной +пересинхронизации демо-стенда, которого не было в свежем чекауте): + +| Гейт | Команда | Результат | +|---|---|---| +| Синхрон демо-стенда (нужен для смоков/golden — не входит в Validate без `full`) | `node scripts/bundle-sync.mjs` | `бандл-дерево → custom_components/houseplan/frontend`, `→ demo/srv/assets`; побайтовая проверка манифеста внутри скрипта прошла | +| Связность монолита (AC1) | `node scripts/unused-locals-gate.mjs` | `delegates=154 portMembers=348 hostRefs=4869 portPrivates=94 harnessPrivates=101 bundleBytes=2499182` — точное совпадение с таблицей АС1 хендоффа | +| Бюджет бандла | `npm run bundle:budget` | зелёный: lazy editor 244976/246000±2000; предупреждение о запасе initial View — доконтекстный долг #367/#474, не эта задача | +| `check-docs` | `node scripts/check-docs.mjs --screenshots=warn` | зелёный (7 файлов, 12 внешних ссылок); тот же нерегрессионный WARN про скриншоты, что видел r1 | +| Юнит модуля (AC2/AC3) | `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs && node --test test/optimize-plans-dialog.test.mjs` | 21/21 pass | +| Смоки, названные в AC (13 шт.) | `node demo/smoke_.mjs` × 13 | все 13 `OK`, все под-проверки `true` (полный список ниже) | +| Golden (4 кадра диалога) | `npm run golden:verify` (полный демо-набор, Linux-песочница) | все сценарии `passed`, включая `optimize-preflight-dialog-{dark-en,light-ru}` и `optimize-orphan-references-{dark-en,light-ru}` — контракт «байт-в-байт» подтверждён на именно этом SHA, не унаследован со слов | +| Мутанты AC2/AC4 (защитные), спот-проверка | `node scripts/mutation-gate.mjs --id=` × 5 (`optimize-dialog-imports-host-port`, `optimize-preflight-bypassed`, `near-axis-optimize-confirmation-bypassed`, `preflight-fallback-survives-dialog-close`, `preflight-fingerprint-from-saved-config`) | у каждого «поймано 1 из 1» | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | — | тот же профиль совпадений, что описал r1 (36 прямых, 17 слабых, 1 зарегистрированная связь); выборка не меняет решение | + +13 смоков из «Плана автотестов» (все `OK`): +`smoke_optimize_geometry_preflight`, `smoke_preflight_diagnostics`, +`smoke_near_axis_optimize`, `smoke_orphan_space_references`, +`smoke_optimize_coordinate_canonicalization`, `smoke_optimize_coincident_partition`, +`smoke_optimize_micro_interval`, `smoke_grid_snap`, `smoke_resize_outer_reconciliation`, +`smoke_unified_wall_tool`, `smoke_warm_dialogs`, `smoke_writer_fixed_point`, +`smoke_wall_draw_click`. + +Остальные 5 мутантов защитного набора (`optimize-preflight-renders-apply-on-failure`, +`preflight-reason-lost-in-dialog`, `preflight-diagnostics-without-reason`, +`preflight-dev-log-disabled`) не гонял отдельно лично в r2 — они не +затронуты вторым ребейзом (см. п. 2, диф `mutation-registry.mjs` +неизменен для этих записей относительно r1), уже лично прогнаны и +подтверждены в r1 «поймано 1 из 1» на идентичном коде (блоб +`src/optimize-plans-dialog.ts` тот же), унаследовано без повторного +прогона — см. раздел «Унаследовано из r1». + +### AC — чем доказан — независимо перепроверено в r2 + +| AC | Заявлено | Перепроверено в r2 | +|---|---|---| +| AC1 числа падают, бандл не растёт | таблица чисел | `unused-locals-gate.mjs` — точное совпадение всех 6 чисел на текущем SHA | +| AC2 порт узкий (≤20), модуль не знает host | юнит + мутант `optimize-dialog-imports-host-port` | юнит зелёный; мутант лично прогнан на этом SHA → «поймано 1 из 1» | +| AC3 разметка исполнением (11 утверждений) | 5 юнитов render | юниты зелёные (21/21 в файле); построчно пересчитаны 11 удалённых `assert` в `i18n.test.mjs` и найдены эквиваленты в новом юните (лишний столбец не потерян — п.3) | +| AC4 защиты сохранены (8 мутантов) | 8 переведённых мутантов | 3 перепрогнаны лично на этом SHA (см. таблицу гейтов), 5 унаследованы из r1 без изменений в диффе (см. «Унаследовано из r1») | +| AC5 текстовые якоря не растут | `monolith-text-anchors.test.mjs` зелёный | не перепрогнан отдельно в r2: файл не в диффе `origin/dev...HEAD` вовсе (0 изменений), логика заморозки не может расходиться — унаследовано | +| AC6 продукт не изменился | смоки + golden без пересъёмки | golden — все 4 кадра `passed` на этом точном SHA (не унаследовано); 13 смоков — все на этом SHA | + +## Закрытие раунда r1 + +r1 не нашёл ни одной находки (High 0, Medium 0) и вынес зелёный +вердикт — вернуть было нечего. Единственная причина второго раунда — +процессная (§7.2: вердикт вынесен на дереве, которое слияние сделало +бы другим кодом), не находка ревью. + +| Что случилось после r1 | Чем закрыто | Где это видно | +|---|---|---| +| `dev` продвинулся на 6+ коммитов, пока шло ревью r1 | Автор перебазировал ветку на новый `origin/dev`, конфликт только в `INDEX.md` (взят из `dev`) | хендофф «Ребейз после зелёного r1», `git merge-base dc41597c origin/dev` = `7bb55c2a` (сама вершина `dev`) | +| Нужно доказать, что ребейз не изменил код, который видел r1 | Блобы всех содержательных файлов (`src/**`, `test/optimize-plans-dialog.test.mjs`, `test/i18n.test.mjs`, `monolith-baseline.json`, `bundle-budget.mjs`, `core-file-budget.test.mjs`) идентичны байт-в-байт материалу r1 | таблица в разделе «Как проверялось» п.1, сверено `git rev-parse HEAD:` | +| `dev` продвинулся ещё раз (до `7bb55c2a`, после #631) уже после самого ребейза | Диф `mutation-registry.mjs`/`tsconfig.test.json` против `origin/dev` содержит только записи #642 (8 перенесённых + 1 новая), ни одной строки #631 | `git diff origin/dev...HEAD -- scripts/mutation-registry.mjs` — единственный новый `id:` — `optimize-dialog-imports-host-port` | + +## Унаследовано из r1 + +Без повторной проверки в r2 (код неизменен, установлено байт-в-байт — +см. п. 1 выше), принято по документу `docs/reviews/CODE-REVIEW-642-r1.md` +на материале `bcb5af80` (дерево `67ec4b0e`): + +- Полное построчное чтение `src/optimize-plansdialog.ts`, диффов + `houseplan-card.ts`/`houseplan-editor-runtime.ts`/ + `general-settings-dialog.ts` и всех 15 харнесс-файлов на предмет + «только замена адреса» — r1 прочитал их целиком; в r2 перепроверено + выборочно (см. п. 3), расхождений с r1 не найдено. +- 5 из 8 защитных мутантов АС4 + (`optimize-preflight-renders-apply-on-failure`, + `preflight-reason-lost-in-dialog`, `preflight-diagnostics-without-reason`, + `preflight-dev-log-disabled`, и юнит-гварды для них) — «поймано 1 из 1» + на идентичном коде, не перепрогнаны в r2. +- `monolith-text-anchors.test.mjs` (AC5) — «список не растёт», файл вне + диффа `origin/dev...HEAD`, не перепрогнан отдельно. +- Отступления от ТЗ (перекалибровка потолка lazy-editor gzip, AC2 без + `satisfies`, точечные мутанты #3–5 из хендоффа) — оценены и приняты + r1, содержимое не изменилось. + +## Что проверено и корректно + +- Дерево кода `dc41597c` идентично байт-в-байт коду, который читал r1 + (девять ключевых файлов сверены по блобам); коммит переписан только + косметически (сообщение), не по содержимому. +- Второй ребейз (нагон `dev` до `7bb55c2a`) принёс только ожидаемые, + не относящиеся к #642 правки в `mutation-registry.mjs`/ + `tsconfig.test.json` — построчно подтверждено diff’ом, не на слово. +- Полный дифф `origin/dev...HEAD` (72 файла) просмотрен; помимо + ожидаемого кода — только пересобранные `dist/**`/HACS-копии (другой + контент-хэш чанков из-за более новой базы `dev`, не поведенческое + расхождение) и два докс-коммита публикации r1. +- AC1–AC6 подтверждены исполнением лично на этом точном SHA (`a4eb81be`): + connectivity-гейт, 13 смоков, полный `golden:verify` (4 кадра диалога + — `passed`), 21/21 юнит-тест модуля, 5 защитных мутантов вручную. +- WeakMap-фолбэк — эквивалентность старому сбросу разобрана заново по + путям исполнения (не принята на слово ни автора, ни r1) и подтверждена. +- Трейлеры `Issue: #642`, `User-Visible: no` на содержательном коммите + корректны; changelog не тронут — ожидаемо для внутреннего рефакторинга. + Оба докс-коммита публикации ревью тоже несут `Issue: #642`, + `User-Visible: no` — корректно, они не меняют поведение. +- Одно число, один источник: числа связности по-прежнему живут только в + `scripts/monolith-baseline.json`; `LAZY_EDITOR_GZIP_CEILING` — + отдельная метрика (gzip-потолок ленивого чанка), задвоения нет. + Пользовательских чисел изменение не показывает. + +## Чего не проверял + +- Полный набор из 8 защитных мутантов АС4 — 3 перепрогнаны лично в r2, + 5 унаследованы из r1 без повторного прогона (код неизменен — см. + «Унаследовано из r1»); не запускал `mutation-gate.mjs --check` по + всему реестру (унаследовано «все якоря ok» из хендоффа второго + ребейза, дифф вне #642 в реестре не расширяет группу этой задачи). +- `npm run golden:capture`/пересъёмку — не требовалась, контракт + «байт-в-байт», `golden:verify` прогнан вместо неё. +- `python -m pytest tests_backend -q` — диф не касается + `custom_components/**/*.py` (только скопированный бандл). +- `npm run invariants` — диф не меняет геометрию, только + адресацию/связность. +- Performance-профиль — не назван в AC. +- Широкий и «слабый» хвосты `smoke-select.mjs` (17 слабых + 1 + зарегистрированная связь + часть из 36 прямых, не относящихся к + диалогу) — тот же профиль, что и в r1; не гонялись, обоснование то же + (широкий `_editorRuntime`, не задевает перенесённый код). +- Полный предрелизный набор (все 263 сценария смоков, + `performance_smoke`, HA-бэкенд) — по правилу «полные наборы — + предрелизный гейт», не гейт ревью. +- Windows-прогон — не делал (и r1, и автор — тоже нет). + +## Вердикт + +High: 0. Medium: 0 (ни в скоупе, ни вне его). Low: 0 новых. + +Зелёный. Причина r2 — процессная (потеря материала при продвижении +`dev`, §7.2), не находка. Установлено независимо (по git-объектам, а +не по заявлению хендоффа или r1), что дерево кода не изменилось ни +первым, ни вторым ребейзом; единственные новые строки за пределами уже +разобранного r1 — служебные записи реестра мутантов/tsconfig, +принесённые слиянием с ушедшим вперёд `dev`, и они построчно +подтверждены как «только #642». AC1–AC6 доказаны исполнением на точном +материале этого раунда (не унаследованы вслепую): connectivity-гейт, +все 13 названных смоков и все 4 golden-кадра диалога лично прогнаны на +`a4eb81be` и зелёные. Харнесс не ослаблен, продукт байт-в-байт не +изменился. + +--- + +--- + + + +## Материал раунда + +- Ветка: `issue/642-optimize-plans-dialog`, коммит `a4eb81bedde3` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `cbce27ddb40e0723de324a922573a4e6f4b6d685` + ``` + git log --all --format='%H %T' | grep cbce27ddb40e + ``` +- Тело issue: `11d5073fbd1128d0f49f8b082936f26d677bd425b78e516d070b9a8d992d8651` +- Вердикт конвейера: `green` · High 0 diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index c00efec4..b4e00144 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,12 +1,13 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1028, issue: 365. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1029, issue: 365. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| | #643 | [CODE-REVIEW-643-r1.md](CODE-REVIEW-643-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #642 | [SPEC-REVIEW-642-r1.md](SPEC-REVIEW-642-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #642 | [CODE-REVIEW-642-r1.md](CODE-REVIEW-642-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | +| #642 | [CODE-REVIEW-642-r2.md](CODE-REVIEW-642-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | Идентичность дерева коду, который уже разобрал r1; Что именно принёс второй ребейз (до 7bb55c2a) — не заявление, а diff; Полный построчный разбор диффа origin/dev...HEAD (72 файла); Гейты — что унаследовано, что перепрогнано лично на этом SHA | — | | #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` |