diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index c493a9fe..cfdae234 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,9 +1,10 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 167, issue: 78. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 168, issue: 79. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| +| #689 | [SPEC-REVIEW-689-r1.md](SPEC-REVIEW-689-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #688 | [SPEC-REVIEW-688-r1.md](SPEC-REVIEW-688-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #688 | [CODE-REVIEW-688-r1.md](CODE-REVIEW-688-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | AC6 не покрывает спиральную лестницу и второй масштаб печати на уровне PDF-сцены | `test/pdf-scene.test.mjs` `test/stairs.test.mjs` `src/pdf/pdf-scene.ts` | | #687 | [SPEC-REVIEW-687-r1.md](SPEC-REVIEW-687-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-689-r1.md b/docs/reviews/SPEC-REVIEW-689-r1.md new file mode 100644 index 00000000..1c5f7990 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-689-r1.md @@ -0,0 +1,247 @@ +# SPEC-REVIEW-689-r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/689 +- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4) +- **Трек:** полный. Аналитик сам назвал критерий §5, который задача не проходит: + «нарушены критерии `small`: нет влияния на производительность и на + touch-контракт (композиция слоёв, #531/#579/#582, HA Companion)» и «сложность + и риск ≤ 3». Разбор по существу — полный трек выбран корректно, задача трогает + композитинг слоёв и HA Companion tile loss (#582), это не мелкая правка. +- **Материал:** тело issue #689, раздел `## ТЗ`, плюс три комментария владельца: + (1) наблюдение «размытие зависит от масштаба загрузки страницы» с проверкой в + облаке на golden-сцене `opening-symbol-room-wall-light` (headless не + воспроизводит) и диагностическим скриптом для владельца; (2) «причина + подтверждена в реальном Chrome» — конкретное значение `will-change: transform` + снято командой из DevTools, стены стали чёткими немедленно; правило #582 не + откатывается вслепую, нужна замена без фиксации растра; (3) аналитика + (ценность 8/10 · сложность 5/10 · P1 · полный трек) с числами CDP LayerTree + (15.9×/13.2×/12.8× площади сцены) и подтверждением тестового CSS в реальном + браузере. Технических вопросов к владельцу нет — ТЗ дописано в тело issue тем + же комментарием. +- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 +- **Роль:** ревьюер ТЗ (не автор) + +## Скоуп ревью + +Композиция слоёв полноразмерной `houseplan-card` в View при зуме/панорамировании +с фоном день/ночь: (1) устранение зафиксированного растра `.plan-svg` после +первого движения камеры (правило #582, `will-change: transform`) — источник +размытия после зума; (2) сокращение композитных слоёв (`.hp-paper-outline-svg`, +экспозиция overflow #544 у `[data-hp-live-viewbox]`) с квадратичного роста по +зуму до ограниченного бюджета — источник белого мигания на большом зуме; +(3) откат фикса #685 (штриховка через `linearGradient` при `zoom ≠ 1` → +обратно единый ``), который лечил не ту причину. Не-скоуп: статичная +`houseplan-space-card`, мягкость во время самой анимации зума, 2.5D-сцены за +пределами общего с плоской проекцией поля 25%, композиция «нетронутая карточка +→ безопасная» при первом движении камеры (#582), новые настройки. + +**SCOPE-проверка (`docs/SCOPE.md`):** задача закрывает J1 («at a glance» — план +должен оставаться читаемым и не мигать белым при разглядывании стен и +устройств на любом зуме) и защищает от регрессии touch-контракт HA Companion +(`docs/TOUCH-SUPPORT.md`, «Design consequence: View mode is the product for two +of the three personas»). Ничего из «никогда не строить» не задевается — +геометрия, толщина и плотность штриховки прямо объявлены неизменными (К5), +новых слоёв данных/настроек нет. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md` целиком; + по ссылкам конспекта открыт `PROCESS.md` §2.4, §2.5, §4, §7.1, §7.2 (§2.10 не + применялся — это r1). +2. Прочитано тело issue #689 целиком и все три комментария владельца (`gh issue + view 689 --json body,comments`) — диагностика и решения владельца сверены с + итоговым текстом ТЗ, расхождений не найдено (см. «Материал» выше). +3. Сверены обязательные разделы §7.1 в теле issue: Сценарий → Что человек + увидит до/после → Скоуп/Не-скоуп → Контракт поведения К1–К7 → UX → Модель + данных/миграция/i18n → Критерии приёмки AC1–AC7 с указанным способом + доказательства → План автотестов → Производительность и touch → Риски → + Откат → Release-артефакты → «Принято предположительно» — все на месте, в + осмысленном порядке. Раздел «Проблема» формально стоит перед заголовком + `## ТЗ` (разделы «Как проявляется» и «Причины (подтверждены)»), но по + содержанию — это ровно требуемый раздел (симптом, root cause, числа CDP + LayerTree), а не пропуск; тот же паттерн уже принят без замечаний в + SPEC-REVIEW-687-r1 («"Проблема" вынесена в раздел `## Зачем` перед `## ТЗ`… + отделять его как находку было бы придиркой к форме»). +4. Технические утверждения ТЗ сверены с реальным кодом, а не приняты на слово: + - `src/styles/plan.styles.ts:115-117` — правило `.stage.daycycle.hp-safe- + daycycle-outline .plan-svg { will-change: transform; }` существует именно + там, где указано (issue цитирует строку 116, попадающую в это правило). + - `src/styles/plan.styles.ts:231-236` — `.hp-paper-outline-svg { overflow: + visible; … }` подтверждён (issue цитирует 231). + - `src/houseplan-card.ts:10877-10878` — контур `.hp-paper-outline-svg` + рендерится с `data-hp-live-viewbox`, т.е. сейчас участвует в экспозиции + #544 (`src/live-viewport.ts:112-139` — `setLayerProjection` действительно + ставит/снимает `overflow: visible` на всех `[data-hp-live-viewbox]` + элементах без исключений). К3 корректно описывает это как *текущее* + поведение, которое задача меняет через новую метку «клип». + - `demo/smoke_daycycle_layer_budget.mjs` **уже существует** (для #582 на + 100%, CDP LayerTree + screencast) — АС2 расширяет его новым случаем + 800%×DPR2, а не изобретает гейт с нуля; «существующий случай #582 на 100% + остаётся зелёным» — проверяемое, не голословное утверждение. + - `docs/ARCHITECTURE.md:594-609` («Live viewport») подтверждает и механизм + экспозиции #544 (окна 596-597), и абзац про #685-градиент (605-609), + который release-артефакты просят убрать — противоречия с ТЗ нет. + - `docs/WALL-THICKNESS.md:284-307` подтверждает текущий линейный градиент + `#685` (299) и физическую формулу штриховки `wallHatchStepUnits`/ + `HATCH_BASE_STEP_UNITS = 8` (`src/wall-thickness.ts:78`); К5 формула + «2·step/HATCH_BASE_STEP_UNITS» совпадает с действующим кодом паттерна + (`src/houseplan-card.ts:10733`, `stroke-width=${hatchStep / 4}`, что при + `HATCH_BASE_STEP_UNITS=8` даёт то же `2·step/8`). + - Коммиты `13af1d5e` и `52fe4d64`, которые ТЗ просит откатить, существуют в + `git log`, оба с трейлером `Issue: #685`, оба трогают + `src/houseplan-card.ts` (плюс `demo/golden/*`, `docs/ARCHITECTURE.md`, + `docs/WALL-THICKNESS.md`, оба changelog, `scripts/mutation-registry.mjs`) + — ровно то, что описывает К5 и release-артефакты. + - `src/houseplan-card.ts:10733-10734` — сегодняшний код действительно + переключает ``/`` по условию (аналог `zoom ≠ 1`), + подтверждая формулировку К5 «откат к состоянию до `13af1d5e`». + - Golden-сцены `opening-symbol-room-wall-light` и + `static-hatch-openings-*` существуют (`demo/golden/matrix.mjs:653-660`). + - `demo/smoke_daycycle_raster.mjs:27` — `RATIO_CEILING = 2.0` существует + (АС6 ссылается на него по имени, не изобретает порог). + - `demo/performance/budgets-large-house-interaction.json` существует (АС6 + ссылается на существующий, а не гипотетический бюджет). +5. **Реестр браузерных гвардов (АС7).** ТЗ явно требует Node-свидетелей, а не + новых браузерных мутантов, ссылаясь на потолок `200/200` (#659). Проверено: + `docs/testing-notes/mutation-browser-guards.md` действительно фиксирует + «Total 200/200… Growth above the cap fails `mutation-gate --check`» — потолок + реален, ограничение в ТЗ не придумано. План автотестов (п.2) прямо называет + прецедент `test/plan-device-landmarks.test.mjs` (#687) как образец — + прочитан: это разбор CSS-каскада через собственный парсер правил + (`rulesOf()`), а не regex по сырому тексту монолита; комментарий в файле сам + поясняет мотив «#624: no text reads of the monolith» — паттерн уже проверен + и принят на прошлом ревью, не новое изобретение, которое рискует попасть в + заморожённый список текстовых тестов монолита (§2.7). +6. Каноническая документация подсистемы сверена на противоречия: + `docs/TOUCH-SUPPORT.md:53-64` фиксирует контракт «один продвинутый + compositor-путь от первого движения камеры до терминального кадра… это + касается и браузеров, и HA Companion WebView» и ссылку на #582 как «field + acceptance for WebView-specific tile loss» — формулировка канона + имплементационно-нейтральна (не называет `will-change` в лоб), поэтому замена + на `translateZ(0)` (К1) не противоречит канону и не требует его правки; ТЗ + корректно не включает `TOUCH-SUPPORT.md` в release-артефакты. + `docs/SUN.md:74-95` описывает текущий механизм «sibling SVG + `will-change: + filter`» для контура день/ночь — это другое правило (filter-hint), не + `.plan-svg`-`will-change:transform`, конфликта с К1 нет; правки К1/К3 в этом + документе (упомянуты в release-артефактах) обоснованы. + `docs/CANVAS.md` не содержит упоминаний `will-change`/композитинга — не + требует правки, ТЗ и не просит. +7. Числовая арифметика К3/AC2 сверена на непротиворечивость: `clip-path: + inset(-25%)` расширяет клип на 25% с каждой стороны → итоговая площадь + ≈ (1+0.5)² = 2.25× сцены, что укладывается в заявленный порог «≤2.5× во + время жеста»; в покое клип и `overflow: visible` снимаются → ~1×, что + укладывается в «≤1.5× в покое». Контур, не получающий экспозиции никогда, + остаётся ~1× всегда. Расчёт подтверждает, что численные пороги AC2 + достижимы предложенным механизмом, а не выбраны произвольно. +8. `git branch -a`, `git log --all --oneline | grep 689`, `git diff + origin/dev...HEAD` — ветки `issue/689-*` и коммитов с трейлером `Issue: #689` + нет, дифф пуст. Продуктового кода для #689 нет — стадия `spec`, гейты + (`tsc`, `test`, `build`, смоки, golden, инварианты) неприменимы, штатное + состояние, а не находка. + +## Находки + +Не найдено High и Medium. Low не найдено — необычно тщательная фактчекаемость +ТЗ (номера строк, SHA коммитов, имена констант и файлов) не оставила +редакционных зазоров уровня, который стоило бы фиксировать отдельно. + +## Что проверено и корректно + +- Все обязательные разделы §7.1 присутствуют и в осмысленном порядке; «Сценарий» + называет персону (член семьи на планшете / администратор на десктопе), + поверхность (View, фон день/ночь) и момент (после зума, после перезагрузки на + крупном масштабе); «Что человек увидит» — парой фраз до/после без терминов + реализации. +- Контракт К1–К7 внутренне согласован и покрыт: К1→AC1, К2→AC1/AC4 (через + «кадр после зума = кадр свежей загрузки»), К3→AC2/AC3, К4→AC4, К5→AC5, + К6→AC3 (idle-DOM)/AC5 (golden), К7→AC6 (регресс существующих смоуков + #531/#579/#531-viewBox-бюджет). Пробелов «контракт есть, AC нет» не найдено. +- AC1–AC7 однозначны и называют способ доказательства + (`smoke`+владелец / `smoke`+CDP LayerTree / `unit` / владелец / `smoke`+ + `unit`+`golden` / `performance`+`smoke` / `unit` Node-свидетели). AC1/AC4 + корректно и честно ограничены визуальной приёмкой владельца в реальном + GPU-браузере с явным обоснованием («Headless не видит ни размытия, ни + мигания») — тот же паттерн уже принят без замечаний в SPEC-REVIEW-685-r1 + для AC1 «резкость». +- Защитный AC7 не просто говорит «мутанты ловятся», а перечисляет все четыре + конкретные регрессии (возврат `will-change`, возврат `overflow: visible`, + снятие `clip-path`, экспозиция контура) — это и есть требуемое по §7.1 + указание способа доказательства для защитного критерия на этапе spec (полная + таблица «чем краснеет» с результатом прогона — требование этапа code, не + spec). +- Технический выбор реализации корректно вынесен в «Принято предположительно»: + механизм явного слоя (`translateZ(0)` вместо снятия промоушена целиком), + величина поля экспозиции (25%), исключение контура из экспозиции через + метку — всё это не наблюдаемо пользователем и правомерно оставлено на + усмотрение реализатора, ревьюер вправе его оспорить и не находит оснований. +- Владелец лично диагностировал обе причины в реальном Chrome (DevTools- + команды, снятие `will-change` вручную, числа CDP LayerTree) и прописал + решения по каждой из них до передачи в ТЗ — открытых продуктовых вопросов не + осталось, автор прямо это фиксирует, и я не нашёл продуктовой развилки, + которая осталась бы неразрешённой (см. п.5–7 «Как проверялось» — все + технические механизмы дополнительно перепроверены против кода, а не приняты + на слово). +- Риски (регресс #582 tile loss, недостаточность поля 25% при очень быстром + жесте, кратковременное исчезновение свечения контура ≤100 мс, сдвиг golden- + цветов день/ночь, невозможность headless увидеть дефект) названы явно и не + спрятаны внутри AC — качество, которое прямое протестировано выше (п.6, п.7). +- Откат (чистый реверт продуктовых коммитов; реверт отката #685 отдельно не + делается — решение владельца) и Release-артефакты (оба changelog, + `docs/SUN.md`/`docs/ARCHITECTURE.md`/`docs/WALL-THICKNESS.md`, golden с + `Release:`+`Baseline-Reviewed`) — список сверен с фактически задетыми + канонами (п.6) и ничего не упущено, ничего лишнего не добавлено (`docs/ + CANVAS.md`, `docs/TOUCH-SUPPORT.md` в списке нет — и не должны быть). +- Модель данных, миграция, i18n: явное «нет изменений», согласуется с тем, что + диф чисто визуальный/CSS/композитинговый, новых полей конфигурации нет + (`docs/CONFIG-COMPATIBILITY.md` не затрагивается). + +## Чего не проверял + +- Гейты (`tsc --noEmit`, `npm test`, `npm run build`, `check-docs.mjs`, смоки, + `golden:verify`, инварианты) — не прогонял: этап `spec`, ветки/коммитов для + #689 нет, `git diff origin/dev...HEAD` пуст (подтверждено в п.8 «Как + проверялось»). Предмет код-ревью после реализации. +- Не проверял вручную в реальном GPU-браузере само наличие размытия/мигания на + `dev` — это диагностика владельца (три комментария, числа CDP LayerTree, + DevTools-эксперимент), а не факт, который проверяется на этапе spec; принимаю + как обоснованную владельцем предпосылку, а не как то, что мне следует + воспроизводить самостоятельно без ветки с фиксом. +- Не оценивал техническую осуществимость конкретной пары «`translateZ(0)` + + `clip-path: inset(-25%)`» на практике (реальный layout/paint эффект в + Chromium/HA Companion за пределами арифметики площадей п.7) — это по разделу + «Принято предположительно» прямо оставлено на усмотрение реализации и + подлежит АС1/АС2/АС4 на код-ревью, а не проверке ТЗ. +- Не искал в `docs/reviews/INDEX.md` строки предыдущих раундов подсистемы — не + требуется: это r1, §2.10 (объём по дельте) не применяется. + +## Вердикт + +Обязательные разделы ТЗ полны и в осмысленном порядке, контракт К1–К7 без +внутренних противоречий и полностью покрыт критериями приёмки, каждый AC +однозначен и называет способ доказательства (включая честно ограниченную +визуальную приёмку владельца там, где headless не воспроизводит дефект). +Технические утверждения — номера строк, имена констант и файлов, SHA +откатываемых коммитов, существующие гейты и пороги — проверены против +реального кода и документации построчно и оказались точными без исключений; +арифметика численных порогов AC2 (2.25× против заявленного потолка 2.5×) +внутренне согласована с предложенным механизмом. Продуктовых вопросов, ушедших +владельцу без ответа, не найдено; технических развилок, требующих спора с +автором, тоже не найдено. Находок нет. + +Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 · в задаче · Документ: docs/reviews/SPEC-REVIEW-689-r1.md + +--- + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `63c24784b604` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `9c786f7acaaf6c1a5ae960a7f7e734473426aaa6` + ``` + git log --all --format='%H %T' | grep 9c786f7acaaf + ``` +- Тело issue: `57b2345003d4b8f80b2c00dcc327b4ae2832e3de6524d1ac404eb5a32d6c84ab` +- Вердикт конвейера: `green` · High 0