mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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 | — | — |
|
||||
|
||||
@@ -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` →
|
||||
обратно единый `<pattern>`), который лечил не ту причину. Не-скоуп: статичная
|
||||
`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` — сегодняшний код действительно
|
||||
переключает `<pattern>`/`<linearGradient>` по условию (аналог `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
|
||||
|
||||
---
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `63c24784b604` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `9c786f7acaaf6c1a5ae960a7f7e734473426aaa6`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 9c786f7acaaf
|
||||
```
|
||||
- Тело issue: `57b2345003d4b8f80b2c00dcc327b4ae2832e3de6524d1ac404eb5a32d6c84ab`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user