diff --git a/docs/reviews/SPEC-REVIEW-437-r1.md b/docs/reviews/SPEC-REVIEW-437-r1.md new file mode 100644 index 00000000..348fb965 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-437-r1.md @@ -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.` для аддитивных полей (`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` + +--- + + + +## Материал раунда + +- Ветка: `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 + ```