From ceee249210e4d268521ee800dbf2011687d1b490 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 10:09:11 +0300 Subject: [PATCH] docs: review document for #780 Issue: #780 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-780-r1.md | 225 +++++++++++++++++++++++++++++ 2 files changed, 227 insertions(+), 1 deletion(-) create mode 100755 docs/reviews/SPEC-REVIEW-780-r1.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 96758cd0..78fccbe3 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,11 +1,12 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 282, issue: 139. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 283, issue: 140. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| | бета v1.79.0-beta.2 | [SHIP-REVIEW-v1.79.0-beta.2.md](SHIP-REVIEW-v1.79.0-beta.2.md) | пакетное ревью ship · — | ⚪ — | 0 | 0 | — | — | | бета v1.79.0-beta.1 | [SHIP-REVIEW-v1.79.0-beta.1.md](SHIP-REVIEW-v1.79.0-beta.1.md) | пакетное ревью ship · — | ⚪ — | 0 | 0 | — | — | +| #780 | [SPEC-REVIEW-780-r1.md](SPEC-REVIEW-780-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 5 | · Medium — в редакторе устройств нет «существующего контекстного лотка» и модели выделения; · Medium — поведение бэкенда на висящую ссылку не определено и ломает сохранение «старо…; · Medium — запрет поднимать бюджеты исполним только ленивой загрузкой, а ТЗ её не требует; · Medium — смещение от грани не определено для стен нулевой толщины и смешанных лент; · Medium — AC17 не проверяем: нет порогов; · Low — D назван «диаметром устройства пространства», а такой величины нет | `validation.py` `scripts/bundle-budget.mjs` `types.ts` `houseplan-card.ts` | | #775 | [CODE-REVIEW-775-r1.md](CODE-REVIEW-775-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #772 | [CODE-REVIEW-772-r1.md](CODE-REVIEW-772-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #762 | [SPEC-REVIEW-762-r1.md](SPEC-REVIEW-762-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-780-r1.md b/docs/reviews/SPEC-REVIEW-780-r1.md new file mode 100755 index 00000000..0c25a3d7 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-780-r1.md @@ -0,0 +1,225 @@ +# SPEC-REVIEW-780-r1 — LED-ленты: рисование, свечение вдоль ленты, управление + +Вердикт: 🟡 **жёлтый** · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 5 → в задаче · Low: 3 + +Ревью ручное, по поручению владельца (модель конвейера без лимита). Ревьюер — +свежая сессия, не автор ТЗ; черновика реализации нет и не читался (§2.4). + +## Скоуп ревью + +- Материал — тело issue #780 на 2026-10-02T07:03:12Z (раздел `## ТЗ`, §1–§15, + AC1–AC20) и три комментария владельца с решением о редакторе устройств и Q1. +- База сверки — `dev` на `d0c13bc5` (v1.79.0-beta.2), та же, что назвал автор. +- Работа по `docs/SCOPE.md`: домочадец и гость видят, где горит протяжённый + свет, и управляют им из View; администратор размещает. Строка Core user + jobs есть, скоуп не вызывает вопросов. +- Прежние ревью #662 (r1 жёлтый, r2–r4 зелёные, `docs/reviews/INDEX.md`) + судили другой текст, где инструмент жил в редакторе плана; они не переносятся. + +## Как проверялось + +Каждое утверждение ТЗ о существующем коде и документах сверено с деревом +`d0c13bc5`, а не принято на слово: + +| Утверждение ТЗ | Где проверено | Итог | +|---|---|---| +| `resolveGlowAppearance`, `glowAlpha`, `GLOW_FALLOFF`, `GLOW_FADE_MS = 500` | `src/glow-scene.ts:43–46`, `src/logic.ts` | верно | +| Glow — `glow_enabled` пространства и `glow` комнаты (`null` наследует), независим от `fill_mode` | `src/logic.ts:1374`, `src/types.ts:16`, `editors/space-form.ts:542` | верно | +| `glow_radius_cm`, `ripple_color`, `ripple_size`, `display`, `size`, `angle` у маркера | `src/types.ts`, `custom_components/houseplan/validation.py` | верно | +| `houseplan-space-card`: opt-in `light_pools`, независимый `live_states` | `src/space-card.ts`, `docs/ARCHITECTURE.md:506` | верно | +| Свет: окна/колонны/Solid-нули непрозрачны, Dashed прозрачны, двери по степени открытия, проход только при полу с обеих сторон | `docs/LIGHT.md:30–60` | верно | +| `isoEdgeColor` / `isoTileShadow` | `src/iso-tiles.ts` | верно | +| История редактора устройств — position-only | `houseplan-editor-runtime.ts` `_devicePositionHistory`, `_undoDevicePosition` | верно | +| Esc завершает цепочку стен; touch в редакторах — best effort | `docs/USER-GUIDE.ru.md:483` | верно, §4.3/§4.6 согласованы | +| Каталог «На плане» | `src/i18n/ru.json` `device_inbox.tab_on_plan` | верно | +| marker-id remap импорта, копия пространства, projection «Только план» | `import_export.py:1067–1118`, `:258–306`, `:1374` | механизмы есть; `led_strips` в projection добавит задача — это и требует §9 | +| Бэкенд не теряет неизвестные поля пространства | `validation.py` `SPACE_SCHEMA … extra=vol.ALLOW_EXTRA` | верно для бэкенда; фронтенд правит конфиг на месте (`_dropLegacySegments`) — round-trip AC15 уместен | +| «Существующий контекстный лоток» в редакторе устройств | `_renderEditorSecondary()` в `houseplan-editor-runtime.ts:5086–5093` | **неверно** — см. M1 | +| `D` — «базовый диаметр устройства пространства» | `icon_size` — опция карточки (`types.ts:404`), `iconUnit(space)` — `space-geometry.ts:521` | **неточно** — см. L1 | +| «Бюджеты initial View … не повышаются» | `node scripts/bundle-budget.mjs` на `d0c13bc5` | исполнимо только ленивым путём — см. M3 | + +Исполнено: `node scripts/bundle-budget.mjs` → `initial View: 300565 B gzip (… budget 301066 B, headroom 501 B)`. +Остальное — **проверено чтением, не исполнением**: ТЗ кода не несёт. + +## Находки + +### M1 · Medium — в редакторе устройств нет «существующего контекстного лотка» и модели выделения + +§4.7 опирается на «существующий контекстный лоток» редактора устройств, §5 — +на его действия («Показывать значком», «Отвязать», «Удалить ленту»). Его нет: + +``` +_renderEditorSecondary(): … this.host._mode === 'plan' ? this._renderPlanSecondary() + : this.host._mode === 'decor' ? … this._renderDecorSecondary() + : null; +``` + +В режиме устройств вторичная панель — только у групп тулбара; клик по значку +сразу открывает диалог, выделения нет. Значит, задача вводит в редактор +устройств первое выделяемое не-значковое тело и первую контекстную модель — это +работа и контракт, а не переиспользование. Догадка, записанная как факт (§7.1). + +Что нужно в ТЗ: прямо сказать, что для режима устройств добавляется +`_renderDevicesSecondary()` на общем механизме `_editorSecondary` (тот же, что у +плана и декора), что выделяется **только лента** (значок по-прежнему открывает +диалог сразу — это уже записано), как выделение снимается (Esc, клик по пустому +месту, смена инструмента) и что оно не переживает выход из редактора. Добавить +это в доказательство AC3/AC6 (smoke: выделение ленты → лоток → действие → +лоток закрыт; клик по значку при выделенной ленте открывает диалог значка). + +### M2 · Medium — поведение бэкенда на висящую ссылку не определено и ломает сохранение «старого» клиента + +AC1 требует **отклонять** запись с несуществующим маркером, §5 — чтобы удаление +маркера атомарно обнуляло ссылку. Второе выполнит только новый фронтенд. Клиент, +не знающий `led_strips` (закэшированный старый бандл после обновления, второй +открытый браузер, откат версии фронтенда), удалит привязанный маркер штатно — +и новый бэкенд по AC1 отклонит **всё** сохранение: пользователь не может удалить +устройство, причина ему не видна. Бэкенд принимает неизвестные поля именно затем, +чтобы «a stale browser tab cannot fail a save» (`validation.py`, комментарий у +`segments`) — ТЗ этот принцип нарушает. + +Второе неопределённое место — предикат «маркер того же пространства». У маркера +`space` необязателен (`MARKER_SCHEMA: vol.Optional("space"): vol.Any(str, None)`): +что проверяет бэкенд, когда `marker.space` пуст, и выставляет ли привязка +`marker.space` явно — не сказано. + +Что нужно в ТЗ (технический вопрос, решается здесь, не владельцем): развести +**создание** и **последствие**. Например: бэкенд на записи нормализует ссылку на +отсутствующий маркер в `marker: null, active: true` (ровно как §9 уже требует при +импорте) и возвращает счётчик в ответе; отклоняет — неверную форму, дубли id, +повторную привязку одного маркера двумя лентами и ссылку на маркер, который +**существует, но** принадлежит другому пространству. Привязка явно пишет +`marker.space` равным пространству ленты. AC1 и AC15 дополнить случаем «клиент без +поддержки лент удалил привязанный маркер → запись принята, лента стала +непривязанной, View не ломается». + +### M3 · Medium — запрет поднимать бюджеты исполним только ленивой загрузкой, а ТЗ её не требует + +§13.3: «Бюджеты initial View и ленивых чанков не повышаются для прохождения +задачи; при отсутствии лент не загружать тяжёлую LED-геометрию без необходимости». +Измерено на базе: запас стартового графа View — **501 Б gzip** до абсолютной стены +`INITIAL_VIEW_GZIP_BUDGET` (её не двигает и полоса #699). Отрисовка ленты, линейная +видимость и хит-тест во View в 501 Б не поместятся. Формулировка «без +необходимости» оставляет реализатору выбор, которого на деле нет, и красный +`bundle:budget` обнаружится только на код-ревью. + +Что нужно в ТЗ: View-часть LED (проекция, линейный свет, хит-тест, iso-полоса) +живёт в **отдельном ленивом чанке**, который грузится, только когда в показанном +пространстве есть активная лента; в стартовом графе — только загрузчик и проверка +наличия. Новый чанк получает **собственную** строку бюджета в +`scripts/bundle-budget.mjs` с потолком, обоснованным замером (это не «повышение» +существующих). Плюс: что видит пользователь, пока чанк грузится (лента появляется +после загрузки, без вспышки старого значка) и при ошибке загрузки (fail-dark по +§14, остальной View работает). Добавить в AC17: `bundle:budget` зелёный без +правки существующих потолков; план без лент не запрашивает LED-чанк (сетевой +журнал smoke). + +### M4 · Medium — смещение от грани не определено для стен нулевой толщины и смешанных лент + +§3: «лента на физической грани стены рисуется со стороны свободного пола, ось +смещена на `t/2`… на углу смещения соединяются непрерывно». От этого правила +зависят хит-геометрия (§7, AC12 «wall-offset»), точка излучения (§6, epsilon) и +численная приёмка (AC8: «смещение настенной полосы на `t/2`»). Не определено: + +- лента на **оси** стены нулевой толщины (§4.2 привязывает к оси, §6 разрешает + светить в обе стороны): свободного пола две стороны — смещения нет, или в какую + сторону оно есть; +- **смешанная** лента: сегмент на грани, следующий уходит в комнату. Смещённый + сегмент и несмещённый сходятся в вершине со ступенькой `t/2` — разрыв или + непрерывный переход, и как именно; +- критерий «сегмент лежит на грани» — каждый сегмент отдельно, по совпадению с + гранью в численном epsilon (§13.4 различает epsilon и магнит, но здесь не + применён). + +Что нужно в ТЗ: правило по сегменту (на грани толстого тела — смещение на `t/2` +в свободную сторону; на оси нулевой стены и в свободном полу — без смещения), +переход между смещённым и несмещённым сегментом (например, вершина соединяет +оба смещения отрезком/скруглением без шва) и пример в AC8/AC12 со смешанной +лентой и лентой на оси нулевой стены. + +### M5 · Medium — AC17 не проверяем: нет порогов + +AC17: «с 10×5 и предельными 50×50 точками нет зависания, утечки кэша и +перерасчёта… Сравнение с базовым SHA, performance-профиль». «Нет зависания» и +«сравнение» без числа не дают ни красного, ни зелёного: неясно, какой результат +провалит AC. 50 лент × 50 точек — 2 500 вершин с линейной видимостью сквозь +препятствия, это реальный риск, и ТЗ сам ставит его первым в §13. + +Что нужно в ТЗ: назвать профиль (новый `led-strips-v1` в `demo/performance/` или +расширение существующего) и потолки — например, `stateUpdateMs` и `panZoomMs` +для 10×5 в пределах потолков текущего `large-house-interaction-v1`, для 50×50 — +явный `hardMaxMs` первого построения и тёплого обновления; «нерелевантный HA +update не пересчитывает геометрию» — счётчиком recompute = 0, утечка кэша — +ограничением размера после N переключений пространства. Числа выбирает автор по +замеру, но они должны стоять в AC до кода. + +### L1 · Low — `D` назван «диаметром устройства пространства», а такой величины нет + +Размер значка задаёт опция **карточки** `icon_size` (`types.ts:404`, по +умолчанию 2,5), а пространство даёт `iconUnit(space)`; авто-сетка считает +`(icon_size / 100) × iconUnit(space)` (`houseplan-card.ts` `_defaultPositions`). +У `houseplan-space-card` своя `icon_size`. Предлагаемая формулировка: +«`D = (icon_size карточки / 100) × iconUnit(space)` — та же величина, что у +авто-сетки; индивидуальный `marker.size` не участвует». Править в тексте. + +### L2 · Low — AC18 требует docs-снимков, которые ветка задачи не коммитит (#697) + +«Docs-снимки по действующему fingerprint» и в §14 «затронутые docs screenshots»: +по §8 (#697) ветки задач не коммитят `docs/images/**`, кадры обновляет бот на +`dev` перед бетой. AC18 переформулировать: тексты ×4, документы и демо +соответствуют UI; кадры документации — штатно ботом беты. Golden задача +принимает сама — метка `ci:golden` стоит. + +### L3 · Low — «второй клиент» в AC19 без способа доказательства + +Демо-стенд держит конфиг в странице; как smoke докажет «подключение другого +клиента», не сказано. Достаточно backend round-trip (запись → чтение новым +соединением) плюс reload в smoke — так и записать. + +## Что проверено и корректно + +- Обязательные разделы §7.1 на месте: сценарий и «до/после» идут первыми и + без терминов реализации; скоуп и не-скоуп; контракт; UX; модель данных и + миграция; i18n ×4; AC с доказательствами; план автотестов; риски; откат; + release-артефакты. +- Продуктовых вопросов не осталось: место инструмента, преобразование в обе + стороны и Q1 решены владельцем и внесены в тело, а не в комментарии. +- Свет согласован с `docs/LIGHT.md`: размещение и распространение — разные + проверки (§6), Dashed/Solid различаются именно для света, fail-dark при + ошибке клиппинга, нет второго resolver. +- Модель данных: необязательное поле без повышения версии; бэкенд неизвестное + поле пропускает; `active` отделён от `marker.hidden` и от питания; лимиты + включают неактивные формы; remap ссылок при импорте и обнуление в «Только + план» предусмотрены. +- Touch: safety floor редактора (§4.6) не противоречит best effort из + `USER-GUIDE.ru.md:483`; хит-радиус `max(22 CSS px, t/2)` и AC12 согласованы + (20 px внутри, 30 px снаружи). +- Undo: честно названо, что position-only история расширяется, а не + переиспользуется (§4.9, §13.7). +- Визуальный контракт: отличия от макета перечислены явно, численная приёмка + задаёт точку, допуск и сцену; `ci:golden` стоит. + +## Чего не проверял + +- Архив дизайнера и Figma не открывал: соответствие таблицы §3 макету принято + со слов автора (суммы SHA-256 он назвал сверенными). Это проверит AC8 на + код-ревью парными кадрами. +- Реализуемость «без шва и двойной яркости» в WebView/GPU не оценивал — она + доказывается пикселями AC10, не текстом. +- `pytest`, смоки, golden не запускались: кода нет. +- Достаточность 20 AC на одну задачу и 4 цикла код-ревью — вопрос объёма, + решение владельца; продуктовым возражением это не считаю. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `d0c13bc5555a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `d41236afd2cd1035cd2edaf322cf83c375eb5942` + ``` + git log --all --format='%H %T' | grep d41236afd2cd + ``` +- Тело issue: `f9f67c2c8cbdfd2b93ff446c9dad80f4becb1ed264a7ddf682317ffcac0a2638` +- Вердикт конвейера: `yellow` · High 0 · маршрут `fix`