Files
2026-09-26 08:12:01 +00:00

20 KiB
Raw Permalink Blame History

SPEC-REVIEW-661-r1

  • Issue: https://github.com/Matysh/houseplan-card/issues/661
  • Этап: S4-spec-review (ревью ТЗ, PROCESS.md §2.4)
  • Трек: полный. Автор явно назвал критерии §5, которые задача не проходит: новый UX-контракт (переключатель + новый элемент окружения), i18n (4 словаря), более одной поверхности (View/киоск/space-card + общие настройки + бэкенд + пайплайн ассета) — это разбор по существу, а не «обычный трек без названного критерия», так что выбор полного трека корректен.
  • Материал: тело issue #661 в текущей редакции (после решения владельца по п.7 «слой», blocked снят → S4-spec-review) + все 4 комментария: (1) анализ и SCOPE-сверка, (2) вопросы владельцу 1–6 пачкой, (3) ответы владельца 1–6 внесены в тело + новый вопрос 7 (слой луны относительно бумаги плана), (4) решение владельца 7 + снятие blocked.
  • Заход: r1 · блокирующих циклов израсходовано 0 из 4
  • Роль: ревьюер ТЗ (не автор)

Скоуп ревью

Декоративный элемент «луна в текущей фазе» в окружении .hp-day-cycle-env (View, киоск, houseplan-space-card), расширяющий уже принятое окружение «Следует за Солнцем» (#146): расчёт положения/фазы луны в карточке без бэкенда как источника, глобальный переключатель settings.moon в общих настройках, ленивый чанк moon-runtime, пайплайн ассета дизайнера, бэкенд-валидация/ экспорт/support package, i18n, golden/смоки/мутанты. Редакторы не затрагивает.

SCOPE-проверка (docs/SCOPE.md): прямой строки в Core user jobs (J1–J7) для декоративного ночного неба нет; задача — расширение уже принятого декоративного окружения #146 тем же путём, каким #146 само было принято (ambiance для View, который «is the product» для двух из трёх персон). Из списка «никогда не строить» (автоматизации, энергия, 3D/CAD, дашборд-фреймворк, история) ничего не задевается; из «не скоуп» самого #661 явно исключены звёзды/погода/движение по азимуту — та же граница, что #146 «weather independence». Новой скоуп-дыры не вижу; это продолжение прецедента, а не новое исключение, требующее отдельного владельческого решения о scope-fit сверх уже данных решений 1–7.

Как проверялось

  1. Прочитаны целиком docs/SCOPE.md, AGENTS.md, docs/process/REVIEWER.md; по ссылкам конспекта — PROCESS.md §2.4, §2.5, §4, §7.1, §7.2, §2.10 (раздел «повторный раунд» не применялся — это r1).
  2. Прочитано тело issue #661 целиком (gh issue view 661 --json body) и все 4 комментария (gh issue view 661 --json comments) — история вопросов/ответов владельца воспроизведена выше в «Материал».
  3. Сверены обязательные разделы §7.1: Сценарий, Что человек увидит до/после, Проблема, Скоуп и Не-скоуп, Контракт поведения (C1–C9), UX, Модель данных и миграция, i18n, Критерии приёмки AC1–AC12 с указанным способом доказательства (unit/smoke/golden/backend/ревью кода), План автотестов, Риски, Откат, Release-артефакты, «Принятые предположения» — все на месте.
  4. Технические утверждения ТЗ сверены с реальным кодом (не приняты на слово):
    • RAY_ELEVATION_MIN = 3 — src/sun.ts:500; RAY_FADE_MS/«3° threshold, the 2-second fade» — docs/SUN.md:363-379. C1/C4 корректно переиспользуют готовый порог и канон анимации, а не изобретают новый.
    • resolveDayCycle, bgModeOf, _dayCycleTick (30-секундный таймер) — реальные символы (src/sun.ts:162,730; src/houseplan-card.ts:9811-9847; src/space-card.ts:175-210), не выдуманы.
    • Прецедент ленивого чанка iso-scene-render/_isoSceneRuntime существует (src/houseplan-card.ts, src/styles/iso-tiles.styles.ts) — заявленная аналогия для moon-runtime обоснована.
    • Прецедент модели данных settings.volumetric_view?: boolean + vol.Optional("volumetric_view"): bool (custom_components/houseplan/ validation.py:2258, docs/CONFIG-COMPATIBILITY.md:102-106) — модель settings.moon («ровно true включает, false удаляет ключ») копирует реально существующий, а не гипотетический паттерн.
    • initialViewFiles, бюджет ленивых чанков — реальные термины scripts/bundle-budget.mjs/bundle-tree.mjs; AC8 проверяем тем же механизмом, что уже используется для мебели/локалей/PDF/isometric.
  5. Проверена внутренняя непротиворечивость числовых данных C2 (12 опорных точек JPL Horizons): даты/высота/освещённость AC7 golden-кадров («Москва 2026-10-21T18:00Z: растущая 78 %, высота 23°» и «Сидней 2026-10-14T09:00Z: растущая 13 %, зеркально») совпадают построчно с таблицей C2 без расхождений; освещённость монотонно растёт между 10-10 (новолуние, 0.18 %) и 10-25/26 (полнолуние, ~99.7 %), что подтверждает согласованность заявленных «waxing»-точек друг с другом, а не изолированные придуманные цифры. Допуск AC1 (1,5°/2,5 п.п.) шире максимума, заявленного самим автором по итогам сверки с параллаксом (1,39°/1,77 п.п.) — граница не подогнана постфактум и не настолько узка, чтобы тест был хрупким.
  6. Проверено, что южно-полушарное зеркалирование в AC3 согласуется с C5: Сидней (lat = -33.87 < 0) → «зеркально», «освещена левая сторона» — прямое применение правила C5 «правая при waxing и lat ≥ 0; при lat < 0 — зеркально», без противоречия.
  7. Проверено, что решение владельца 7 (луна только над фоном, план — везде поверх луны) действительно и полностью внесено в C6 (слой внутри .hp-day-cycle-env под бумагой плана) и в AC5/AC7 (проверка elementFromPoint, оба golden-кадра намеренно показывают план поверх части диска) — не осталось расхождения между текстом решения и техническим контрактом.
  8. Проверено, что все продуктовые вопросы (1–6 первой пачки + вопрос 7 о слое) получили ответ владельца, внесены в тело («Решения владельца (26.09)» 1–7), и blocked снят — открытых продуктовых вопросов не осталось.
  9. git diff origin/dev...HEAD --stat пуст, git log --oneline -5 показывает только доковые коммиты предыдущих задач — продуктового кода для #661 нет; гейты (tsc, test, build, смоки, golden, инварианты) неприменимы к стадии spec.

Находки

Ни одной High или Medium. Два Low — сняты ревьюером здесь же, без возврата автору.

  • Low-1 (C4, полнота перечня триггеров анимации). C4 перечисляет конкретные случаи, запускающие 2000-мс переход: пересечение 3° в обе стороны, уход освещённости под порог, смена фазы окружения на/с day. Явно не назван случай, когда луна уже видна ночью и админ переключает settings.moon в общих настройках (или per-space/global bg_mode уходит от daynight) — станет ли исчезновение плавным fade-out на 2000 мс или мгновенным (как «нарушение C1 → луны нет» можно прочитать буквально)." AC5 этот путь не тестирует. Почему Low, а не Medium: это редкое административное действие (не часть ежевечернего наблюдения, ради которого пишется контракт), разумное умолчание очевидно — тот же общий механизм перехода видимости, что и для остальных триггеров C3/C4 (fingerprint visible|k|waxing|mirror, а не отдельный случай для каждого источника изменения), и последствие ошибки — не более чем один лишний/отсутствующий fade на редком пути. Снимаю без возврата на цикл; автору стоит добавить одну обобщающую фразу к C4 при реализации («тот же переход для любого изменения visible, независимо от того, какое условие C1 сработало»), но это не блокирует «Готово к разработке».
  • Low-2 (DoR, явное заявление о touch). §2.5 требует explicit «влияние на touch названо (или явно "нет")»; в тексте ТЗ такой явной строки нет, хотя вывод однозначен: элемент декоративный, pointer-events: none (C6), редакторы (единственная touch-чувствительная поверхность продукта) луну не получают вовсе (Скоуп/C9). Противоречия с TOUCH-SUPPORT.md нет. Снимаю как Low; автору стоит добавить одну строку «touch: нет влияния, pointer-events:none» в Риски или Release-артефакты для полноты DoR-чек-листа будущих ревью, без возврата на цикл сейчас.

Что проверено и корректно

  • Все обязательные разделы §7.1 присутствуют и в правильном порядке; первые два раздела (Сценарий, Что человек увидит) отвечают на вопрос «какая персона, какая поверхность, какой момент» и «что видно, без терминов реализации» — соответствуют требованию §7.1.
  • Скоуп/Не-скоуп разделены чётко и совпадают с решениями владельца и с уже установленной границей #146 («weather independence», отсутствие звёзд/погоды).
  • Контракт поведения C1–C9 самосогласован: пороги (3°, 3 %) переиспользуют существующие константы и канон docs/SUN.md, а не вводят новые магические числа; слой (C6) точно воспроизводит финальное решение владельца 7; ленивая загрузка (C7) и бюджет слоёв (C8) привязаны к существующим бюджетным гейтам (bundle:budget, smoke_daycycle_layer_budget), не к новым придуманным метрикам.
  • Каждый AC1–AC12 однозначен и указывает способ доказательства (unit / unit+ smoke / smoke / golden / backend / ревью кода); граничные значения заданы числом по обе стороны порога (AC2: 2,9°→нет, 3,0°→да; k 0,029→нет, 0,03→да) — ни один AC не сформулирован расплывчато («выглядит нормально», «примерно так»).
  • Модель данных и миграция копируют реально существующий паттерн volumetric_view бит-в-бит (наличие ключа = вкл, отсутствие/false/иное = выкл, false при сохранении удаляет ключ), совместимость со старым фронтендом/бэкендом описана явно и соответствует docs/CONFIG-COMPATIBILITY.md.
  • i18n-таблица дана для ru/en с ключами, de/fr — прямыми переводами в прозе; AC10 требует паритет-теста по всем четырём словарям.
  • Риски называют главный внешний блокер (ассет дизайнера → blocked на S5-ready до получения арта) и бюджет старта (≤500 Б gzip), что закрывает DoR-пункт «влияние на производительность названо».
  • Откат описан на двух уровнях (переключатель выкл / полное удаление модуля) и согласован с политикой «не удалять файл пользователя на догадке» (тут не применимо — луна не создаёт пользовательских файлов) и с правилом SUN.md «обновление не меняет вид существующего плана».
  • Все продуктовые вопросы (1–7) заданы владельцу пачкой с предложенным умолчанием каждый раз, ответы внесены в тело, blocked корректно использовался и снят — открытых вопросов не осталось.
  • Мутанты (7 штук) целятся в конкретные защитные точки контракта (порог, новолуние, зеркалирование, гейт фазы, ленивая загрузка, параллакс, слой) — не для одной лишь «отчётности», а как реальный план для будущего код-ревью.

Чего не проверял

  • Гейты (npx tsc --noEmit, npm test, npm run build, node scripts/check-docs.mjs, смоки, golden:verify, инварианты) — не прогонял: этап spec, git diff origin/dev...HEAD пуст, продуктового кода для #661 ещё нет. Предмет код-ревью после реализации.
  • Не пересчитывал сам укороченный ряд Мёса/топоцентрическую поправку на параллакс против JPL Horizons — доверился заявленной автором сверке (12 точек, максимумы 1,39°/1,77 п.п. с поправкой, до 2,14° без неё) в объёме, достаточном для проверки, что допуск AC1 (1,5°/2,5 п.п.) не подогнан и не чрезмерно узок; независимую эфемеридную проверку не делал.
  • Не проверял существование/качество ассета дизайнера — он ещё не заказан, это явно названный автором внешний риск с блокировкой на S5-ready, а не предмет ревью ТЗ.
  • Не проверял осуществимость нового golden-механизма (поле сценария moon: { clock, latitude, longitude }, window.__hpTest.clock для фиксации часов окружения) построчно по существующему demo/golden/matrix.mjs — это новая инфраструктура, явно отмеченная как «принято предположительно, поменять свободно»; её реализуемость — предмет код-ревью, а не спек-ревью.
  • Не проверял заново продуктовую правомерность решений владельца 1–7 (угол, умолчание переключателя, утренние сумерки, вариант A ассета, порог новолуния, размер, слой) — это прямые владельческие решения, ревьюер их не оспаривает, только сверяет, что технический контракт им соответствует (см. «Как проверялось» п.7).

Вердикт

Обязательные разделы ТЗ полны, каждый AC однозначен и указывает способ доказательства, технические утверждения проверены по реальному коду и не оказались догадками, выданными за факт, продуктовые вопросы закрыты владельцем без остатка. Два найденных Low не влияют на реализуемость и сняты здесь же с запиской для автора, без возврата на цикл.

Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче


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

  • Ветка: dev, коммит 1ecf96d596af — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 213b7d2756d53d06b3dd310ea68ad6c963f20d37
    git log --all --format='%H %T' | grep 213b7d2756d5
    
  • Тело issue: 77ff1c023b01033841f4fffc73cbbb86c5ef3d37b91b3d2260d9538b729dc625
  • Вердикт конвейера: green · High 0