From 6392179013ff7ed2d984298ffe24fea075fe225b Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 10:15:23 +0000 Subject: [PATCH] docs: review document for #372 Issue: #372 User-Visible: no --- docs/reviews/SPEC-REVIEW-372-r1.md | 190 +++++++++++++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-372-r1.md diff --git a/docs/reviews/SPEC-REVIEW-372-r1.md b/docs/reviews/SPEC-REVIEW-372-r1.md new file mode 100644 index 00000000..3221c554 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-372-r1.md @@ -0,0 +1,190 @@ +# SPEC-REVIEW-372-r1 + +Issue: [#372](https://github.com/Matysh/houseplan-card/issues/372) — «Remove empty +header spacing when `title` is empty in `houseplan-space-card`» +Этап: spec (PROCESS.md §2.4) · заход r1 · блокирующих циклов израсходовано 0 из 4 +ТЗ: `docs/specs/372-space-card-empty-title.md`, коммит `edb9b6b3` (ветка +`issue/372-space-card-empty-title`) +Трек: полный (аналитика назвала критерий `small`, который задача не проходит — +новый UX-контракт для `title`) + +## Скоуп ревью + +Первый заход по этой задаче — предыдущего вердикта нет, разбор полный. +Проверялось: соответствие `docs/SCOPE.md`, обязательные разделы ТЗ по §7.1, +однозначность и доказуемость каждого AC, отсутствие догадок, выданных за факт, +корректность продуктового вопроса и решения владельца, трейлеры коммита. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§2, §3, §4, §7, §8, §12). +2. Прочитано тело issue #372 и все 4 комментария: аналитика → продуктовый вопрос + Q1 → решение владельца (Default) → публикация ТЗ. +3. Прочитан `docs/USER-GUIDE.ru.md` §18 (`houseplan-space-card`) — терминология + `title`/`show_button`/`button_target` в ТЗ совпадает с действующим гайдом. +4. Прочитан `docs/CANVAS.md` §4, §4.1, §6 — канонический документ подсистемы, + так как задача трогает `spaceFrame`/`contentFrame`, разделяемые с иконочным + масштабированием. +5. Каждое фактическое утверждение ТЗ сверено с текущим кодом на `dev`-состоянии + рабочей копии (сам ТЗ-коммит кода не трогает): + - `src/space-card.ts:812,827` — `title = config.title !== undefined ? config.title : sp?.title || ''`; + `.hp-static-title` рендерится только если `title` truthy → подтверждён факт + «явная пустая строка уже не создаёт header». + - `src/space-geometry.ts:419-429` (`spaceFrame`) — симметричный `pad = 0.05` + на всех четырёх сторонах; fallback на `space.vb` без паддинга либо на + legacy unit square — подтверждён факт «полоса — верхняя часть 5%-го поля» + и корректность контракта п.6 («если content frame отсутствует, compact + mode не выдумывает новый crop»). + - `src/space-render.ts:274-275,543` — `vb = [fr.x, fr.y, fr.w, fr.h]` идёт + прямо в `viewBox`; `space-render.ts` импортируется только из + `space-card.ts` → подтверждена локальность правки (не задета полная + `houseplan-card`). + - `src/houseplan-card.ts:5516-5517` — тот же `spaceFrame` вызывается без + переопределения pad → допустимость «необязательного параметра» в ТЗ + (раздел «Принято предположительно») не меняет дефолтный путь других + потребителей. + - `src/space-geometry.ts:457-499` (`iconUnit`, `iconCqw`) — масштаб иконок + считается от `contentFrame(items, {pad:0})` и делится на `vb[2]` (ширину), + а не высоту. Контракт ТЗ (п.4: «меняются только `viewBox.y` и `height`, + ширина неизменна») технически не задевает формулу размера иконок и + `declump`-раскладку — риск «иконки поедут» в ТЗ закрыт корректно. + - `src/space-render.ts:541` — `style="aspect-ratio:${vb[2]}/${vb[3]}..."` — + подтверждает, что уменьшение `height` уменьшит и высоту сцены/карточки, как + заявлено в «Что человек увидит». + - `demo/docs/` не содержит упоминаний `space-card` → подтверждена гипотеза + ТЗ «канонические PNG документации не содержат static space card, поэтому + `check-docs` не даст визуальной регрессии по этой карточке». + - `docs/specs/README.md` diff — строка добавлена корректно, ссылка и путь + совпадают. +6. Файлы, названные в разделе «Затронутые файлы», существуют: + `demo/smoke_space_card.mjs`, `test/space-geometry.test.mjs`. +7. Коммит `edb9b6b3` проверен `git show -s --format=full`: трейлеры + `Issue: #372` / `User-Visible: no` корректны для документации без изменения + поведения (класс C, PROCESS.md §1/AGENTS.md). + +Гейты кода не гонялись — на этапе spec-ревью нет диффа продуктового кода; +единственный диф — `docs/specs/**` и `docs/specs/README.md`, класс C. + +## Проверка по SCOPE.md + +Сценарий закрывает J1 (`docs/SCOPE.md`: «Show the whole home... live spatial +overview») в его компактном read-only варианте — `houseplan-space-card` явно +описана в SCOPE.md как View-поверхность. Изменение чисто визуальное, не +расширяет интерактивность, не затрагивает лок-инвариант, не входит в +Out-of-scope список. Соответствует. + +## Проверка обязательных разделов (§7.1) + +Присутствуют все обязательные разделы: сценарий · что человек увидит до/после · +проблема · скоуп и не-скоуп · контракт поведения (9 пунктов) · UX и +доступность · модель данных/миграция/совместимость · i18n · AC1–AC8 с +доказательством · план автотестов (7 шагов) · риски (привязаны к AC) · откат · +release-артефакты. Дефектов нет. + +Первые два раздела («Сценарий», «Что человек увидит») отвечают на требуемые +вопросы: персона, поверхность, момент; и «одной фразой, без терминов +реализации» — выполнено, реализационные детали (`spaceFrame`, `viewBox`) +вынесены в раздел «Проблема», как и предписано структурой. + +## Проверка AC + +Все 8 AC пронумерованы, каждый называет способ доказательства (`unit`, +browser `smoke`, «ревью кода», CI). Формулировки однозначны и допускают +проверку числом (координаты `viewBox`, `getBoundingClientRect()`), а не +описательно. AC5 отдельно защищает вырожденные/fallback случаи от NaN/ +Infinity — соответствует существующему коду (`spaceGeometry.ts` уже обрабатывает +`DEGENERATE`/`SANE_LIMIT`, ТЗ не выдумывает новый механизм, а требует его +сохранения). + +Замечаний по AC нет. + +## Проверка на догадки, выданные за факт + +Раздел «Проблема» содержит числовые утверждения (`topGap = 0 px` при пустом +title, `topGap = 37 px` при непустом) — они взяты из аналитического +комментария к issue («Проверенный факт»), а не из самого ТЗ, и подтверждаются +кодом (см. «Как проверялось», п.5). Формулировки контракта поведения — не +предположения, а прямое отражение того, что уже верно для существующего кода +(`spaceFrame`, `iconCqw`) плюс явное решение владельца по Q1. + +Раздел «Принято предположительно, поменять свободно» корректно ограничен +непродуктовыми деталями (имя helper, точное место арифметики, конкретная +fixture, способ переиспользования `spaceFrame`) — эти решения не наблюдаемы +пользователем и по §7.1 их не нужно выносить владельцу. + +Единственная пограничная зона — поведение `title` с одними пробелами +(whitespace-only). ТЗ явно относит её к «Не-скоуп» («значение, отличное от +точной пустой строки, сохраняет текущую семантику») и не нормализует. Это +можно было бы счесть продуктовым пограничным случаем (§7.1 перечисляет +«поведение в пограничном случае» как повод спросить владельца), но исходный +текст issue и Q1 говорят именно о `title: ""`, альтернативное поведение не +запрошено пользователем, а решение сохраняет статус-кво (наименьшее +удивление) и явно задокументировано, а не скрыто. Не считаю это находкой — +уровень Low, снимаю с записью: граница названа, обоснована и не меняет +видимое поведение относительно текущего `dev`. + +## Проверка вопроса владельцу и его решения + +Q1 задан корректно по форме §7.1: что неясно · что изменится от ответа · +предлагаемый вариант по умолчанию (Default/Альтернатива). Вопрос продуктовый +(влияет на видимый контракт `title`), не технический. Issue корректно ушёл в +`S3-spec` + `blocked` на время ожидания и разблокирован после ответа владельца. +ТЗ включает принятое решение (Default) без искажений. + +## Проверка трейлеров и артефактов + +- `Issue: #372` / `User-Visible: no` на коммите `edb9b6b3` — корректно, диф не + меняет пользовательское поведение (только `docs/specs/**`). +- `docs/specs/README.md` — запись добавлена, путь и заголовок совпадают. +- Ссылка issue ↔ ТЗ на месте с обеих сторон (тело issue ссылается на файл ТЗ в + ветке; файл ТЗ ссылается на issue). + +## Находки + +Нет находок уровня High или Medium. Одна Low-находка (whitespace-only title, +см. выше) рассмотрена и снята с записью прямо в этом документе — правка ТЗ не +требуется. + +## Что проверено и корректно + +- Полнота обязательных разделов ТЗ (§7.1). +- Однозначность и доказуемость всех 8 AC. +- Фактические утверждения о текущем поведении (`topGap`, `pad=0.05`, + использование `spaceFrame`/`iconCqw`) — подтверждены чтением + `src/space-card.ts`, `src/space-geometry.ts`, `src/space-render.ts`, + `src/houseplan-card.ts`. +- Локальность правки: `space-render.ts` используется только `space-card.ts`; + `spaceFrame` также используется в `houseplan-card.ts` без переопределения + `pad`, поэтому необязательный параметр не сломает дефолтный путь полной + карточки — заявленный в ТЗ риск «случайно меняется полная карточка» + технически обоснованно закрывается выбранным подходом. +- Независимость масштаба иконок (`iconUnit`/`iconCqw`) от высоты + `viewBox` — компакт-режим меняет только `y`/`height`, что не пересекается с + формулой размера иконок (делится на ширину `vb[2]`, не на высоту). +- Отсутствие static space card в канонических docs-скриншотах — заявление ТЗ + о нулевом визуальном дифе `check-docs` подтверждено по дереву `demo/docs/`. +- Продуктовый вопрос Q1 задан и решён по процессу, решение отражено в ТЗ без + искажений. +- Трейлеры коммита `edb9b6b3` и запись в `docs/specs/README.md`. +- Названные файлы для тестов (`demo/smoke_space_card.mjs`, + `test/space-geometry.test.mjs`) существуют. + +## Чего не проверял + +- Продуктовый код не написан на этом этапе — код-ревью впереди, при переходе в + `S6`→`S7`. Технической реализуемости контракта я касался только для того, + чтобы убедиться, что ТЗ не строится на неверных фактах о коде (не для + оценки будущей реализации). +- Гейты `typecheck`/`test`/`build`/`golden`/`check-docs`/смоки не гонялись — + на spec-ревью нет диффа `src/**`, единственный диф — `docs/specs/**`. +- Не проверял историю других issue (#150, #214 и т.д.), кроме тех, что нужны + для контекста самого процесса ревью (эта задача — r1, «Унаследовано» не + применимо). + +## Вердикт + +Зелёный. ТЗ полное, каждый AC доказуем и однозначен, факты о текущем +поведении подтверждены чтением кода, продуктовый вопрос корректно решён +владельцем до написания ТЗ, догадок за фактами не найдено. Единственная +пограничная зона (whitespace-only title) рассмотрена и снята Low-находкой без +правки ТЗ.