Files
houseplan-card/docs/reviews/SPEC-REVIEW-561-r1.md
2026-09-12 22:47:16 +00:00

16 KiB
Raw Permalink Blame History

SPEC-REVIEW-561-r1

Скоуп

Issue #561 (bug, P3, S4-spec-review, полный трек — критерий small «сложность/риск ≤3» не пройден, о чём аналитика прямо сказала). Предмет: устойчивость identity локальных настроек сводной панели (show/icon_scale/font_scale) в нативном HA Masonry после полного reload/rebalancing колонок, когда stableSummaryPlacementSlot() строит ключ из фактического DOM-пути (включая номер визуальной колонки), а не из канонического порядка hui-masonry-view.cards. ТЗ живёт в теле issue, раздел ## ТЗ, как того требует #517.

Материал ревью: тело issue #561 на момент разбора (нормализованный sha256 тела фиксирует сам конвейер в блоке якорей — вручную не пересчитывал). Рабочее дерево репозитория — b26d9694 (origin/dev), совпадает с материалом, названным в задании.

Как проверялось

  • Прочитаны docs/SCOPE.md (J1/J6, персоны, touch-контракт), AGENTS.md, PROCESS.md §1–§7 целиком, docs/TOUCH-SUPPORT.md целиком.
  • Прочитано тело issue #561 целиком (постановка риска + ## ТЗ) и оба комментария (S2 — аналитика, Вопрос владельцу Q1) через gh issue view 561 --json body,comments.
  • Прочитан текущий код, который ТЗ обязуется заменить: src/summary-panel-identity.ts (stableSummaryPlacementSlot, structuralPath, enclosingNativeCard) и src/summary-panel-runtime-loaded.ts (preferenceKey, loadLocal, saveLocal, использование storage_unavailable).
  • Прочитан src/summary-panel.ts (summaryLocalKey, parseSummaryLocal, SUMMARY_PANEL_LEGACY_SCALE_KEY) — проверка, что «legacy scale seed» в AC5 это существующий отдельный глобальный ключ houseplan_card_kiosk_v1, а не спорный per-card DOM-ключ, то есть решение владельца по Q1 (старые DOM-path ключи не переносятся) этому не противоречит.
  • Прочитан существующий регресс-тест test/summary-panel.test.mjs (#437 placement identity survives Masonry reflow and inner-card remount, строки 249–268): подтверждено буквально то, что говорит аналитика — тест переставляет уже созданные firstWrapper/secondWrapper внутри masonry.children (same-wrapper reflow), но никогда не создаёт новые wrapper-объекты для полного reload. Значит новый AC1 (новые view/card/column объекты) — это не дублирование существующего покрытия, а реально недостающий кейс.
  • Прочитан архивный docs/specs/437-summary-panel.md §8.2 (строки 419–443): исходный контракт #437 уже требовал для Masonry «индекс в исходном cards, не номер визуальной колонки» — то есть #561 закрывает именно расхождение между тем контрактом и фактической реализацией, а не придумывает новый.
  • Проверено существование всех файлов, которые ТЗ называет затронутыми: src/summary-panel-identity.ts, src/summary-panel-runtime-loaded.ts, test/summary-panel.test.mjs, scripts/mutation-gate.mjs, demo/smoke_summary_panel.mjs (и соседние smoke_summary_*) — все на месте.
  • Проверен текст summary.storage_unavailable (src/summary-panel-i18n.ts:45,113,181,249): «These screen settings work for this session but cannot be saved in this browser» — достаточно нейтрален, чтобы AC4 мог переиспользовать его для «identity ещё не разрешена», не вводя новый i18n-ключ, как ТЗ и заявляет.
  • Код продукта не менял и не правил — только ревью ТЗ.

Находки

Medium (в скоупе задачи — возврат автору, отдельный issue не заводится)

M1. ТЗ не называет явно влияние на touch/View/kiosk — обязательный, блокирующий пункт DoR (§2.5: «влияние на touch по docs/TOUCH-SUPPORT.md (View и киоск — блокирующие)»).

  • Файл: тело issue #561, раздел ## ТЗ.
  • Слово «touch» и «kiosk»/«киоск» не встречается в тексте ТЗ ни разу (gh issue view 561 --json body -q '.body' | grep -ni "touch\|kiosk\|киоск" — пусто).
  • docs/TOUCH-SUPPORT.md прямо называет сводную панель поддерживаемой View- поверхностью и требует, чтобы она «remain usable after backgrounding, resize, orientation changes and warm remount», а киоск — «Primary supported environment» для View. Ротация планшета в киоске — это ровно тот случай, который контракт п.2 ТЗ описывает технически («адаптивный переход между разным числом Masonry-колонок») и который покрывают AC1/AC3, просто без слова «touch/kiosk» рядом.
  • Из-за этого DoR-пункт формально не закрыт: пятый пункт чек-листа «Готово к разработке» (§2.5) требует явного утверждения, а не подразумеваемого. Это не находка о реальном дефекте поведения — по существу сценарий уже покрыт AC1/AC3 — а находка о неполноте самого ТЗ как артефакта, обязательного для перехода в S5-ready.
  • Как закрыть: одно-два предложения в разделе 2 или 6, прямо говорящие Touch/kiosk: …, с указанием, какой AC доказывает переживание ориентации/resize на киоск-планшете (по факту — уже написанные AC1/AC3), либо явное «не влияет, потому что…», если авторы считают иначе.

Low (снимаю с записью, не блокирует)

L1. Разделы «сценарий» и «что человек увидит до/после» не оформлены отдельными подзаголовками первыми, как того требует PROCESS.md §7.1 («Два первых раздела — продуктовые, и они идут первыми не случайно»). По содержанию оба вопроса закрыты: раздел 1 («Проблема и цель») называет персону через контекст #437/SCOPE J1/J6 и условия появления бага, раздел 2 («Пользовательский контракт», особенно пункты 1–2) формулирует «что человек увидит» без терминов реализации («местные настройки остаются независимыми», «сохраняют настройку именно своей карточки»). Снимаю: смысл присутствует, реорганизация — не более чем польза для читаемости, откладывать задачу ради неё нецелесообразно.

Что проверено и корректно

  • Риск не выдан за факт (проверка на «пользователь заявил догадку решением»): ключевое утверждение аналитика — расхождение с реальным hui-view.ts/ hui-masonry-view.ts пинованной версии 20260729.7 — подтверждено конкретными ссылками на исходники HA с номерами строк в комментарии S2, а не голым заявлением. Технические детали контракта раздела 3 (canonical index, порядок обхода composed ancestors, поведение WeakMap-кеша) прослеживаются к реальному коду summary-panel-identity.ts, который они заменяют, — ни одного пункта, придуманного без опоры на существующий код или подтверждённое поведение HA.
  • Единственный продуктовый вопрос (Q1) задан и решён предсказуемо: дефолт («не переносить неоднозначные ключи, обычные defaults, пользователь один раз перенастраивает») зафиксирован в разделе 2 п.5 как решение владельца и совпадает с рекомендованным по умолчанию вариантом из вопроса — не более широкое и не более узкое решение, чем спрашивалось.
  • Технические допущения промаркированы, а не выданы за факт: раздел 8 явно помечает как «принято предположительно, менять свободно» — форму версии/имени slot (masonry-v2:<index>), сохранение после явного reorder (уже было решено в #437, не ново), выбор «unresolved лучше эвристической записи».
  • Каждый AC1–AC8 указывает способ доказательства (unit/unit/unit/unit-runtime/ unit/unit/mutation/frontend) и сформулирован как проверяемое утверждение, а не пожелание: у AC1 явно назван негативный якорь («старый slot C никогда не равен новому slot B» — это прямое повторение воспроизведённого дефекта, а не общая фраза).
  • AC7 — именно защитный AC с названным мутантом (замена canonical-index на visual-column index / снятие fail-closed guard), что закрывает системную дыру, которую сам процесс называет причиной прошлых проблем (#435, #423): дефектный guard, зелёный без единого мутанта. Здесь мутант закладывается уже на этапе спецификации.
  • Скоуп не размыт: явно исключены новый card_id, серверная миграция, перенос предпочтений после ручного reorder (наследие #437), расширение на Sections/unknown wrappers. Ничего из этого не просочилось в AC под видом «заодно поправим».
  • Откат и release-артефакты названы: откат — вернуть прежний resolver и тесты, без миграции данных; changelog RU+EN, docs/ARCHITECTURE.md/docs/TESTING.md затронуты по существу (источник identity меняется).
  • i18n и совместимость сервера/конфигурации явно закрыты («не меняются») — соответствует тому, что фактически меняется только клиентский resolver.
  • Формат: "small" не подходит и это названо явно аналитиком («сложность и риск 5/10… не проходит критерий small «сложность и риск ≤3»; затрагивается browser-local compatibility-контракт ключей») — соответствует требованию §338 называть нарушенный критерий, а не просто выбрать полный трек.

Чего не проверял

  • Не запускал никакие гейты (typecheck/test/build/mutation-gate) — на этапе ревью ТЗ кода ещё нет, реализация не начата (S6-in-progress не наступил), проверять нечего.
  • Не проверял реальное поведение hui-masonry-view.cards в живом Home Assistant — доверился ссылкам на исходники HA 20260729.7 в комментарии аналитика (конкретные файлы и диапазоны строк), сам HA-фронтенд не разворачивал.
  • Не проверял # 493 и #552 построчно на предмет пересечения — принял утверждение аналитика об отсутствии дублирования на основании описанных в issue отличий (repeated setConfig/lifecycle vs E2E journeys vs identity resolution), не вычитывал сами issue #493/#552.
  • Не оценивал, отвечал ли владелец на Q1 именно комментарием в GitHub — в треде issue отдельного ответа нет, но раздел 2 п.5 ТЗ фиксирует решение текстом, совпадающим с предложенным дефолтом; процесс не требует, чтобы решение владельца обязательно приходило тем же каналом, что вопрос.

Вывод

Единственная блокирующая (в бюджете цикла) находка — M1, чисто оформительская по существу (сценарий уже спроектирован и покрыт AC1/AC3), но формально пропущенный обязательный пункт DoR. High-находок нет. Вердикт — жёлтый: автор дописывает явное указание touch/kiosk-влияния (или явное «не применимо» с обоснованием) в тот же раздел ТЗ, без пересмотра AC/контракта, и документ уходит на второй заход.


Материал раунда

  • Ветка: dev, коммит b26d9694025e — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 22783006accc33717edd960c94098bf536d89e25
    git log --all --format='%H %T' | grep 22783006accc
    
  • Тело issue: 5c619c9775f834a6607d8623a303f1ba412d1f0cf9c4f4fa8b0e2fcf50cef47b
  • Вердикт конвейера: yellow · High 0