From 27517db8a4b933e54368927851f76352567d5568 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 08:12:01 +0000 Subject: [PATCH] docs: review document for #661 Issue: #661 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-661-r1.md | 214 +++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-661-r1.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 0839dfb4..0a1ebfff 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,9 +1,10 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1077, issue: 380. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1078, issue: 381. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| +| #661 | [SPEC-REVIEW-661-r1.md](SPEC-REVIEW-661-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #660 | [SPEC-REVIEW-660-r1.md](SPEC-REVIEW-660-r1.md) | spec · r1 | 🔴 красный | 2 | 1 | Раздел ## ТЗ в теле issue отсутствует целиком; Изменение прямо противоречит двум местам; AC «расстояние уменьшено ровно вдвое» не | `docs/process/AUTHOR.md` `REVIEWER.md` `test/core-file-budget.test.mjs` `scripts/smoke-select.mjs` `demo/helpers/hp-test.mjs` `docs/UX-MODES.md` `docs/reviews/SPEC-REVIEW-647-r1.md` | | #660 | [SPEC-REVIEW-660-r2.md](SPEC-REVIEW-660-r2.md) | spec · r2 | 🔴 красный | 1 | 0 | Скоуп п.3 переносит крестик внутрь .modes, но .modes | `src/styles/chrome.styles.ts` `src/houseplan-card.ts` `docs/USER-GUIDE.ru.md` `docs/UX-MODES.md` `src/houseplan-editor-runtime.ts` `src/header-menu.ts` `smoke_mobile_view_header.mjs` | | #660 | [SPEC-REVIEW-660-r3.md](SPEC-REVIEW-660-r3.md) | spec · r3 | 🟢 зелёный | 0 | 0 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-661-r1.md b/docs/reviews/SPEC-REVIEW-661-r1.md new file mode 100644 index 00000000..41938312 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-661-r1.md @@ -0,0 +1,214 @@ +# 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