mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,246 @@
|
||||
# SPEC-REVIEW-437-r1
|
||||
|
||||
Issue: [#437](https://github.com/Matysh/houseplan-card/issues/437) — конфигурируемая сводная панель поверх плана.
|
||||
Ветка: `issue/437-summary-panel`. Материал: тело issue #437, все 7 комментариев,
|
||||
`docs/SCOPE.md`, `docs/specs/437-summary-panel.md`, `docs/specs/README.md` на
|
||||
`HEAD = 4a1aac16` (заход r1, первый раунд — раздел «Унаследовано» не требуется).
|
||||
Трек: полный (P2/feature; критерии `small` не пройдены явно названы автором:
|
||||
новый UX-контракт, несколько поверхностей, shared/local persistence, touch,
|
||||
performance).
|
||||
|
||||
## Скоуп проверки
|
||||
|
||||
Диапазон `ed9ee026..4a1aac16` — три файла, только класс C (документация):
|
||||
|
||||
```
|
||||
docs/SCOPE.md | 8 +-
|
||||
docs/specs/437-summary-panel.md | 720 ++++++++++++++++++++++++++++++++
|
||||
docs/specs/README.md | 1 +
|
||||
```
|
||||
|
||||
Продуктовый код (`src/**`, `custom_components/**/*.py`) не тронут — это этап
|
||||
ТЗ, гейты код-ревью здесь не применяются.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны PROCESS.md (§1–§10.4), AGENTS.md, `docs/SCOPE.md` целиком.
|
||||
2. Прочитано тело issue #437 целиком и все 7 комментариев (аналитика 07.09,
|
||||
ответы владельца Q1–Q5, дополнение о скрытии при нехватке места, занятие
|
||||
автором, сдача на ревью).
|
||||
3. Прочитан `docs/specs/437-summary-panel.md` целиком (720 строк, 14 разделов,
|
||||
AC1…AC26).
|
||||
4. Сверены технические утверждения ТЗ с фактическим кодом на этом дереве
|
||||
(`dev@ed9ee026`, идентично текущему src/**, так как diff — только docs):
|
||||
- `src/houseplan-panel.ts` — подтверждено: `narrow` сеттер только
|
||||
`toggleAttribute` на себе, в `_card` не пробрасывается (строки 59–62)
|
||||
→ утверждение §3/§5.3 о недостающей проброске верное, не догадка.
|
||||
- `_kioskScale` применяется только при `this._kiosk` (строки 6206–6207,
|
||||
11812) → утверждение §8.3 о необходимости расширения на обычный View
|
||||
верное.
|
||||
- `_canManageConfiguration`, `_sendConfigCandidate`, `_roomArea`,
|
||||
`_cleanFloor`, `_renderKioskDialog`, `innerContourForRoom`,
|
||||
`floorMinusBodies`, `geometryArea` — все существуют по указанным
|
||||
сигнатурам.
|
||||
- `ha-binding-status.ts` экспортирует `haRegistrySnapshot`,
|
||||
`activeRegistryHass`, `fullRegistryHass`, `resolveHaBindingStatus` —
|
||||
согласуется с §7.2.
|
||||
- `websocket_api.py` — `houseplan/config/get` уже возвращает
|
||||
capability-флаги того же вида, что предлагает ТЗ (`support_api`,
|
||||
`decor_assets_api`, строки ~1391–1392); добавление
|
||||
`summary_panel_api: 1` (§9.2) — не новая конструкция, а повтор
|
||||
установленного паттерна.
|
||||
- `docs/CONFIG-COMPATIBILITY.md` подтверждает установленный паттерн
|
||||
`settings.<namespace>` для аддитивных полей (`settings.zigbee_topology`,
|
||||
`settings.show_room_tooltip`) — предложенный `settings.summary_panel`
|
||||
ему соответствует.
|
||||
- `docs/TOUCH-SUPPORT.md` — формат обязательной декларации
|
||||
(`Touch editor: supported / best effort / not exposed`) соблюдён
|
||||
дословно в §5.4.
|
||||
- `docs/UX-MODES.md` — «редакторы блокированы в киоске даже для admin»
|
||||
подтверждено (строка 270) — совпадает с §8.1.
|
||||
- Терминология i18n-таблицы §10 (например, `kiosk.icon_scale` →
|
||||
«Размер значков устройств») сверена посимвольно с
|
||||
`src/i18n/ru.json`/`en.json`/`de.json` — совпадает.
|
||||
- Числовые примеры §5.2 (baseline 162 px, границы 303/304×640,
|
||||
185/186×800, 245/246×800 для киоска) арифметически проверены и сходятся.
|
||||
5. Проверена индексация: `docs/specs/README.md` содержит строку на #437;
|
||||
AC-таблица содержит ровно AC1…AC26 без пропусков и дублей (`grep`).
|
||||
6. `node scripts/check-docs.mjs` — pass (7 файлов, 12 внешних ссылок),
|
||||
воспроизведено самостоятельно, не только со слов автора.
|
||||
7. Проверено количество и состав языковых файлов: `src/i18n/{en,ru,de,fr}.json`
|
||||
— по 1257 ключей в каждом (равный паритет), `docs/CHANGELOG*.md` и
|
||||
`CONTRIBUTING.md` подтверждают, что французский — полноценная
|
||||
поддерживаемая локаль с автоматическим parity-тестом.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи, не блокирует само по себе, но требует правки этим же раундом)
|
||||
|
||||
**M1 — Раздел 10 «i18n и визуальное соответствие» пропускает французскую
|
||||
локаль, которая является полноценной поддерживаемой (не легаси/не частичной).**
|
||||
|
||||
- Файл: `docs/specs/437-summary-panel.md`, строки 542–571 (заголовок раздела и
|
||||
таблица RU/EN/DE).
|
||||
- Текст ТЗ: «Все новые runtime строки в **RU, EN и DE** (существующая немецкая
|
||||
локализация сохраняется)…», далее — трёхколоночная таблица RU/EN/DE без FR.
|
||||
- Факт: `src/i18n/fr.json` существует, содержит ровно 1257 ключей — столько
|
||||
же, сколько en/ru/de.json (проверено `python3 -c "json.load(...)"`), т.е.
|
||||
это не отстающий и не частичный файл, а локаль с полным паритетом.
|
||||
`docs/CHANGELOG.md:545` и `docs/CHANGELOG.ru.md:581`: «House Plan заговорил
|
||||
по-французски: полный перевод сообщества… выбирается явно и включается
|
||||
автоматически для французских профилей». `docs/USER-GUIDE.ru.md:209–210`
|
||||
документирует французский как штатный язык интерфейса.
|
||||
`CONTRIBUTING.md:14–28`: «A shipped UI language has three matching parts…
|
||||
The registry drives … parity tests. The tests reject missing or extra
|
||||
locale files; frontend dictionaries also fail on mismatched keys…» — то
|
||||
есть в проекте есть автоматический гейт паритета ключей по всем
|
||||
зарегистрированным локалям, включая `fr`.
|
||||
- Сценарий отказа: если реализация будет вестись буквально по §10 этого ТЗ,
|
||||
новые ключи (заголовок панели, «Показывать панель», «Добавить блок»,
|
||||
системные показатели и т.д. — минимум 15 строк из таблицы плюс
|
||||
неперечисленные состояния загрузки/ошибок) появятся в en/ru/de.json, но не
|
||||
в fr.json. Это либо (а) красный `npm test` на этапе код-ревью того же
|
||||
issue из-за несовпадения набора ключей между локалями — тогда автору
|
||||
придётся добавлять FR-перевод «в последний момент» без ТЗ на формулировки,
|
||||
либо (б) сознательное расхождение реализации с этим ТЗ. Оба исхода —
|
||||
дефект самого ТЗ, а не реализации: раздел, которому поручено перечислить
|
||||
языковые требования, даёт неполный список.
|
||||
- Это не архитектурное решение и не продуктовый вопрос владельцу — все
|
||||
формулировки RU/EN/DE уже даны, нужен четвёртый столбец FR (перевод есть
|
||||
кому делать: прецедент — «полный перевод сообщества» уже используется для
|
||||
остальных строк продукта). Правка укладывается в этот же документ, без
|
||||
расширения скоупа.
|
||||
- **Как закрыть:** добавить колонку FR в таблицу §10 (или отдельную
|
||||
таблицу), убрать формулировку «RU, EN и DE» на «RU, EN, DE и FR
|
||||
(все четыре — существующие поддерживаемые локали)», и явно отметить, что
|
||||
parity-тест (`npm test`) покрывает контроль набора ключей по всем
|
||||
зарегистрированным локалям.
|
||||
|
||||
### Low (снимаются с записью либо правятся автором по желанию — не блокируют)
|
||||
|
||||
**L1 — Термин `viewAllowed` в формуле §5.3 не введён явно.**
|
||||
|
||||
- Файл: `docs/specs/437-summary-panel.md`, строка 251:
|
||||
`effectiveVisible = viewAllowed && localShow && mobileAllowed && fits`.
|
||||
- В отличие от `fits` (явно определён строкой 204) и `mobileAllowed` (явно
|
||||
определён строкой 252 тут же), `viewAllowed` больше нигде в документе не
|
||||
вводится и не упоминается текстом. Смысл разумно выводится из §4.1:
|
||||
«В редакторах плана, устройств и подложки overlay и его View-контролы
|
||||
скрыты» — то есть `viewAllowed` похоже означает «карточка сейчас не в
|
||||
одном из трёх редакторов». Формула — часть контракта, из которого прямо
|
||||
выводится AC1/AC14/AC20, поэтому отсутствие явного определения — не
|
||||
фатально (единственное разумное прочтение уже есть в тексте на пять
|
||||
абзацев выше), но противоречит собственной точности документа, который в
|
||||
остальных местах вводит каждый предикат формулы явно.
|
||||
- **Решение ревьюера:** Low, не блокирует — оставляю на усмотрение автора;
|
||||
одна фраза («viewAllowed — не редактор, см. §4.1») закрыла бы вопрос
|
||||
окончательно, но обязательной не считаю, так как единственное прочтение
|
||||
уже присутствует в тексте.
|
||||
|
||||
**L2 — Систематически пропущены пробелы перед числами в ряде мест текста.**
|
||||
|
||||
- Примеры (подтверждено на уровне байтов через `od -c`, это не артефакт
|
||||
просмотра): «шаг5%, Reset100%» (строка 446), «ширина336» и «высота161»
|
||||
(строки 230–231), «измеренным верхним пределом72» (строка 232),
|
||||
«local scales50/100/300%» (строка 612).
|
||||
- Арифметику это не портит — я independently пересчитал все числовые примеры
|
||||
§5.2 (162 = 2+48+20+2+43+11+36; 161/162 и 245/246 с учётом insets/control
|
||||
reserve) и они сходятся, то есть смысл не искажён для внимательного
|
||||
чтения, но формат местами затрудняет беглое чтение точных
|
||||
инженерных цифр — ровно того, что этот документ в остальном делает
|
||||
образцово аккуратно.
|
||||
- **Решение ревьюера:** Low, снимаю с записью — не блокирует, косметика,
|
||||
можно поправить при любой следующей правке файла заодно с M1.
|
||||
|
||||
### Не найдено High
|
||||
|
||||
Ни одной находки, которая делает AC невыполнимым, непроверяемым или
|
||||
противоречащим SCOPE/PROCESS/каноническим документам подсистемы. В
|
||||
частности:
|
||||
|
||||
- Все обязательные разделы §7.1 PROCESS.md присутствуют (сценарий, что
|
||||
видит человек, скоуп/не-скоуп, контракт, UX, модель данных/миграция,
|
||||
i18n, AC с доказательством, план автотестов, риски, откат,
|
||||
release-артефакты).
|
||||
- Границы SCOPE (`docs/SCOPE.md` строки 115–122) процитированы и соблюдены
|
||||
дословно: read-only, три системных показателя, без произвольных карточек,
|
||||
формул, истории, действий с устройствами.
|
||||
- Продуктовые вопросы (Q1–Q5) закрыты решениями владельца до старта
|
||||
написания ТЗ; ни один открытый продуктовый вопрос в документе не остался.
|
||||
Технические допущения (namespace/version, per-card identity key,
|
||||
fit-baseline 280×162, narrow-bridge для старых HA, id/naming) явно собраны
|
||||
в §14 «Принято предположительно» и помечены как решаемые вердиктом
|
||||
ревьюера, а не владельцем — ровно то, что требует AGENTS.md.
|
||||
- AC1…AC26 пронумерованы без пропусков/дублей, у каждого — проверяемое
|
||||
поведение и способ доказательства (unit/backend/smoke), у AC с защитным
|
||||
характером (лимиты, guard прав, geometry invariants) — указан
|
||||
отрицательный свидетель словами («убрать fit-gate ⇒ …», «сравнение всего
|
||||
объекта или fallback-first ломают тест», «plan-filter mutation → красный»
|
||||
и т.д.), что на этой стадии (ТЗ, не код) — корректный уровень строгости.
|
||||
- Ни одно утверждение о текущем поведении продукта не оказалось
|
||||
непроверенной догадкой, выданной за факт — все проверенные технические
|
||||
ссылки на код/API/документы (раздел «Как проверялось», пункт 4)
|
||||
подтвердились.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Соответствие SCOPE-исключения (`docs/SCOPE.md`) и формулировки ТЗ/issue —
|
||||
дословное совпадение по объёму (три системных показателя, read-only,
|
||||
«не general dashboard framework»).
|
||||
- Полнота обязательных разделов ТЗ по §7.1 PROCESS.md.
|
||||
- Корректность всех проверенных технических ссылок на существующий код и
|
||||
API (см. «Как проверялось», п.4) — ни одна не оказалась неточной.
|
||||
- Внутренняя согласованность контракта показа/скрытия панели, независимости
|
||||
scope блока и системных показателей (Q3), политики сломанных ссылок (Q5)
|
||||
между телом issue, комментариями владельца и инженерным ТЗ — расхождений
|
||||
не найдено.
|
||||
- Индекс `docs/specs/README.md` обновлён корректно.
|
||||
- `node scripts/check-docs.mjs` — pass, воспроизведено самостоятельно.
|
||||
- Численные fixture-примеры §12 (счётчик устройств, площадь с
|
||||
пересечениями/копиями этажей) — конкретны и пригодны для unit-тестов без
|
||||
дополнительной интерпретации.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты код-ревью (`typecheck`, `npm test`, `npm run build`,
|
||||
`no-new-any`, golden, smoke, `pytest tests_backend`,
|
||||
`model-invariants`) — не запускал: этап ТЗ, диапазон правок только
|
||||
документация (класс C), продуктовый код не менялся, эти гейты
|
||||
относятся к код-ревью следующего этапа и станут применимы после начала
|
||||
реализации (которую владелец явно запретил начинать до отдельной
|
||||
команды).
|
||||
- Скриншоты/визуальные артефакты Dashboard 5 — сверка макетов дизайнера с
|
||||
реальным рендером не проводилась и не может быть проведена на этой
|
||||
стадии: продукта, который можно отрендерить, ещё нет (сам документ прямо
|
||||
говорит об этом в §2 и в комментарии автора).
|
||||
- Правильность будущей реализации `viewAllowed`/`fits`/registry-снапшота и
|
||||
прочих инженерных деталей §14 — они помечены как «принято
|
||||
предположительно, менять свободно» и по правилам AGENTS.md подлежат
|
||||
оспариванию на код-ревью, а не на ревью ТЗ.
|
||||
- French-переводы конкретных новых строк (сам перевод) — вне scope ревью
|
||||
ТЗ; закрывается автором вместе с M1 при правке раздела 10.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Единственная находка уровня Medium (M1, i18n) — в скоупе задачи, чинится
|
||||
правкой раздела 10 в этом же документе без изменения контракта или AC.
|
||||
High-находок нет. Итог — жёлтый вердикт, возврат автору на правку этим же
|
||||
циклом.
|
||||
|
||||
`Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче · Документ: docs/reviews/SPEC-REVIEW-437-r1.md`
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/437-summary-panel`, коммит `4a1aac16fefa` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `8d913a72959085e2d2d6b1cef0bda7c1fd065b31`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 8d913a729590
|
||||
```
|
||||
- ТЗ `docs/specs/437-summary-panel.md`, блоб `fc6650a59de278b770561cd63e5e92862256aafa`
|
||||
```
|
||||
git log --all --find-object=fc6650a59de278b770561cd63e5e92862256aafa -- docs/specs/437-summary-panel.md
|
||||
```
|
||||
Reference in New Issue
Block a user