mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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 | — | — |
|
||||
|
||||
@@ -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 → в задаче
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `1ecf96d596af` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `213b7d2756d53d06b3dd310ea68ad6c963f20d37`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 213b7d2756d5
|
||||
```
|
||||
- Тело issue: `77ff1c023b01033841f4fffc73cbbb86c5ef3d37b91b3d2260d9538b729dc625`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user