docs: review document for #662

Issue: #662
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-26 08:37:45 +00:00
parent 27517db8a4
commit 422221fe93
2 changed files with 296 additions and 1 deletions
+2 -1
View File
@@ -1,9 +1,10 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1078, issue: 381. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1079, issue: 382. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|---|---|---|---|---:|---:|---|---|
| #662 | [SPEC-REVIEW-662-r1.md](SPEC-REVIEW-662-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | «Входящие» не существует в UI, и текущая классификация каталога кладёт неразмещённую ле…; «Не скоуп» отсутствует внутри формального ## ТЗ | `src/device-inbox.ts` `USER-GUIDE.ru.md` `docs/USER-GUIDE.ru.md` `demo/smoke_led_strip_bind.mjs` `device-inbox.ts` `SPEC-REVIEW-661-r1.md` |
| #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` |
+294
View File
@@ -0,0 +1,294 @@
# SPEC-REVIEW-662-r1
- **Issue:** https://github.com/Matysh/houseplan-card/issues/662
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
- **Трек:** полный. Автор явно назвал критерии §5, которые задача не проходит:
новый UX-контракт (новый вид маркера с собственной геометрией и хит-тестом),
более одной поверхности (три рендерера + два редактора + бэкенд), влияние на
touch (новый тип цели касания «ломаная» во View/киоске — блокирующая зона
TOUCH-SUPPORT), влияние на перф (линейный источник света: K свипов
видимости на ленту) — разбор по существу, выбор полного трека корректен.
- **Материал:** тело issue #662 в текущей редакции (после снятия `blocked`) +
все 4 комментария: (1) аналитика/SCOPE/трек, (2) вопросы владельцу 1–6
пачкой, (3) ответы владельца 1–6 внесены в тело + новые вопросы 7–9
(привязка сразу после рисования; судьба значка размещённого устройства;
объём правки геометрии), (4) решение владельца 7–9 внесено, `blocked` снят
→ `S4-spec-review`.
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
- **Роль:** ревьюер ТЗ (не автор)
## Скоуп ревью
Показ протяжённого источника света (LED-лента) как ломаной вдоль стен вместо
значка в точке: геометрия рисуется в редакторе плана новым инструментом
«LED-лента» (контракт цепочки стен), привязывается к обычному `light.*`
маркеру через существующий диалог «Добавить устройство»; во View/киоске/
`houseplan-space-card` лента рисуется как капсульная полоса on/off/unavailable
с хит-тестом по всей длине, участвует в fill «Свечение» как линейный источник
(объединение вееров видимости), поднята в 2.5D по правилам плиток устройств.
Бэкенд: схема `led_strips`, конфиг-инвариант «один маркер — не более одной
ленты», per-space export/import, support package. Редактор устройств —
диалог маркера без выбора вида; ленты не перетаскиваются там.
**SCOPE-проверка (docs/SCOPE.md):** прямая строка — J1 («room fills
(light/temp/LQI)», «live spatial overview») и J7-смежная точность
представления источника; сам автор в аналитике формулирует это как устранение
конкретной лжи на плане («подсветка гарнитура длиной 3 м рисуется как лампа
посреди столешницы»), что бьёт в основную работу продукта «взглянул и понял,
что где горит». Из списка «никогда не строить» ничего не задевается: адресные
эффекты/сегменты явно исключены в «Не скоуп»; редакторы затронуты только для
расстановки — View остаётся продуктом для двух персон. Новой scope-дыры не
вижу.
## Как проверялось
1. Прочитаны целиком `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md`
и по ссылкам конспекта — PROCESS.md §7.1 (обязательные разделы, цепочка
вопросов владельцу), §2.4, §2.5, §4, §7.2, §2.10 (не применялся — это r1).
2. Прочитано тело issue #662 целиком (`gh issue view 662 --json body`) и все 4
комментария (`gh issue view 662 --json comments`) — история решений
владельца воспроизведена в «Материал» выше.
3. Сверены обязательные разделы §7.1: Сценарий, Что человек увидит до/после,
Проблема, Скоуп, Контракт поведения (C1–C12), UX-тексты, Модель данных и
миграция, Критерии приёмки AC1–AC14 с указанным способом доказательства
(backend/unit/smoke/golden/ревью кода), План автотестов, Риски, Откат,
Release-артефакты, «Принятые предположения» — почти все на месте; см.
Находка 2 про «Не скоуп».
4. Технические утверждения ТЗ построчно сверены с реальным кодом, а не приняты
на слово:
- `resolveGlowAppearance`, `GLOW_FALLOFF`, `GLOW_FADE_MS = 500`,
`glowAlpha` — реальны (`src/glow-scene.ts:46`, `src/logic.ts`).
- `visibilityPolygon`/`splitAtIntersections` — реальны
(`src/light-visibility.ts`, `src/glow-scene.ts`); `clipCache` с LRU-лимитом
— реален (`src/glow-scene.ts:105-160`).
- `device-hit-owner.ts` существует, капсула + порог 44 px (`cellSize = 44`)
— реальная константа (`src/device-hit-owner.ts:165-167,292`), пороги
AC5 (20/30 px) согласуются с предложенным `max(22 px, половина толщины)`.
- `glow-blend.ts` (`resolvedSvgScreenBlend`, `svgScreenBlendSupported`) —
реальный зонд деградации `screen`, используется в
`houseplan-card.ts`/`space-card.ts`; `.glow-pools` уже несёт
`blend-screen`/`blend-normal` на уровне всей группы пятен — вложенная
`isolation:isolate`+`lighten`-группа для капсул одной ленты — новый, но
технически состоятельный слой поверх существующего механизма (CSS
compositing поддерживает вложенные группы), и риск деградации назван явно.
- `MarkupTool` — реальный union-тип в `houseplan-editor-runtime.ts:341`
(`'select' | 'draw' | 'column' | 'merge' | 'split' | 'resize' | 'opening'
| 'wallthick' | 'delroom'`); добавление литерала `'strip'` — прямое
расширение по образцу. Инструмент «Стены», описанный в аналитике как
прецедент цепочки (тап добавляет точку, Shift — 45°, `pointercancel`/
второй палец/пинч/подавленный синтетический клик не добавляют), —
дословно совпадает с `docs/TOUCH-SUPPORT.md:151-162` и с реальными
i18n-ключами `markup.hint_start`/`markup.hint_points`
(`src/i18n/ru.json:157-158`) — ни один термин не выдуман.
- `docs/ISOMETRIC.md:307-328` — «Raised tiles», подъём `0.075 D` — то же
число, что и C9 предлагает переиспользовать для ленты (не новая
константа).
- MARKER_SCHEMA (`custom_components/houseplan/validation.py:1826+`)
подтверждает все поля, которые C2 объявляет «не действуют» для маркера
ленты (`display`, `ripple_color`, `ripple_size`, `size`, `angle` — строки
1941-1946) и все поля, которые «действуют» (`hidden`, `tap_*`,
`controls`, `is_light`, `light_entity`, `toggle_entity`, `glow_color`,
`glow_radius_cm`, `value_badge`/`value_source`, `use_climate_temp`) —
ни одно имя поля не выдумано, а `space` уже существует как отдельное от
`layout` поле маркера (строка 1835) — модель C1/C4 «маркер со `space`, но
без `layout`» технически возможна уже сегодня, а не требует новой схемы.
- Per-space export (`_project_plan_only_space`, `_marker_owned` в
`import_export.py:151-360`) реально существует и фильтрует маркеры по
положению — расширение на `led_strips` в C11 корректно называет реальный
механизм, а не гипотетический.
- AC14 называет только реально существующие документы
(`docs/DEVICE-PRESENTATION.md`, `docs/LIGHT.md`,
`docs/DEVICE-LIGHT-SETTINGS-MATRIX.ru.md`, `docs/ARCHITECTURE.md`
— разделы «Device markers»/«Markup editor» существуют дословно под этими
заголовками, `docs/ISOMETRIC.md`, `docs/CONFIG-COMPATIBILITY.md`,
`docs/TOUCH-SUPPORT.md`); раздел «Linear sources» в LIGHT.md пока не
существует — ТЗ корректно описывает его как новый, не выдаёт за
существующий.
5. Проверена внутренняя непротиворечивость числовых порогов: толщины
`0,12 D`/`0,08 D`, хит `22 px`/половина толщины, выборка `100 см`/`≤8`
точек, `MAX_MARKERS=2000`/`MAX_POLY_POINTS=500` (validation.py:1198,1222) —
лимиты ленты (`2…50` точек, `≤50` лент) заметно консервативнее уже принятых
потолков, конфликта нет.
6. Проверена цепочка вопросов владельцу: пачка 1 (6 вопросов) и пачка 2
(3 вопроса) заданы с предложенным умолчанием по каждому пункту, ответы
внесены в «Решения владельца» 1–9, `blocked` корректно ставился/снимался,
открытых продуктовых вопросов не осталось.
7. Прогнаны дешёвые гейты по требованию раздела гейтов: `git diff
origin/dev...HEAD --stat` пуст, `git status --short` чист,
`git rev-parse HEAD` = `27517db8a4b933e54368927851f76352567d5568` — точное
совпадение с материалом. Гейты (`tsc`, `test`, `build`, смоки, golden,
инварианты) неприменимы: продуктового кода для #662 ещё нет, стадия
`spec`.
8. Сверена классификация каталога устройств (`src/device-inbox.ts:190-260`)
против модели C2/C4/C5/AC8 — см. Находку 1.
## Находки
Один Medium (в скоупе, чинится автором без возврата на отдельный issue) и
один Low (снят здесь же).
### Medium-1. «Входящие» не существует в UI, и текущая классификация каталога кладёт неразмещённую ленту в другую вкладку
- **Файл/раздел:** тело issue #662, `## ТЗ` → «Модель данных и миграция»
(«Входящие» устройств его не показывают»), C5 («маркер остаётся в конфиге
как неразмещённый — виден во «Входящих» устройств»), AC8 («Отвязать» →
устройство во «Входящих»).
- **Что не так:** реальные вкладки каталога устройств — ровно четыре:
`on_plan`/`available`/`hidden`/`readd` (`src/device-inbox.ts:13`), в
`USER-GUIDE.ru.md:1081` они подписаны «На плане», «Доступны», «Скрытые»,
«Доступны снова». Вкладки «Входящие» не существует нигде — ни в коде, ни в
`docs/USER-GUIDE.ru.md`, ни в i18n (`grep "Входящ" src/i18n/ru.json` находит
только несвязанную строку `backup.import_detail.dropped_links`). Это прямое
нарушение требования брать терминологию видимого поведения из
`docs/USER-GUIDE.ru.md`, а не изобретать её (AGENTS.md «Read this first»).
- **Хуже того — вопрос не только в названии.** Реальная классификация
(`src/device-inbox.ts:219-224`) кладёт запись в `on_plan`, если существует
`live`-маркер (запись в `config.markers`, не `removed`, не `hidden`) —
**независимо от наличия `layout`**; в `available` запись попадает, только
если `live`-маркера нет вовсе (см. ветку `else if (candidate) category =
'available'`, куда управление не доходит, если `live` истинен). Маркер
ленты, отвязанной или удалённой по C5 («маркер остаётся неразмещённым»,
запись `config.markers` не трогается, `removed` не проставляется), —
это ровно случай «`live`, но без `layout`»: по действующему коду он
попадёт на вкладку **«На плане»**, а не «Доступны»/«Входящие», при этом
визуально ничего не будет отрисовано (ни значка, ни ленты) — фантомная
строка «на плане», которую нечем найти на самом плане.
- **Как воспроизвести (мысленный прогон по коду, не исполнение):**
`led_strips[i].marker = null` → маркер остаётся в `config.markers` с
`space`, без `layout`, `removed` не установлен → в
`buildDeviceInboxRows` (`src/device-inbox.ts:200-224`) `liveByBinding` его
подхватывает → `category = 'on_plan'` (строка 222) → каталог покажет его во
вкладке «На плане», хотя ни один рендерер (View/2.5D/space-card) ничего не
рисует для маркера без `layout` и без активной ленты.
- **Почему Medium, не High:** это не продуктовый вопрос (владелец решил
поведение — «снова не размещено», п. 8–9 решений), а техническая
недосказанность контракта: не хватает одного явного правила — например,
«маркер со `space`, без `layout` и не referenced ни одной лентой,
классифицируется как `available`» (меняет `src/device-inbox.ts:219-224`) —
и правки термина «Входящие» → «Доступны» по всему ТЗ и AC8. И то, и другое —
в пределах компетенции автора (техническое решение по PROCESS.md §7.1,
«где хранится состояние… — решает автор»), без обращения к владельцу.
Но пока это не зафиксировано явно, AC8 не может быть проверен: смок
`demo/smoke_led_strip_bind.mjs` не будет знать, какую вкладку/строку
утверждать, и разночтение всплывёт только на код-ревью — дороже, чем сейчас.
- **Что делать:** заменить «Входящие» на «Доступны» (реальная вкладка) во всех
трёх местах (модель данных, C5, AC8) **и** явно дописать в C5 или в
«Принятые предположения», как классификатор `device-inbox.ts` отличает
«маркер представлен лентой» (остаётся `on_plan`, это ожидаемо и корректно,
пока лента жива) от «маркер осиротел после отвязки/удаления ленты»
(должен переклассифицироваться в `available`, иначе он зависает фантомом
на вкладке «На плане»).
### Low-1. «Не скоуп» отсутствует внутри формального `## ТЗ`
- Раздел `## Не скоуп (кандидаты в отдельные issue)` расположен **до**
заголовка `## ТЗ`, в аналитической части issue. §7.1 требует «скоуп и
не-скоуп» как разделы самого ТЗ (раздел `## ТЗ`, #517); внутри `### Скоуп`
под `## ТЗ` пункта «не-скоуп» нет вовсе. Для сравнения — в #661 (тот же
трек, тот же ревьюер-конспект) не-скоуп дан строкой прямо внутри `###
Скоуп` под `## ТЗ` (`SPEC-REVIEW-661-r1.md`, «Скоуп/Не-скоуп разделены
чётко»). Содержательно у #662 всё названо (сегменты/эффекты адресных лент,
вставка/удаление вершин, высота крепления, произвольная форма гирлянды, тап
в space-card) и совпадает с решениями владельца — вопросов к содержанию нет,
только к месту. **Почему Low:** содержание присутствует и однозначно,
ближайший заголовок над ним прямо озаглавлен «Не скоуп» и явно относится к
этой же задаче — риск, что автор реализации или код-ревьюер его не найдёт,
минимален. Снимаю без возврата на цикл; автору стоит при следующей правке
текста (например, вместе с Medium-1) продублировать одну строку
«Не скоуп: …» в `### Скоуп` под `## ТЗ`, чтобы раздел был самодостаточным.
## Что проверено и корректно
- Все обязательные разделы §7.1, кроме отмеченного в Low-1, присутствуют и в
правильном порядке; Сценарий и Что человек увидит отвечают на «какая
персона/поверхность/момент» и «что видно без терминов реализации».
- Контракт C1–C12 самосогласован и почти полностью переиспользует
существующие механизмы (см. «Как проверялось» п.4) вместо изобретения
параллельных: геометрия ломаной — как у стен/комнат (`coordinate-
canonicalization.ts`), хит-тест — новый тип цели в уже существующем
`device-hit-owner.ts`, свечение — линейный кандидат в уже существующем
`glow-scene.ts`/`light-visibility.ts`, привязка — существующий диалог
«Добавить устройство», 2.5D — существующая формула поднятых плиток.
- Каждый AC1–AC14, кроме AC8 (см. Medium-1), однозначен и называет способ
доказательства (backend/unit/smoke/golden/ревью кода); граничные значения
заданы числом по обе стороны порога (AC1: `<2`/`>50` точек, `>50` лент;
AC5: 20 px → лента, 30 px → нет).
- Модель данных: новый необязательный массив без миграции, политика
неизвестных полей для старого фронтенда/бэкенда описана явно и совпадает с
реальным поведением конфиг-загрузки; раздел `docs/CONFIG-COMPATIBILITY.md`
назван в AC14 для формализации.
- Поля маркера, которые лента «отключает» или «включает» (C2), построчно
совпадают с MARKER_SCHEMA (см. «Как проверялось» п.4) — ни одно не
выдумано.
- i18n-таблица дана для ru/en с ключами по существующей конвенции
(`title.markup_<key>`, `markup.hint_<tool>_*`), паритет de/fr обещан
переводом; AC13 требует тест паритета по всем четырём словарям.
- Риски называют главные технические неопределённости честно (перф линейного
света, смешивание `lighten`/деградация, торец на диагоналях, новое
состояние «маркер без layout» — что как раз материал Medium-1, автор его
заметил, но не закрыл до уровня «AC можно проверить»), откат описан на двух
уровнях и не противоречит правилу «не удалять файл/данные пользователя на
догадку».
- Мутанты (8 штук) целятся в конкретные защитные точки будущего кода (порог
хита, деление радиуса пополам, обрезка по одной точке вместо веера, снятие
`lighten`, сохранение `layout` при привязке, добавление точки при пане на
touch, уникальность маркера на бэкенде, отрисовка непривязанной ленты) —
реальный план для код-ревью, а не отчётность.
- Продуктовые вопросы 1–9 заданы владельцу пачками с предложенным умолчанием
каждый раз, ответы внесены в тело, `blocked` использовался и снят корректно
— открытых продуктовых вопросов не осталось.
## Чего не проверял
- Гейты (`npx tsc --noEmit`, `npm test`, `npm run build`,
`node scripts/check-docs.mjs`, смоки, `golden:verify`, `model-invariants`) —
не прогонял: `git diff origin/dev...HEAD --stat` пуст, продуктового кода для
#662 нет, стадия `spec`. Предмет код-ревью после реализации.
- Не оценивал осуществимость самого механизма «капсулы `lighten` внутри
изолированной группы + внешний `screen`» построчно (не писал прототип) —
это новая, но правдоподобная композиция существующих CSS-примитивов
(`isolation`, `mix-blend-mode`), явно отмеченная как риск с планом
деградации; предмет код-ревью, не спек-ревью.
- Не проверял реализуемость объединения вееров видимости из нескольких точек
ломаной (`visibilityPolygon` сейчас строится из одной точки) построчно —
плановое расширение помечено «принято предположительно, поменять
свободно»; это код-ревью, не спек-ревью.
- Не проверял иконку `mdi:led-strip-variant` на существование в used
MDI-наборе карточки — декоративная деталь кнопки инструмента, не влияет на
AC.
- Не проверял продуктовую правомерность решений владельца 1–9 (ломаная вместо
отрезка, видимость выключенной ленты, радиус вдвое, тап по всей длине, лента
на полу в 2.5D, без ручек в v1, привязка сразу после рисования, судьба
значка, объём правки геометрии) — прямые владельческие решения, ревьюер их
не оспаривает, только сверяет соответствие контракта (см. «Как проверялось»
п.6).
## Вердикт
Обязательные разделы почти полны (один Low по месту, не по содержанию),
контракт C1–C12 глубоко переиспользует существующие механизмы и не содержит
догадок, выданных за факты, — каждое нетривиальное техническое утверждение
сверено с реальным кодом и подтвердилось. Но один AC (AC8) и три места текста
(модель данных, C5, AC8) называют несуществующую вкладку каталога устройств
«Входящие» и, что важнее названия, не формулируют правило, по которому
осиротевший маркер ленты (со `space`, без `layout`, не удалённый) должен
попасть в `available`, а не зависнуть фантомом в `on_plan` по действующей
логике `device-inbox.ts`. Это техническая недосказанность в пределах
компетенции автора, не продуктовый вопрос — но без неё AC8 не проверяем.
Возвращаю автору без отдельного issue (Medium в скоупе).
Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `27517db8a4b9` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `8eaeacefa90fcaebfe8444496827be42e6e06012`
```
git log --all --format='%H %T' | grep 8eaeacefa90f
```
- Тело issue: `960ef8ab4f944382a75391de287ebf208e2000449760a9c84c12793c820412c6`
- Вердикт конвейера: `yellow` · High 0