mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 13:18:58 +00:00
@@ -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 | — | — |
|
||||
|
||||
Executable
+225
@@ -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 цикла код-ревью — вопрос объёма,
|
||||
решение владельца; продуктовым возражением это не считаю.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `d0c13bc5555a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `d41236afd2cd1035cd2edaf322cf83c375eb5942`
|
||||
```
|
||||
git log --all --format='%H %T' | grep d41236afd2cd
|
||||
```
|
||||
- Тело issue: `f9f67c2c8cbdfd2b93ff446c9dad80f4becb1ed264a7ddf682317ffcac0a2638`
|
||||
- Вердикт конвейера: `yellow` · High 0 · маршрут `fix`
|
||||
Reference in New Issue
Block a user