From 422221fe9342c88d35ab389515960ba05552ea86 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 08:37:45 +0000 Subject: [PATCH] docs: review document for #662 Issue: #662 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-662-r1.md | 294 +++++++++++++++++++++++++++++ 2 files changed, 296 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-662-r1.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 0a1ebfff..4e3baff9 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -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` | diff --git a/docs/reviews/SPEC-REVIEW-662-r1.md b/docs/reviews/SPEC-REVIEW-662-r1.md new file mode 100644 index 00000000..7adcbc9b --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-662-r1.md @@ -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_`, `markup.hint__*`), паритет 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 → в задаче + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `27517db8a4b9` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `8eaeacefa90fcaebfe8444496827be42e6e06012` + ``` + git log --all --format='%H %T' | grep 8eaeacefa90f + ``` +- Тело issue: `960ef8ab4f944382a75391de287ebf208e2000449760a9c84c12793c820412c6` +- Вердикт конвейера: `yellow` · High 0