diff --git a/docs/reviews/SPEC-REVIEW-561-r1.md b/docs/reviews/SPEC-REVIEW-561-r1.md new file mode 100644 index 00000000..3cbd403e --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-561-r1.md @@ -0,0 +1,173 @@ +# 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:`), сохранение после явного 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