20 KiB
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.
Как проверялось
- Прочитаны целиком
docs/SCOPE.md,AGENTS.md,docs/process/REVIEWER.md; по ссылкам конспекта — PROCESS.md §2.4, §2.5, §4, §7.1, §7.2, §2.10 (раздел «повторный раунд» не применялся — это r1). - Прочитано тело issue #661 целиком (
gh issue view 661 --json body) и все 4 комментария (gh issue view 661 --json comments) — история вопросов/ответов владельца воспроизведена выше в «Материал». - Сверены обязательные разделы §7.1: Сценарий, Что человек увидит до/после, Проблема, Скоуп и Не-скоуп, Контракт поведения (C1–C9), UX, Модель данных и миграция, i18n, Критерии приёмки AC1–AC12 с указанным способом доказательства (unit/smoke/golden/backend/ревью кода), План автотестов, Риски, Откат, Release-артефакты, «Принятые предположения» — все на месте.
- Технические утверждения ТЗ сверены с реальным кодом (не приняты на слово):
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.
- Проверена внутренняя непротиворечивость числовых данных 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 п.п.) — граница не подогнана постфактум и не настолько узка, чтобы тест был хрупким.
- Проверено, что южно-полушарное зеркалирование в AC3 согласуется с C5:
Сидней (
lat = -33.87 < 0) → «зеркально», «освещена левая сторона» — прямое применение правила C5 «правая при waxing и lat ≥ 0; при lat < 0 — зеркально», без противоречия. - Проверено, что решение владельца 7 (луна только над фоном, план — везде
поверх луны) действительно и полностью внесено в C6 (слой внутри
.hp-day-cycle-envпод бумагой плана) и в AC5/AC7 (проверкаelementFromPoint, оба golden-кадра намеренно показывают план поверх части диска) — не осталось расхождения между текстом решения и техническим контрактом. - Проверено, что все продуктовые вопросы (1–6 первой пачки + вопрос 7 о слое)
получили ответ владельца, внесены в тело («Решения владельца (26.09)» 1–7), и
blockedснят — открытых продуктовых вопросов не осталось. 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/globalbg_modeуходит отdaynight) — станет ли исчезновение плавным fade-out на 2000 мс или мгновенным (как «нарушение C1 → луны нет» можно прочитать буквально)." AC5 этот путь не тестирует. Почему Low, а не Medium: это редкое административное действие (не часть ежевечернего наблюдения, ради которого пишется контракт), разумное умолчание очевидно — тот же общий механизм перехода видимости, что и для остальных триггеров C3/C4 (fingerprintvisible|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— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
213b7d2756d53d06b3dd310ea68ad6c963f20d37git log --all --format='%H %T' | grep 213b7d2756d5 - Тело issue:
77ff1c023b01033841f4fffc73cbbb86c5ef3d37b91b3d2260d9538b729dc625 - Вердикт конвейера:
green· High 0