From c39a8fe88ef019bf947e5671d8744e7da0c5b653 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 05:38:04 +0000 Subject: [PATCH] docs: review document for #718 Issue: #718 User-Visible: no --- docs/reviews/CODE-REVIEW-718-r1.md | 235 +++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-718-r1.md diff --git a/docs/reviews/CODE-REVIEW-718-r1.md b/docs/reviews/CODE-REVIEW-718-r1.md new file mode 100644 index 00000000..0f85974c --- /dev/null +++ b/docs/reviews/CODE-REVIEW-718-r1.md @@ -0,0 +1,235 @@ +# CODE-REVIEW-718-r1 + +Материал: `git log --oneline origin/dev..HEAD` и `git diff origin/dev...HEAD`, +SHA `b0751497d4fa7f5174ae2a1f2645858d4f552feb` (ветка `issue/718-moon-any-background`). +Трек `ask`, заход r1, блокирующих циклов 0/4. Мутанты по диффу на материале не +запрашивались (по доктрине #709 мутанты в разработке не гоняются, их проверяет +ночь). + +## Скоуп + +ТЗ (тело issue #718, решения владельца 30.09): луна видна при любом фоне, не +только «Следует за Солнцем»; на статичном фоне — отдельный слой `.hp-moon-sky`; +строка статуса в «Общих настройках» под `gs.moon_hint`. Контракт К1–К10, AC1–AC16, +план автотестов и четыре новых мутанта — всё из тела issue. Спек-ревью прошло +зелёным (r1, «Готово к разработке», находок нет). + +Диапазон коммитов: `b998b0b3` (фича, `User-Visible: yes`, оба CHANGELOG в том же +коммите), `1c495448` (2 golden-эталона, `Baseline-Reviewed`+`Release`), +`c2ca6a55` (docs-only, отпечаток скриншотов), `b0751497` (починка флака AC5 + +повторный отпечаток). Трейлеры `Issue:`/`User-Visible:` на месте во всех +некласс-C коммитах; `User-Visible: yes` ровно там, где меняются оба CHANGELOG. + +## Как проверялось + +Прочитаны построчно: `src/moon.ts`, `src/moon-gate.ts`, `src/moon-runtime.ts`, +`src/houseplan-card.ts` (хуки `_dayCycleTick`/`_syncDayCycleClock`/рендер сцены), +`src/space-card.ts`, `src/space-render.ts`, `src/houseplan-editor-runtime.ts`, +`src/editors/general-settings-dialog.ts`, новый `src/editors/moon-status.ts`, +`scripts/bundle-budget.mjs`, `scripts/mutation-registry.mjs` (4 новых мутанта), +`test/moon.test.mjs`, `test/moon-settings.test.mjs`, `test/bundle-assets.test.mjs`, +`demo/smoke_moon_static.mjs`, `demo/smoke_moon_status.mjs`, +`demo/smoke_daycycle_layer_budget.mjs`, `demo/golden/matrix.mjs`, +`demo/golden/harness.mjs`, `docs/SUN.md`, `docs/CONFIG-COMPATIBILITY.md`, +`docs/USER-GUIDE.ru.md`/`.md`, `docs/CHANGELOG.ru.md`/`.md`, i18n-словари всех +четырёх языков. + +Каждый класс риска по промпту (ux: новые ключи `gs.moon_status_*`) сверен с +AC14 и `test/i18n-dead-keys.test.mjs` — ключи вызываются литералами +(`src/editors/moon-status.ts` держит `MOON_STATUS_KEYS` как объект, не шаблонную +строку), тест это явно проверяет (`doesNotMatch(/gs\.moon_status_\$\{/)`). + +**Таблица AC9 (таблица TЗ) пересчитана вручную** против `moonStatusOf` и +независимо против теста `test/moon.test.mjs` («#718 AC9»), который сверяет +тексты не со словарём, а с буквальной таблицей UX из ТЗ (константа `UX` в +тесте, не импорт из JSON) — так тест не может совпасть с багом в словаре. +Проверены все 14 строк таблицы, включая порядок причин, зажим `min(round,2)` +и прошивку знака (−0 → «0», минус — U+2212). + +**CI.** Validate на `b0751497` зелёный +(https://github.com/Matysh/houseplan-card/actions/runs/36819695788) — +typecheck/unit/build/bundle-policy. Job `smoke` и `golden` в этом прогоне +**skipped** (механизм переиспользования по хешу входов, не формальность — +см. «Гейты» ниже): проверил источники переиспользования предметно, а не со слов +автора. + +## Находки + +Нет High. Нет Medium. + +Низкое, не блокирует, снимаю с записью: +- de/fr тексты статуса используют неразрывный пробел перед «%» (fr — ещё и + перед «:»/«;»), а ru/en — обычный, как в тексте ТЗ. Это стандартная + типографика языка, не ошибка: AC14 требует дословного совпадения только для + ru/en, de/fr «переводятся при реализации» (раздел i18n ТЗ), и тест + `test/moon-settings.test.mjs` сверяет плейсхолдеры, а не байты, для этих + двух языков. Снимаю, автор сам назвал это в «Отклонениях». +- `resolveDayCycle` при статичном фоне считается дважды за рендер (один раз + для `_dayCycleState`/окружения, что даёт `null`, и один раз внутри + `_moonSkyState`). Чистая функция, дёшево, тот же паттерн уже есть у + окружения — не дефект, не AC. +- Диалог после warm revive открывается без строки статуса: автор нашёл это по + ходу реализации и завёл отдельным issue + [#731](https://github.com/Matysh/houseplan-card/issues/731) (`polish`, + `S5-ready`, `track:show`, ссылка на #718) — корректная практика, не находка + этого ревью. + +## Критерии приёмки — доказательства + +| AC | Способ | Проверено | Красный на старом поведении | +|---|---|---|---| +| AC1 | smoke `smoke_moon_static.mjs` | Прочитан: один `.hp-moon.on`, родитель `.hp-moon-sky` — первый ребёнок `.stage`, окружения и `.hp-paper-outline-svg` нет, фон не меняется, проба пикселя #661 (план поверх луны частично) | Да — автор пометил свидетелем, подтверждено: без слоя сцена вообще не рисует `.hp-moon-sky` | +| AC2 | smoke | Чанк не грузится днём/при `moon:false`; появление/исчезновение с фейдом 2с на пуше `sun.sun` | — | +| AC3 | smoke, Part B, часы браузера, `timezoneId:'UTC'` | 17:59→18:00:30 луна приходит, 07:59→08:00:30 (другая дата) та же логика, уход на 40° убирает класс `on` | Да — без тикера статичного фона луна не появилась бы без пуша hass; есть мутант `moon-static-clock-tick-off` (guard — именно этот smoke) | +| AC4 | smoke, переключение вкладок `daynight`↔`static`, превью сегмента в диалоге | bbox совпадает ±1px, `running===0` (без активного `CSSTransition`) | — | +| AC5 | smoke, переход View↔редактор (#101) | `opacity` слоя синхронна `--hp-mode-view-weight` (±0.02 по кадрам), после перехода `.hp-moon` нет, возврат — сразу `opacity:1`. Тест дожидается конца перехода (`!_modeTransitionBusy && !class mode-transition`) — починка флака из r1 (AC5 читался 2 кадра после `setMode`, ловил промежуточный кадр) | — | +| AC6 | smoke, `houseplan-space-card`, свой статичный фон | `.hp-static-stage > .hp-moon-sky > .hp-moon`, стекинг (план поверх части диска), часы браузера без `sun.sun` | — | +| AC7 | smoke `smoke_daycycle_layer_budget.mjs` | Число композитных слоёв со статичной луной равно числу без неё (CDP `LayerTree`) | — | +| AC8 | golden, `ci:golland`-эталоны `static-bg-moon-gibbous-white-light`/`static-bg-moon-crescent-south-dark`, matrix v70 | Harness проверяет `data-moon-k`, правильного родителя по `bgMode`, ждёт `[data-moon-status]`, горизонтальное переполнение на 390px | Эталоны приняты `Baseline-Reviewed` (прогон 36790529482), кадры просмотрены автором, описание совпадает с геометрией К3 | +| AC9 | unit, `test/moon.test.mjs` | Все 14 строк таблицы ТЗ пересчитаны вручную, тексты RU/EN сверены с буквальной таблицей (не со словарём) | Мутанты `moon-status-order`, `moon-status-clamp-off` | +| AC10 | unit | Эквивалентность `status.reason==='shown' ⇔ moonView(...).visible`, каждый час октября, 2 города, 6 вариантов `sun.sun` (5 значений + отсутствие) | — | +| AC11 | smoke (диалог, en) | Текст «Now: shown (24°…, 79% lit)» равен `data-moon-k×100` плана; тумблер не меняет текст; Save неактивна, закрытие без вопроса; смена `sun.sun`/координат меняет причину при повторном открытии | — | +| AC12 | smoke, `page.route` задержка/отказ | Нет строки, пока грузится (подпись = `gs.moon_hint`); приходит ≤2с; отказ — без строки и исключений, Save/Cancel работают; закрыли-открыли — один снимок второго открытия; preloaded чанк — строка в первом кадре | — | +| AC13 | unit + bundle:budget | `lazyMoonFiles` не пересекается ни со стартовым, ни с `lazyEditorFiles` (новая проверка `assertBundleBudget`, новый тест `bundle-assets.test.mjs`); авторский замер +176 Б gzip к стартовому графу (бюджет ТЗ ≤500 Б, потолок 301 066 не поднят) — числа из коммит-сообщения `b998b0b3`, не перепроверялись отдельным прогоном (см. «Чего не проверял») | Прочитано, не исполнено числом | +| AC14 | unit, `test/moon-settings.test.mjs`, `i18n-dead-keys` | 7 ключей во всех 4 словарях, непустые, один набор плейсхолдеров, RU/EN дословно из ТЗ; снимок черновика (`generalDraftKey`) не меняется появлением строки (K7, risk 5) | — | +| AC15 | unit `test/moon.test.mjs` | Инвертированная проверка #661 AC2 («static → nothing» теперь «static → собственный слой»); редактор (`viewWeight:0`) и неявное/невключённое `moon` дают `null`/`nothing`; днём чанк не запрашивается (`host.updates===0`) | Мутант `moon-static-sky-off` | +| AC16 | ревью кода | `docs/SUN.md` (новый раздел «Static background», «Weight», «Status line»), `docs/CONFIG-COMPATIBILITY.md` (раздел Moon переписан под «любой фон»), `USER-GUIDE.ru.md`/`.md` §15 (таблица условий, таблица строк статуса), оба CHANGELOG — все сверены построчно с К1–К10, терминология совпадает с существующим разделом §15 (не изобретена) | Проверено чтением | + +Четыре новых мутанта (`moon-static-sky-off`, `moon-static-clock-tick-off`, +`moon-status-order`, `moon-status-clamp-off`) — `find`-паттерны сверены +построчно с текущим кодом, каждый матчится ровно один раз (например, +`moon-static-clock-tick-off` целится в блок `_syncDayCycleClock`, где следом +идёт `const needsTimer` — в `_dayCycleTick` такого продолжения нет, поэтому +паттерн не задевает соседнюю функцию со строкой `dayCycleClock(...)` дословно +такой же). Браузерный гвард `moon-static-clock-tick-off` корректно добавлен в +`docs/testing-notes/mutation-browser-guards.md` (85→86, итог 201/200) — по +owner-decision #699 это ориентир, а не стена: `BROWSER_GUARD_LIMIT` в +`scripts/mutation-browser-policy.mjs` только предупреждает (`overLimit`), не +красит `--check`. Защита по таблице §2.7 «AC · чем доказан · чем краснеет» для +всех четырёх защитных AC (AC1/AC15, AC3, AC9×2) закрыта именованным мутантом — +пустых третьих столбцов нет. + +## Контракт К1–К10 — сверка с кодом + +- К1 (условия показа, bg_mode исключён) — `moon.ts:moonView`/`moonShownAt` не + принимают `bg_mode`; видимость определяется только фазой/высотой/освещённостью/ + координатами. Подтверждено. +- К2 (daynight без изменений) — `MOON_CSS` только добавляет селекторы + (`.hp-moon-sky` правила), существующее правило `.hp-day-cycle-env{container-type:size}` + слито в общий селектор без потери семантики; сам `.hp-moon` остаётся + последним ребёнком `.hp-day-cycle-env`, когда `sky === undefined` в + `moonLayer`. Golden-сцены #661 не сдвигаются (harness различает ветки по + `scenario.bgMode`). +- К3 (слой) — `.hp-moon-sky` первый ребёнок `.stage`/`.hp-static-stage`, box + `inset:0`, без `z-index`/`filter`/`will-change`; подтверждено разметкой в + `moon-runtime.ts:renderMoonSky` и CSS. +- К4 (движение/переходы) — `opacity` слоя равна `dayCycleWeight` + (`--hp-mode-view-weight`), та же переменная, что у окружения + (`houseplan-card.ts:10729`/`10876` — один источник, не дублированное число); + переключение фона не пересоздаёт элемент (один и тот же `.hp-moon` просто + оказывается в новом родителе в одном рендере). +- К5 (фаза/тикер) — `dayCycleClock(env, sky)` сравнивает окружение целиком + (fingerprint), слой луны — только по фазе; `env`/`sky` взаимоисключающие по + построению (оба используют один и тот же `_effBgMode()`/`daynight`), так что + `env ?? sky` никогда не выбирает не то значение. +- К6 (загрузка чанка) — `withMoon` — одна точка входа с общим `loading`-промисом; + `moonLayer` и `openMoonStatus` оба идут через неё; второго `import()` нет + (проверено и тестом bundle: ровно одно вхождение `__HOUSEPLAN_MOON_RETRY_ASSET__` + осталось неизменным, т.к. строка ретрая не продублирована). +- К7 (строка статуса) — WeakMap `openings` по host, вне `_settingsDialog`; + закрытая до прихода результата открытие — дроп (тест `moon-settings.test.mjs`); + `aria-describedby` есть, `aria-live` нет (тест явно это проверяет). +- К8 (одно число — один источник, §8) — `moonStatusOf`/`moonStatus` используют + тот же `moonShownAt`, что `moonView`; AC10 — явная проверка эквивалентности + на большом наборе точек, а не декларация. +- К9 (бандл) — `assertBundleBudget` получил новую проверку + `lazyEditorFiles ∩ lazyMoonFiles = ∅`, есть негативный тест, который ловит + регрессию (`bundle-assets.test.mjs`). +- К10 (`gs.moon_hint`) — текст поменян на «при любом фоне» во всех 4 языках. + +## Что проверено и корректно + +- Условие видимости луны полностью отвязано от `bg_mode` (К1), при этом + `daynight`/`static` остаётся маршрутизирующим флагом только для выбора слоя + рендера — не смешано с видимостью. +- Mutually exclusive `env`/`sky` построены на одном и том же `_effBgMode()` во + всех трёх рендер-путях (full card, space-render, space-card) — не три + независимых копии условия, которые могли бы разойтись. +- `viewWeight` для слоя — то же вычисленное значение, что идёт в CSS-переменную + окружения (не отдельная константа «1» где-то закралась). +- i18n: литеральные ключи (#502), один набор плейсхолдеров во всех 4 словарях, + RU/EN текст побайтово из ТЗ. +- Трейлеры: `Issue`/`User-Visible` на каждом некласс-C коммите; `User-Visible: yes` + строго с обоими CHANGELOG в одном коммите; golden-коммит несёт + `Release`+`Baseline-Reviewed` (не `-Local`, есть ссылка на настоящий прогон). +- Число «+176 Б gzip» (бюджет стартового графа) и число «79 %» (освещённость в + примерах UX) каждое имеет один источник: бюджет считает один скрипт + (`bundle-budget.mjs` по факту сборки), 79% в тексте статуса и 79% на + `data-moon-k` плана — один и тот же `moonIllumination`, AC10 это явно + проверяет (не два текста с разными числами). + +## Чего не проверял + +- **Смоки и golden не перегонял лично** — опирался на CI. Job `smoke` и + `golden` в Validate на `b0751497` были **skipped** (не ошибка, а штатное + переиспользование по хешу входов, `reuse-smoke-`/`reuse-golden-`). + Прошёл по цепочке источника: + - smoke: источник — прогон + [36818757797](https://github.com/Matysh/houseplan-card/actions/runs/36818757797), + SHA `2ab57fc1` той же ветки, **все 3 шарда зелёные** (включает + `smoke_moon_static.mjs`, `smoke_moon_status.mjs`, + `smoke_daycycle_layer_budget.mjs` — сортировка по имени файла, шардирование + не пропускает ни один файл). + - golden: источник — прогон + [36810137364](https://github.com/Matysh/houseplan-card/actions/runs/36810137364). + Сам прогон завершился `failure` целиком (упал `preflight` и один шард smoke + из-за несвязанного флака `presentedFramesHaveNoWhiteTile`, как и описал + автор), но job `golden` в нём — **success** независимо (шаги/job в этом + конвейере красятся раздельно, см. AGENTS.md). Ключ переиспользования — + хеш содержимого golden-входов, поэтому совпадение ключа с текущим деревом + доказывает побитовое равенство, а не совпадение по времени. + - Не стал перезапускать `smoke-select.mjs --base/--head` вручную: оба + источника reuse — полные прогоны (все смоки, не выборка), что сильнее + выборки по диффу. +- **Бюджетные числа AC13** (+176 Б, +433 Б, +327 Б, hostRefs 4885→4888) взял из + текста коммита `b998b0b3`, не пересчитывал `npm run build` + `bundle:budget` + самостоятельно — Validate (typecheck/test/build) на материале зелёный, а + тест на пересечение графов (новый, проверен как «умеет падать» по diff + `test/bundle-assets.test.mjs`) гарантирует инвариант, которого достаточно + для AC13 первого пункта. Второй пункт (конкретная величина роста ≤500 Б) — + доверился числу автора без перезапуска. +- **Мутанты не прогонял** — по правилам раунда они не гоняются в разработке + (#709); проверил только, что `find`-паттерны реестра соответствуют реальному + коду (построчно, включая уникальность матча). +- **`pytest tests_backend`** не гонял — диффа Python в этой задаче нет + (`vol.Optional("moon"): bool` не менялся, по ТЗ и по факту диффа). +- **`npm run invariants`** не гонял — геометрия плана/комнат не меняется (новый + слой — не геометрическая сущность, стекинг проверен смоками AC1/AC6 через + `elementFromPoint`, не через файл конфигурации). +- **Performance-профили** — не названы в AC, не гонял. +- Не проверял вручную (глазами) сами PNG двух новых golden-кадров — доверился + записи `Baseline-Reviewed` в коммите `1c495448` со ссылкой на прогон и + текстовым описанием каждого кадра. + +## Вердикт + +Зелёный. AC1–AC16 доказаны (автотестом либо разобраны чтением с явной +пометкой), защитные AC закрыты именованными мутантами без пустых столбцов, +контракт К1–К10 сверен построчно с кодом, трейлеры и оба CHANGELOG на месте, +CI на материале зелёный (цепочка reuse прослежена до настоящих исполненных +прогонов smoke/golden). Находок, блокирующих или требующих правки в рамках +задачи, нет; три низких замечания сняты с записью. + +--- + + + +## Материал раунда + +- Ветка: `issue/718-moon-any-background`, коммит `b0751497d4fa` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `a643212cfa232be612832cd5c4b4a7e37299f0a6` + ``` + git log --all --format='%H %T' | grep a643212cfa23 + ``` +- Тело issue: `71a4be01f5dd57a0c50deb9ea5df0a86585f54efef75a9f66ba7a87d99ce9059` +- Вердикт конвейера: `green` · High 0 · маршрут `fix`