mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -1,11 +1,12 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1058, issue: 373. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1059, issue: 374. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| #649 | [SPEC-REVIEW-649-r1.md](SPEC-REVIEW-649-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | в скоупе задачи (возвращается автору); принято ревьюером с записью, правки не требует | `lab.js` `houseplan-card.ts` |
|
||||
| #649 | [SPEC-REVIEW-649-r2.md](SPEC-REVIEW-649-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #648 | [SPEC-REVIEW-648-r1.md](SPEC-REVIEW-648-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | «Сценарий» не называет персону и; AC10 называет provenance-gate | `docs/SCOPE.md` `scripts/validate-commit-provenance.mjs` `validate.yml` |
|
||||
| #647 | [SPEC-REVIEW-647-r1.md](SPEC-REVIEW-647-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 2 | П.4 ТЗ переносит на новый слот прежнее; ТЗ не упоминает и не защищает документированный | `src/styles/dialogs.styles.ts` `src/styles.ts` `smoke_glow_blending.mjs` `smoke_test_facade.mjs` `smoke_unified_wall_tool.mjs` `docs/UX-MODES.md` `docs/reviews/CODE-REVIEW-195-r1.md` `src/styles/chrome.styles.ts` |
|
||||
| #647 | [SPEC-REVIEW-647-r2.md](SPEC-REVIEW-647-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #647 | [CODE-REVIEW-647-r1.md](CODE-REVIEW-647-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
|
||||
@@ -0,0 +1,257 @@
|
||||
# SPEC-REVIEW-648-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/648 —
|
||||
«Sections: вертикальный resize карточки и container-owned высота плана»
|
||||
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
|
||||
- **Трек:** полный (`P2`, автор явно обосновал в аналитике: новый UX-контракт,
|
||||
несколько поверхностей — HA grid API, layout CSS, переходы режимов, browser
|
||||
smoke — и host/input-риск; лёгкий трек не заявлялся)
|
||||
- **Материал:** тело issue #648, раздел `## ТЗ` (снимок на момент ревью,
|
||||
2026-09-25T11:40:53Z, комментарий `#issuecomment-5831785019`), плюс
|
||||
комментарий владельца с вопросами и ответами `#issuecomment-5831745868`.
|
||||
Кода ещё нет — `S4-spec-review` только что применена, ветка
|
||||
`issue/648-sections-card-resize` в самом ТЗ упомянута, но не является
|
||||
предметом этого ревью.
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
|
||||
- **Роль:** ревьюер ТЗ (не автор)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
ТЗ меняет контракт `getGridOptions()` основной карточки House Plan
|
||||
(`custom:houseplan-card`) с `{ columns: 'full' }` на `{ columns: 'full',
|
||||
rows: 10, min_rows: 6 }`, добавляет container-owned высоту сцены для
|
||||
Sections/grid-layout (аналогично уже существующей `panel-host`-цепочке) и
|
||||
переводит переходы View ↔ Plan/Devices/Backdrop на расчёт от фактической
|
||||
высоты карточки вместо `window.innerHeight`. Это осознанная отмена прежнего
|
||||
контракта #486 («rows, min_rows, max_rows и принудительная высота не
|
||||
задаются»), одобренная в этом же issue.
|
||||
|
||||
Первый вопрос — какую строку `docs/SCOPE.md` задача обслуживает. Прямой
|
||||
Core-job строки нет: это инфраструктурная/UX-правка поверхности, на которой
|
||||
живут J1–J7 для персоны Home admin (Sections — сейчас основной тип
|
||||
dashboard в HA, «Desktop browser… reference and recommended editing
|
||||
environment»). Задача не расширяет функциональность плана и не входит в
|
||||
список «Out of scope», поэтому конфликта со SCOPE.md нет; это ожидаемо для
|
||||
инфраструктурной polish-задачи P2, аналогично прецеденту #647-r1 (п.1 «как
|
||||
проверялось» того документа).
|
||||
|
||||
Проверялось: обязательные разделы §7.1, однозначность и доказуемость
|
||||
каждого AC, соответствие «Аналитики текущей реализации» фактическому коду
|
||||
`origin/dev` на материале ревью, согласие с прежним контрактом #486 и
|
||||
корректность его отмены, терминология (`docs/USER-GUIDE.ru.md`), отсутствие
|
||||
незакрытых продуктовых вопросов владельцу, отсутствие догадок, выданных за
|
||||
факт.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md`,
|
||||
PROCESS.md §2.3–§2.5, §4, §7.1, §7.2, §8.
|
||||
2. Прочитано тело issue #648 целиком и оба комментария владельца
|
||||
(вопросы Q1–Q3 с предложенными default'ами и последующее подтверждение
|
||||
«Q1–Q3 — defaults», плюс аналитика/оценка).
|
||||
3. Сверены семь фактических утверждений раздела «Аналитика текущей
|
||||
реализации» с `origin/dev` (SHA `dc6018ed`) чтением, а не на слово:
|
||||
- `getGridOptions()` действительно возвращает только `{ columns: 'full'
|
||||
}`, `getCardSize()` — `12`
|
||||
(`src/houseplan-card.ts:3750-3754`);
|
||||
- `houseplan-space-card` (`src/space-card.ts`) не имеет `getGridOptions`
|
||||
вовсе — подтверждено grep, совпадает с «не входит» и с AC1;
|
||||
- container-owned цепочка `panel-host` реально существует и совпадает с
|
||||
описанием «host → ha-card → .stage, включая min-height: 0 и flex»:
|
||||
`src/styles/base.styles.ts:40-79` (`:host([panel-host])`, `ha-card`
|
||||
flex-column, `.stage`/`.empty` `flex: 1 1 auto; min-height: 0`);
|
||||
- обычная dashboard-карточка действительно берёт высоту сцены от
|
||||
`100dvh`: `src/boot-soft-layout.ts:35`
|
||||
(`stage.style.height = calc(100dvh - headerHeight)`);
|
||||
- переход режимов для не-panel карточки действительно считается от
|
||||
`window.innerHeight`: `src/houseplan-card.ts:1342`
|
||||
(`this.panelHost ? this.clientHeight : innerHeight`);
|
||||
- `ResizeObserver` + `_refitView()` реально существуют и являются
|
||||
единственным путём refit: `src/houseplan-card.ts:4044-4046,
|
||||
4442` (`_roViewport`, `_refitView`);
|
||||
- прежний контракт #486 действительно запрещал `rows`/`min_rows`/
|
||||
`max_rows`: `docs/specs/486-house-plan-panel.md:462`. Все семь
|
||||
утверждений подтверждены буквально, ни одно не оказалось догадкой.
|
||||
4. Прочитан `docs/CONFIG-COMPATIBILITY.md` целиком (шапка, «Status
|
||||
meanings») — реестр покрывает только персистентные поля House Plan
|
||||
(`scripts/config-field-registry.mjs`); HA-шный `grid_options` в его
|
||||
область не входит, поэтому заявление ТЗ «отдельной миграции нет»
|
||||
корректно и не требует новой записи в реестре.
|
||||
5. Проверена терминология: `docs/USER-GUIDE.ru.md:472` (таблица режимов
|
||||
«Подложка») и раздел 10 «Устройства» подтверждают, что ТЗ использует
|
||||
принятые пользовательские названия режимов Plan/Devices/Backdrop
|
||||
(«План»/«Устройства»/«Подложка»), не изобретает новые.
|
||||
6. Проверено существование опор «плана автотестов»: `demo/smoke_houseplan_
|
||||
panel.mjs` существует (подтверждает «существующий panel smoke» в
|
||||
AC7/п.3 плана автотестов), `scripts/smoke-select.mjs` существует.
|
||||
7. Проверено, что заявленная в AC10 доказательная опора `provenance-gate`
|
||||
реально проверяет: `scripts/validate-commit-provenance.mjs` — по коду
|
||||
это коммит-трейлеры (`Issue:`, `User-Visible:`, `Release:`,
|
||||
`Baseline-Reviewed*`), а не содержимое changelog. Находка ниже.
|
||||
8. Проверено, что все три вопроса владельца (Q1 default-высота/минимум,
|
||||
Q2 однократный default для существующих карточек, Q3 поведение
|
||||
редакторов на маленькой высоте) закрыты ответом «defaults» и их
|
||||
значения буквально отражены в контракте (K1, K9, K6/K10
|
||||
соответственно) — открытых продуктовых вопросов не осталось.
|
||||
|
||||
Гейты не гонялись: этап `spec`, продуктового кода к задаче ещё нет
|
||||
(`S4-spec-review` только что применена), диапазон ревью — текст ТЗ в теле
|
||||
issue, а не диапазон коммитов. Это ожидаемо для ревью ТЗ, а не пропуск
|
||||
(см. §8: гейты §8 относятся к код-ревью).
|
||||
|
||||
## Находки
|
||||
|
||||
### Low-1 (снято решением ревьюера). «Сценарий» не называет персону и
|
||||
поверхность по словарю `docs/SCOPE.md` буквально
|
||||
|
||||
**Файл:** тело issue #648, раздел «Сценарий».
|
||||
|
||||
PROCESS.md §7.1 требует, чтобы раздел «Сценарий» явно отвечал «какая
|
||||
персона (`docs/SCOPE.md`), на какой поверхности, в какой момент это
|
||||
встретит». Текущий текст («Пользователь добавляет основную карточку House
|
||||
Plan в dashboard типа Sections… В режиме редактирования dashboard
|
||||
пользователь тянет штатную ручку HA…») не называет персону («Home admin»)
|
||||
и поверхность («Desktop browser») явными словами из таблицы персон
|
||||
SCOPE.md.
|
||||
|
||||
**Почему не блокирует.** Ответ восстанавливается однозначно из контекста:
|
||||
редактирование dashboard в HA технически доступно только пользователю с
|
||||
правом редактирования, а SCOPE.md прямо называет desktop-браузер
|
||||
«reference and recommended editing environment» для всех редакторов;
|
||||
двух прочтений сценарий не допускает, и ни один AC от этого не становится
|
||||
недоказуемым. Снимаю без правки текста; рекомендация автору на будущее —
|
||||
называть персону/поверхность явно словами из таблицы SCOPE.md, как это
|
||||
сделано, например, в закрытом #647.
|
||||
|
||||
### Low-2 (снято решением ревьюера). AC10 называет `provenance-gate`
|
||||
доказательством пункта, который эта проверка не покрывает
|
||||
|
||||
**Файл:** тело issue #648, таблица AC, строка AC10.
|
||||
|
||||
**Формулировка ТЗ:** «Доказательство: `check-docs`, code review и
|
||||
provenance-gate» — для наблюдаемого результата «RU/EN changelog
|
||||
согласованы и кредитуют `@pando80`».
|
||||
|
||||
**Что не так.** `scripts/validate-commit-provenance.mjs` (единственный
|
||||
скрипт с этим именем в репозитории, гейт `provenance` в `validate.yml`)
|
||||
проверяет исключительно трейлеры коммита (`Issue:`, `User-Visible:`,
|
||||
`Release:`, `Baseline-Reviewed*`/`-Local`) и подпись/происхождение baseline
|
||||
— он не читает текст changelog и никак не может подтвердить, что в нём
|
||||
упомянут `@pando80`. Названная как доказательство автоматика этот
|
||||
конкретный факт не проверяет; реально это заявление в AC10 доказывается
|
||||
только чтением («code review»).
|
||||
|
||||
**Почему не блокирует.** «code review» в этой же строке — правильный и
|
||||
достаточный способ доказательства для текстового упоминания в changelog;
|
||||
пропуск не делает AC10 недоказуемым, только слегка вводит в заблуждение
|
||||
про степень автоматизации. Снимаю без правки текста; рекомендация автору —
|
||||
убрать `provenance-gate` из этой строки или заменить его точным описанием
|
||||
(«code review сверяет текст обоих changelog»).
|
||||
|
||||
Ни одна находка не поднимается до Medium: обе — неточность формулировки,
|
||||
не создающая недоказуемый или неоднозначный AC, и ревьюер вправе снять Low
|
||||
своим решением (PROCESS.md §2.4).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все обязательные разделы §7.1 присутствуют: сценарий · что человек
|
||||
увидит до/после · проблема · скоуп и не-скоуп · контракт поведения (K1–
|
||||
K12) · UX · модель данных и миграция · i18n · AC1–AC10 с указанием
|
||||
доказательства · план автотестов · риски · откат · release-артефакты.
|
||||
- Все пункты DoR (§2.5), которые применимы на этом этапе, закрыты явно:
|
||||
i18n — «новых UI-строк нет» (K не добавляет ключей); миграция —
|
||||
«отдельной миграции нет», проверено против `CONFIG-COMPATIBILITY.md` (см.
|
||||
«как проверялось» п.4); влияние на производительность названо явно
|
||||
(«новый постоянный observer или polling запрещён», используется
|
||||
существующий `ResizeObserver`); влияние на touch названо (AC8, раздел
|
||||
«Performance и touch» — pan/pinch/click-координаты после resize);
|
||||
release-артефакты перечислены поимённо (оба changelog, USER-GUIDE RU/EN,
|
||||
ARCHITECTURE, TESTING, STATUS-FEATURES); откат описан конкретно
|
||||
(возврат `getGridOptions()` к `{ columns: 'full' }`, удаление
|
||||
grid-ветки).
|
||||
- Открытых продуктовых вопросов владельцу не осталось: Q1–Q3 заданы одним
|
||||
батчем с default'ами (по правилу §7.1) и закрыты ответом «defaults»;
|
||||
их значения дословно перенесены в контракт (K1 ← Q1, K9 ← Q2, K6/K10 ←
|
||||
Q3). Ни одна открытая техническая развилка не переслана владельцу —
|
||||
единственный оставшийся технический вопрос (как именно карточка
|
||||
определяет, что она размещена в grid/Sections-layout, а не в Masonry —
|
||||
K3/K11) корректно оставлен автору как техническая деталь и явно назван
|
||||
в разделе «Принято предположительно, поменять свободно» («техническое
|
||||
имя/тип reactive layout property и способ отражения grid-сигнала в
|
||||
CSS») — это ровно то место, где PROCESS.md §7.1 требует «принято
|
||||
предположительно», а не решения владельца, поскольку пользователь этот
|
||||
механизм не наблюдает; наблюдаемое поведение (grid-layout получает
|
||||
container-owned высоту, Masonry/panel-host — нет) остаётся проверяемым
|
||||
через AC7 независимо от выбранного механизма.
|
||||
- Отмена контракта #486 («rows не задаются») явно названа, обоснована и
|
||||
привязана к прямому одобрению владельца в этом же issue — не тихая
|
||||
правка чужого решения.
|
||||
- Контракт K2 (≈632 px / ≈376 px) явно помечен как справочная информация о
|
||||
текущей формуле HA, а не жёсткая константа для продуктового кода
|
||||
(«House Plan не дублирует эти пиксельные формулы в продуктовом коде»);
|
||||
AC2 требует сравнения с реально измеренным rect фикстуры, а не с этими
|
||||
числами буквально — не хрупко.
|
||||
- AC1–AC9 однозначны и называют способ доказательства (`unit`/`browser
|
||||
smoke`) и, где применимо к защитному поведению (AC4, AC5, AC9),
|
||||
называют отрицательную мутацию, которая должна покраснить конкретный
|
||||
smoke — пустых столбцов доказательства нет ни в одной строке таблицы.
|
||||
- Скоуп и не-скоуп не пересекаются и не оставляют серых зон: явно
|
||||
исключены sidebar panel, kiosk, Masonry, `houseplan-space-card`,
|
||||
собственный контрол высоты, dashboard YAML-миграция, изменение
|
||||
ширины/колонок Sections, буквальный cherry-pick, изменение
|
||||
zoom/pan-контракта вне реакции на resize.
|
||||
- Внешний вклад `@pando80` корректно учтён: перенесена идея (rows/
|
||||
min_rows + flex-высота), а не буквальный cherry-pick, с явным
|
||||
объяснением, почему форкнутый CSS недостаточен (не покрывает всю цепочку
|
||||
host→card→stage и переходы редакторов) — соответствует «Технические
|
||||
поверхности» и не создаёт риска слепого копирования стороннего кода.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты §8 (`typecheck`, `npm test`, `npm run build`, `check-docs.mjs`,
|
||||
browser smoke, golden, инварианты) — не прогонял: этап `spec`, кода к
|
||||
задаче ещё нет, диапазон ревью — текст ТЗ, а не коммиты. Предмет
|
||||
код-ревью после реализации.
|
||||
- Не проверял в браузере фактическое поведение Home Assistant Sections
|
||||
(реальные px на строку, наличие/отсутствие внешнего сигнала о
|
||||
grid-layout для custom-элемента) — на этом этапе кода нет, и HA как
|
||||
внешняя система не является материалом ревью; K2 корректно помечен как
|
||||
«около» и не хрупкий, а способ детектирования grid-контекста (K3/K11)
|
||||
прямо оставлен автору как техническая деталь, подлежащая проверке в
|
||||
код-ревью через AC7 (существующий panel smoke + новый соседний сценарий
|
||||
+ unit) и защитную мутацию.
|
||||
- Не оценивал производительность конкретной реализации (задача явно
|
||||
запрещает новый polling/observer, но не гарантирует отсутствие
|
||||
регрессии до появления кода) — это войдёт в код-ревью вместе с AC9
|
||||
(30 изменений высоты, bounded counters).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Найдено 0 High, 0 Medium. Два Low-замечания (неточность словаря в разделе
|
||||
«Сценарий»; неточная ссылка на `provenance-gate` в доказательстве AC10)
|
||||
сняты решением ревьюера с записью выше — обе не создают недоказуемый или
|
||||
неоднозначный AC и не блокируют переход в «Готово к разработке». Открытых
|
||||
продуктовых вопросов владельцу не осталось; единственная оставшаяся
|
||||
техническая развилка (механизм детектирования grid-layout) правильно
|
||||
оформлена как «принято предположительно, поменять свободно» и не требует
|
||||
решения владельца.
|
||||
|
||||
**Зелёный.** ТЗ готово к разработке.
|
||||
|
||||
---
|
||||
|
||||
Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/648-sections-card-resize`, коммит `dc6018ed5a0c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `6c27ae8bbfa0a56ba1de6ff59bf3f2cd41ec41f2`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 6c27ae8bbfa0
|
||||
```
|
||||
- Тело issue: `8400466150bde494c01aef14b7d5961be36d721b08a28c69e800bd88ac645154`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user