diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 9d261a48..7129c2d0 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -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 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-648-r1.md b/docs/reviews/SPEC-REVIEW-648-r1.md new file mode 100644 index 00000000..1bac0d3f --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-648-r1.md @@ -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 + +--- + + + +## Материал раунда + +- Ветка: `issue/648-sections-card-resize`, коммит `dc6018ed5a0c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `6c27ae8bbfa0a56ba1de6ff59bf3f2cd41ec41f2` + ``` + git log --all --format='%H %T' | grep 6c27ae8bbfa0 + ``` +- Тело issue: `8400466150bde494c01aef14b7d5961be36d721b08a28c69e800bd88ac645154` +- Вердикт конвейера: `green` · High 0