Files
houseplan-card/docs/reviews/CODE-REVIEW-666-r1.md
T
2026-09-26 12:11:51 +00:00

16 KiB
Raw Blame History

CODE-REVIEW-666-r1

Issue: #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 — 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