diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 475b8420..63ac7c6a 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,9 +1,10 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1084, issue: 384. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1085, issue: 385. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| +| #665 | [SPEC-REVIEW-665-r1.md](SPEC-REVIEW-665-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | заявленная правка docs/ISOMETRIC.md | `docs/ISOMETRIC.md` `docs/adr/160-isometric-stage3-overlays.md` `check-docs.mjs` `ISOMETRIC.md` | | #664 | [CODE-REVIEW-664-r1.md](CODE-REVIEW-664-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #663 | [SPEC-REVIEW-663-r1.md](SPEC-REVIEW-663-r1.md) | spec · r1 | 🟡 жёлтый | 1 | 0 | §6.2 (wall snap) и §8 (canonicalization/Optimize) описывают два; AC7 называет четыре состояния сломанной; AC13 не называет конкретный бюджет | `docs/CANVAS.md` `docs/WALL-THICKNESS.md` | | #663 | [SPEC-REVIEW-663-r2.md](SPEC-REVIEW-663-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-665-r1.md b/docs/reviews/SPEC-REVIEW-665-r1.md new file mode 100644 index 00000000..8a0dbcf0 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-665-r1.md @@ -0,0 +1,213 @@ +# SPEC-REVIEW-665-r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/665 +- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4) +- **Трек:** лёгкий (`small`), но не `trivial` — авторская аналитика (комментарий + 1) сама называет причину: правка задевает цель касания поднятой подписи + комнаты в 2.5D View/киоске (блокирующая зона `TOUCH-SUPPORT.md`), поэтому её + сохранение доказывается отдельным AC, а не декларацией — отсюда полноценное + ревью ТЗ, а не короткий трек. +- **Материал:** тело issue #665 (единственная редакция, раздел `## ТЗ`) + + комментарий владельца (аналитика/оценка, метка `S4-spec-review`, вопросов + владельцу нет). +- **Заход:** r1 · блокирующих циклов израсходовано 0 из 2 +- **Роль:** ревьюер ТЗ (не автор) + +## Скоуп ревью + +CSS-баг в 2.5D View: `.stage.projection-iso.mode-view .roomlabel` использует +`min-height: 44px` + `justify-content: center` как способ дать поднятой подписи +комнаты минимальную цель касания. Побочный эффект — расстояние от имени +комнаты до строки показателей (`.rlmetrics`, абсолютно позиционирована от +низа контейнера подписи) зависит от того, насколько 44 px больше высоты +самого имени, то есть меняется при зуме. Во Flat такого эффекта нет — там +контейнер не растягивается, и зазор — постоянные `0,15em`. Контракт: перенести +44×44 px минимум с самого блока подписи на невидимый `::before` (приём уже +применяется для `.dev`/`.oplock`), вернув геометрию блока подписи к +поведению Flat. Не в скоупе: размер шрифта подписи на зуме, состав +показателей, раскладка столкновений поднятых подписей. + +**SCOPE-проверка (`docs/SCOPE.md`):** правка обслуживает J1 («show the whole +home… room fills… values») и опирается на узкое исключение #89 (2.5D — +детерминированная проекция того же состояния/геометрии, что и Flat, без +второй модели): здесь она восстанавливает именно это равенство, устраняя +единственное расхождение геометрии подписи между Flat и 2.5D. Новой +scope-дыры не создаёт: правка не меняет состав показателей, не вводит новую +интерактивность и не расширяет исключение #89. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `docs/process/REVIEWER.md`, по ссылкам — + PROCESS.md §2.4, §2.5, §7.1, §7.2, §4 (§2.10 не применялся — это r1). +2. Прочитано тело issue #665 целиком (`gh issue view 665 --json body`) и + единственный комментарий владельца (`--json comments`) — вопросов + владельцу нет, подтверждаю по тексту. +3. Сверены обязательные разделы §7.1: сценарий (персона «домочадцы и киоск», + поверхность 2.5D View, момент — зум) — на месте и без терминов реализации; + «что человек увидит до/после» — одной фразой каждое; проблема и причина + воспроизведены исполнением (демо-стенд, таблица замеров на 4 уровнях зума); + скоуп/не-скоуп — явно; контракт C1–C3 — однозначен; модель + данных/миграция/i18n — явное «нет»; AC1–AC3 — каждый с указанным способом + доказательства (smoke/smoke/мутант); откат — «вернуть правило»; + release-артефакты — changelog RU+EN, golden (условно, с оговоркой о приёмке + по CI), скриншоты документации. Явного заголовка «Риски» и «UX» нет, но + содержание покрыто (см. находку Low-1 ниже — только по одному пункту, доке). +4. **Перепроверены фактические утверждения ТЗ по коду на SHA `942b9ede` + (== origin/dev), а не приняты на слово:** + - `src/styles/plan.styles.ts:692-702` — правило `.stage.projection-iso + .mode-view .roomlabel { min-width: 44px; min-height: 44px; + justify-content: center; … }` существует буквально как процитировано в + issue. + - `src/styles/plan.styles.ts:555-570,647-649` — `.roomlabel` это flex-column + c `gap: 0.15em`; `.rlname`/`.rlmetrics` (`src/houseplan-card.ts:12077, + 12087`) — прямые дети; `.rlmetrics` вынута из потока + (`position: absolute; top: calc(100% + 0.15em)`), поэтому единственный + flex-элемент — `.rlname`, и в 2.5D `justify-content: center` при + `min-height: 44px` центрирует имя в блоке высотой 44 px, а + `top: calc(100% + 0.15em)` считается от высоты **контейнера** + (`.roomlabel`), а не от низа имени. Математика issue + (`(44 − высота_имени)/2 + 0.15em`) воспроизводится по коду точно, не на + веру. + - `demo/smoke_isometric_contract.mjs:36-47` (`owns44`) — уже берёт + `Math.max(rect.width/height, ::before width/height)`, то есть уже + нейтрален к тому, откуда взялся 44-px минимум (сам блок или его + `::before`). Утверждение AC2 «`owns44` уже учитывает `::before`» — + подтверждено чтением, не принято на слово. + - `src/iso-scene-render.ts:229-264` (`isoRaisedOverlayHalfSize`, кейс + `'room-label'`) — считает половину футпринта из `nameHeight`/текстовых + метрик шрифта, константы `44` в функции нет. Утверждение C3 «раскладка + столкновений уже считает от шрифта, а не от 44 px» — подтверждено. + - `src/styles/plan.styles.ts:541-551` (`.oplock::before`) — приём + invisible-`::before` с `width/height: max(44px, 100%)`, + `transform: translate(-50%, -50%)`, `pointer-events: auto` уже применяется + в проекте буквально в предложенном автором виде — «принятое предположение» + не гипотетическое, а копия рабочего паттерна. + - `src/houseplan-card.ts:12058-12060` — узел с + `data-hp-iso-overlay-kind="room-label"` (тот, что проверяет `owns44`) — + это и есть сам `.roomlabel`, а не обёртка вокруг него; перенос + минимального размера на его собственный `::before` не рассинхронизирует + смок с реальным деревом DOM. +5. Проверен единственный сомнительный пункт «Затронутые файлы» — заявленная + правка `docs/ISOMETRIC.md` («строка про цель касания подписи»): найдена + находка Low-1 ниже. +6. `git branch -a` / `git log --all --oneline | grep 665` — ветки `issue/665-*` + и коммитов с трейлером `Issue: #665` не существует; `git diff + origin/dev...HEAD` пуст (HEAD == origin/dev == материал). Продуктового кода + для #665 нет — стадия `spec`, гейты (`tsc`, `test`, `build`, смоки, golden, + инварианты) неприменимы к этому этапу, это штатное состояние, а не находка. + +## Находки + +### Low-1 (снят без возврата) — заявленная правка `docs/ISOMETRIC.md` +(«строка про цель касания подписи») не соответствует текущему документу + +**Что не так.** Раздел «Затронутые файлы» называет `docs/ISOMETRIC.md` со +пометкой «строка про цель касания подписи», подразумевая правку существующей +строки. Проверено чтением всего файла: `docs/ISOMETRIC.md` не содержит ни +числа `44`, ни слова `touch`/`target`/`tap` применительно к подписи комнаты, ни +упоминания приёма `::before`. Единственное место в проекте, где написано «keeps +its existing axis-aligned minimum 44 CSS-pixel target», — это +`docs/adr/160-isometric-stage3-overlays.md:60`, ADR-документ (заморожена +история решения), и эта фраза не завязана на конкретный CSS-механизм +(`min-height` vs `::before`) — после правки она останется истинной без +изменений. + +**Почему не High/Medium.** Это не открытый продуктовый вопрос и не +противоречие контракту: ни один AC не зависит от этой правки, `check-docs.mjs` +не привязывает конкретную CSS-реализацию к тексту `ISOMETRIC.md`. Худший +исход — исполнитель не найдёт заявленную строку, ничего не тронет в этом файле +и это не сломает ни один AC. + +**Что нужно (не блокирует).** При реализации либо снять этот пункт из +«Затронутые файлы» (в `docs/ISOMETRIC.md` сейчас нечего редактировать), либо +явно решить, что добавляется новая строка (например, рядом с «Device markers, +room labels/cards and opening-lock badges keep their canonical floor anchors…», +`docs/ISOMETRIC.md:220`) с фактом «label touch target size lives on an +invisible `::before`, matching `.dev`/`.oplock`» — тогда это новый факт, а не +правка существующего. Оставляю на усмотрение исполнителя; не создаю отдельный +issue (Low, в скоупе, не блокирует). + +## Что проверено и корректно + +- Обязательные разделы §7.1 присутствуют по существу и в разумном порядке для + однострочного CSS-бага: сценарий и «что человек увидит» — продуктовые, без + терминов реализации; проблема подкреплена исполненной репродукцией (реальный + демо-стенд, а не гипотеза); скоуп/не-скоуп разделены явно. +- **Причина бага перепроверена по коду, а не принята на веру** — воспроизведена + вся цепочка: `min-height`+`justify-content:center` на flex-контейнере с одним + in-flow элементом + `.rlmetrics` абсолютно спозиционирована от контейнера, а + не от имени (см. «Как проверялось» п.4). Табличные замеры в issue (4 уровня + зума, отношение `gap/высота_имени` падает с 2,0 до 0,16 в 2.5D и держится на + ~0,09–0,1 во Flat) согласуются с этой моделью без противоречий. +- Контракт C1–C3 однозначен и без пересечений: C1 — числовая цель (`0,15em`, + учёт `--rl-name`/`--rl-meta`), C2 — сохранение 44×44 px через `::before`, + C3 — явный запрет трогать `isoRaisedOverlayHalfSize`/Flat/редакторы/ + `houseplan-space-card`. Каждый пункт проверяем независимо от двух других. +- **AC1** (smoke, gap/name-height ratio на двух зумах, допуск 0,02) — численно + привязан к уже исполненной таблице замеров, метод измерения (`gap = + rlmetrics.top − rlname.bottom`) уже указан и воспроизводим. +- **AC2** (существующая `raisedTargetsOwn44Pixels`/`owns44`) — заявление, что + проверка уже нейтральна к источнику 44 px, подтверждено чтением + `demo/smoke_isometric_contract.mjs:36-47`, а не принято как факт со слов + автора. +- **AC3** (мутант — вернуть `min-height: 44px`, красит AC1) — механизм + find/replace для мутации CSS-строки уже используется в + `scripts/mutation-registry.mjs` для аналогичных `min-height: 44px`-правил; + подход реализуем. +- «Принятые предположения» — не гипотетический дизайн с нуля, а копия рабочего + паттерна `.oplock::before` (`src/styles/plan.styles.ts:541-551`), уже + проверенного в проде для другого раскрытого элемента; технический риск + реализации низкий. +- Откат, i18n, миграция, перф — покрыты явным «нет»/«вернуть правило», без + зазора для домысливания. +- Открытых продуктовых вопросов нет — подтверждаю: контракт (C1: расстояние + как во Flat; C2: сохранить цель 44 px) отвечает на оба вопроса, которые + вообще мог бы задать владелец («что человек должен видеть» и «что не должно + сломаться»), и делает это без домысливания — ожидание владельца прямо + процитировано в теле issue («в 2.5D расстояние… не меняется — как в плоском + виде»). + +## Чего не проверял + +- Гейты (`tsc --noEmit`, `npm test`, `npm run build`, `check-docs.mjs`, смоки, + `golden:verify`, инварианты) — не прогонял: этап `spec`, ветки/коммитов для + #665 нет, `git diff origin/dev...HEAD` пуст. Предмет код-ревью после + реализации. +- Не проверял `golden`-матрицу построчно на предмет того, есть ли там кадр + именно 2.5D + `label_temp`/`label_light` включёнными на комнате с коротким + именем (условие ТЗ «изменятся, если есть в матрице»); нашёл, что фикстуры с + этими флагами существуют в `demo/golden/harness.mjs:621,657-660`, но не + сверял, попадает ли конкретно 2.5D-кадр под порог видимого изменения — это + разумно оставить `golden:verify` на код-ревью. +- Не оспаривал числовой допуск AC1 (0,02) по существу — это техническая деталь + реализации теста, не продуктовый контракт; при коде-ревью его стоит + сверить на устойчивость (не флейкует ли на реальных шрифтах/DPR), но это не + предмет ревью ТЗ. + +## Вердикт + +Обязательные разделы полны, причина бага и оба смок-факта (AC1 таблица замеров, +AC2 уже-нейтральный `owns44`, C3 уже-от-шрифта `isoRaisedOverlayHalfSize`) +перепроверены по коду, а не приняты на слово автора — расхождений не найдено. +Единственная находка (Low-1, неточная ссылка на несуществующую строку в +`docs/ISOMETRIC.md`) не блокирует ни один AC и снимается без возврата автору. +Открытых продуктовых вопросов нет, откат и release-артефакты названы. Готово к +разработке. + +Вердикт: зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0 → в задаче + +--- + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `942b9ede6789` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `4fd2ba833946aa89eac3733667f2d15b952b24c5` + ``` + git log --all --format='%H %T' | grep 4fd2ba833946 + ``` +- Тело issue: `8da493f662bb94963a92e6ca40c224f16235947efbd137e809e313833e01cc97` +- Вердикт конвейера: `green` · High 0