mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-28 19:01:34 +00:00
@@ -1,9 +1,10 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1068, issue: 376. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1069, issue: 377. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| #654 | [SPEC-REVIEW-654-r1.md](SPEC-REVIEW-654-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | «Release-артефакты» не называют обновление docs/ISOMETRIC.md | `docs/ISOMETRIC.md` `docs/CHANGELOG.md` `docs/CHANGELOG.ru.md` `docs/reviews/INDEX.md` |
|
||||
| #651 | [SPEC-REVIEW-651-r1.md](SPEC-REVIEW-651-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | устаревшая формулировка «экспериментальный 2.5D-вид» противоречит текущему статусу функции | `CHANGELOG.md` `CHANGELOG.ru.md` `docs/ISOMETRIC.md` `docs/USER-GUIDE.ru.md` `docs/STATUS.md` `USER-GUIDE.ru.md` |
|
||||
| #651 | [SPEC-REVIEW-651-r2.md](SPEC-REVIEW-651-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #651 | [CODE-REVIEW-651-r1.md](CODE-REVIEW-651-r1.md) | code · r1 | 🟡 жёлтый | 0 | 2 | AC1 «другая комната исключена» не доказан ни тестом, ни мутацией; AC4 «деградированный fallback группы» не доказан ни тестом, ни мутацией; неточная формулировка в комментарии к реализации (снято без правки) | `src/iso-overlays.ts` `test/iso-overlays.test.mjs` `scripts/mutation-registry.mjs` |
|
||||
|
||||
@@ -0,0 +1,261 @@
|
||||
# SPEC-REVIEW-654-r1
|
||||
|
||||
**Issue:** [#654](https://github.com/Matysh/houseplan-card/issues/654) — «2.5D: цвет бумаги читается `getComputedStyle` в `render()` — на первом кадре все полы «тёмные»; вероятная вспышка Flat→2.5D при холодной загрузке дашборда»
|
||||
**Этап:** spec (ревью ТЗ, PROCESS.md §2.4)
|
||||
**Заход:** r1 · блокирующих циклов израсходовано (после этого вердикта): 1 из 4
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Жёлтый.** High: 0 · Medium: 1 (в скоупе, возвращается автору) · Low: 0.
|
||||
|
||||
Единственная находка — «Release-артефакты» ТЗ не называют обновление
|
||||
`docs/ISOMETRIC.md` («Activation»), хотя задача вводит новый, ранее нигде не
|
||||
документированный элемент этого контракта: план скрыт загрузочной
|
||||
поверхностью, пока ленивый 2.5D-рантайм не установлен (в т.ч. в kiosk, где
|
||||
общий boot veil сегодня отключается мгновенно). Остальная часть ТЗ —
|
||||
обязательные разделы §7.1, однозначность и доказуемость AC1–AC6, явный блок
|
||||
технических предположений — выполнена корректно, а фактические технические
|
||||
предпосылки (`getComputedStyle` на `.hp-paper`, `_effectiveProjection()`,
|
||||
`_isoSceneRuntimeLoader`, поведение kiosk в `setConfig()`) проверены по коду
|
||||
`dev` и подтвердились буквально.
|
||||
|
||||
## Скоуп проверки
|
||||
|
||||
1. Какую строку `docs/SCOPE.md` закрывает задача — J1 («живой обзор дома») в
|
||||
рамках уже принятого узкого исключения #89 (детерминированная 2.5D-презентация
|
||||
той же геометрии, без второй модели/свободной камеры); задача не расширяет
|
||||
это исключение, а чинит дефект внутри него.
|
||||
2. Обязательные разделы ТЗ по PROCESS.md §7.1 (сценарий и «что человек увидит»,
|
||||
проблема, скоуп/не-скоуп, поведенческий контракт, UX, данные/миграция,
|
||||
i18n, AC1…ACn с доказательством, план автотестов, риски, откат,
|
||||
release-артефакты).
|
||||
3. Однозначность и проверяемость каждого AC (AC1–AC6) и то, что заявленный
|
||||
способ доказательства (unit/AST/smoke) реален, а не ссылается на
|
||||
несуществующий код.
|
||||
4. Соответствие фактических технических утверждений ТЗ действительному
|
||||
состоянию `src/houseplan-card.ts`, `src/iso-materials.ts` на `dev` (это не
|
||||
код-ревью диффа — диффа ещё нет; это проверка, что ТЗ не выдаёт догадку за
|
||||
факт о существующем поведении).
|
||||
5. Терминология видимого поведения — против `docs/USER-GUIDE.ru.md` и
|
||||
канонического `docs/ISOMETRIC.md` (обязательное чтение для видимого
|
||||
поведения по инструкции ревью).
|
||||
6. Наличие открытых продуктовых вопросов, которые ТЗ решило само вместо
|
||||
вынесения владельцу, и их обоснованность.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Тело issue #654 и комментарии получены через `gh issue view 654 --json
|
||||
body,comments,labels,state` (MCP `mcp__github__get_issue`/`get_issue_comments`
|
||||
были недоступны без разрешения пользователя в этой сессии — `gh` CLI как
|
||||
эквивалентный путь к тому же публичному API).
|
||||
- Прочитаны целиком `docs/SCOPE.md`, `docs/process/REVIEWER.md`, `AGENTS.md`
|
||||
(обязательный порядок чтения ревьюера), раздел PROCESS.md §7.1 (обязательные
|
||||
разделы ТЗ, шаблон вердикта).
|
||||
- Прочитан целиком `docs/ISOMETRIC.md` (в первую очередь «Activation» и Stage 6
|
||||
«Raised tiles» — источник формулы `isoLightFloorRooms`/luma > 0.55).
|
||||
- Код `dev` (`git rev-parse HEAD` = `803c0f0eaab3aea1ea9a87215c35b638a679dafb`)
|
||||
проверен построчно на все фактические утверждения ТЗ:
|
||||
- `src/houseplan-card.ts:10701` — буквально совпадает с цитатой в issue
|
||||
(`getComputedStyle(this.renderRoot.querySelector('.hp-paper') ?? this).fill`
|
||||
внутри `_renderBody()`, вызываемой из `render()`).
|
||||
- `src/houseplan-card.ts:6002-6008` (`_effectiveProjection()`) — подтверждён
|
||||
ранний `return finish('flat')`, пока `!this._isoSceneRuntime`.
|
||||
- `src/houseplan-card.ts:2568` (`connectedCallback`) — подтверждена
|
||||
предзагрузка `if (this._isoEnabled) void this._ensureIsoSceneRuntime();`.
|
||||
- `src/houseplan-card.ts:3095` (`setConfig()`) — подтверждено, что kiosk
|
||||
немедленно гасит `_booting`/`_bootFading` («kiosk: 100dvh, nothing to
|
||||
settle»), т.е. общий boot veil в kiosk сегодня действительно не
|
||||
появляется — ровно то, что называет issue.
|
||||
- `src/houseplan-card.ts:332-337` (`BOOT_MIN_MS/BOOT_QUIET_MS/BOOT_MAX_MS`,
|
||||
`_bootWatch()` `:6356-6373`) — существующий boot-veil таймер имеет
|
||||
безусловный потолок `BOOT_MAX_MS = 1200` мс. Это прямое технической
|
||||
напряжение с «переиспользовать существующий bootveil» (если ленивый чанк
|
||||
ставится дольше 1.2 с, безусловное снятие veil обнажит Flat-кадр вопреки
|
||||
AC3) — ТЗ уже закрывает этот риск не как факт, а как открытый технический
|
||||
выбор в разделе «Принято предположительно»: «использование существующего
|
||||
`bootveil` **либо** визуально идентичного внутреннего состояния
|
||||
iso-pending». Разбор ниже, в «Что проверено и корректно».
|
||||
- `src/iso-materials.ts:91-99` (`isoLightFloorRooms`) — подтверждено, что
|
||||
функция уже является чистой и принимает явный `Rgb`, DOM не трогает;
|
||||
баг живёт только в вызывающем коде (`houseplan-card.ts:10701`), что
|
||||
напрямую подтверждает реалистичность AC1 (unit-тест на чистую функцию уже
|
||||
возможен без новой архитектуры).
|
||||
- `test/iso-stage6.test.mjs:7,43` — `isoLightFloorRooms` уже юнит-тестируется
|
||||
без DOM, ссылка AC1 не на пустое место.
|
||||
- `demo/smoke_volumetric_setting.mjs` (90 строк, без `page.reload()`) —
|
||||
подтверждена цитата issue «проверяет только переключение без
|
||||
перезагрузки».
|
||||
- `demo/smoke_isometric_contract.mjs`, `demo/smoke_iso_tiles.mjs`,
|
||||
`demo/smoke_iso_theme_walls.mjs` — существуют (план автотестов ссылается
|
||||
на реальные файлы, не выдуманные).
|
||||
- `docs/reviews/INDEX.md` проверен на прецедент: обе смежные задачи (#651,
|
||||
#649) в списке затронутых файлов называли `docs/ISOMETRIC.md`, а ревью #651
|
||||
прямо содержало Medium-находку за расхождение терминологии ТЗ с этим же
|
||||
каноническим документом — это основа находки ниже, а не единичное мнение.
|
||||
- Код продукта не менялся, гейты (`tsc`/`npm test`/`npm run build`) не
|
||||
запускались: на этапе `spec` предмет ревью — текст ТЗ, диффа ещё нет (ветка
|
||||
реализации не создана).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе) — «Release-артефакты» не называют обновление `docs/ISOMETRIC.md`
|
||||
|
||||
**Файл:** тело issue #654, раздел `## ТЗ`, подраздел «Release-артефакты» (и
|
||||
смежно — «Контракт поведения и UX», п. 1–2).
|
||||
|
||||
**Воспроизведение:**
|
||||
- «Release-артефакты» ТЗ перечисляют только: `docs/CHANGELOG.md` +
|
||||
`docs/CHANGELOG.ru.md` (да), «пользовательское руководство/i18n/config
|
||||
migration: нет», golden/perf/security — без единого упоминания
|
||||
`docs/ISOMETRIC.md`.
|
||||
- При этом «Контракт поведения и UX» вводит новый пункт жизненного цикла,
|
||||
которого сегодня в `docs/ISOMETRIC.md` («Activation») нет ни в каком виде:
|
||||
«Пока ленивый 2.5D-рантайм загружается, план не показывается в
|
||||
промежуточном Flat-виде… Это относится и к kiosk, где общий boot veil
|
||||
обычно отключён» — то есть kiosk получает *новое* видимое поведение
|
||||
(кратковременная загрузочная поверхность), которого у него сегодня нет
|
||||
вовсе (`setConfig()` гасит `_booting`/`_bootFading` безусловно, см. «Как
|
||||
проверялось»).
|
||||
- Текущий текст `docs/ISOMETRIC.md:10-28` («Activation») описывает только:
|
||||
когда включена ленивая загрузка чанка, что переключение без перезагрузки
|
||||
мгновенно, и фингерпринт-фолбэк — ни слова о том, что скрывается сам план на
|
||||
время установки чанка при холодном старте, и что это распространяется на
|
||||
kiosk, где обычный boot veil не работает.
|
||||
|
||||
**Почему это находка, а не придирка к формулировке.** `docs/ISOMETRIC.md` —
|
||||
канонический документ подсистемы (AGENTS.md, «Reading this first»; PROCESS.md
|
||||
§2.10 требует смотреть его строки перед разбором подсистемы), и обе соседние
|
||||
задачи по той же подсистеме (#649, #651 — см. `docs/reviews/INDEX.md`)
|
||||
называли именно этот файл среди затронутых при заметно менее существенных
|
||||
изменениях поведения (терминология, geometry). Здесь же вводится реальный
|
||||
новый инвариант жизненного цикла — «первый кадр 2.5D скрыт до готовности
|
||||
рантайма, включая kiosk» — который следующий разработчик, открывший только
|
||||
`docs/ISOMETRIC.md`, не увидит и рискует нарушить неосознанно (например, при
|
||||
следующей правке `_bootWatch()`/`BOOT_MAX_MS` или логики `setConfig()` для
|
||||
kiosk). Отсутствие этого пункта в «Release-артефактах» — это ровно тот
|
||||
паттерн, что уже стоил Medium-находки в #651.
|
||||
|
||||
**Чем закрывается.** Добавить в «Release-артефакты» строку об обновлении
|
||||
`docs/ISOMETRIC.md` (раздел «Activation» — или новый короткий подраздел о
|
||||
first-frame gating), фиксирующую: план скрыт до готовности ленивого рантайма
|
||||
при `volumetric_view: true`, это распространяется на kiosk, terminal failure
|
||||
безопасно возвращает Flat. Формулировка — на усмотрение автора, это
|
||||
техническая правка текста ТЗ, а не новый продуктовый вопрос владельцу.
|
||||
|
||||
**Класс:** Medium, в скоупе задачи — правится прямо в теле ТЗ, отдельный issue
|
||||
не заводится (#202).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Обязательные разделы §7.1** — присутствуют все: «Сценарий и видимое
|
||||
изменение» (отвечает на оба продуктовых вопроса — какая персона/поверхность/
|
||||
момент и что человек видит одной фразой, без терминов реализации), «Проблема
|
||||
и причины», «Скоуп»/«Не скоуп», «Контракт поведения и UX», «Модель данных и
|
||||
совместимость», «i18n», «Критерии приёмки» (AC1–AC6 — см. ниже),
|
||||
«План автотестов и доказательств», «Производительность и touch» (не
|
||||
обязателен, но уместен для render-path фикса), «Риски», «Принято
|
||||
предположительно, поменять свободно», «Откат», «Release-артефакты».
|
||||
- **Однозначность и доказуемость AC1–AC6.** Каждый AC называет конкретный вид
|
||||
доказательства (unit/AST/smoke) и наблюдаемый критерий, а не самоцель
|
||||
реализации:
|
||||
- AC1 — чистая функция классификации без DOM; белая бумага + светлая заливка
|
||||
→ light-floor, тёмная → нет; cold-reload smoke сверяет первый кадр с
|
||||
установившимся. Подтверждено технически: `isoLightFloorRooms` уже чистая
|
||||
(см. «Как проверялось»), баг — только в вызове.
|
||||
- AC2 — отсутствие `getComputedStyle`/`getBoundingClientRect` в
|
||||
`render`/`_renderBody`/`willUpdate` и в iso render-helper поверхностях,
|
||||
AST-проверка + браузерный счётчик обращений на повторных HA-рендерах без
|
||||
смены темы/режима. Способ доказательства (AST, не regex по тексту монолита)
|
||||
соответствует требованию ревьюера не полагаться на текстовые якоря.
|
||||
- AC3 — атомарность холодного старта при искусственно задержанном чанке, для
|
||||
обычной карточки и kiosk; критерий (`.stage.projection-iso`, устойчивый
|
||||
viewBox) — наблюдаемый и не расплывчатый.
|
||||
- AC4 — безопасный отказ: конечная загрузочная поверхность, доступный Flat,
|
||||
настройка не меняется, приватные данные не в диагностике — опирается на
|
||||
уже существующий контур `EditorRuntimeLoader`/`safeRuntimeDiagnostic`, не
|
||||
изобретает новый механизм отказа.
|
||||
- AC5 — точная инвалидация memo по бумаге/fills/rooms; переход в редактор и
|
||||
обратно не переносит цвет одного режима в классификацию другого.
|
||||
- AC6 — соседние контракты (`volumetric_view` как есть, Flat не запрашивает
|
||||
чанк, structural fallback, zoom/устройства) явно перечислены как
|
||||
регрессионный периметр.
|
||||
- **Технические предпосылки ТЗ проверены по коду, а не приняты на слово.**
|
||||
Все процитированные строки и функции (`_renderBody`/`render`,
|
||||
`_effectiveProjection`, `connectedCallback`-предзагрузка,
|
||||
`setConfig()`-поведение kiosk, `EditorRuntimeLoader`) существуют буквально
|
||||
там, где заявлено, и ведут себя так, как описано — включая менее очевидное
|
||||
наблюдение issue про kiosk (`setConfig()` действительно гасит `_booting`
|
||||
раньше, чем что-либо успевает отрисоваться). Догадок, выданных за факт, в
|
||||
части воспроизведения дефекта не найдено.
|
||||
- **Раздел «Принято предположительно» реалистично закрывает найденное
|
||||
техническое напряжение.** `BOOT_MAX_MS = 1200` мс — безусловный потолок
|
||||
существующего boot veil — потенциально конфликтует с «дождаться ленивого
|
||||
рантайма» при медленной сети. ТЗ не скрывает эту развилку: явно оставляет
|
||||
автору выбор между переиспользованием `bootveil` и отдельным
|
||||
`iso-pending`-состоянием, а ревьюер вправе его оспорить (§7.1). Технической
|
||||
ошибки/недосказанности здесь нет — граница ответственности проведена верно.
|
||||
- **Отсутствие открытых продуктовых вопросов проверено, а не принято на
|
||||
слово.** Кроме разобранной выше Medium-находки, ни одного места, где догадка
|
||||
о видимом поведении подана как факт без пометки предположения, не найдено.
|
||||
Формулировка «Продуктовых вопросов для владельца нет» в аналитике —
|
||||
обоснована: единственная пограничная зона (новое поведение kiosk) уже прямо
|
||||
зафиксирована как решение в контракте, а не спрятана.
|
||||
- **Соответствие `docs/SCOPE.md`.** Задача остаётся строго внутри узкого
|
||||
исключения #89 (никакой новой камеры/модели/настройки), корректно исключает
|
||||
#651 (дрейф значков при зуме) и `_pointInRoom` из скоупа со ссылкой на
|
||||
конкретные issue.
|
||||
- **i18n** — новых строк нет, что верно: явно запрещён новый текст/кнопка/
|
||||
индикатор (Контракт поведения, п. 1).
|
||||
- **Модель данных/совместимость** — эфемерный runtime-кэш экземпляра карточки,
|
||||
без схемы/YAML/localStorage/миграции; согласуется с характером фикса
|
||||
(render-path + lifecycle-гейт, не персистентное состояние).
|
||||
- **Откат** — простой revert продуктового коммита, без обратной миграции;
|
||||
корректно для чисто presentation-слойного изменения.
|
||||
- **Артефакты плана автотестов существуют.** `demo/smoke_isometric_contract.mjs`,
|
||||
`demo/smoke_iso_tiles.mjs`, `demo/smoke_iso_theme_walls.mjs`,
|
||||
`demo/smoke_volumetric_setting.mjs` — реальные файлы, не выдуманные ссылки;
|
||||
последний подтверждённо не покрывает `page.reload()`, что и обосновывает
|
||||
необходимость нового/расширенного smoke по AC3.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `tsc`/`npm test`/`npm run build` и прочие гейты §8 не запускал — на этапе
|
||||
`spec` предмет ревью текст ТЗ, реализации ещё нет (ветка задачи не создана
|
||||
на момент ревью).
|
||||
- Не проверял браузерное поведение вручную — реализации нет, нечего
|
||||
запускать; это работа код-ревью следующего этапа.
|
||||
- Не читал построчно весь `src/iso-scene-render.ts`/`src/iso-sun.ts`/
|
||||
`src/iso-tiles.ts` — только точечно то, что нужно для проверки фактических
|
||||
утверждений ТЗ (существование `isoLightFloorRooms`, `_isoSceneRuntime`,
|
||||
boot-veil таймеров). Полный аудит «какие именно файлы входят в 2.5D
|
||||
render-helper поверхности» для AC2 — задача код-ревью, когда появится AST-тест
|
||||
и станет видно его фактический периметр.
|
||||
- Не оценивал real-world длительность загрузки чанка `iso-scene-render`
|
||||
(совпадает ли она обычно с `BOOT_MAX_MS`) — это эмпирический вопрос
|
||||
реализации/код-ревью, а не спецификации; отметил риск как технически
|
||||
реальный, но признал его закрытым явным «принято предположительно», а не
|
||||
находкой.
|
||||
- Полные наборы golden/perf/smoke не прогонял — кода для прогона ещё нет; это
|
||||
часть «Плана автотестов» и войдёт в работу при реализации (§2.6) и
|
||||
последующем код-ревью (§2.7).
|
||||
|
||||
## Рекомендация автору
|
||||
|
||||
Одна точечная правка текста ТЗ — дополнить «Release-артефакты» пунктом об
|
||||
обновлении `docs/ISOMETRIC.md`. Правка не требует нового цикла анализа по
|
||||
существу поведенческого контракта: сам контракт (AC1–AC6, риски, допущения)
|
||||
корректен и не меняется.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `803c0f0eaab3` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `b4eeb2b0840189d8031f291834e57fab092af49b`
|
||||
```
|
||||
git log --all --format='%H %T' | grep b4eeb2b08401
|
||||
```
|
||||
- Тело issue: `bd46753eac9a5a3b908addb0135b2054eea85c66f89eda4b4f8bdae8d80d78cd`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user