diff --git a/docs/reviews/CODE-REVIEW-385-r3.md b/docs/reviews/CODE-REVIEW-385-r3.md new file mode 100644 index 00000000..f66e90fa --- /dev/null +++ b/docs/reviews/CODE-REVIEW-385-r3.md @@ -0,0 +1,161 @@ +# CODE-REVIEW-385-r3 + +- Issue: https://github.com/Matysh/houseplan-card/issues/385 +- Заход: r3 · блокирующих циклов израсходовано до этого раунда: 1/4 +- Материал: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD` + на SHA `e561e5fc5d37d71d583a3c675a83112d50bf3d13` + (`origin/dev` = `fbbea475`, ветка полностью включает `dev` — конфликта + merge-base нет, `S6→S7` без ребейза на этот раз) +- Предыдущий раунд: `docs/reviews/CODE-REVIEW-385-r2.md`, вердикт **жёлтый**, + SHA `2e77fe09a554692bc23bf1f2497c2ae6c8c02059` (SHA назван в документе r2 — + не находка в этот раз; в самом тексте issue-комментария #10 SHA буквально + не напечатан, только в артефакте) + +## Почему разбор по дельте, а не заново + +Единственный новый коммит с момента r2 — `e561e5fc` («fix: drop the +unreachable virtual-radio guard, keep honest evidence (#385 r2-M1)»), +адресующий ровно и только находку r2-M1. Delta не ребейз (`origin/dev` не +продвинулся), не меняет контракт поведения и не задевает новую подсистему — +локальнее самой задачи. По §2.9/§2.10 разбор ограничен дельтой: +переисследуются только AC1 (соседний код в том же файле, проверено, что не +задет) и AC2 (предмет находки); AC3–AC6 наследуются из r2 без повторного +исполнения там, где сами файлы не изменились. + +``` +git diff 2e77fe09a554692bc23bf1f2497c2ae6c8c02059..HEAD --stat +``` +даёт: `src/houseplan-editor-runtime.ts` (11 строк), `demo/smoke_value_face_source.mjs` +(14 строк), плюс синхронный пересобранный класс D (`dist/**`, +`custom_components/houseplan/frontend/**`) и `docs/images/screenshots.json` +(fingerprint bump). Класса A/B за пределами этих двух файлов дельта не +касается; `docs/reviews/CODE-REVIEW-385-r2.md` уже был частью дерева на +момент r2 (коммит `c872c935` лёг между материалом r2 и `e561e5fc`, публикация +документа, не код). + +## Содержание правки + +```diff +- if (d.binding === 'virtual') { +- this.host._markerDialog = { ...d, bindingMode: 'virtual', bindingOpen: false }; +- return; +- } +``` +убран из обработчика `@change` радиокнопки «virtual» (`houseplan-editor-runtime.ts:12251-12266`), +заменён комментарием, почему сброс здесь всегда легитимен. Синхронно из +`demo/smoke_value_face_source.mjs` убрано поле `sameVirtualKeepsSource` и +шаги, которые его вычисляли (искусственная установка sentinel-source + +повторный клик по уже отмеченной радиокнопке). + +Это буквально вариант «а» из предложенных r2 (см. r2, находка Medium): +убрать недостижимый guard и вакуумную строку смока, а не переквалифицировать +AC2 на «проверено чтением» при сохранении мёртвого кода. + +## Как проверялось (гейты, лично на этом SHA) + +| Гейт | Команда | Результат | +|---|---|---| +| typecheck | `npx tsc --noEmit` | чисто | +| unit | `npm test` | 1593 tests, 1592 pass / 0 fail / 1 skipped | +| build + 3 копии бандла | `npm run build`; `npm run bundle:sync` | без diff, `git status --short` пуст после сборки | +| check-docs | `node scripts/check-docs.mjs` | зелёный (7 файлов, 10 внешних ссылок) | +| bundle:budget | `npm run bundle:budget` | initial View 277 994 / 300 000 Б gzip (запас 22 006 Б; на 1 Б меньше, чем в r2 — ожидаемо, дельта убирает код) | +| no-new-any | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | 20 новых строк в 2 файлах (было 23 в r2 — дельта чистит код), новых `any` нет | +| smoke-select | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | тот же результат, что в r2: 33 слабые связи по общему имени `_markerDialog` (НЕОПРЕДЕЛЁННОСТЬ), ни одного прямого совпадения кроме уже прогнанного `smoke_value_face_source.mjs` | +| smoke_value_face_source | `node demo/smoke_value_face_source.mjs` (после `bundle:sync`) | все **15** оставшихся полей `true` (поле `sameVirtualKeepsSource` намеренно удалено вместе с кодом, который оно проверяло) | +| мутант AC1 | `node scripts/mutation-gate.mjs --id=same-binding-click-resets-source` | **1/1 поймано** — соседний, нетронутый дельтой guard кандидат-листа по-прежнему тестируется честно | +| golden:verify | — | не прогонял: `imageSha256` во всех сценариях `screenshots.json` не изменились между r2 и r3 (сверено построчно диффом), поменялся только `sourceFingerprint`/`sourceSha256` — дифф не меняет рендер | +| инварианты модели | — | не прогонял: дельта не трогает геометрию/`layout`/`marker.space`/толщину стен | +| backend pytest (AC5) | — | не прогонял: дельта не касается `import_export.py`, AC5 не затронут — наследуется из r2 без повторной попытки (см. «Унаследовано») | +| мутант AC4, process-gate | — | не прогонял: дельта не касается `process-gate.mjs`, AC4 не затронут | + +## Разбор по AC — только то, что дельта задевает + +**AC1** (клик по тому же кандидату в списке — no-op). Код не тронут дельтой +(`:12305-12325` идентичен r2). Перепроверено мутантом +`same-binding-click-resets-source` — красный при мутации, зелёный без неё — +**всё ещё доказано автотестом, который умеет падать**. Соседство изменённого +кода в том же файле не задело эту ветку — подтверждено чтением диффа и +исполнением мутанта. + +**AC2** (виртуальная радио-ветка). Ровно предмет находки r2-M1. Дельта +убирает guard и вакуумную проверку. Структурное обоснование из r2 +(радиокнопка не эмитит `change`, если уже отмечена — проверено r2 голым +Playwright-тестом на этом же Chromium) **не переисполнялось заново мной**, но +и не должно: удалённый код был доказательством *отсутствия* дефекта, а не +источником риска — с его удалением исчез сам объект спора. Осталось +проверить две вещи: (1) что комментарий, объясняющий инвариант, на месте — +да, в обоих файлах; (2) что удаление не задело `bindingResetToAuto` (реальная +смена на virtual всё ещё сбрасывает) — смок подтверждает, поле `true`. + +Итог: **AC2 закрыт как «проверено чтением/структурным разбором r2, не +автотестом»** — честная запись, а не заявление о тесте, которого нет. Смока +для этой ветки больше не существует, и это правильно: смок на недостижимую +ветку был бы ложным свидетельством, ровно тем, что нашла r2. Это не +регрессия дисциплины «тест умеет падать» — ветки, которую можно было бы +протестировать честно, больше не существует. + +**AC3–AC6**: файлы, которые их доказывают (`devices.ts`, `process-gate.mjs`, +`import_export.py`, гейты), дельтой не затронуты — наследуются из r2 без +повторного исполнения (см. ниже). AC6 (гейты/бюджет) переисполнен целиком +выше в таблице, так как он не привязан к конкретному файлу, а к состоянию +дерева. + +## Закрытие раунда r2 + +| Находка r2 | Чем закрыта | Где видно | +|---|---|---| +| **M1** (Medium, в скоупе): AC2 заявлен доказанным смоком (`sameVirtualKeepsSource`), но ветка структурно недостижима реальным кликом — тест не умеет падать, хендофф заявил не то, что есть | Вариант «а»: guard и вакуумная проверка смока удалены, оставлен объясняющий комментарий; заявление об AC2 неявно понижено до «проверено структурным разбором», без ложной ссылки на тест | `src/houseplan-editor-runtime.ts:12252-12261` (комментарий вместо guard); `demo/smoke_value_face_source.mjs:173-183` (поле и шаги удалены); коммит `e561e5fc`, трейлер `Issue: #385` | +| L1 (Low, r1, оставлена автору): дублирующийся `one(name)` в `process-gate.mjs` | Не в скоупе этой дельты — `process-gate.mjs` не менялся | без изменений, как и раньше | + +## Унаследовано из r2 (`docs/reviews/CODE-REVIEW-385-r2.md`, SHA `2e77fe09a554692bc23bf1f2497c2ae6c8c02059`) + +Принято без повторного исполнения в этом раунде — файлы не менялись дельтой: + +- **AC3** (`src/devices.ts:889-898`, условные спреды в `rewriteMarkerControlReferences`) — юнит-доказательство и разбор r2 в силе. +- **AC4** (`scripts/process-gate.mjs:109-121`, общий `isReleaseCommit`) — юнит со шпионом и мутант `release-proof-computed-for-every-commit` r2 в силе; L1 (дубль `one()`) остаётся неисправленной низкоприоритетной находкой на усмотрение автора. +- **AC5** (`custom_components/houseplan/import_export.py:504-527`, парная нейтрализация при экспорте) — код не менялся с версии, которую r1 гонял исполнением (446 passed), r2 подтвердил чтением; в этой песочнице backend-харнесс так же недоступен (`.venv-backend` отсутствует), исполнением не перепроверял. +- Таблица «Закрытие раунда r1» и заключение «третьего места сброса нет» (grep по `bindingMode:`/`binding: `) — дельта не добавляет новых точек записи `_markerDialog`, наследую без повтора. +- Оценка «одно число — один источник»: задача не вводит новых видимых пользователю величин — в силе, дельта не меняет вывод. + +## Что проверено и корректно + +- Единственный коммит дельты несёт `Issue: #385`, `User-Visible: no` — + корректно: удаление недостижимого кода не меняет наблюдаемое поведение, + ченджлоги не тронуты (и не должны быть). +- Три копии бандла (`dist/`, `custom_components/.../frontend/`, + `demo/srv/assets/`) синхронны после сборки на этом SHA. +- Удаление поля смока не оставляет висячих ссылок: `sameVirtualKeepsSource` + встречается только в исторических документах ревью (r1/r2), не в коде. +- `checkAll` (`demo/serve.mjs:29-31`) итерирует по фактическим ключам + возвращённого объекта — удаление поля не создаёт риска ложно-зелёной + проверки несуществующего ключа. +- Гард кандидат-листа (AC1, нетронутая часть того же файла) не задет дельтой + — подтверждено мутантом. + +## Чего не проверял и почему + +- **Backend pytest** (AC5, `tests_backend/test_ha_import_export.py`) — файл + не в дельте, наследую из r2 (там же не исполнялся из-за отсутствия + харнесса в песочнице, закрыт чтением); полный HA-харнесс — предрелизный + гейт. +- **`golden:verify`** — не прогонял: `imageSha256` во всех сценариях + `screenshots.json` идентичны между r2 и r3, дифф не меняет рендер. +- **Инварианты модели** (`scripts/model-invariants.mjs`) — дельта не трогает + геометрию/`layout`/`marker.space`/толщину стен. +- **Мутант AC4** (`release-proof-computed-for-every-commit`) — `process-gate.mjs` + не в дельте, AC4 не затронут. +- **31 из 33 «слабых» смоков** от `smoke-select.mjs` — та же оценка, что в + r2: общее имя `_markerDialog`, диф не касается ничего вне логики + binding-выбора, покрытой прогнанным `smoke_value_face_source.mjs`. +- **`performance_smoke` и полный `mutation-gate`** — не в AC, дельта убирает + код, а не добавляет чувствительные к перфу пути; предрелизный гейт. + +## Вердикт + +**Зелёный.** Единственная находка r2 (Medium M1) закрыта по существу — +недостижимый код и ложное свидетельство о нём убраны, а не замаскированы; +честная классификация доказательства AC2 восстановлена. High и новых Medium +нет. Разбор ограничен дельтой согласно §2.10: AC1 подтверждён повторно +(мутант в том же файле), AC2 — предмет находки, AC3–AC6 наследуются из r2 без +повторного исполнения. Задача готова к очереди на пре-релиз.