diff --git a/docs/reviews/CODE-REVIEW-629-r2.md b/docs/reviews/CODE-REVIEW-629-r2.md new file mode 100644 index 00000000..9b757295 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-629-r2.md @@ -0,0 +1,157 @@ +# CODE-REVIEW-629-r2 + +Вердикт: **зелёный** · заход r2 · блокирующих циклов 0/4 · High: 0 · Medium: 0 + +## Материал + +- Ветка `issue/629-smoke-facade`, вершина **`d90725815a1b042144bbdca5274a897e6860c68a`** (рабочая копия на нём, `git status --short` чисто). +- Диапазон `git log --oneline origin/dev..HEAD` — четыре коммита: + - `035399e7` feat(hooks): `data-hp="mode-tab" data-mode` на вкладках режимов (класс A) + - `fa319e37` test(harness): фасад `window.__hpTest` и гейт `no-new-private-writes` (класс B/C) + - `a977f2c2` docs: публикация документа ревью r1 (артефакт конвейера) + - `d9072581` build(#629): ребейз на dev после #615 — пересборка бандла, индекс ревью (класс D) +- Все четыре коммита несут `Issue: #629`, `User-Visible: no` — корректно (изменений, видимых пользователю карточки, нет; changelog не требуется). +- Validate на этом SHA — success (run 36081944233): `typecheck`, `npm test`, `npm run build` со сверкой копий бандла и `check-docs.mjs` этим прогоном подтверждены. Тем не менее я перегнал `npm run build` + `bundle-sync` + `bundle-tree` лично, чтобы смоки шли на свежесобранном бандле, а не на закоммиченном — `git status --short` после пересборки чистое, т.е. они побайтно совпадают. + +## Скоуп + +Не изменился с r1: инструментальная задача, ни одна персона `docs/SCOPE.md` продукт напрямую не видит. Выигрывает разработчик/ревьюер смоков (гейт `no-new-private-writes`, фасад `window.__hpTest`) и косвенно — надёжность регрессионного покрытия редакторов. Продукт получает один атрибут `data-hp="mode-tab"` на вкладках режимов, который не участвует ни в одном селекторе стилей. + +## Почему это r2, а не продолжение r1 + +r1 получил зелёный вердикт дважды по существу не переделываемого кода, но не смог слиться в `dev` из-за конфликта дважды подряд (комментарии issue 05:34:29 и 00:08:52): после первого зелёного ревью ветка была переиграна на `dev` (влились #642, #627, #617, #618), затем ещё раз — после того как Validate с мутантами на этом ребейзе упал (флейк, разобран автором как невоспроизводимый) и потребовал второго ребейза поверх `dev` уже с #615. Дельта с r1 — ребейз на ушедший вперёд `dev`; по правилу §2.10 это НЕ локальная дельта («ребейз на ушедший вперёд dev» прямо назван как случай, где разбор остаётся полным). Поэтому ниже — полный разбор AC, но с явной опорой на то, что бо́льшая часть файлов задачи побайтно не менялась с r1. + +## Закрытие раунда r1 + +r1 не содержал находок (High: 0, Medium: 0) — таблица «находка | чем закрыта» пуста, закрывать нечего. Единственная отмеченная r1 «мелочь без блокирующего веса» (G1 формально не ловит запись через `for (c._x of list)`) не была поднята до Medium и не требовала правки; в дельте r1→r2 эта часть гейта (`scripts/no-new-private-writes.mjs`) не менялась (см. ниже), так что предмет замечания не изменился. + +## Дельта r1 → r2 (объявление и разбор) + +SHA материала r1 (`e5662b0b188119e276866354aa995601dc49a4aa`) не резолвится в этой рабочей копии (ветка была дважды переиграна, история недостижима) — это ожидаемый случай (#634/§2.10), а не находка. Материал ищется по блобам, перечисленным в `docs/reviews/CODE-REVIEW-629-r1.md` («Материал раунда»). Сверка `git hash-object` текущих файлов с этими 12 блобами: + +| Файл | Блоб r1 | Блоб r2 (HEAD) | Статус | +|---|---|---|---| +| `scripts/no-new-private-writes.mjs` | `82586f4c…` | `82586f4c…` | не менялся | +| `demo/helpers/hp-test.mjs` | `2d1632ef…` | `2d1632ef…` | не менялся | +| `demo/srv/demo.html` | `1c02e893…` | `1c02e893…` | не менялся | +| `demo/serve.mjs` | `67f7c625…` | `67f7c625…` | не менялся | +| `demo/smoke_test_facade.mjs` | `a9de89d8…` | `a9de89d8…` | не менялся | +| `demo/smoke_area_relocation.mjs` | `ff00faff…` | `ff00faff…` | не менялся | +| `demo/smoke_glow.mjs` | `07db5f34…` | `07db5f34…` | не менялся | +| `demo/smoke_grid_snap.mjs` | `3332bab7…` | `76665323…` | **изменился** — слияние с #642 | +| `src/houseplan-card.ts` | `ec905c50…` | `5c890e6f…` | **изменился** — перенос атрибута на строку `#642` | +| `docs/data-hp-contract.json` | `75abb945…` | `75abb945…` | не менялся | +| `test/no-new-private-writes.test.mjs` | `dc1783b2…` | `dc1783b2…` | не менялся | +| `test/hp-test-facade.test.mjs` | `3d340be6…` | `3d340be6…` | не менялся | + +Плюс `scripts/mutation-registry.mjs` (в материал r1 отдельно не занесён блобом, но менялся дальше по цепочке ребейзов как append-only реестр всего проекта) — проверен отдельно ниже. + +Итого предметная дельта r1→r2 — ровно два файла задачи плюс сгенерированный бандл и `docs/reviews/INDEX.md`. Разбираю оба: + +**1. `demo/smoke_grid_snap.mjs`.** Причина изменения — текстовый конфликт при ребейзе: #642 (другая задача, слилась в `dev` независимо) сменила адрес вызова диалога «Оптимизировать планы» на `c._editorRuntime.optimizePlans.open()/run()`, #629 в тех же местах перевела подготовку сцены на фасад (`hp.setServerConfig`, `hp.setLayout`, `hp.switchSpace`). Автор оставил оба изменения. Прочитано построчно и прогнано: + - `hp.setServerConfig(...)`, `hp.setLayout(...)`, `hp.switchSpace(...)` перед каждым вызовом `_editorRuntime.optimizePlans.open()/run()` — подготовка сцены идёт через фасад, сам вызов диалога — через публичный член `_editorRuntime` (не приватное поле карточки в смысле G1: сегмент `_editorRuntime` начинается с `_`, но обращение идёт к его собственному публичному методу `.optimizePlans.open()`, а не к присваиванию `_editorRuntime.* = …`; G1 ловит присваивания/`++`/`--`/`delete`, а не вызовы методов на непокрытых G5 объектах — вызов не входит ни в исключаемые G5 имена, ни в записи, поэтому гейту нечего ловить, и это не находка: AC1–AC5 не про вызовы вообще, кроме четырёх именованных в G5, и `_editorRuntime.optimizePlans.open/run` в их числе нет). + - Оставшиеся 12 записей в приватные поля (`c._drag`, `c._path`, `c._decorDraft`, `_decorMove`, `_decorTextDialog`, `_openingDialog`, `_alignDialog`) — состояние жестов, ровно то подмножество, которое ТЗ (хендофф r1, AC10) явно оставило нетронутым: полей из запрещённого AC10 списка (`_serverCfg, _layout, _regSignature, _cfgEpoch, _modelCache, _frame, _tool, _mode, _space, _markerDialog, _spaceDialog, _roomDialog`) среди них нет — проверено `grep` по обоим спискам, совпадений нет. + - Гейт задачи и счётчик проверок пересчитаны лично (см. таблицу ниже) — 0 нарушений, список имён проверок (35) не сократился. + +**2. `src/houseplan-card.ts`.** Причина изменения — не в этой задаче: #642 опустила потолок `test/core-file-budget.test.mjs` до фактического размера файла, и лишняя строка с атрибутом `mode-tab` подняла бы размер на единицу сверх нового потолка. Автор перенёс `data-hp="mode-tab" data-mode=${m}` на ту же строку шаблона, что и существующий `data-editor-navigation=${m}` (`src/houseplan-card.ts:10806`) — атрибут тот же самый, только без отдельной строки. Прочитано и подтверждено: `grep -n "mode-tab\|data-editor-navigation" src/houseplan-card.ts` показывает оба атрибута рядом на одной строке шаблона, третий — `data-editor-navigation="view"` — не тронут; `node --test test/core-file-budget.test.mjs` зелёный (7/7, «потолки заданы для двух ядер и ни для чего больше», «потолки — числа в этом файле, а не вычисление от текущего размера»), т.е. изменение не подняло и не обошло бюджет ядра. Семантика P1/P2 ТЗ не изменилась: атрибут по-прежнему рендерится только вместе с самими вкладками, не участвует в CSS/JS-селекторах. + +**3. `scripts/mutation-registry.mjs`.** Append-only реестр, менялся при каждом ребейзе из-за параллельно вливающихся задач. Все 7 мутантов задачи (`private-writes-ignores-update-expressions`, `-credits-any-field`, `-accepts-bare-marker`, `-skips-covered-calls`, `room-settings-click-does-not-open`, `hp-dialog-escape-does-not-close`, `config-updated-event-ignored`) найдены на месте, якоря актуальны, и я лично прогнал каждый через `mutation-gate.mjs --id=` на HEAD — все 7 «поймано 1 из 1» (см. таблицу). + +## Унаследовано из r1 (не перепроверялось повторным чтением) + +Файлы, чьи блобы побайтно совпадают с материалом r1 (таблица выше — «не менялся»), приняты по r1-документу без повторного чтения кода: разбор G1–G6 гейта, реализация фасада (F1–F4), содержимое фикстуры `demo.html`/`serve.mjs`, содержимое `smoke_area_relocation.mjs`/`smoke_glow.mjs`, контракт `docs/data-hp-contract.json`, юниты `no-new-private-writes.test.mjs`/`hp-test-facade.test.mjs`, документы (`docs/TESTING.md`, `PROCESS.md` §2.7, `AGENTS.md`, `docs/STYLING-HOOKS.md` §7.4) — всё это `docs/reviews/CODE-REVIEW-629-r1.md`, материал `tree 5c7ec84a6816b887e17ab07b53db5c79988cc294`. Я тем не менее лично перезапустил их исполняемые доказательства (гейт, юниты, смоки, мутанты — см. таблицу), поскольку это дёшево и подтверждает, что ребейз не сломал поведение; повторно не читал их код построчно, раз он идентичен. + +## Как проверялось (гейты, лично прогнанные на `d90725815a1b042144bbdca5274a897e6860c68a`) + +| Гейт | Команда | Результат | +|---|---|---| +| Гейт задачи, CLI | `node scripts/no-new-private-writes.mjs --base origin/dev --head HEAD` | 0; «556 добавленных строк в 5 файл(ах), новых записей нет» | +| Сборка (свежий бандл под смоки) | `npm run build` (tsc --noEmit + rollup) | 0 | +| Синхронизация бандла | `node scripts/bundle-sync.mjs`; `node scripts/bundle-tree.mjs dist custom_components/houseplan/frontend` | 0; verified 30 assets; `git status --short` после — чисто (побайтно = закоммиченному) | +| Мутант AC1 | `mutation-gate.mjs --id=private-writes-ignores-update-expressions` | поймано 1/1 | +| Мутант AC2 | `mutation-gate.mjs --id=private-writes-credits-any-field` | поймано 1/1 | +| Мутант AC3 | `mutation-gate.mjs --id=private-writes-accepts-bare-marker` | поймано 1/1 | +| Мутант AC4 | `mutation-gate.mjs --id=private-writes-skips-covered-calls` | поймано 1/1 | +| Мутант AC6 | `mutation-gate.mjs --id=room-settings-click-does-not-open` | поймано 1/1 | +| Мутант AC6 | `mutation-gate.mjs --id=hp-dialog-escape-does-not-close` | поймано 1/1 | +| Мутант AC6 | `mutation-gate.mjs --id=config-updated-event-ignored` | поймано 1/1 | +| Смок AC6 | `node demo/smoke_test_facade.mjs` | 13/13 OK | +| Смок AC10 (изменившийся файл дельты) | `node demo/smoke_grid_snap.mjs` | 35/35 OK | +| Счётчик проверок AC10 | `HP_SMOKE_CHECKS=1 node demo/smoke_grid_snap.mjs` | 35 имён, не сократился относительно r1 | +| core-file-budget (причина переноса атрибута) | `node --test test/core-file-budget.test.mjs` | 7/7 pass | +| AC9 | `node scripts/unused-locals-gate.mjs` | «мёртвого кода нет», все числа равны базе | +| AC9 | `npm run bundle:budget` | 0 (предупреждение о запасе 11172 Б — старый долг #367/#474, не от этой задачи) | +| AC5 проводка | `node scripts/check-inputs.mjs --coverage` | 0 | +| AC11 | `node scripts/check-docs.mjs --screenshots=warn` | passed; WARN о старом отпечатке скриншотов — не от этой задачи (см. «чего не проверял») | +| Связь дифф↔смоки | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «НЕОПРЕДЕЛЁННОСТЬ» — ожидалось ТЗ (§12), закрыто адресной выборкой выше по изменившимся файлам дельты | +| Трейлеры | `git show -s --format='%B'` на всех 4 коммитах | `Issue: #629`, `User-Visible: no` на каждом | +| Версия хука | `since: "1.78.0-beta.2"` vs `package.json` (`1.78.0-beta.1`) и теги (`v1.78.0-beta.1` последний) | впереди на единицу, коллизии нет | + +Плюс чтение кода: `demo/smoke_grid_snap.mjs` целиком (слитый файл), `src/houseplan-card.ts` вокруг строки 10806 (перенос атрибута), commit-сообщения всех 4 коммитов диапазона, `docs/reviews/INDEX.md` (актуализирован `reviews-index.mjs`, строки r1 на месте, r2 добавит шаг публикации). + +## Находки + +Нет ни одной находки High/Medium/Low. + +## Что проверено и корректно + +- Дельта r1→r2 полностью объясняется техническим слиянием при ребейзе (два конфликта: смок с параллельной задачей #642, потолок размера ядра), а не переработкой решения задачи — оба случая прочитаны и перепроверены исполнением выше. +- Продуктовая правка (`data-hp="mode-tab"`) осталась той же самой по семантике (P1/P2 ТЗ): рендерится только вместе с вкладками, не участвует в стилях, `User-Visible: no` корректен на всех коммитах. +- Гейт `no-new-private-writes` не даёт 0 нарушений «случайно»: строки в `smoke_grid_snap.mjs`, добавленные слиянием с #642 (`_editorRuntime.optimizePlans.*`), не являются ни присваиванием (G1), ни одним из четырёх покрытых фасадом вызовов (G5) — гейту действительно нечего ловить, это не дыра в его контракте. +- AC10 для `smoke_grid_snap` не деградировал: список из 12 оставшихся приватных записей — ровно подмножество «состояния жестов», названное ТЗ как не подлежащее переводу, ни одно из запрещённых AC10 полей туда не попало. +- Все 7 мутантов задачи по-прежнему поймано 1/1 — рефакторинг реестра мутантов (append-only слияние с параллельными задачами) не сломал якоря. +- `core-file-budget` зелёный после переноса строки: потолок и факт совпадают, обхода бюджета нет. +- Трейлеры и версия хука в порядке, коллизий с уже выпущенными тегами нет. + +## Чего не проверял + +- Golden не снимал и не сверял: атрибут не рисуется, задача явно не затрагивает рендер (#455, §10 ТЗ). Не изменилось с r1. +- Полную регрессионную выборку харнесса (14+ смоков из хендоффов r1, соседей #642/#627/#617/#618/#615) не перегонял заново: эти смоки зависят от файлов (`demo.html`, `serve.mjs`, `hp-test.mjs`), которые в дельте r1→r2 не менялись (побайтно те же блобы) — уже дважды прогнаны автором на последующих ребейзах и один раз мной в r1. Прогнал вместо этого адресно оба изменившихся в дельте файла (`smoke_grid_snap`, плюс `smoke_test_facade` как независимая проверка фасада) и `core-file-budget` как тест, объясняющий причину правки продукта. +- `performance_smoke` и полную матрицу 267 смоков — предрелizный гейт, не гейт ревью. +- `python -m pytest tests_backend` — правки `custom_components/**/*.py` нет. +- Инварианты модели — задача не трогает геометрию/ссылки на неё. +- `via: 'x'` на настоящем `ha-dialog` (F4) — не исполним на этом стенде (#505 вне скоупа), не менялось с r1. +- `docs:capture`/`docs:accept -- --identical` — WARN о старом отпечатке скриншотов не специфичен для этой задачи (правки `src/` в принципе инвалидируют отпечаток), обязателен только перед кандидатом беты. +- Полный `npm test`/`tsc --noEmit` с нуля не перегонял отдельным шагом сверх того, что уже подтвердил зелёный Validate на этом же SHA (run 36081944233) — по инструкции раунда это разрешённое сужение; `npm run build` я всё же перегнал лично (см. таблицу), чтобы иметь гарантированно свежий бандл под смоки. + +--- + + + +## Материал раунда + +- Ветка: `issue/629-smoke-facade`, вершина `d90725815a1b042144bbdca5274a897e6860c68a`. +- Дерево вершины: `2522ddc3cd83eabf9c987fe99fec5667ff0e8d4c`. +- Блоб-якоря дельты (файлы задачи, отличные от r1): + ``` + blob 76665323ee94911009e8c33e96c0a15bb822bf03 demo/smoke_grid_snap.mjs + blob 5c890e6fd8f9f2ee62bd99d8fe46495c95c0c513 src/houseplan-card.ts + ``` +- Блоб-якоря, унаследованные без изменений (совпадают с `docs/reviews/CODE-REVIEW-629-r1.md`): + ``` + blob 82586f4c8307234f445d8cf462a00494549687ba scripts/no-new-private-writes.mjs + blob 2d1632ef41b62a6834dc9983d71c9801c4e749b8 demo/helpers/hp-test.mjs + blob 1c02e893566812e5920f00a814369b81dd739b0b demo/srv/demo.html + blob 67f7c6250dfe9c40727655a9bfd6b6cd3311a877 demo/serve.mjs + blob a9de89d8a54b0b91755079651dafc20ad662d1fa demo/smoke_test_facade.mjs + blob ff00faff12ef00d1eb7cb9822d37d1645fe07fdd demo/smoke_area_relocation.mjs + blob 07db5f348c3af542d28783ae07636a2b1d065017 demo/smoke_glow.mjs + blob 75abb94555f9c0dd6f1d5644e5d66f15727752dd docs/data-hp-contract.json + blob dc1783b2c1aa38211ce33b2f194ec7de100f48ee test/no-new-private-writes.test.mjs + blob 3d340be6048798e2f7b1310346214af689941253 test/hp-test-facade.test.mjs + ``` +- Вердикт этого раунда: `green` · High 0 · Medium 0 + +--- + + + +## Материал раунда + +- Ветка: `issue/629-smoke-facade`, коммит `d90725815a1b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `2522ddc3cd83eabf9c987fe99fec5667ff0e8d4c` + ``` + git log --all --format='%H %T' | grep 2522ddc3cd83 + ``` +- Тело issue: `00fd8d5cfe7f3778ae654cefa41d7042e6f514b9a75b2db5eaa26c253a73deb9` +- Вердикт конвейера: `green` · High 0 diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 21de010a..c58153f2 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,6 +1,6 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1040, issue: 366. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1041, issue: 366. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| @@ -32,6 +32,7 @@ | #630 | [CODE-REVIEW-630-r1.md](CODE-REVIEW-630-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #629 | [SPEC-REVIEW-629-r1.md](SPEC-REVIEW-629-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #629 | [CODE-REVIEW-629-r1.md](CODE-REVIEW-629-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | +| #629 | [CODE-REVIEW-629-r2.md](CODE-REVIEW-629-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — | | #627 | [SPEC-REVIEW-627-r1.md](SPEC-REVIEW-627-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | избыточное (не противоречивое) условие в AC2; влияние на touch не названо явным пунктом | `docs/TOUCH-SUPPORT.md` | | #627 | [CODE-REVIEW-627-r1.md](CODE-REVIEW-627-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | demo/smoke_danger_confirmation.mjs не переведён на ожидание составного гейта; диалог оп… | `demo/smoke_danger_confirmation.mjs` `src/houseplan-card.ts` `de.ts` `smoke_danger_confirm_branches.mjs` | | #627 | [CODE-REVIEW-627-r2.md](CODE-REVIEW-627-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |