mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 20:29:00 +00:00
@@ -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-<hash>`/`reuse-golden-<hash>`).
|
||||
Прошёл по цепочке источника:
|
||||
- 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). Находок, блокирующих или требующих правки в рамках
|
||||
задачи, нет; три низких замечания сняты с записью.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/718-moon-any-background`, коммит `b0751497d4fa` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `a643212cfa232be612832cd5c4b4a7e37299f0a6`
|
||||
```
|
||||
git log --all --format='%H %T' | grep a643212cfa23
|
||||
```
|
||||
- Тело issue: `71a4be01f5dd57a0c50deb9ea5df0a86585f54efef75a9f66ba7a87d99ce9059`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
Reference in New Issue
Block a user