diff --git a/docs/reviews/CODE-REVIEW-745-r1.md b/docs/reviews/CODE-REVIEW-745-r1.md new file mode 100644 index 00000000..5c0580c0 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-745-r1.md @@ -0,0 +1,193 @@ +# CODE-REVIEW-745-r1 + +Issue: #745 · Трек: `show` · Заход: r1 · блокирующих циклов 0/2 +Материал: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD` +SHA материала: `ca708d8443aee387895a0547d4a6c52fb95e816f` (сверено `git rev-parse HEAD` перед итогом — совпадает) +Один коммит на ветке `issue/745-space-card-room-keys` поверх `dev` (`7ce3f662`, #742 уже влит). + +## Скоуп диффа + +``` +demo/smoke_space_card.mjs | 153 +++++++++++++++++++++++++- +docs/CHANGELOG.md | 3 + +docs/CHANGELOG.ru.md | 3 + +docs/testing-notes/mutation-browser-guards.md | 5 +- +scripts/mutation-registry.mjs | 13 +++ +src/space-render.ts | 12 +- +src/styles/plan.styles.ts | 9 +- +7 files changed, 189 insertions(+), 9 deletions(-) +``` + +Ровно поверхность из ТЗ: `src/space-render.ts`. Правка — список фигур комнат +карточки пространства (`space-render.ts:534–570`) стал +`keyed(space.id, repeat(shownRooms, (r, index) => r.id || index, (r) => {…}))` +вместо голого `map()`: буквально та же форма, что #742 поставил в +`houseplan-card.ts:10952` для основной карточки (`src/houseplan-card.ts:10950-10952` +сверено построчно). Переход `.room { transition: 0.12s }` +(`plan.styles.ts`) не тронут — только дополнен комментарий про карточку +пространства. Остальные файлы: свидетель (новый раздел существующего смока, +а не новый файл — как и объявлено «принято предположительно»), один мутант и +его запись в `mutation-browser-guards.md`, CHANGELOG RU/EN. + +## Как проверялось + +Рабочая копия уже стояла на материале (`ca708d84`). Дешёвые гейты (`tsc +--noEmit`, `npm test`, `npm run build`) подтверждены зелёным Validate на +этом SHA ([run 36858825861](https://github.com/Matysh/houseplan-card/actions/runs/36858825861)) — +не перегонял; смоки/golden/мутанты на этом SHA Validate не гонял вовсе +(ветка задачи, пуш не PR/кандидат — политика `validate.yml`: «настоящую +приёмку там делает код-ревью»), поэтому все браузерные проверки ниже я +прогнал сам. + +| Гейт | Результат | Примечание | +|---|---|---| +| `npx tsc --noEmit`, `npm run build` | не перегонял отдельно | Validate зелёный на `ca708d84`; `npm run bundle:sync` ниже всё равно прогоняет `build` как часть сборки бандла для смоков — тоже чисто | +| `npm run bundle:sync` (сборка + копия в `demo/srv/assets`) | ✅ | нужно для честного запуска браузерных смоков — `demo/srv/assets` не обновляется автоматически | +| `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs` | ✅ | тестовая сборка для `test-build/` | +| `npm run gate:small` | ✅ зелёный, 126 с, упало 0 | сборка+typecheck, no-new-any, no-new-private-writes, smoke-select, `npm test` (юниты), bundle-policy --verify, bundle:budget, lint:unused — все шаги `ok` | +| `demo/smoke_space_card.mjs` (новый раздел, AC1–AC2) | ✅ зелёный на правке | прогнан лично на честно собранном бандле; все 9 полей `keyedRooms` совпали с ожидаемыми значениями дословно (см. ниже) | +| Мутант `space-card-rooms-rendered-without-keys` — `node scripts/mutation-gate.mjs --id=space-card-rooms-rendered-without-keys` | ✅ «заявленный тест покраснел на мутанте» | гейт сам применяет патч во временном дереве, собирает и гоняет гард — не доверие авторскому отчёту, а независимое исполнение | +| `node scripts/mutation-gate.mjs --check --changed=origin/dev..HEAD` | ✅ exit 0, `browser guards: 204/200` (WARN, не fail — ориентир, не лимит, #699) | новый id размечен в `mutation-browser-guards.md`, `missingReasons` пуст | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | выполнен | см. решение по строкам ниже | +| `demo/smoke_space_card_bg.mjs` (AC3) | ✅ | | +| `demo/smoke_space_card_decor_capability.mjs` (AC3) | ✅ | | +| `demo/smoke_moon_static.mjs` (AC3) | ✅ | | +| `demo/smoke_room_settings.mjs` (прямое совпадение smoke-select) | ✅ | | +| `demo/smoke_space_switch_transitions.mjs` (companion-свидетель #742, не назван в AC, но делит `plan.styles.ts`) | ✅ | подтверждает, что правка не расшатала прецедент #742 | +| `npm run golden:verify` | не прогонялся | метки `ci:golden` на issue нет; тот же вывод, что и на #742 (`CODE-REVIEW-742-r1.md`: «golden не заказывается») — дефект чисто транзитный (видим только в первом кадре перехода), кадр в покое (стили/разметка/порядок узлов в DOM) не меняется: `keyed`/`repeat` не добавляют обёрточных узлов и сохраняют тот же порядок, что и прежний `map()` | +| `python -m pytest tests_backend` | не прогонялся | дифф не касается `custom_components/**/*.py` | +| `npm run invariants` | не прогонялся | дифф не меняет геометрию комнат и ссылки на неё — только идентичность DOM-узлов списка | + +### `smoke-select` — решение по строкам + +``` +Изменено файлов src/**: 2 · символов проекта на изменённых строках: 2 +Матрица: 286 смоков · порог «широкого» символа: больше 57 смоков + +Прямое совпадение (1): demo/smoke_room_settings.mjs ← roomFillModeOf +Не учитывались как широкие: setConfig +``` + +- **Прямое совпадение (1)** — `smoke_room_settings.mjs` прогнан, зелёный. +- **`setConfig` как «широкий»** — не прогонял отдельно. Совпадение идёт не + из продуктового кода диффа (там `setConfig` не менялся), а из тестового + файла: новый раздел смока сам многократно вызывает `el.setConfig(...)` на + карточке пространства. `setConfig` — общий метод HA-карточек, матрица + отметила бы десятки несвязанных смоков; решение осознанное, не ищу + скрытую связь там, где символ пришёл из теста, а не из правки. + +### Доказательство негатива (свидетель умеет падать) + +Не подменял файл руками — использовал официальный путь: `node +scripts/mutation-gate.mjs --id=space-card-rooms-rendered-without-keys`. Гейт +сам: (1) прогнал чистый смок на правке — `ok`; (2) применил патч мутанта +(внутренний `repeat` заменён на голый `map()`, оставив внешний `keyed`) во +временном дереве, пересобрал бандл и прогнал тот же смок — `ok +space-card-rooms-rendered-without-keys: заявленный тест покраснел на +мутанте». Итог: «поймано 1 из 1». Это ровно AC2 (путь B) — инструмент +подтверждает, что без внутреннего ключа тест красит правильную причину, а +не падает случайно. + +Отдельно глазами проверил баланс скобок патча реестра: исходная строка +держит два открытых `(` (`keyed(`, `repeat(`) и один `{`; патч меняет +`repeat(` на `.map(`, то есть тоже даёт два `(` и один `{` — закрывающая +`}));` на `space-render.ts:570` остаётся синтаксически верной и для +мутанта, и для оригинала (структурная проверка `mutation-gate --check` +подтвердила это же: «якорь найден 1 раз», сборка мутанта не падает +компиляцией). + +## AC — разбор + +| AC | Вердикт | Как доказано | +|---|---|---| +| AC1 (путь A — смена `space` в `setConfig` того же элемента) | ✅ выполнен | Раздел смока: фикстура держит ожидаемую форму (`pathAFixtureHolds: true` — `r1` без заливки/`polygon`, `garden`/`g1` с заливкой/`polygon`), после `setConfig({ space: 'garden' })` и одного кадра: нет `CSSTransition` на `[data-hp="room"]` (`pathANoRoomTransition: []`), ни один старый узел не остался подключён под чужим `data-id` (`pathANoRoomNodeOutlivesTheSwitch: []` — внешний `keyed(space.id,…)` отбрасывает всё поддерево целиком при смене `space`), новый узел `g1` рождается сразу с итоговым `rgb(96, 125, 139) / 0.18` (`pathANewRoomBornInItsFill`, совпадает с `DEFAULT_CUSTOM_FILL` в `src/logic.ts:1301`) | +| AC2 (путь B — событие конфигурации меняет состав комнат; плюс настоящий переход не сломан) | ✅ выполнен, красит мутант `space-card-rooms-rendered-without-keys` | Вставка комнаты первой в `f1.rooms` через `__hpTest.setServerConfig` (путь, которым правка с другого устройства доходит до карточки — то же событие, на которое подписан `onConfigChange` → `_load()` в `space-card.ts`): список вырос (`pathBRoomListGrew: true`), ни один из прежних 4 узлов не подменён (`pathBNoRoomNodeSwapped: []` — внутренний ключ `repeat` по `r.id`), переходов не было (`pathBQuietRooms: []`). Настоящая смена `custom_fill` после этого по-прежнему идёт на том же узле (`pathBRealFillChangeKeepsTheRoomNode: true`) и анимируется (`pathBRealFillChangeAnimates: true`) — это тот самый контроль «ложный фикс `transition:none` не прошёл бы», подтверждён отдельно прогоном `smoke_space_switch_transitions.mjs` (companion #742) | +| AC3 (ничего не сломано, выпуск) | ✅ выполнен | `gate:small` зелёный; `smoke_space_card`, `smoke_space_card_bg`, `smoke_space_card_decor_capability`, `smoke_moon_static` зелёные лично; `smoke-select` учтён построчно; мутант в реестре рядом с #742, пойман своим guard’ом (проверено исполнением, не только `--check`); `mutation-gate --check` зелёный (204/200 — предупреждение по #699, не отказ); CHANGELOG RU/EN в том же коммите, текст дословно совпадает с ТЗ; `golden` не заказан и не нужен (см. таблицу выше) | + +Единственное число, которое дифф делает видимым пользователю дважды — +`DEFAULT_CUSTOM_FILL` (`#607d8b` / `0.18`, `src/logic.ts:1301`): оно не +менялось этим диффом, у него один источник (`logic.ts`), и смок читает его +же значение через `getComputedStyle`, а не дублирует константу независимо +(дублирование есть только в самом свидетеле — `FINAL_FILL` в +`smoke_space_card.mjs`, это сверено вручную при разборе AC1, расхождения +нет). + +## Находки + +Нет. High: 0. Medium: 0. + +## Что проверено и корректно + +- Форма правки — буквально та же, что #742 поставил в `houseplan-card.ts`: + `keyed(space.id, repeat(shownRooms, (r, index) => r.id || index, …))`; + импорты `keyed`/`repeat` добавлены туда, где их не было (`space-render.ts:10-11`). +- Идентификаторы фикстуры в новом разделе смока (`f1`, `garden`, `r1`–`r4`, + `g1`) — не придуманы: это демонстрационные данные проекта, используемые + уже десятками других смоков (`smoke_iso_floor_switch.mjs`, + `smoke_merge_split.mjs`, `smoke_infinite_canvas.mjs` и др.) и реальные + SVG-подложки `demo/srv/assets/f1.svg`, `garden.svg`. +- Пустой `id` ключуется числовым индексом (`r.id || index`) — тот же приём, + что в #742, по `Map`-семантике `repeat` число никогда не совпадёт со + строковым `id`. +- `.room { transition: 0.12s }` не тронут; AC2-проверка настоящей смены + заливки подтверждает, что это не фикс через `transition: none`. +- `roomShapes` используется ровно один раз (`space-render.ts:964`), встроен + в `` тем же способом `${roomShapes}`, что и раньше — `keyed`/`repeat` + не добавляют обёрточных DOM-узлов, порядок фигур не меняется. +- `shownRooms` вынесен в переменную и используется один раз — не тронул + остальные независимые фильтры по `space.rooms` (например, `glowBaseShapes` + ниже по файлу). +- Мутант реестра синтаксически корректен для обеих версий (считал баланс + скобок вручную, см. «Доказательство негатива»), guard пойман исполнением, + а не только текстовой проверкой `--check`. +- Трейлеры `Issue: #745`, `User-Visible: yes` на коммите; CHANGELOG RU/EN + правлены в том же коммите, тексты дословно совпадают с текстом ТЗ. +- `no-new-private-writes`: свидетель пишет конфиг только через существующий + `__hpTest.setServerConfig`, как и заявлено отклонением от ТЗ (п. «Путь B + свидетель проверяет через `__hpTest.setServerConfig`, а не записью в + `_snap.config`») — подтверждено прогоном соответствующего шага `gate:small`. +- Рабочая копия репозитория после всех проверок чиста (`npm run + bundle:clean`, `git status --short` пуст, `HEAD` = `ca708d84`). + +## Маршрут вердикта (§5) + +Все критерии пройдены: сложность и риск низкие (копия уже проверенного на +#742 паттерна), одна поверхность (`src/space-render.ts`), нет миграции +конфига, нет нового UX-контракта (устраняется баг в рамках уже описанного +поведения), нет влияния на производительность/touch-контракт, ожидаемое +поведение зафиксировано тем же правилом #525/#534/#742 и текстом в CHANGELOG. +`route: fix`. + +## Чего не проверял + +- Полную сверку бандла из трёх независимых копий — не требовалась: Validate + зелёный на этом SHA подтверждает `build`; локальная пересборка (`bundle:sync` + для смоков + `--id` мутанта) прошла ещё дважды без расхождений. +- 285 «слабых»/неотобранных смоков вне прямого совпадения `smoke-select` — + единственная прямая связь (`smoke_room_settings`) прогнана; `setConfig` + как источник совпадения разобран и сознательно не расширен (см. выше). +- `golden:verify`, `pytest tests_backend`, `npm run invariants`, + performance-профили — не требуются по диффу/AC (нет правки Python, нет + правки геометрии комнат, метки `ci:golden` нет, перф не затронут). Решение + по golden совпадает с прецедентом #742 на идентичном паттерне правки. +- Ручная проверка в браузере (глазами) — доказательство построено на + `getComputedStyle`/`getAnimations`, то есть на тех же сигналах, которые + увидел бы человек на первом кадре; отдельно её не повторял. +- Пакетное/ночное ревью, release-review — вне объёма этого раунда (§8: + объём гейтов соразмерен задаче). + +--- + +--- + + + +## Материал раунда + +- Ветка: `issue/745-space-card-room-keys`, коммит `ca708d8443ae` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `799d5ef74e6afa50d64480013db2799c42b848d0` + ``` + git log --all --format='%H %T' | grep 799d5ef74e6a + ``` +- Тело issue: `8c17362760b68e61c8c6b00cda0e18656cacfb312c80e5828034f3cc79599971` +- Вердикт конвейера: `green` · High 0 · маршрут `fix`