diff --git a/docs/reviews/CODE-REVIEW-666-r1.md b/docs/reviews/CODE-REVIEW-666-r1.md new file mode 100644 index 00000000..759f486a --- /dev/null +++ b/docs/reviews/CODE-REVIEW-666-r1.md @@ -0,0 +1,177 @@ +# CODE-REVIEW-666-r1 + +**Issue:** [#666](https://github.com/Matysh/houseplan-card/issues/666) — «Шапка: крестик × активного редактора — внутри подсветки вкладки» +**Трек:** `trivial` (короткий) · **Заход:** r1 · **Блокирующих циклов:** 0 из 2 +**Материал:** `3bdd3c5163d05c1717bb719dda8f081d4c4ae5a7` (1 коммит поверх `dev` `42f437ca`), рабочая копия — на нём +**Вердикт:** зелёный · High: 0 · Medium: 0 + +## Скоуп + +Один коммит, класс A+B: `src/styles/chrome.styles.ts` (заливка активной вкладки +растянута на `::after` поверх промежутка группы и слота ×), `demo/smoke_toolbar_stable_width.mjs` +(пиксельные пробы), `scripts/mutation-registry.mjs` (мутант +`toolbar-active-highlight-stops-at-tab`), `docs/CHANGELOG.md`/`.ru.md`, +`docs/images/screenshots.json` (только отпечаток источника). DOM +(`src/houseplan-card.ts`, `editorClose`) не тронут — контракт #647/#660 +(`slot.previousElementSibling === активная вкладка`) сохранён. + +Три AC из тела issue: +- **AC1** — заливка покрывает промежуток и слот ×, за слотом заливки нет. +- **AC2** — геометрия (`.modetab`, `.editor-close-slot`, `.closex`, ширины) не изменилась. +- **AC3** — цвет × на заливке = цвет текста вкладки. + +## Как проверялось + +### Гейты, подтверждённые Validate на этом SHA (не перегонял) + +Validate run [36239668263](https://github.com/Matysh/houseplan-card/actions/runs/36239668263) — +`completed success`, `heavy=false` (проверил сам, `gh run view --json jobs` + +лог job «Классификация изменённых файлов»: `heavy=false`, `mutants_requested=true`, +ручной `workflow_dispatch`). Зелёные job на этом SHA: +- «Фронтенд: типы, юниты, мутанты, синхрон бандла» — `tsc --noEmit`, `npm test`, `bundle-policy --verify`, `no-new-any`. +- «Предполёт: документация, провенанс, процесс» — `check-docs.mjs` и трейлеры. +- «Мутанты по диффу» (6/6 шардов) — включая новый мутант. В логе явно: + `ok чистый прогон: node demo/smoke_toolbar_stable_width.mjs` (все 6 шардов) и + `ok toolbar-active-highlight-stops-at-tab: заявленный тест покраснел на мутанте` + (шард 3/6). Это исполнение, не заявление автора — я прочитал сырой лог сам. + +**Важно:** job «Смоки в браузере» и «Golden-кадры» на этом прогоне — +`skipped` (не `success`), потому что `heavy=false` (ручной запуск с +`full=false`). Значит браузерный смок-набор и полный golden на этом SHA +Validate **не прогонял** — это моя обязанность по инструкции ревью, и я её +выполнил ниже. + +### Гейты, прогнанные мной лично в этом ревью + +| Гейт | Команда | Результат | +|---|---|---| +| Build (для смока/golden нужен свежий бандл) | `npm run build` | ok, 22.4s | +| Синхрон копий стенда | `node scripts/bundle-sync.mjs` | ok | +| Смок из AC (браузер, целевой) | `node demo/smoke_toolbar_stable_width.mjs` | `OK` — все ширины (1400…390) × все редакторы (plan/devices/decor): `highlightCoversGapAndSlot`, `highlightStopsAtSlot`, `crossReadsOnHighlight` — везде `true`; существующие геометрические проверки (`widthAndTabsStable`, `closeSlotLivesInsideModes`, `crossHitTarget`, `edgeClickCloses` и т.д.) — тоже `true` | +| Связь диффа со смоками | `node scripts/smoke-select.mjs --base 42f437ca --head 3bdd3c51` | `НЕОПРЕДЕЛЁННОСТЬ` (0 смоков связано доказуемо — чисто CSS-дифф без символов, которые отслеживает выборка). Не разрешение ничего не гонять: смок, названный в AC (`smoke_toolbar_stable_width`), прогнан отдельно и напрямую (выше) | +| Golden, полный набор (диф трогает рендер) | `npm run golden:verify` | 163 `passed`, **12 `different`** (список и разбор — ниже) | + +**Полный список 12 `different`:** `panel-wide-plan-editor-dark-en`, +`room-label-parity-plan-dark`, `room-label-parity-plan-light`, +`geometry-plan-editor-dark`, `safe-resize-handles-clamp-light`, +`safe-resize-handles-clamp-dark`, `tray-medium-group-en`, +`tray-medium-selection-ru`, `furniture-variants-dark`, +`furniture-variants-light`, `room-temperature-dialog-desktop-en`, +`decor-color-popover-desktop-en`. + +Я не поверил на слово диагностике автора (один кадр `geometry-plan-editor-dark`, +1153 px) и разобрал **все 12** через `artifacts/golden/golden-report.json` плюс +собственный скрипт на канале различий (маркер diff-пикселя — `rgb(255,0,180)`, +объявлен в `demo/golden/run.mjs:374-376`; серые «неотличающиеся» пиксели идут с +альфой 90 и не в счёт). Для каждого кадра построил bounding box маркированных +пикселей: + +| Сцена | differingPixels / ratio | bbox маркера | +|---|---|---| +| panel-wide-plan-editor-dark-en | 926 / 0.09% | x∈[456,491] y∈[75,105] | +| geometry-plan-editor-dark | 926 / 0.09% | x∈[771,806] y∈[20,50] | +| room-label-parity-plan-dark | 926 / 0.10% | x∈[124,159] y∈[72,102] | +| room-label-parity-plan-light | 1380 / 0.15% | x∈[23,159] y∈[72,102] | +| safe-resize-handles-clamp-light | 1380 / 0.13% | x∈[53,189] y∈[72,102] | +| safe-resize-handles-clamp-dark | 926 / 0.09% | x∈[154,189] y∈[72,102] | +| tray-medium-group-en | 725 / 0.12% | x∈[124,157] y∈[72,98] | +| tray-medium-selection-ru | 725 / 0.12% | x∈[501,534] y∈[67,93] | +| furniture-variants-dark | 725 / 0.10% | x∈[407,440] y∈[74,100] | +| furniture-variants-light | 725 / 0.10% | x∈[501,534] y∈[72,98] | +| room-temperature-dialog-desktop-en | 723 / 0.09% | x∈[125,157] y∈[74,100] | +| decor-color-popover-desktop-en | 723 / 0.12% | x∈[408,440] y∈[72,98] | + +Во всех 12 сценах различие — компактный прямоугольник ~35×30 px в полосе +шапки (`y` 20…105), т.е. ровно зона активной вкладки + промежуток + ×, без +единого пикселя за пределами шапки (не задета геометрия плана, комнат, диалогов, +трея). Затем я визуально сверил `artifacts/golden/actual/panel-wide-plan-editor-dark-en.png` +с `demo/golden/baselines/panel-wide-plan-editor-dark-en.png` (кроп шапки, +масштаб ×4): на baseline «×» — серый, снаружи синей таблетки, отдельно от неё +(в точности баг из тела issue); на actual — «×» белый, внутри одной сплошной +скруглённой заливки без шва, а рамка фокуса теперь обводит вкладку целиком +вместе с ×. Это прямое исполняемое доказательство AC1/C1/C3 — не пересказ +смок-утверждений, а разница пикселей, которую я сам увидел. + +Эталоны (`demo/golden/baselines/**`) **не обновлены** в этом коммите — это +ожидаемо и объявлено в самом ТЗ («Release-артефакты: golden-кадры редакторов… +изменятся… приёмка эталонов — по Linux CI на кандидате беты»), т.е. не +пропуск, а сознательно отложенное действие класса D (PROCESS §12/§13: +сгенерированное коммитится только релизным промоушеном или приёмкой с +доказательством ревью на полном Linux-артефакте). Урок 2026-09-18 +(`docs/LESSONS.md`, CODE-REVIEW-598-r1 H1) требовал не ревьюить поверх +непринятых эталонов, когда приёмка была частью самого AC той задачи — здесь +приёмка explicitly не часть AC #666, поэтому 12 «different» не читаю как +находку, а как подтверждённое, точное, ожидаемое отличие. + +### AC → доказательство (перепроверено чтением) + +| AC | Чем доказан | Чем краснеет | +|---|---|---| +| AC1 (заливка покрывает промежуток и слот) | `smoke_toolbar_stable_width` (прогнан мной) + golden 12/175 сцен, разница подтверждена как заголовочная зона | мутант `toolbar-active-highlight-stops-at-tab` (`right: 0` вместо `calc(-1*(sp-1+slot))`) — в CI **исполнено**: чистый прогон зелёный, на мутанте красный, лог прочитан | +| AC2 (геометрия неизменна) | тот же смок: `widthAndTabsStable`, `closeSlotLivesInsideModes`, `modeToZoomGapMatchesSpec`, `crossHitTarget`, `edgeClickCloses` — все `true` на 7 ширинах | существующие мутанты `toolbar-close-*` не тронуты (новый мутант — отдельный `id`, найденная строка `--hp-editor-close-size: 24px;` в файле уникальна, поэтому её патч продолжает находить цель после переноса объявления в `.modes` — проверено чтением `mutation-registry.mjs:7632`) | +| AC3 (цвет × = цвет текста вкладки) | `crossReadsOnHighlight: true` во всех комбинациях | **не мутировано отдельно** (автор сам это пишет в хендоффе). Я не стал патчить `src/**` (ревьюеру запрещено править продуктовый код), поэтому закрываю чтением: правило `.modetab.active + .editor-close-slot .closex { color: var(--text-primary-color, #fff); }` — единственный источник этого цвета для × на подсветке; без него действует базовое `.editor-close-slot .closex { color: var(--hp-muted); }`, а `--hp-muted` = `var(--secondary-text-color, #8aa0b3)` — заведомо другое значение, чем `--text-primary-color` (`#fff`) в любой теме HA, которую я нашёл в репозитории. Значит `crossColour === tabColour` красится при удалении правила. Отмечаю как «проверено чтением, не исполнением» — самостоятельного мутанта на этот конкретный AC нет, но брешь не приводит к находке: логика однозначна и не зависит от рантайма | + +## Что проверено и корректно + +- Один источник размера слота (`--hp-editor-close-size`) — теперь объявлен + один раз в `.modes`, читается и слотом, и заливкой; никакого дублирования + числа (§8, «одно число — один источник»). +- `isolation: isolate` на `.modes` ограничена этим маленьким контейнером + (только вкладки + слот ×, без вложенных диалогов/оверлеев с собственным + `z-index`) — проверил `grep z-index` по всем `src/styles/*.ts`: ничего + внутри `.modes` не имеет своего позиционирования, конфликтов со стекингом + нет. +- Мобильная ветка (≤ 480 px) не тронута: `.modetab { display: none }` скрывает + и сам таб, и его `::after` разом — подтверждено чтением существующего + медиа-запроса (не менялся) и смоком (`w390_*` — все зелёные, включая + `noCrossInsideTabs`). +- View: в DOM ни один `.modetab` не получает класс `active`, когда + `this._mode === 'view'` (map идёт только по `['plan','devices','decor']`, + `view` там нет) — значит `.modetab.active::after` в принципе не существует + в View; «заливки у слота нет» доказано структурой шаблона, а не только + побочным эффектом стилей. +- Терминология changelog («крестик активного редактора») совпадает с + `docs/USER-GUIDE.ru.md:251` — не изобретена заново. +- Трейлеры коммита: `Issue: #666`, `User-Visible: yes`, оба changelog + (RU+EN) в том же коммите. +- Бандл не закоммичен (класс D, ожидаемо согласно #657 — обычная задача не + тащит `dist/**`). + +## Чего не проверял + +- `pytest tests_backend` — диф не касается `custom_components/**/*.py`, не + запускал. +- `node scripts/model-invariants.mjs` — геометрия модели и ссылки на неё не + менялись (чистый CSS), не запускал. +- Performance-профили — не названы в AC, диф не связан с рендер-циклом + устройств/геометрии. +- Полный `demo/smoke_*.mjs` (276 файлов) — `smoke-select.mjs` не нашёл + доказанной связи (НЕОПРЕДЕЛЁННОСТЬ), а AC называет ровно один смок; прогнал + его напрямую. Остальные не гонял. +- Мутацию AC3 «вручную» (временный патч `src/styles/chrome.styles.ts` и + повторный прогон) — не стал вносить изменения в продуктовый код даже + временно; закрыл AC3 чтением (см. таблицу выше). +- Повторную приёмку golden-эталонов — по объявлению самого ТЗ это + предрелизная (Linux CI/кандидат беты) обязанность, не гейт этого ревью. + +## Итог + +Три AC доказаны — два исполнением (мутант в CI: чистый+красный, смок лично +перепрогнан), один чтением с однозначным обоснованием. Диф локальный, +корректный, без побочных эффектов за пределами объявленной зоны — подтверждено +не только смоком, но и пиксельным разбором всех 12 отличающихся golden-сцен и +визуальным сравнением до/после. Находок нет. + +--- + + + +## Материал раунда + +- Ветка: `issue/666-active-tab-close`, коммит `3bdd3c5163d0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `a24c553fa7d582765d217596bbf20a519a61cf91` + ``` + git log --all --format='%H %T' | grep a24c553fa7d5 + ``` +- Тело issue: `da355dc7a92e263f1b17a812ebde8f2605b9d7332b5cbda5538a299451b24573` +- Вердикт конвейера: `green` · High 0