From cd7c53cd974ea2f1bc32ca06c650e207a76750e4 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:15:07 +0000 Subject: [PATCH] docs: review document for #86 Issue: #86 User-Visible: no --- docs/reviews/CODE-REVIEW-86-r3.md | 250 ++++++++++++++++++++++++++++++ 1 file changed, 250 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-86-r3.md diff --git a/docs/reviews/CODE-REVIEW-86-r3.md b/docs/reviews/CODE-REVIEW-86-r3.md new file mode 100644 index 00000000..b7e0d08a --- /dev/null +++ b/docs/reviews/CODE-REVIEW-86-r3.md @@ -0,0 +1,250 @@ +# CODE-REVIEW-86-r3 + +- **Issue:** #86 — Тексты подсказок к настройкам, партия 1 +- **ТЗ:** `docs/specs/086-settings-help-content-party1.md` (актуализировано 2026-08-30, зелёный SPEC-REVIEW-86-r1) +- **SHA материала:** `96bd3e3eccaaa57fae8a46159b90eb0702e2066b` (`origin/dev...HEAD`, 10 коммитов: `009f784d` `82585520` `30df567d` `d2d70df5` `ffd61def` `85e0f09f` `dbfaf48b` `414933e2` `0fc9b985` `96bd3e3e`) +- **Трек:** полный (нарушен критерий light-track «одна поверхность», зафиксировано автором в шапке ТЗ) +- **Заход:** r3 · блокирующих циклов израсходовано 2/4 (r1 — красный, r2 — жёлтый) + +## Почему разбор полный, а не по дельте + +Конвейер привёл ветку к `dev` до этого ревью: поверх материала r2 (`18ab3779`) +легли новые коммиты `origin/dev` (branch base сдвинулась с прежней `8819e390`/`ba568763` +на текущую `d14cf769`, тип `#44`). Это переписало все SHA веткb — я лично +проверил: все восемь SHA, перечисленных в шапке `CODE-REVIEW-86-r2.md` +(`fa031920`, `5994c929`, `b4c5083f`, `9e559b10`, `be6bd5f6`, `4015f7f8`, +`10f441aa`, `18ab3779`), дают `fatal: Not a valid object name` в текущей +истории (`git cat-file -e ` на каждом). Это ровно случай PROCESS.md +§2.10/§7.2 «ребейз на ушедший вперёд dev — после ребейза это другой код», +поэтому разбираю AC1–AC9 заново на `96bd3e3e`, а не только находку r2. + +Я сопоставил новые SHA со старыми по содержимому коммитов (сообщение + +диф), чтобы не потерять картину: `009f784d`→(было `fa031920`, спека), +`82585520`→(`5994c929`, актуализация спеки), `30df567d`→(`b4c5083f`, feat), +`d2d70df5`→(`9e559b10`, no-new-any), `ffd61def`→(`be6bd5f6`, golden accept, +закрытие H2), `85e0f09f`→(`4015f7f8`, docs review r1), `dbfaf48b`→(`10f441aa`, +cold-onboarding smoke, закрытие M1-r1), `414933e2`→(`18ab3779`, docs accept, +закрытие H1), затем новый коммит r2-документа `0fc9b985`, и поверх — новый +для r3 коммит `96bd3e3e` (200%-zoom smoke, попытка закрытия M1-r2). Содержимое +каждой пары (кроме родителя) идентично по `git show -- `, +проверил построчно на трёх крупнейших (`30df567d`, `ffd61def`, `414933e2`). + +## Скоуп + +Diff (`origin/dev...HEAD` за вычетом сгенерированных `dist/**`, +`custom_components/**/frontend/**`) добавляет 11 согласованных Party-1 +`hp-help`-подсказок в `houseplan-editor-runtime.ts` и дублирует 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 и 200%-browser-zoom сценариями; +правит `CHANGELOG`×2, `USER-GUIDE.ru.md`, `TESTING.md`, `docs/specs/README.md`; +принимает 6 golden-эталонов party1 (+2 не связанных с #86, для уже слитой +#383) и полный десятикадровый набор `docs/images/**` после рейза; несёт два +собственных документа ревью (`docs/reviews/CODE-REVIEW-86-r1.md`, +`-r2.md`) как часть истории ветки. + +Единственная содержательная новая работа с момента r2 — коммит `96bd3e3e` +(browser-zoom smoke); всё остальное — тот же контент, что и в r2, просто с +новыми SHA после рейза. + +## Закрытие раунда r2 + +| Находка r2 | Чем закрыта | Где это видно | +|---|---|---| +| **M1** (Medium, в скоупе) — AC7 «browser zoom 200%» не доказан ничем: ни автотестом, ни golden-кадром, хотя ТЗ §13/§14 требует оба канала | **Частично.** Коммит `96bd3e3e` добавляет в `demo/smoke_help_affordance.mjs` реальный browser-смок: `launch({width:390,height:900}, 2)` — тот самый renderer-контракт «780 физических px → 390 CSS px при DPR 2», который сама ТЗ §14 описывает как метод; открывает `gs.bg_mode.help`, проверяет 5 фактов (`zoomApplied`, `triggerInside`, `tooltipInside`, `noHorizontalDialogOverflow`, `stageStable`). Я прогнал `node demo/smoke_help_affordance.mjs` лично — все пять `party1BrowserZoom200*` = `true`, тест не тавтологичен: `inside()` зависит от реального `getBoundingClientRect()`, при отсутствии триггера/тултипа вернул бы `false`. **Golden-часть не закрыта**: `git diff origin/dev...HEAD -- demo/golden/matrix.mjs` — пусто, никакой 200%-zoom/`deviceScaleFactor`-сценарий для party1-диалогов не добавлен ни в r2, ни в r3. См. новую находку M1 ниже — это тот же пробел ТЗ, наполовину закрытый. | + +## Унаследовано из r2 + +Формально разбор в этом заходе полный (см. выше), поэтому ничего не принято +только на основании прежнего документа без собственной проверки на `96bd3e3e`. +Из r2 без повторной аргументации, но с повторной проверкой факта, взято: +- механизм `hp-help`/`hp-dialog` (#68) не форкнут — перепроверено (`git diff + origin/dev...HEAD` по `src/hp-help*`, `src/hp-dialog*` пуст) на текущем SHA, + а не принято на слово из `CODE-REVIEW-86-r2.md`; +- список Party-1 из 11 ключей и их каноничные RU/EN тексты — сверены заново + построчно с §6 ТЗ и с тестом `issue 86 Party 1 has the exact help inventory` + на `96bd3e3e`, прогнан лично; +- содержимое `docs/images/**` — открыл `06-device-editor.png` и + `03-space-create.png` заново глазами на текущем SHA (не переиспользовал + скриншот из r2-документа): оба показывают новые ⓘ-иконки («Controls other + light sources», «Is this device a light source?», «Scale (grid cell + size)», «Zero-thickness walls»). `check-docs.mjs` зелёный на `96bd3e3e`. + +Находка L1 (низкая, снята без правки в r1/r2 — `.help`-ключ не совпадает +буквально с ключом label) не задета ни рейзом, ни новым коммитом; решение +прежних раундов остаётся в силе, повторно не аргументирую. + +## Как проверялось + +Зелёного Validate на `96bd3e3e` на момент ревью не найдено — прогнал дешёвые +и целевые гейты лично: + +| Гейт | Команда | Результат | +|---|---|---| +| 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` | ок, `git status` после — чисто (три копии бандла уже были синхронны) | +| 3 копии бандла | `cmp dist/houseplan-card.js custom_components/.../houseplan-card.js`, `cmp dist/houseplan-assets.json custom_components/.../houseplan-assets.json`, `cmp dist/houseplan-card.js demo/srv/assets/houseplan-card.js` | идентичны (все три) | +| 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` | зелёный (7 файлов, 10 внешних ссылок); визуально перепроверил 2 из 10 кадров лично (см. «Унаследовано») | +| Выбор смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 12 «прямое совпадение», 18 «слабая связь» — тот же список, что и в r2 (diff содержательно не изменился, кроме теста) | +| Смоки прямого совпадения (12/12) | `smoke_help_affordance` (включая новый cold-onboarding и 200%-zoom блоки), `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 зелёные | +| Мутационный гейт задачи | `node scripts/mutation-gate.mjs --id=settings-help-party1-placement-removed` | поймано 1 из 1, гейт-чек (`npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs && node --test ...`) сам по себе чистый | +| **Golden** | `npm run golden:verify` (полный лог, без `tail`, второй прогон отфильтрован `grep` на именованные сценарии) | `exitCode=0`; явно проверил, что все 8 связанных с #86/#383 сценариев (`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`, `furniture-transform-light`, `furniture-transform-dark`) — все `passed`; ни одного `different`/`missing` во всём прогоне | +| DE/FR полный перевод | ручное чтение всех 11 `.help` + `.aria` в `de.json`/`fr.json` | смысл передан полностью, пропусков предложений нет | +| 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 «представительный reviewed golden» для browser zoom 200% так и не появился (Medium, в скоупе) + +ТЗ формулирует AC7 как **двухканальное** доказательство: «Узкий viewport +390 px и browser zoom 200% не обрезают tooltip/trigger и не меняют stage +geometry. **Доказательство: smoke + representative reviewed golden**» (§13), +а план автотестов (§14) отдельно и явно перечисляет golden-требование: +«Golden: … light/dark and 200% zoom; **full Linux artifact review +required**» — отдельной строкой от смок-плана про тот же 200%-zoom +layout-pass. + +r2 закрыл ровно половину: `96bd3e3e` добавил рабочий смок (проверил лично, +см. таблицу выше и «Закрытие раунда r2»). Golden-половина не появилась ни в +r2, ни в этом коммите: + +- `git diff origin/dev...HEAD -- demo/golden/matrix.mjs` — **пусто**, ни один + сценарий не добавлен и не изменён этим diff'ом вообще; +- `grep -n "zoom\|deviceScaleFactor\|dpr2" demo/golden/matrix.mjs` — единственное + совпадение `dpr2: true` относится к неродственному сценарию + `tunnelContinuity` (геометрия проёмов, строка 484), а `large-house-zoom-*` + (строки 130–131, 743–746) — это zoom канвы плана (`zoom: 0.4/2.5`), а не + browser-zoom диалога, и не про #86 (уже отмечено ещё в r2); +- ни один из 8 принятых по #86/#383 golden-кадров (см. таблицу «Как + проверялось») не задаёт `deviceScaleFactor`/увеличенный `fontSize` — + все они сняты при обычном масштабе. + +Это тот же пробел ТЗ, что и M1 из r2, просто теперь наполовину закрыт: сам +факт «trigger/tooltip не обрезаются при 200% zoom» теперь доказан +исполнением (смок), но собственное требование ТЗ о **golden-кадре** с +человеческим ревью реального рендера при 200% zoom остаётся неисполненным +после двух раундов внимания именно к этому AC. Я не могу заменить это +решение своим чтением CSS или доверием к смоку: ТЗ называет golden отдельным +обязательным каналом доказательства, а не альтернативой смоку, и это +продуктовое решение автора ТЗ (какая степень доказательства нужна для +рендер-регрессий), а не техническая деталь реализации, которую покрывает +свобода §7.1. + +**Как воспроизвести:** `git diff origin/dev...HEAD -- demo/golden/matrix.mjs` +→ пустой diff; `grep -n "zoom\|deviceScaleFactor" demo/golden/matrix.mjs` → +только неродственные совпадения. + +**Что нужно:** добавить в `demo/golden/matrix.mjs` минимум один +representative party1-сценарий (например тот же `gs.bg_mode.help` в общих +настройках или один space-диалог) с `deviceScaleFactor: 2` и уменьшенным +`viewport.width` (390 CSS px, как в смоке) для light и dark тем — как прямо +требует §14 — принять через `npm run golden:accept -- --reviewed` на полном +Linux-артефакте, приложить ссылку на прогон. Альтернатива — явно +зафиксировать в ТЗ (правкой документа, не просто в хендоффе) решение +владельца/автора о том, что golden-канал для этого под-AC избыточен и смока +достаточно, с обоснованием; тогда это перестанет быть находкой. Пока такой +записи нет ни в ТЗ, ни в решениях владельца по #86, действующий контракт — +тот, что в §13/§14. + +### L1 — `.help`-ключ не совпадает буквально с ключом видимого label (Low, снимается без правки) + +Унаследовано из ревью ТЗ (SPEC-REVIEW-86-r1) и CODE-REVIEW-86-r1/r2: +`space.cell_cm.help` соседствует с `space.scale_label`, `space.fill_mode.help` +— с `space.fill_label`. Не создаёт двусмысленности контракта, тот же паттерн +уже был у пилотов #68, находится в рамках технической свободы реализации по +PROCESS.md §7.1. Не задето ни рейзом, ни коммитом `96bd3e3e`. Оставляю без +правки. + +## Что проверено и корректно + +- **AC1 (инвентарь).** `grep -oE "_help\('[^']+\.help'\)"` по обоим рантаймам + на `96bd3e3e` — ровно 11 Party-1 ключей плюс существующие 8 пилотных + `marker.*` из #68, новых мест за пределами §6 нет. 5 `space.*`-ключей + идентичны между `houseplan-editor-runtime.ts` и + `houseplan-onboarding-runtime.ts`. Доказано тестом `issue 86 Party 1 has + the exact help inventory` (прогнал, зелёный) и мутационным гейтом + `settings-help-party1-placement-removed` (прогнал лично — мутант пойман). +- **AC2/AC3 (канонический текст, parity).** RU/EN в `en.json`/`ru.json` + побайтово совпадают с таблицей §6 ТЗ — сверил тестом (жёсткий + `assert.equal`, прогнал, зелёный). DE/FR прочитаны целиком лично — передают + тот же смысл без пропусков предложений. `.help.aria` пары присутствуют во + всех 4 словарях, тест `i18n: every literal help call has body and full + aria keys in every language` зелёный (проверяет объединённый + `helpSource` — оба рантайма). +- **AC4 (без дублей).** `marker.controls_hint`, `gs.bg_daynight_hint`, + `gs.north_hint`, `space.zero_wall_help` удалены из всех 4 словарей и обоих + рантаймов — тест `issue 86 removes explanatory hints…` зелёный, `grep` + лично подтвердил отсутствие. +- **AC5 (все поверхности).** General (`gs.glow_radius`, `gs.bg_mode`, + `gs.north`), Space (`space.cell_cm`, `space.zero_wall_style`, + `space.bg_mode`, `space.north`, `space.fill_mode`), Marker + (`marker.controls.help`) и Device catalog + (`device_inbox.show_hidden.help`) — все присутствуют в diff'е шаблонов; + открытие подтверждено `smoke_help_affordance` (все `party1*Inventory` + флаги `true`) и визуально — открыл `06-device-editor.png`, + `03-space-create.png`, а через `golden:verify` подтвердил ещё 6 кадров + (`device-inbox-*`, `toggle-entity-dialog-*`, + `space-room-color-popover-desktop-ru`) без наложений/обрезки. +- **AC6 (input/a11y).** Переиспользует lifecycle #68 без форка; `smoke_help_affordance` + (прогнал, зелёный) покрывает hover/focus/click/Escape/disabled/Popover-fallback, + включая `keyboardFocus: true`. +- **AC7 (узкий viewport 390 px + 200% zoom).** Часть про 390 px доказана и + смоком (`party1ShowHiddenHelpInsideNarrowViewport`), и golden + (`device-inbox-narrow-ru-dark`, `toggle-entity-dialog-mobile-ru`, оба + `passed`). Часть про 200% zoom теперь доказана смоком + (`party1BrowserZoom200*`, все 5 флагов `true`, прогнал лично), но не + golden-каналом — см. находку M1. +- **AC8 (модель не меняется).** Прочитан весь diff `src/**` — ни одного + изменения вне JSX-шаблонов, CSS и `_help()`-геттеров; `strictNumber`, + `touchSpaceDisplay`, `@change`/`@input` обработчики не тронуты. `npm test` + (1610/1611 pass) включает существующие config-serialization снапшоты — без + регрессий. +- **AC9 (партии 2/3 отсутствуют).** `src/**` вне i18n просмотрен построчно — + только `houseplan-editor-runtime.ts`, `houseplan-onboarding-runtime.ts`, + `dialogs.styles.ts`; ни одного `_help()` вне §6 сверх уже существующих + пилотов; в i18n нет ключей `opening.*`/`wallthick.*`/`decor.*`/`device.binding.*` + партий 2/3. +- **Одно число — один источник.** Diff не добавляет и не меняет ни одной + пользовательской величины — только текст подсказок и разметка; правило + неприменимо. `test/single-source-numbers.test.mjs` прошёл в составе `npm test`. +- **Трейлеры и changelog.** `Issue: #86` на всех 10 коммитах; `User-Visible: yes` + только на `30df567d`, в нём же правки обоих changelog (проверил `git show + 30df567d -- docs/CHANGELOG.md docs/CHANGELOG.ru.md`). Коммиты, трогающие + `demo/golden/baselines/**` (`ffd61def`) и `docs/images/**` (`414933e2`), + несут `Release:`+`Baseline-Reviewed:` со ссылками на зелёные прогоны. + `docs/specs/README.md` содержит строку #86 → файл спеки. +- **process-gate.mjs (офлайн).** Пройден, 0 предупреждений. + +## Вывод + +Контент-часть партии 1 (11 подсказок, 4 локали, source contract, +mutation-тест, cold-onboarding поведение) корректна и без регрессий после +второго рейза — я лично пересобрал бандл, прогнал `golden:verify` полным +логом, все 12 прямых смоков и заново открыл документационные кадры, а не +принял их на слово из документа r2. + +Новый коммит `96bd3e3e` реально закрывает исполняемую часть находки r2 (M1): +поведение при 200% browser zoom теперь доказано смоком, который умеет +падать. Но собственное требование ТЗ §13/§14 о **golden-кадре** для этого же +under-AC — отдельный, явно названный канал доказательства — так и не +выполнено ни в r2, ни здесь. High-находок нет, поэтому вердикт снова +жёлтый, а не красный: то же самое требование ТЗ, что и в r2, теперь +закрыто наполовину вместо нуля, и чинится в этом же issue без нового +цикла спецификации.