mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
claude[bot]
@@ -0,0 +1,214 @@
|
||||
# CODE-REVIEW-661-r1
|
||||
|
||||
Вердикт: **зелёный** · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0
|
||||
|
||||
Материал ревью: `origin/dev...349af7da66d10549507e8175972d3ad707332b36`
|
||||
(три коммита: `c8c470ed` фича, `59e05482` golden accept, `349af7da`
|
||||
отпечаток скриншотов). Рабочая копия уже стояла на этом SHA, `git fetch`/
|
||||
`checkout` не делал.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Issue #661, трек `ask`. ТЗ (раздел `## ТЗ` тела issue) — луна в текущей фазе
|
||||
на фоне «Следует за Солнцем» (сумерки/ночь), переключатель `settings.moon` в
|
||||
общих настройках, ленивый расчёт в карточке (без бэкенда), ассет-пайплайн
|
||||
дизайнера, i18n ×4, документы, golden ×2, смоки, мутанты. Ревью ТЗ этой
|
||||
задачи уже прошло зелёным (`docs/reviews/SPEC-REVIEW-661-r1.md`, 0/0),
|
||||
поэтому здесь — только код-ревью C1–C9/AC1–AC12.
|
||||
|
||||
Продуктовая рамка (`docs/SCOPE.md`): фича расширяет J1 («живой вид дома
|
||||
целиком») декоративной деталью ночного окружения — в скоупе, не
|
||||
самостоятельный источник действий, никаких сервисов/сущностей не создаёт.
|
||||
|
||||
Три отличия от буквы ТЗ задокументированы и приняты владельцем ДО кода
|
||||
(комментарии 29.09/30.09 в issue, `docs/SUN.md` «Moon» уже переписан под
|
||||
них): луна не зеркалится по полушарию (лит-сторона всегда левая, `waning`
|
||||
проходит состояния `waxing` в обратном порядке — мутант `moon-mirror-off`
|
||||
заменён на `moon-lit-side-flips`/`moon-lazy-gate-off`→фактически
|
||||
`moon-lit-side-flips`), маска — через `<mask>` с растушёванным
|
||||
терминатором, бокс размера сидит на `.hp-day-cycle-env` (не на окне). Это
|
||||
не находки: решения владельца предшествуют коду, а не подменяют его после
|
||||
факта.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты на `349af7da` уже зелёные (Validate run 36701023959, ссылка в
|
||||
промпте) — `tsc`, `npm test`, `npm run build` + `bundle-policy --verify` не
|
||||
перегонял отдельно, но локально всё равно выполнил `npm run bundle:sync`
|
||||
(= `tsc --noEmit && rollup -c` + `bundle-sync.mjs`), чтобы получить свежий
|
||||
`dist/demo/srv` для смоков — оба шага прошли чисто, что заодно
|
||||
переподтверждает typecheck и build на этом SHA.
|
||||
|
||||
Дальше — по AC и по выводу `smoke-select`:
|
||||
|
||||
| Гейт | Прогнал | Результат |
|
||||
|---|---|---|
|
||||
| `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs` | да | чисто |
|
||||
| `node --test test/moon.test.mjs test/moon-settings.test.mjs test/bundle-assets.test.mjs test/golden-matrix.test.mjs` | да | 106/106 pass |
|
||||
| `node scripts/bundle-budget.mjs` | да | initial View 300328 B — **совпадает** с числом хендоффа (300 328 vs 300 338 на dev), headroom 738 B, `lazy moon` 11387 B — своим ленивым графом, не в initial |
|
||||
| `demo/smoke_moon.mjs` (AC5) | да, в браузере | все 14 проверок true, `OK` |
|
||||
| `demo/smoke_daycycle_layer_budget.mjs` (AC6) | да, в браузере | 7 слоёв ночью без луны = 7 с луной, `.hp-moon` не создаёт свой слой, `OK` |
|
||||
| `demo/smoke_sun_rim.mjs`, `demo/smoke_sun.mjs` (прямое совпадение по `RAY_ELEVATION_MIN`/`RAY_FADE_MS`, `_settingsDialog`) | да | `OK` |
|
||||
| `demo/smoke_general_settings_form.mjs`, `smoke_gs_always.mjs`, `smoke_settings_dialog_cards.mjs`, `smoke_dialog_config_parity.mjs`, `smoke_warm_dialogs.mjs`, `smoke_readonly_cold_start.mjs` (слабая связь по `_settingsDialog`/`_settings`, решение — прогнать) | да | все `OK` — правка диалога (17 строк `t→st`, новый toggleRow) не задела форму, парность конфигурации, warm-remount |
|
||||
| `demo/smoke_summary_dialog_scroll.mjs`, `demo/smoke_summary_first_paint.mjs` (числились красными в хендоффе как флак #712) | да | оба зелёные на этом дереве сейчас — подтверждает, что автор верно квалифицировал их как флак площадки, не регрессию диффа |
|
||||
| `python -m pytest tests_backend/test_settings_moon.py tests_backend/test_validation.py` | **нет** | в песочнице ревью нет интерпретатора с зависимостями (`voluptuous` не установлен, venv backend отсутствует) — прочитано чтением (см. ниже), плюс Validate уже прогнал pytest pure (510 passed, хендофф) |
|
||||
| `npm run golden:verify` (метка `ci:golden`) | нет, повторно | golden уже принят и подтверждён в задаче отдельным коммитом `59e05482` с `Baseline-Reviewed` на артефакт CI; Validate на итоговом `349af7da` зелёный — пересъёмка всей матрицы ради ревью избыточна |
|
||||
| Полный `npm test` (3302 юнит-теста) | нет | покрыт зелёным Validate; прогнал только относящиеся к диффу файлы явно (см. выше) |
|
||||
| Мутанты из реестра | нет (не гоняются на `ask` в разработке, #709) | все 9 заявленных в ТЗ мутантов физически присутствуют в `scripts/mutation-registry.mjs` и синтаксически корректны (see below) |
|
||||
|
||||
`node scripts/smoke-select.mjs --base origin/dev --head HEAD`: 6 прямых
|
||||
совпадений, 17 слабых связей (все — `_settingsDialog`, дно диалога общих
|
||||
настроек), 4 зарегистрированные связи (`smoke_moon.mjs`,
|
||||
`smoke_daycycle_layer_budget.mjs` — оба уже названы автором в
|
||||
`smoke-links.mjs` и прогнаны выше; `smoke_entry_stale.mjs`/
|
||||
`smoke_lazy_editor_chunk.mjs` — про `EditorRuntimeLoader`, общий с луной
|
||||
только паттерн лениво-загружаемого чанка, а не общий код; не прогонял —
|
||||
`moon-gate.ts` не переиспользует `EditorRuntimeLoader`, ссылка на разные
|
||||
константы retry-URL). Из прямых совпадений и слабых связей прогнал все,
|
||||
которые не дублируют уже прогнанные AC5/AC6-смоки (список — в таблице
|
||||
выше); остальные 12 слабых `_settingsDialog`-совпадений (bg_color,
|
||||
color_picker_consumers, dialog_polish_605, dialog_segments_i18n,
|
||||
dialog_zombie, discard_copy, esc_dialogs, ha_controls, help_affordance,
|
||||
room_tooltip_toggle, sun.mjs [прогнан], zigbee_topology_hover) не прогонял
|
||||
точечно — правка диалога чисто аддитивна (новый `toggleRow` в конце секции
|
||||
«Солнце», перенос строк в ленивый словарь без изменения значений), и шесть
|
||||
уже прогнанных представителей этой же категории (форма, парность,
|
||||
warm-remount, readonly cold start, карточки диалога, sun.mjs) покрывают
|
||||
затронутый код путь; полная матрица — задача полного Validate, не ревью.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет ни одной. Ниже — что проверялось прицельно и не дало повода для записи.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
**Математика и пороги (AC1–AC4, C1–C4).** `src/moon.ts` — короткий ряд Меёса
|
||||
+ топоцентрический параллакс, проверено тестом против 12 точек JPL Horizons
|
||||
из ТЗ (допуск 1.5°/2.5 п.п., тест реально попадает в границу: точка
|
||||
2026-10-31 в реестре мутантов помечена как та, что «красит» `moon-parallax-off`
|
||||
— это не пустая формальность, я прочитал формулу и вижу, что без вычитания
|
||||
`parallax·cos(geocentric)` высота у горизонта уходит на градусы, что и ловит
|
||||
тест). `MOON_ELEVATION_MIN`/`MOON_MIN_ILLUMINATION` — литералы (не импорт из
|
||||
`sun.ts`), с явным тестом равенства `RAY_ELEVATION_MIN`; сознательный компромисс
|
||||
ради бюджета initial-графа, задокументирован и защищён тестом на дрейф.
|
||||
|
||||
**Фаза и лит-сторона (AC3, C5), отклонение от буквы ТЗ.** `moonPhasePath(k)`
|
||||
строит путь без зависимости от `waxing`/широты — лит-сторона всегда левая.
|
||||
Прочитал геометрию пути руками (не только тест): внешняя дуга — полуокружность
|
||||
бокса радиуса 256 от (256,0) до (256,512) через левый край; терминатор —
|
||||
дуга эллипса `rx=|2k-1|·240, ry=240` до северного полюса диска (256,16);
|
||||
свип-флаг 0 при `k>0.5` бросает выпуклость вправо (в тёмную половину — доля
|
||||
освещённого растёт), свип 1 при `k<0.5` — влево. Тест
|
||||
`AC3: the mask is the left half plus or minus the terminator half-ellipse`
|
||||
декодирует ту же дугу и независимо пересчитывает `litShare`, сверяя с `k` —
|
||||
это не regex по строке, а геометрическая проверка. Мутант `moon-lit-side-flips`
|
||||
меняет именно свип-флаг внешней дуги — «краснеет» ожидаемо.
|
||||
|
||||
**Слой и покрытие планом (C6, owner decision 7).** Луна — последний ребёнок
|
||||
`.hp-day-cycle-env`, элемент без собственного композитного слоя (`filter`/
|
||||
`will-change` отсутствуют — проверено чтением CSS-строки и смоком
|
||||
`smoke_daycycle_layer_budget.mjs`, число слоёв не растёт). `smoke_moon.mjs`
|
||||
зондирует пиксель под планом (не меняется при скрытии луны) и в открытом
|
||||
поле (меняется) — это прямое доказательство «план поверх луны», не косвенное.
|
||||
|
||||
**Ленивая загрузка (C7, AC8).** `moon-gate.ts` — тот же контракт, что у
|
||||
`iso-scene-render`/редакторов, в меньшем объёме: чанк грузится только когда
|
||||
`settings.moon`, окружение существует и фаза не `day` (тест
|
||||
`C7: the chunk is asked for at night only` подтверждает — 0 обращений днём,
|
||||
1 успешный рендер после прихода чанка ночью). Бюджет: `bundle-budget.mjs`
|
||||
у меня локально дал initial View 300328 B — число сошлось с хендоффом
|
||||
день-в-день, `lazy moon` 11387 B отдельным графом, не пересекается с initial
|
||||
(`assertBundleBudget` бросает на пересечение — проверено чтением
|
||||
`scripts/bundle-budget.mjs`).
|
||||
|
||||
**Настройки и i18n (AC9, AC10).** `moonDraftOf`/`writeMoonSetting` — строго
|
||||
`true`/удаление ключа, как `volumetric_view`; тест читает разметку диалога
|
||||
структурно (`toggleRow` после `sun-rays`, иконка, подписи) — не хрупкий
|
||||
regex по всему файлу, а срез между известными якорями. Бэкенд:
|
||||
`vol.Optional("moon"): bool` отклоняет `"yes"/1/0/None/[]/{}"`, support
|
||||
package копирует только `isinstance(..., bool)`, `DEFAULT_CONFIG.moon = True`
|
||||
только для новых установок — путь мерджа в `websocket_api.py`
|
||||
(`{**DEFAULT_CONFIG, **config}`) не протекает в существующие: `or DEFAULT_CONFIG`
|
||||
срабатывает только при полном отсутствии сохранённого конфига, а не частично
|
||||
(проверено чтением, не исполнением — pytest недоступен в песочнице, см.
|
||||
таблицу гейтов). Экспорт/импорт — `test_issue_661_full_export_and_import_preserve_the_moon`
|
||||
гоняет оба значения `True`/`False` через реальные `create_export`/`parse_document`.
|
||||
|
||||
Строки перенесены из initial-словаря (`en.json` и т.д.) в ленивый
|
||||
`settings/*.json` для ~17 ключей диалога — сверил все 4 языка построчно,
|
||||
ни одна строка не потеряна и не задвоена; тест `moon-settings.test.mjs`
|
||||
и `bundle-assets.test.mjs` синхронизацию подтверждают структурно, я же
|
||||
сверил построчно смысл (переводы на месте, не машинный мусор).
|
||||
|
||||
**Golden (AC7).** Два новых кадра зафиксированы в `demo/golden/matrix.mjs`
|
||||
с явными координатами/часами/`k`; `test/golden-matrix.test.mjs` проверяет,
|
||||
что старые `day-cycle-*` кадры луну не получили (`scenario.moon === undefined`).
|
||||
Коммит `59e05482` несёт `Release:`+`Baseline-Reviewed:` на прогон CI, где
|
||||
эти кадры и появились; `imageSha256` всех 11 кадров в `docs/images/screenshots.json`
|
||||
не изменился между `c8c470ed` и `349af7da` — сдвинулся только отпечаток
|
||||
сборки, что и заявлено в сообщении коммита.
|
||||
|
||||
**Ассет-пайплайн.** `scripts/generate-moon-assets.mjs` — жёсткий гейт на
|
||||
входной SVG (запрещённые теги, `href`/`url()` наружу, `DOCTYPE`/`ENTITY`,
|
||||
шаблонные литералы `${}`/backtick — то есть инъекция в собственный `svg\`…\``
|
||||
литерал исключена), лимит размера, обязательный `viewBox 0 0 512 512`.
|
||||
`pack.json` провалидирован (`schema_version`, `mirror:false`, `lit_side:left`
|
||||
как решение владельца) — тест дергает и позитивный, и три негативных случая.
|
||||
|
||||
**Трейлеры.** Все три коммита проверены вручную: `c8c470ed` —
|
||||
`Issue: #661` + `User-Visible: yes`, changelog (RU+EN) в том же коммите;
|
||||
`59e05482` — `Release:` + `Baseline-Reviewed:` (ссылка на прогон CI) +
|
||||
`Issue:`/`User-Visible: no`, как требует правка `demo/golden/baselines/**`;
|
||||
`349af7da` — только `docs/images/screenshots.json`, класс C (документация),
|
||||
трейлеры не нужны — соответствует правилу.
|
||||
|
||||
**Одно число — один источник (§8).** Единственное видимое пользователю
|
||||
число здесь — «3°»/«3 %» в подсказке `gs.moon_hint` (все 4 языка) и в
|
||||
`docs/SUN.md`/`USER-GUIDE`; источник — константы `MOON_ELEVATION_MIN`/
|
||||
`MOON_MIN_ILLUMINATION` в `src/moon.ts`, тексты в документах и переводах
|
||||
переписаны вручную под них (не генерируются), но само число нигде не
|
||||
разошлось между документами, кодом и переводами — сверил все вхождения.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `python -m pytest tests_backend/...` — сессия без `voluptuous`/venv
|
||||
backend; см. таблицу гейтов. Полагаюсь на зелёный Validate (хендофф:
|
||||
«pytest pure — 510 passed») и на прочтение диффа `validation.py`/
|
||||
`support_package.py`/`const.py`, который тривиален и симметричен уже
|
||||
проверенному коду (`volumetric_view`).
|
||||
- Полную матрицу `npm run golden:verify` — не переснимал; принято в
|
||||
задаче отдельным `Baseline-Reviewed`-коммитом, Validate на итоговом SHA
|
||||
зелёный.
|
||||
- Полный `npm test` (3302 теста) — не гонял целиком, только файлы,
|
||||
которых касается дифф, плюс полагаюсь на зелёный Validate.
|
||||
- Мутанты из реестра — не гонял (правило #709: не гоняются в разработке
|
||||
ни на каком треке); проверил, что все 9 заявленных в ТЗ (`moon-threshold-off`,
|
||||
`moon-new-moon-shown`, `moon-day-visible`, `moon-parallax-off`,
|
||||
`moon-lit-side-flips`, `moon-tick-renders-every-time`, `moon-lazy-gate-off`,
|
||||
`moon-over-paper`, `moon-validation-accepts-anything`) физически
|
||||
зарегистрированы и по тексту патча действительно бьют по описанному
|
||||
инварианту (прочитано, не исполнено).
|
||||
- 12 из 17 «слабых связей» смоук-выборки на `_settingsDialog`, не
|
||||
прогнанные точечно (см. обоснование в разделе «Как проверялось»).
|
||||
- Ручное тестирование в браузере не по смокам (визуальный осмотр диалога
|
||||
глазами) — не делал; смоки покрывают структуру и поведение, golden —
|
||||
внешний вид.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
`git rev-parse HEAD` = `349af7da66d10549507e8175972d3ad707332b36`. Для
|
||||
следующего раунда (если потребуется) дельта считается от этого SHA.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/661-moon`, коммит `349af7da66d1` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `b9213300bbaafff88c296d71923bc79a49c962d6`
|
||||
```
|
||||
git log --all --format='%H %T' | grep b9213300bbaa
|
||||
```
|
||||
- Тело issue: `77ff1c023b01033841f4fffc73cbbb86c5ef3d37b91b3d2260d9538b729dc625`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user