diff --git a/docs/reviews/CODE-REVIEW-86-r2.md b/docs/reviews/CODE-REVIEW-86-r2.md new file mode 100644 index 00000000..c21173d1 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-86-r2.md @@ -0,0 +1,243 @@ +# CODE-REVIEW-86-r2 + +- **Issue:** #86 — Тексты подсказок к настройкам, партия 1 +- **ТЗ:** `docs/specs/086-settings-help-content-party1.md` (актуализировано 2026-08-30, зелёный SPEC-REVIEW-86-r1) +- **SHA материала:** `18ab3779980e1d22f2cdc4a6da12a4ef2c1a4df9` (`origin/dev...HEAD`, 8 коммитов: `fa031920` `5994c929` `b4c5083f` `9e559b10` `be6bd5f6` `4015f7f8` `10f441aa` `18ab3779`) +- **Трек:** полный (нарушен критерий light-track «одна поверхность», зафиксировано автором в шапке ТЗ) +- **Заход:** r2 · блокирующих циклов израсходовано 1/4 (r1 — красный) + +## Почему разбор полный, а не по дельте + +Между r1 (красный, материал `bd27fca1`→`c862de65`) и этим заходом ветка была +**перебазирована на ушедший вперёд `origin/dev`** (комментарий автора: «ветка +перебазирована на актуальный dev `8819e390`»; конфликт возник в +`custom_components/houseplan/frontend/**`, `dist/**`, `docs/images/**`, +`docs/CHANGELOG*.md` — то есть в сгенерированных деревьях и релизных +артефактах, задетых параллельно слитой #383). Предыдущие SHA (`bd27fca1`, +`489cc8b6`, `c862de65`, `8633187d`, `3274ac39`) не существуют в текущей истории +(git rebase их переписал), в чём я убедился лично: `git show -s ` на них +даёт `fatal: ambiguous argument`. Это ровно случай PROCESS.md §2.10/§7.2 «ребейз +на ушедший вперёд dev — после ребейза это другой код», поэтому разбираю AC1–AC9 +заново, а не только находки r1. + +Отдельно: конвейер уже один раз пытался прогнать ревью до этого рейза и +остановился на конфликте («Ревью не запускалось» в issue) — тот заход не +состоялся и бюджет циклов не потратил (в issue это явно записано), поэтому +текущий заход законно `r2`, а не `r3`. + +## Скоуп + +Diff добавляет 11 согласованных Party-1 `hp-help`-подсказок в +`houseplan-editor-runtime.ts` (масштаб пространства, общий Glow-радиус, режим +заливки, роль источника света — обновлён текст пилота #68, управление другими +источниками, общий/локальный север, общий/локальный фон, стиль стен нулевой +толщины, «Показывать скрытые на плане») и дублирует 5 space-подсказок в +отдельном `houseplan-onboarding-runtime.ts`; переводит их на en/ru/de/fr; +удаляет 4 старых `.rhint`; добавляет unit-тесты инвентаря/parity/no-duplicate, +мутационный тест `settings-help-party1-placement-removed`, расширяет +`smoke_help_affordance.mjs` cold-onboarding сценарием; правит `CHANGELOG`×2, +`USER-GUIDE.ru.md`, `TESTING.md`, `docs/specs/README.md`; принимает 6 +golden-эталонов (+2 не связанных с #86, для уже слитой #383) и полный +десятикадровый набор `docs/images/**` после рейза. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **H1** (High) — `docs/images/screenshots.json` подделан, PNG не пересняты | Коммит `18ab3779` заменяет **все 10** PNG (байты изменились) и все 10 `imageSha256`/`sourceSha256` в манифесте, `oxipng: null → "oxipng 10.2.0"` — признак настоящей пересъёмки через `docs:accept`, а не ручной правки JSON; трейлеры `Release:`+`Baseline-Reviewed:` со ссылкой на прогон 33306281294 | `git show 18ab3779 --stat -- docs/images/` (11 файлов, включая JSON); я лично открыл `docs/images/03-space-create.png` и `06-device-editor.png` — на обоих видны новые иконки-подсказки («Scale (grid cell size)» ⓘ, «Zero-thickness walls» ⓘ, «Controls other light sources» ⓘ) | +| **H2** (High) — `golden:verify` красный, 6 эталонов не приняты | Коммит `be6bd5f6` принимает ровно эти 6 (`device-inbox-desktop-en-light`, `device-inbox-desktop-ru-dark`, `device-inbox-narrow-ru-dark`, `toggle-entity-dialog-desktop-en`, `toggle-entity-dialog-mobile-ru`, `space-room-color-popover-desktop-ru`) + 2 несвязанных `furniture-transform-*` для #383, с `Release:`+`Baseline-Reviewed:` (прогон 33305056892) | Я прогнал `npm run golden:verify` лично на `18ab3779` (полный лог, без truncation) — `exitCode=0`, все 6 сценариев `passed`; визуально открыл `device-inbox-desktop-en-light.png`, `toggle-entity-dialog-desktop-en.png`, `space-room-color-popover-desktop-ru.png` — новые ⓘ-иконки на месте, без наложений и обрезки | +| **M1** (Medium, в скоупе) — cold-onboarding путь 5 space-подсказок не доказан исполнением | Коммит `10f441aa` добавляет в `smoke_help_affordance.mjs` реальный запуск `launchColdView()` с пустым `_serverCfg.spaces`, дожидается `_onboardingRuntime && !_editorRuntime`, проверяет инвентарь всех 5 `space.*.help`, открытие одной подсказки, read-only черновика и то, что editor runtime не подгрузился | Я прогнал `node demo/smoke_help_affordance.mjs` лично — `party1ColdOnboardingInventory/HelpOpens/HelpIsReadOnly/KeepsEditorLazy` все `true`, `checkAll` требует `true` по умолчанию (падает при `false`), `OK` | +| **L1** (Low, снят без правки) — `.help`-ключ не совпадает буквально с ключом label | Не задета рейзом и правками r1→r2, решение прежнее в силе | `space.cell_cm.help` рядом с `space.scale_label`, `space.fill_mode.help` рядом с `space.fill_label` — как и в r1 | + +## Унаследовано из r1 + +Так как разбор в этом заходе полный (см. выше — ребейз на ушедший вперёд dev), +формально всё перепроверено заново на текущем SHA, а не унаследовано без +проверки. Из r1 без повторной **аргументации** взято только: +- сам факт, что механизм `hp-help`/`hp-dialog` (#68) не форкнут и не изменён — + перепроверено тем же способом (`git diff` по `src/hp-help*`, `src/hp-dialog*` + пуст) и подтверждено заново, а не принято на слово; +- список Party-1 из 11 ключей и их каноничные RU/EN тексты — сверены заново + построчно с §6 ТЗ и с тестом `issue 86 Party 1 has the exact help inventory`. + +Никакой пункт r1 не принят по одному лишь заявлению автора без собственной +проверки в этом заходе. + +## Как проверялось + +Дешёвые гейты Validate на `18ab3779` формально зелёные (прогон 33306392784), +но при разборе job'ов выяснилось, что тяжёлые job'ы (`Фронтенд: типы, юниты, +мутанты, синхрон бандла`, `Golden-кадры…`, `Смоки в браузере…`) там **skipped +(переиспользованы)** через `scripts/gate-reuse.mjs` — то есть Validate НЕ +прогнал их заново на этом SHA, а сверил хеш входов с прошлым успешным прогоном. +Механизм легитимный (ключ = контент-хеш `src/**`+гарнитура job'а, не имя ветки +или SHA — прочитал `scripts/gate-reuse.mjs`), но раз я это заметил, не +полагаюсь на устную переиспользованную метку и прогнал сам: + +| Гейт | Команда | Результат | +|---|---|---| +| Typecheck | `npx tsc --noEmit` | зелёный | +| Unit | `npm test` | 1611 тестов, 1610 pass, 1 skip (тот же приватный `#281`-фикстур-скип, что и на `origin/dev`, к #86 не относится), 0 fail | +| Build + sync | `npm run build`, `npm run bundle:sync` | ок | +| 3 копии бандла | `cmp dist/houseplan-card.js custom_components/.../houseplan-card.js`, `cmp dist/houseplan-assets.json custom_components/.../houseplan-assets.json` | идентичны (третья копия `demo/srv/assets` пересобрана тем же `bundle:sync`) | +| Bundle budget | `npm run bundle:budget` | initial View 280 304 B гзип, бюджет 300 000 — в норме | +| no-new-any | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | 119 добавленных строк в 3 файлах, новых `any` нет | +| check-docs | `node scripts/check-docs.mjs` | зелёный, и в этот раз обоснованно — см. закрытие H1 | +| Выбор смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 12 «прямое совпадение», 18 «слабая связь» | +| Смоки прямого совпадения (12/12) | `smoke_help_affordance`, `smoke_bg_color`, `smoke_color_picker_consumers`, `smoke_device_inbox`, `smoke_dialog_zombie`, `smoke_disabled_device`, `smoke_esc_dialogs`, `smoke_gs_always`, `smoke_ha_controls`, `smoke_hidden_flag`, `smoke_motion_sense`, `smoke_sun` | все 12 зелёные, включая новый cold-onboarding блок | +| Мутационный гейт задачи | `node scripts/mutation-gate.mjs --id=settings-help-party1-placement-removed` | мутант пойман | +| **Golden** | `npm run golden:verify` (полный лог, без `tail`) | `exitCode=0`, 0 `different`/`missing`, все 6 задетых #86 сценариев `passed` | +| process-gate (офлайн) | `node scripts/process-gate.mjs` | пройден, 0 предупреждений (проверка статуса issue пропущена — нет `--issues`) | + +**Что не прогонял и почему:** +- 18 «слабых связей» из `smoke-select` (общее имя `_config`) — все про + геометрию/локали/optimize, diff их логики не касается, сигнала для + расширения после 12/12 прямых совпадений нет; +- `python -m pytest tests_backend -q` — diff не касается + `custom_components/**/*.py`; +- остальные ~193 браузерных смока и `performance_smoke` — задача не задевает + геометрию/перф-чувствительные пути, полный прогон здесь избыточен (§8: + полные наборы — предрелизный гейт); +- `model-invariants` — diff не трогает геометрию, ссылки, `layout`, + `marker.space`, `open_spans`; неприменимо. + +## Находки + +### M1 — AC7 «browser zoom 200%» не доказан ничем (Medium, в скоупе) + +ТЗ §13 формулирует AC7: «Узкий viewport 390 px **и browser zoom 200%** не +обрезают tooltip/trigger и не меняют stage geometry. Доказательство: smoke + +representative reviewed golden», а план автотестов §14 отдельно требует +golden «light/dark and 200% zoom; full Linux artifact review required». + +Проверил оба канала: + +- `grep -n "zoom" demo/smoke_help_affordance.mjs` — **пусто**: файл проверяет + только `{ width: 390, height: 780 }` с `deviceScaleFactor=1` (2-й аргумент + `launch`), то есть узкий viewport, но никакого 200%-zoom; +- ни один из 6 принятых в рамках закрытия H2 golden-сценариев + (`device-inbox-*`, `toggle-entity-dialog-*`, + `space-room-color-popover-desktop-ru`, см. `demo/golden/matrix.mjs:383-397, + 765-778, 795-797`) не задаёт zoom-эмуляцию; в матрице вообще нет ни одного + сценария, который проверял бы party1-триггеры при 200% browser zoom + (`large-house-zoom-250-dark` — это zoom канвы плана, а не browser-zoom + диалога, и не про эту задачу). + +Находка r1 упомянула эту часть AC7 мимоходом («200%-zoom и «reviewed golden» +часть AC7 не доказана — см. H2»), но фактическое закрытие H2 (`be6bd5f6`) +приняло именно и только эти 6 уже существовавших в плане сценариев — ни один +из них 200%-zoom не тестирует, поэтому проблема осталась ровно там же, где +была, просто перестала быть видна за красным `golden:verify`. + +Прочитал добавленный CSS (`src/styles/dialogs.styles.ts`, новые +`.helpfieldlabel`, `.help-inline-label`, `.device-inbox-filter-help`): все три +класса используют `display: flex/inline-flex`, `flex-wrap: wrap`, +`min-width: 0` и `overflow-wrap: anywhere` на вложенном `