mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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 без
|
||||
повторного исполнения. Задача готова к очереди на пре-релиз.
|
||||
Reference in New Issue
Block a user