mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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:<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/контракта, и документ уходит на второй заход.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `b26d9694025e` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `22783006accc33717edd960c94098bf536d89e25`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 22783006accc
|
||||
```
|
||||
- Тело issue: `5c619c9775f834a6607d8623a303f1ba412d1f0cf9c4f4fa8b0e2fcf50cef47b`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user