30 KiB
SPEC-REVIEW-146-r1
- Issue: https://github.com/Matysh/houseplan-card/issues/146
- ТЗ под ревью:
docs/specs/146-four-phase-sun-background.md(коммит558dae9, веткаissue/146-four-phase-sun-background) - Роль: ревьюер ТЗ (не автор), этап
S4-spec-review - Трек: обычный (не
small/trivial) — сложность/риск 8/10, множество поверхностей (full View, kiosk, статическая карточка, defaults, storage migration, import/export, i18n, golden/performance), файл ТЗ вdocs/specs/создан корректно. - Цикл: r1/4
Скоуп ревью
Проверялось соответствие ТЗ:
docs/SCOPE.md— попадание в Core user jobs (J1), отсутствие расширения скоупа за пределы декоративного фона;PROCESS.md§2.4, §2.5 (DoR), §7.1 (обязательные разделы), §3/§12 (запреты, в т.ч. «догадка вместо решения»);AGENTS.md— классы файлов, ветка, трейлеры коммита ТЗ;- каноническому документу подсистемы
docs/SUN.md(действующий непрерывныйdayPhase(), компас-гейт,planDim, wedge-контракт, которые ТЗ обязано либо заменить, либо явно сохранить); docs/USER-GUIDE.ru.md— терминология «Следует за солнцем», раздел «15. Солнце и фон день/ночь»;docs/TOUCH-SUPPORT.md— контракт View/kiosk/editors;docs/CONFIG-COMPATIBILITY.md— паттерн «materialize», идемпотентная миграция, preview vs apply, same-instance/foreign import;docs/ARCHITECTURE.md— существование#101(src/mode-transition.ts) и#131(cold-start), на которые ссылается ТЗ;- полному тексту issue #146 (тело + 5 комментариев: два уточнения владельца, аналитика с Q1–Q3, решение владельца по Q1–Q3, хендофф автора);
- приложенным к issue файлам (
SPECIFICATION.md,interactive-prototype.html,README.md) — на предмет того, что численные визуальные токены ТЗ (палитра, контур, положение декоративного света) не выдуманы, а прослеживаются к реальному источнику, при этом текст issue сохраняет приоритет там, где он расходится с вложениями (сам issue это оговаривает явно).
Как проверялось
- Прочитан весь тред issue #146 через
gh issue view --json body,comments: исходное описание, два уточнения владельца («текст issue приоритетен над вложениями»; «если высота недоступна — ориентируемся по времени суток»), аналитика с тремя батчированными продуктовыми вопросами Q1–Q3 (каждый с явным default), решение владельца «принимаю все предложенные defaults», финальный хендофф автора со ссылкой на коммит558dae9. Вопросы заданы корректно — только продуктовые («как различать рассвет/сумерки», «чем управлять положением света», «что делать с существующими планами»), ни одного технического вопроса владельцу не эскалировано. - Сверены обязательные разделы ТЗ (§7.1 PROCESS.md) построчно — таблица ниже.
- Прочитан
docs/SUN.mdцеликом: подтверждено, что действующийdayPhase()— непрерывная интерполяция поBG_STOPS, зависящая отnorth_deg, сplanDim(brightness(.9)на весь.zoomwrapночью) и 45-секундным CSS transition. ТЗ §3 корректно описывает это как исходную техническую базу для замены (не голословно) и явно требует удаленияplanDim-потребителей (§10, §14.1) и снятия compass-гейта именно с фона (§4.5, §11.4), сохраняя compass-гейт для оконных лучей (§7.1 отдельно, §11.4). - Скачаны и прочитаны все три вложения issue (
SPECIFICATION.md,interactive-prototype.html,README.md) для проверки, что численные визуальные токены ТЗ не изобретены заново:- все 14 hex-цветов §9 ТЗ (
#aabdd1,#e8c8b7,#dce9ef,#cbdce3,#48536c,#9a7380,#111a27,#1f2f3eи др.) присутствуют дословно в CSSinteractive-prototype.html; - все rgba-токены (
horizon,sun,vignette, три уровня outline) таблиц §9 сверены построчно с--hp-horizon,--hp-sun-color,--hp-vignette,--hp-shadow-core/softи--hp-outline-glowпрототипа для каждой из четырёх фаз (day— базовый блок,[data-time="dawn"],[data-time="dusk"],[data-time="night"]) — совпадение полное и в цвете, и в привязке near/mid/far к core/soft/glow; - размер декоративного света
250 CSS px(§8.3) найден буквально (width: 250px; height: 250px) в CSS прототипа; - константы клок-дуги §8.2 (
8 + progress·84,78 − sin(progress·π)·64, диапазон05:00–21:00) совпадают дословно сSPECIFICATION.md§7.1 (sunrise=300,sunset=1260, те же формулы) — не изобретены; - переходная длительность
1100 msиcubic-bezier(.22, .61, .36, 1)(§11.1) совпадают дословно сSPECIFICATION.md§7; - обнаружено, что в CSS прототипа определены (но нигде не применяются)
переменные
--hp-plan-filter/--hp-plan-opacityсbrightness/ contrast/saturate/hue-rotateпо фазам — то есть в самом прототипе есть мёртвый код, который менял бы план. ТЗ (§10, инвариант неизменного плана) корректно не переносит это в контракт и прямо запрещает ровно такой набор фильтров на дереве плана — соответствует тексту issue («сам план не тонируется») иSPECIFICATION.md§5/§12, а не тому единственному месту прототипа, которое им противоречит.
- все 14 hex-цветов §9 ТЗ (
- Прочитан
docs/CONFIG-COMPATIBILITY.mdиdocs/specs/050-config-export- import.md: паттерн «materialize», «same-instance/foreign import», «preview не пишет storage» — не изобретены заново, это термины уже существующей подсистемы, ТЗ §12 использует их правильно. - Прочитан
docs/ARCHITECTURE.md:968(## View/editor transition ownership (#101)) и:720(#131, «initial snapshot does not depend on live-sync subscriptions») — оба issue, на которые ссылается §11.3/§14.9 ТЗ, реальны и описывают именно то поведение, которое ТЗ обязывает сохранить. - Прочитаны действующие i18n-ключи (
src/i18n/ru.json,en.json):gs.bg_daynight,gs.bg_daynight_hint,gs.sun_missing,gs.north_hintсуществуют буквально с теми именами, которые ТЗ §13 требует обновить — ключи не выдуманы, а текущий текст хинта («Needs the compass below») действительно противоречит новому «работает без компаса» и корректно включён в release-артефакты (§18) как подлежащий правке в том же коммите. - Прочитан
docs/USER-GUIDE.ru.md§15 («Солнце и фон день/ночь», строка 899: «Для солнечных функций нужныsun.sunи направление севера»). Подтверждено намеренное расхождение с новым контрактом: ТЗ прямо требует обновить этот раздел в release-артефактах (§18) — не пропущено. - Проверено наличие тестовой инфраструктуры для заявленных способов
доказательства:
test/i18n.test.mjsсуществует (AC12 реалистичен),demo/smoke_sun*.mjs(5 файлов) существуют как база для «targeted production-bundle smoke» (§16.3),npm run golden:verify/golden:accept -- --reviewed— существующие команды (AGENTS.md, PROCESS.md §8) — доказательства AC не ссылаются на несуществующие механизмы. - Пересчитаны границы контракта фазы (§7.2) и fallback (§7.3) на
непротиворечивость:
(-∞,-6]night,(-6,6)dawn/dusk поrising,[6,+∞)day — полное покрытие без пересечения;[300,480)/[480,1080)/[1080,1260)/ остальное — то же самое для минут суток. Обе шкалы математически корректны и совпадают с §4 (решение владельца). - Найдено единственное реальное расхождение внутри самого ТЗ — см. Medium-1 ниже; проверено, что оно не отражено в §21 («принятые технические предположения»), то есть не помечено как оспоримое предположение.
- Проверены трейлеры и class-принадлежность:
git diff --stat origin/dev...HEADпоказывает толькоdocs/specs/146-four-phase-sun- background.mdиdocs/specs/README.md(класс C, ни одного файла класса A — продуктовый код не тронут доS5-ready, правило №1 AGENTS.md соблюдено); коммит558dae9несётIssue: #146иUser-Visible: no— корректно для документа ТЗ.
Обязательные разделы (§7.1 PROCESS.md)
| Раздел | Есть | Комментарий |
|---|---|---|
| Сценарий (персона/поверхность/момент) | ✅ | §1 — домашний администратор/kiosk, View и kiosk, ежедневный взгляд на дом |
| Что человек увидит до/после | ✅ | §2, «До:»/«После:» — см. Low-2 (длина и термины реализации) |
| Проблема (с подтверждённой причиной) | ✅ | §3, семь пунктов, все сверены с docs/SUN.md (см. «Как проверялось» п.3) |
| Скоуп / не-скоуп | ✅ | §5 / §6, явные границы (без нового mode-токена, без geo/weather API, без editor-контракта) |
| Контракт поведения | ✅ | §7 (источник/фаза) + §8 (декоративный свет) + §9 (визуал) + §10 (инвариант плана) + §11 (переходы/surfaces) |
| Модель данных и миграция | ✅ | §12 — schema не расширяется, идемпотентная миграция, import/export матрица |
| UX, i18n, accessibility, touch | ✅ | §13 — существующие ключи названы поимённо, touch/forced-colors учтены — см. Low-3 |
| AC1…ACn с доказательством | ✅ | §15, 16 штук, у каждого назван способ доказательства и исполнитель |
| План автотестов | ✅ | §16, разбит на unit/backend/smoke/golden с конкретными сценариями |
| Риски | ✅ | §19, таблица вероятность/влияние/мера, 11 строк |
| Откат | ✅ | §20 — пользовательский (выбрать static) и технический (revert) отдельно |
| Release-артефакты | ✅ | §18, конкретный список документов и оба changelog в одном User-Visible: yes коммите |
Все обязательные разделы присутствуют и содержательны. Дополнительно есть раздел «Решения владельца» (§4, дословно фиксирует принятые Q1–Q3) и явный блок «Принятые технические предположения — можно менять без продуктового ревью» (§21, 12 пунктов) — именно то разделение продуктового и технического, которого требует PROCESS.md §7.1. Формулировка §21.12 «Открытых продуктовых вопросов нет» подтверждается содержанием: все три вопроса из аналитики (различение dawn/dusk, источник позиции декоративного света, судьба существующих конфигов) закрыты явными owner-решениями до написания этого файла.
Находки
Medium-1 — azimuth тайно расширяет owner-решение о fallback-триггерах на саму фазу
Файл: docs/specs/146-four-phase-sun-background.md:123-134 (§7.1),
:422-425 (AC2) против :64-69 (§4.1-4.2) и :603-608 (§21.1-21.2)
Решение владельца (issue #146, комментарий 2026-08-14T13:28:11Z) и его
дословный пересказ в §4.1-4.2 ТЗ называют ровно два условия полного
clock-fallback: «Если elevation или rising отсутствуют/некорректны». Формула
самой фазы (§7.2) действительно использует только elevation и rising —
azimuth в ней не участвует вовсе, он нужен исключительно для позиции
декоративного света (§8.1).
Однако §7.1 («Валидный real-sun sample») вводит третье условие,
отсутствующее в owner-тексте: невалидный/нечисловой azimuth тоже
«атомарно» отправляет в clock-fallback всю day-cycle — то есть и фазу,
которую он математически не определяет. AC2 закрепляет это как обязательный
unit-тест («отсутствие/garbage любого из elevation/azimuth/rising атомарно
включает clock-fallback»), делая расширение постоянной частью контракта.
Воспроизведение: sun.sun с валидными elevation=20 и rising=true
(должно быть day, если бы azimuth не гейтил фазу отдельно), но
azimuth=NaN (например, временный сбой конкретной интеграции, которая
считает elevation/rising отдельно от azimuth) — по букве §7.1/AC2 card
покажет фазу по местным часам браузера, а не day, хотя есть
корректные реальные данные для однозначного определения фазы. Владелец
такой случай не разбирал: его ответ на Q1 говорит только о недоступности
высоты или направления движения.
Это ровно тот класс дефекта, о котором предупреждает PROCESS.md §7.1: «Догадка, записанная как факт, — худший вид дефекта: она проходит ревью, потому что выглядит решением». Расширение не отмечено в §21 как «принятое техническое предположение, можно менять свободно» — то есть выдано за решённый факт, а не за предположение, которое ревьюер вправе оспорить.
На практике реальный sun.sun от ядра HA всегда обновляет azimuth и
elevation одним циклом (оба поля или есть, или оба недоступны при
unavailable), поэтому вероятность живого расхождения низкая — это и
удерживает находку на уровне Medium, а не High: она не ломает заявленный
сценарий и не требует возврата ТЗ на цикл.
Решение ревьюера: Medium, заведён отдельный issue
#147 со ссылкой на
#146, метки bug/P3/S1-new. Не блокирует S5-ready; решается либо
уточнением владельца, либо явным переносом пункта в §21 при реализации.
Low-2 — «что человек увидит» длиннее одной фразы и использует термины реализации
Файл: docs/specs/146-four-phase-sun-background.md:30-42 (§2)
PROCESS.md §7.1 требует «одной фразой, без терминов реализации». Раздел
написан четырьмя предложениями и содержит конкретные implementation-термины
(«1100 ms», «alpha-aware контур», «локальные часы браузера»). По существу
требование выполнено — читатель понимает, что видно до/после, без
двусмысленности — но это дальше от буквы правила, чем прецедент в
SPEC-REVIEW-141-r1 (тот был просто «два предложения вместо одного», без
числовых деталей реализации).
Решение ревьюера: Low, не блокирует. Косметическая правка на усмотрение автора при следующей редакции.
Low-3 — нет буквальной touch-editor метки по docs/TOUCH-SUPPORT.md
Файл: docs/specs/146-four-phase-sun-background.md:369-389 (§13)
docs/TOUCH-SUPPORT.md требует явную метку Touch editor: supported /
best effort / not exposed от «новых спецификаций editor-фич». Задача
#146 editor-фичей не является (editors явно в не-скоупе, §6, и §13 прямо
говорит «Editor остаётся desktop-first и не получает новых действий») —
формально требование правила не адресовано этому ТЗ буквально. Тем не менее
одна строка Touch editor: not exposed сняла бы даже формальное сомнение.
Решение ревьюера: Low, не блокирует. Необязательная косметическая правка.
Что проверено и корректно
- Соответствие
docs/SCOPE.md: задача закрывает J1 («показать дом одним взглядом» — фон как декоративный временной контекст без искажения цветов пола/стен/устройств) и не расширяется на смежные Core user jobs; явно не трогает J2/J3/J5/J7 и не создаёт нового публичного mode-токена сверх утверждённогоstatic | daynight(§6). - Легитимность полного трека: сложность/риск 8/10, множество
поверхностей, влияние на миграцию/i18n/perf/touch — критерии
small/trivial(§5/§5.1 PROCESS.md) не выполняются ни по одному пункту, полный трек и файл вdocs/specs/выбраны верно. - Продуктовые вопросы закрыты по процессу, без утечки технических
вопросов владельцу: Q1 (dawn/dusk внутри одного диапазона высоты), Q2
(источник позиции декоративного света), Q3 (судьба существующих
конфигов) — каждый в форме «что неясно · что изменится · default»,
batched одним комментарием, с явным
blocked+S3-specдо ответа. Технических вопросов владельцу не задано ни одного. - Технический диагноз §3 не голословен — все семь пунктов сверены с
docs/SUN.mdи совпадают (см. «Как проверялось» п.3). - Визуальные токены §9 не выдуманы — построчно прослежены к
interactive-prototype.html(см. «Как проверялось» п.4); опасный побочный путь прототипа (--hp-plan-filterна плане) правильно проигнорирован, а не перенесён в контракт. - Инвариант неизменного плана (§10) прямо запрещает весь набор
color-фильтров и overlay на дереве плана и требует удаления
dayPhase().planDim— устраняет главный риск регресса, названный в таблице рисков (§19, строка 1). - Совместимость (§12) корректно использует существующий паттерн
«materialize» вместо runtime-fallback (
docs/CONFIG-COMPATIBILITY.md); явно разведены три сценария (существующий global без поля →static; явныйdaynight→ новая семантика того же токена; новое пространство в старой установке →daynight) — ровно то, что запросил владелец в Q3, без скрытой пятой ветки. - Import/export (§12.4) использует существующую терминологию подсистемы
(
docs/specs/050-config-export-import.md: preview/apply, same-instance/ foreign) корректно, не изобретая параллельный механизм. - Архитектурные ссылки на #101/#131 точны — оба существуют в
docs/ARCHITECTURE.mdи описывают именно то поведение (owner transition timeline; cold-start без второго рендера), которое ТЗ обязывает сохранить. - i18n (§13, §18) называет существующие ключи (
gs.bg_daynight,gs.bg_daynight_hint,gs.sun_missing,gs.north_hint) поимённо и корректно определяет, что их текущий текст («нужен компас») противоречит новому контракту и должен быть обновлён — без добавления третьего публичного режима. - Non-scope (§6) корректно отсекает соседние соблазны: геолокация/
weather API, ручной phase selector, изменение геометрии/порогов оконных
лучей, day/night в редакторах, персистентность вычисленной фазы,
production
?time=— каждый с обоснованием, почему не эта задача. - AC1–AC16 однозначны и снабжены способом доказательства из
допустимого по §2.5 PROCESS.md набора (
unit/backend/smoke/golden/ «ревью кода», плюс «backend migration tests» и «pixel regression» как уточнённые подвидыbackend/golden); ни один AC не оставляет открытым, чем именно он доказывается. - Release-артефакты (§18) перечисляют конкретные существующие
документы (
SUN.md,USER-GUIDE.ru.md,ARCHITECTURE.md,CONFIG-COMPATIBILITY.md,TESTING.md) и оба changelog в одномUser-Visible: yesкоммите — соответствует правилу 11 PROCESS.md; golden корректно ограниченgolden:accept -- --reviewedпо полному Linux- артефакту (§16.4), perf/golden/smoke верно отнесены к пре-релизному, а не implementation-гейту (AC16, §8/§11.4 PROCESS.md). - Откат (§20) корректно опирается на отсутствие необратимой миграции:
явный
static— немедленный пользовательский откат, revert implementation-коммита — технический, без потери данных. - Трассируемость:
docs/specs/README.md:50обновлён тем же коммитом; веткаissue/146-four-phase-sun-backgroundи трейлеры (Issue: #146,User-Visible: no) корректны для документа класса C.git diff --stat origin/dev...HEADне содержит ни одного файла класса A — продуктовый код не тронут доS5-ready(правило №1 AGENTS.md).
Чего не проверял
- Не проверял реализуемость «одного pure resolver» (§14.1) как конкретной структуры данных/API, конкретных имён helper-функций, CSS custom properties или способа crossfade (registered custom properties vs два bounded layer, §21.5) — по тексту ТЗ это явно свободное техническое решение автора кода, не предмет ревью ТЗ.
- Не запускал автотесты,
golden,performanceили browser-смоки — на этапеspecэто не требуется; существование тестовой инфраструктуры (test/i18n.test.mjs,demo/smoke_sun*.mjs, командыgolden:verify/golden:accept) проверено чтением файловой системы иpackage.json, а не исполнением. - Не проверял осуществимость «одного shared fallback ticker» на несколько
карточек (§21.9) по реальному коду
space-card.ts/houseplan-card.ts— это явно помечено в ТЗ как свободное техническое предположение, подлежащее доказательству тестами на этапе реализации/код-ревью. - Не проверял точность числовых оценок аналитики (7/10 · 4/10 · 8/10 · P1) по существу — это поле владельца (PROCESS.md §2.2), уже принято явным решением владельца до написания ТЗ.
- Не проверял детали алгоритма определения «внешней стены» и геометрии
оконных лучей (
RAY_MIN_COS,RAY_FADE_ENDи т.д.) — задача explicitly не меняет эту геометрию (§6), затронута только независимость фона отnorth_deg, что и было предметом проверки. - Не проверял корректность реализации
--hp-plan-filter/--hp-plan-opacityв самом прототипе как продуктового кода — это чужой одноразовый макет (вложение issue), а не часть репозитория; проверено только то, что ТЗ не унаследовало этот фрагмент в свой контракт.
Вердикт
Зелёный. High: 0, Medium: 1 (заведён отдельным issue #147, не блокирует), Low: 2 (косметика, не блокируют, оставлены с записью в этом документе). ТЗ решает заявленный сценарий J1 полностью, визуальные токены прослежены к источнику, а не изобретены, продуктовые вопросы закрыты владельцем по процессу без утечки технических вопросов.