german/houseplan-card
1
0
Fork 0
mirror of https://github.com/Matysh/houseplan-card synced 2026-09-29 03:09:36 +00:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

docs: review document for #649

Browse Source 
Issue: #649
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-25 12:34:36 +00:00
parent b7079ead80
commit f68878cc44
2 changed files with 218 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/reviews/INDEX.md
+2 -1
View File
@@ -1,9 +1,10 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1056, issue: 372. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1057, issue: 373. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|---|---|---|---|---:|---:|---|---|
| #649 | [SPEC-REVIEW-649-r1.md](SPEC-REVIEW-649-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | в скоупе задачи (возвращается автору); принято ревьюером с записью, правки не требует | `lab.js` `houseplan-card.ts` |
| #647 | [SPEC-REVIEW-647-r1.md](SPEC-REVIEW-647-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 2 | П.4 ТЗ переносит на новый слот прежнее; ТЗ не упоминает и не защищает документированный | `src/styles/dialogs.styles.ts` `src/styles.ts` `smoke_glow_blending.mjs` `smoke_test_facade.mjs` `smoke_unified_wall_tool.mjs` `docs/UX-MODES.md` `docs/reviews/CODE-REVIEW-195-r1.md` `src/styles/chrome.styles.ts` |
| #647 | [SPEC-REVIEW-647-r2.md](SPEC-REVIEW-647-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
| #647 | [CODE-REVIEW-647-r1.md](CODE-REVIEW-647-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
docs/reviews/SPEC-REVIEW-649-r1.md
+216
View File
@@ -0,0 +1,216 @@
# SPEC-REVIEW-649-r1
Этап: spec (PROCESS.md §2.4). Заход r1, блокирующих циклов израсходовано 0 из 4
(перед этим раундом).
## Скоуп
Issue #649 — продолжение #644: 2.5D выходит из experimental и становится
публичным режимом; в 2.5D меняются плитки маркеров/замков (объём, торец, тень
на полу), свет из окон (мягкая заливка по солнцу вместо проекции Flat-клиньев),
чинятся два бага декора/стен (толщина линий мебели, независимость цвета стен от
темы) и добавляется переключатель `settings.volumetric_view` в «Общих
настройках». Flat (2D), `houseplan-space-card` и оба редактора не меняются.
Материал ревью — тело issue #649, раздел `## ТЗ` (редакция r1, 25.09), плюс
комментарий «Аналитика» с ответами владельца на Q1–Q3 от 25.09. ТЗ соответствует
полному треку (лёгкий трек явно отклонён в комментарии «Аналитика» с указанием
нарушенных критериев §5 — сложность 9/10, несколько поверхностей, новое
конфиг-поле с backend-валидацией, новый публичный UX-контракт, влияние на
производительность и touch).
Работа сделана в скоупе `docs/SCOPE.md`: 2.5D-презентация канонического плана —
поименованное исключение #89; задача не вводит вторую модель, не трогает
декор-редактор и не расширяет его семантику.
## Как проверялось
1. Прочитаны `docs/SCOPE.md`, `docs/process/REVIEWER.md`, `AGENTS.md`,
`PROCESS.md` §2.4/§2.5/§7.1/§7.2/§2.10, тело issue #649 целиком, комментарий
«Аналитика».
2. Проверены обязательные разделы ТЗ по §7.1 (сценарий, что видно до/после,
проблема, скоуп/не-скоуп, контракт поведения, UX, модель данных и миграция,
i18n, AC1…AC13 с доказательством, план автотестов, риски, откат,
release-артефакты) — все присутствуют по существу (сценарий и «до/после»
объединены в одном абзаце, но обе продуктовые вещи в тексте есть: «что видит
администратор» и «что видит домочадец»).
3. Каждое утверждение раздела «Проблема (по коду `dev@b7079ead`)» и раздела
«Эталон и единицы» сверено с кодом на этом же SHA (рабочая копия уже на нём):
- `_renderSunRays`, `furnitureScreenScale`/`furniturePlanScreenScale`,
`openingInnerFaceOffsetFromIndex`, `windowLit`, `planSunAngle`,
`gridVisualUnits`, `ISO_WALL_HEIGHT`, `isoRaisedOverlayHalfSize`,
`wall_fill`, `houseplan_card_view_v1`, `projection-toggle`, `LABS_FLAGS` —
все существуют, `grep -rn` по `src/`;
- `src/houseplan-card.ts:8414` — `furnitureScreenScale = this._renderProjection
=== 'iso' ? 1 : furniturePlanScreenScale(...)` — точное совпадение с текстом
issue и с формулировкой AC8/мутанта «`iso ? 1 : …` возвращён»;
- `src/styles/plan.styles.ts:338-368` — правила `.stage.theme-dark .iso-*`,
строка `757` — `.stage.projection-iso.theme-dark.mode-view .roomlabel` —
совпадает с заявленными диапазонами «~338–390» и «~740–760»;
- `src/labs.ts` — `LABS_FLAGS` содержит ровно один флаг `iso`; утверждение
«`hp_alpha` остаётся механизмом без экспериментов» после удаления `iso`
подтверждается — массив станет пустым, а не будет удалён весь механизм;
- `docs/SUN.md:208` — `RAY_MIN_COS = 0.05` — подтверждает решение «принято
предположительно»: порог окна `windowLit()` = 0.05, а не 0.08 лаборатории;
- `docs/ISOMETRIC.md` — камера 0°/20°, `wallHeight=84`, поворот створки
`50° × openingAmount` — подтверждает «Общие условия» (камера
20°/0°/84/50° сохраняется, это не новое число);
- `src/houseplan-card.ts:2302-2305` (`_desiredProjection`) — iso уже сегодня
доступен только при `this._mode === 'view'`; заявление «редакторы и
`houseplan-space-card` — всегда Flat» не новое ограничение, а действующее
поведение, и `src/space-card.ts`/`src/space-render.ts` вовсе не содержат
projection-ветвления — задаче не нужно ничего менять для этого пункта;
- `src/editors/general-settings-dialog.ts:86-100` — карточка «Отображение»
сегодня содержит ровно два тумблера в порядке `gs-room-tooltip` →
`gs-radar-live` («Show live presence on the plan»); заявление «третьим
пунктом после …» в ТЗ арифметически верно;
- `src/i18n/{en,ru,de,fr}.json:766-767` — ключи `view.volumetric`/
`view.flat` действительно существуют и будут освобождены, как написано;
- backend-прецедент: `custom_components/houseplan/validation.py:2256`
(`vol.Optional("show_room_tooltip"): bool`), `support_package.py:155-157`,
`scripts/config-schema.json:431` — ровно тот шаблон, который ТЗ предлагает
повторить для `volumetric_view`;
- `demo/performance/budgets-large-house-isometric.json`,
`budgets-isometric-stage3-dense.json` — профили из AC13 существуют.
4. Отдельно проверена внутренняя непротиворечивость числовых формул раздела 1
(плитка/торец/тень) и раздела 2 (свет): пересчёт лабораторных единиц в базы
**D** и **H** (радиус 22/80=0.275 D, длина луча 910/218.8≈4.16 H,
215/218.8≈0.98 H, 408/218.8≈1.865 H, blur 14/218.8≈0.064 H) — совпадает
поразрядно.
5. Не запускались никакие гейты (typecheck/test/build) — на этапе ревью ТЗ
продуктовый код ещё не пишется, гейты неприменимы (§2.5, §8).
## Находки
### Medium — в скоупе задачи (возвращается автору)
**M1. Тень плитки: не определена opacity для пересечения «тёмная тема + светлый
пол», хотя это одна из четырёх утверждённых опорных комбинаций (вложение 13).**
Раздел «1. Иконки-плитки», подраздел «Тень на полу»:
> Opacity: белые тела 0.34, цветные тела и бейджи 0.50. **Тёмная тема — 0.40**
> (цветные 0.50).
> …
> **Светлый пол**: opacity 0.30 (цветные 0.42), размытие 0.1375 D, сдвиг
> (0.1 D, 0.425 D).
Тема и пол — независимые переменные (пол вычисляется по яркости заливки
комнаты/бумаги плана, тема — по UI; ничто в ТЗ не привязывает одно к другому,
и пример из «Эталон и единицы» специально существует для случая «тёмная тема,
светлый пол» — вложение 13, `13-lab-sketch07-darktheme-lightfloor-approved.png`,
утверждено владельцем). Текст задаёт модификатор темы (0.40/0.50, без
привязки к полу) и модификатор пола (0.30/0.42, без привязки к теме), но не
говорит, что происходит при обеих модификациях сразу: остаётся 0.40 (тема
победила), становится 0.30 (пол победил) или это третья, ещё не названная
величина. Blur и сдвиг для этой комбинации тоже не названы отдельно —
неявно предполагается, что там побеждает правило пола (0.1375 D / сдвиг 0.425 D),
раз только у пола есть blur/сдвиг-модификатор, но для opacity такой
однозначности нет, потому что модификатор есть у обеих переменных.
Это не гипотетическая придирка: в «Аналитика» написано «Кадры сняты во всех
четырёх сочетаниях темы и пола» — то есть у автора ТЗ физически есть
референсный кадр с точным числом для этой комбинации (из `lab.js`/
`applyTheme()` или скриншота), просто оно не перенесено в текст. AC4 отдельно
требует «сдвиг/opacity/размытие по полу **и** теме» (обе переменные явно
названы как определяющие), а AC12 требует side-by-side сравнение именно с
вложением 13 — то есть на кодревью потребуется точное число, а разработчик,
не имея его, либо угадает, либо придумает своё правило приоритета, которое
затем не совпадёт с рефересом без объяснения (сам AC12 называет это находкой).
**Чем закрывается:** одна строка в ТЗ, например «Тёмная тема + светлый пол:
opacity N (цветные M), размытие/сдвиг как у светлого пола» — с числом,
проверенным по `lab.js`/`applyTheme()`, а не придуманным заново. Дешёвая правка,
без пересмотра остальной части раздела.
### Low — принято ревьюером с записью, правки не требует
**L1. Строка кода для группы `iso-floor-scene` в разделе «Проблема» указана
неточно.** Текст: «`houseplan-card.ts` ~9870, группа `iso-floor-scene`»; на
SHA `b7079ead` фактическая строка — `10901`
(`grep -n "iso-floor-scene" src/houseplan-card.ts`). Расхождение ~1000 строк в
файле на 12882 строки — не влияет ни на один AC (это справочный указатель для
реализующего агента, не проверяемое утверждение поведения), другие
координаты того же раздела (`8414`, `338-368`, `757`) точны. Оставляю без
правки: не блокирует, автор может поправить попутно или проигнорировать.
## Что проверено и корректно
- Обязательные разделы ТЗ по §7.1 присутствуют по существу (см. «Как
проверялось», п. 2).
- Все технические утверждения раздела «Проблема» и «Эталон и единицы» о
текущем состоянии кода подтверждены чтением кода на SHA `b7079ead` (список
выше) — ни одно не оказалось догадкой, выданной за факт.
- Открытые продуктовые вопросы закрыты: Q1–Q3 из тела issue отвечены
владельцем в чате 25.09 и зафиксированы в комментарии «Аналитика» с прямой
ссылкой на источник; ответы корректно перенесены в текст ТЗ (виртуальные
устройства без пунктира; отдельной настройки яркости нет; переключателя на
карте нет).
- Раздел «Принято предположительно» корректно отделяет техническое решение
(можно менять свободно) от продуктового (уже решено владельцем) — ни одно
техническое решение оттуда не выглядит замаскированным продуктовым вопросом.
- Таблица AC1…AC13 — для каждого защитного AC заполнены все три столбца
(AC · чем доказан · чем краснеет), пустых «чем краснеет» нет (§2.7, #435);
для AC12 (сравнение с макетом, не защита) корректно используется другой тип
свидетеля — сравнение, а не мутация, что соответствует исключению из §2.7.
- AC13 (производительность) корректно вынесен в предрелизный гейт, а не
заявлен как проверяемый на этом ревью — совпадает с практикой AGENTS.md
(«Gates», performance/golden/HA — предрелизно).
- Модель данных: поле `settings.volumetric_view` (bool, default false,
хранится только при true) повторяет уже принятый в проекте паттерн
(`show_room_tooltip`, `sun_ray_origin` — см. `docs/CONFIG-COMPATIBILITY.md`);
backend-план (`validation.py`/`support_package.py`/`config-schema.json`)
указывает точные прецедентные места.
- Явно названный откат: `volumetric_view=false` возвращает Flat без
перезагрузки, старый бандл игнорирует поле по схеме — соответствует §2.5.
- i18n: заголовок/описание даны на en/ru/de/fr; ключ `view.volumetric`/
`view.flat` действительно существует и корректно помечен к освобождению.
- Дискрепанс дизайн-референса замечен и разрешён самим автором заранее:
вложение 11 показывает янтарную рамку наведения (правило лаборатории для
светильников), а ТЗ явно фиксирует, что нормативен цвет `#0C82F0` из пакета
иконок, вложение 11 — эталон только формы/подъёма. Без этой оговорки
ревьюер кода почти наверняка завёл бы ложную находку на кодревью.
## Чего не проверял
- Не открывал архив лаборатории (`first-floor-lab-2026-09-25-sketch07.zip`) и
не сверял `lab.js`/`lab.css` построчно с текстом ТЗ — кроме найденной выше
комбинации, доверился утверждению автора о полном прочтении (это его
ответственность по «Материалы прочитаны целиком»); частичная перепроверка
других чисел (радиус, длина луча, blur) по единицам D/H сошлась без
расхождений (см. «Как проверялось», п. 4), что даёт основания доверять
остальным цифрам, но не является гарантией на 100% чисел документа.
- Не проверял математически формулу параллелограмма луча
`s = n·L + t·L·(dir·t)/(dir·n)` с нуля (геометрический вывод из физики
теней) — это проверяемо только исполнением на кодревью через
`smoke_iso_sun`/`test/iso-sun*.test.mjs`, а не чтением спецификации.
- Не запускал `npx tsc --noEmit`, `npm test`, `npm run build` — на этапе
ревью ТЗ продуктовый код не меняется, гейты неприменимы.
- Не сверял восемь вложений (09–16) попиксельно с текстом ТЗ — это работа
AC12 на кодревью (side-by-side, `docs/design/649-25d-stage6/ACCEPTANCE.md`),
не задача ревью текста.
- Не проверял `scripts/config-field-registry.mjs` на предмет обязательности
регистрации нового поля `volumetric_view` — `docs/CONFIG-COMPATIBILITY.md`
прямо говорит, что реестр пока не покрывает все публичные поля, так что
отсутствие явного плана регистрации в ТЗ не расценено как находка.
## Вердикт
Ровно одна находка Medium в скоупе задачи, High нет. Возвращается автору для
однострочного уточнения M1; остальное ТЗ пригодно к разработке без изменений.
Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `b7079ead801f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `27ed2efaa7081238ee82e7c0d965016363dcba54`
```
git log --all --format='%H %T' | grep 27ed2efaa708
```
- Тело issue: `6d3993469405770c498f080b1f6e13b0fe2c05c511c48ccd12bc62884fd3c151`
- Вердикт конвейера: `yellow` · High 0