Files
2026-09-30 10:37:20 +00:00

19 KiB
Raw Permalink Blame History

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.


Материал раунда

  • Ветка: issue/661-moon, коммит 349af7da66d1 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: b9213300bbaafff88c296d71923bc79a49c962d6
    git log --all --format='%H %T' | grep b9213300bbaa
    
  • Тело issue: 77ff1c023b01033841f4fffc73cbbb86c5ef3d37b91b3d2260d9538b729dc625
  • Вердикт конвейера: green · High 0