mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-05 06:08:59 +00:00
@@ -1,9 +1,10 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 194, issue: 93. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 195, issue: 94. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| #713 | [SPEC-REVIEW-713-r1.md](SPEC-REVIEW-713-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | К6 перечисляет три места переноса камеры между проекциями, но доказательство/AC покрыва… | `src/houseplan-card.ts` |
|
||||
| #711 | [CODE-REVIEW-711-r1.md](CODE-REVIEW-711-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #709 | [CODE-REVIEW-709-r1.md](CODE-REVIEW-709-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | canon TESTING.md противоречит себе | `docs/TESTING.md` `scripts/smoke-select.mjs` `scripts/pre-push-gate.mjs` `TESTING.md` `process-digests.test.mjs` |
|
||||
| #709 | [CODE-REVIEW-709-r2.md](CODE-REVIEW-709-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
|
||||
@@ -0,0 +1,184 @@
|
||||
# SPEC-REVIEW-713-r1
|
||||
|
||||
**Issue:** #713 · «Плоские предметы декора и разный сдвиг устройств при переключении в 2.5D»
|
||||
**Этап:** spec (PROCESS.md §2.4) · **Трек:** `ask` · **Заход:** r1 · блокирующих циклов израсходовано 0/4 до этого раунда
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Тело issue #713, раздел `## ТЗ` (первая полная редакция, комментарий владельца
|
||||
30.09 «Сделано: первая полная редакция ТЗ…», сессия `session_018qZfe7YS4rqEMKoVeS3GKd`).
|
||||
- Четыре комментария владельца/автора от 2026-09-30 (10:30–11:10 UTC): замер
|
||||
трёх вариантов подъёма значков (А/Б/текущий) на `buildIsoOverlayRenderScene`,
|
||||
решение владельца «вариант Б», решения по проекции/названиям/переносу камеры/
|
||||
тени плитки.
|
||||
- Канонический документ подсистемы: `docs/ISOMETRIC.md` (текущее поведение).
|
||||
- Код прочитан только для проверки, что ТЗ не выдаёт догадку за факт: код
|
||||
продукта не менялся, ветки/коммитов к этой задаче нет (метка `S4-spec-review`,
|
||||
что и ожидается на этом этапе).
|
||||
|
||||
## Скоуп задачи
|
||||
|
||||
Баг: в 2.5D пол (и лежащий на нём декор/мебель) сжимается по `cos 20°`
|
||||
относительно центра сцены, а значки устройств поднимаются раскладкой #651 с
|
||||
индивидуальным вектором на кластер (до 48 px), из-за чего декор наезжает сам на
|
||||
себя, а сдвиг значков не одинаков. ТЗ переводит проекцию на «третий путь»: пол
|
||||
не преобразуется вовсе, стены и значки поднимаются строго вверх на
|
||||
`H·sin 20°`. Это закрывает J1/J6 (`docs/SCOPE.md`) — «объёмный вид не должен
|
||||
врать о геометрии плана» — и совпадает с существующим обещанием подсказки
|
||||
`gs.volumetric_view_hint` («Decor and the flat plan stay unchanged»).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Обязательные разделы §7.1 — все присутствуют: сценарий, что человек увидит до/
|
||||
после, проблема, скоуп/не-скоуп, контракт поведения (К1–К9), UX, модель данных
|
||||
и миграция, i18n, критерии приёмки AC1–AC10 с методом доказательства, план
|
||||
автотестов, риски, откат, release-артефакты. Разделов, ссылающихся на
|
||||
несуществующее поведение или придуманную терминологию, не найдено.
|
||||
|
||||
Технические утверждения ТЗ сверены с реальным кодом, а не приняты на веру
|
||||
(находка «догадка как факт» ищется именно так):
|
||||
|
||||
- **К1 (пол — тождество, `isoPlaneMatrix`).** Прочитан `src/iso-projection.ts`.
|
||||
Текущая матрица при `rotDeg=0`: `d = xyScale·cos(tilt)`, `f` включает
|
||||
`-z·zScale·sin(tilt)` — т.е. компрессия по `cos 20°` уже локализована в одном
|
||||
месте (`d`), а высотный член уже использует `sin 20°`, как и требует К1
|
||||
(«это столько же, сколько сейчас: 28,73 единицы»). Правка технически
|
||||
локальна и не меняет высоту стен — заявление ТЗ подтверждается чтением, а не
|
||||
декларацией.
|
||||
- **К7 (ключ глубины проёма).** Прочитан `src/iso-openings.ts:376-379`: текущий
|
||||
`cameraDepth = (x·sin(rot)+y·cos(rot))·sin(tilt) + z·cos(tilt)` — при
|
||||
`rot=0` это ровно `y·sin20° + z·cos20°`, как и написано в ТЗ до правки; новый
|
||||
ключ `s·y + z` — согласованное упрощение. Отдельно прочитан
|
||||
`src/iso-walls.ts:107,147,154,206`: очередь стен сортируется по проекции
|
||||
экранного Y (`projectPlanPoint(...)[1]`), а не по этой формуле — она
|
||||
автоматически наследует исправление К1 и не требует отдельного пункта в ТЗ.
|
||||
К7 корректно ограничен только вертикальными поверхностями проёма.
|
||||
- **К4 / «один и тот же Δ для всех» vs плитка #649.** Проверено, не ломает ли
|
||||
инвариант «сдвиг один для всех» существующий подъём плитки `0.075·D`
|
||||
(`src/iso-tiles.ts`, `ISO_TILE.lift = 6/80`). Опасение было: `D` — диаметр
|
||||
конкретного маркера, который зависит от `marker.size` (per-device поле,
|
||||
живой диалог `houseplan-editor-runtime.ts:7224`, `presentation.scale` в
|
||||
`device-presentation.ts:787`), и тогда `0.075·D` был бы разным для маркеров
|
||||
разного размера, что противоречило бы К4 и AC3. Опровергнуто чтением
|
||||
`src/iso-tiles.ts:7-8` и `styles/iso-tiles.styles.ts:10` /
|
||||
`iso-tiles.ts:143-145`: `--iso-lift` считается от `--iso-d`, а `--iso-d`
|
||||
строится из `--device-base-size` (константа **пространства**, а не
|
||||
`--dev-size`, которая единственная включает `--dev-scale`/`marker.size`).
|
||||
Значит подъём плитки одинаков для всех маркеров пространства независимо от
|
||||
индивидуального размера — К4/AC3 не противоречат существующему коду.
|
||||
(Ложная тревога снята чтением, оставляю в документе как пример проверенного,
|
||||
а не декларированного факта.)
|
||||
- **Перенос камеры (К6), тёплый памятник.** Прочитан
|
||||
`src/houseplan-card.ts:2331-2358` (`_convertProjectionView`,
|
||||
`_syncVolumetricSetting`) и `:3292-3340` (`_warmAdoptViewport`). Подтверждено:
|
||||
ветка `sameProjection` уже сравнивает `vp.projection` с текущей эффективной
|
||||
проекцией (`:3322`) — сценарий «памятник сохранён в другой проекции»
|
||||
действительно достижим (например, настройка сменилась пуш-обновлением между
|
||||
сохранением памятника и восстановлением), а не гипотетический краевой
|
||||
случай.
|
||||
- **`gs.volumetric_view_hint`.** Проверено в `src/i18n/settings/ru.json:54` и
|
||||
`en.json:54` — текст действительно уже обещает «Декор и обычный план не
|
||||
меняются» на всех языках; ТЗ верно называет его текущим враньём, которое
|
||||
правка делает правдой без изменения строк (раздел i18n «нет новых ключей»
|
||||
подтверждён — no diff needed).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе, чинится в этой же задаче)
|
||||
|
||||
**M1 — К6 перечисляет три места переноса камеры между проекциями, но
|
||||
доказательство/AC покрывает только два.**
|
||||
|
||||
К6 называет три ситуации: (а) сохранение настройки на той же сцене, (б) вход
|
||||
из Просмотра 2.5D в редактор (и возврат), (в) **восстановление снимка
|
||||
Просмотра и тёплого памятника (`vp.projection`), сохранённых в другой
|
||||
проекции**. AC5 — единственный кандидат на доказательство К6 — по тексту
|
||||
покрывает только (б): «Вход в редактор плана из Просмотра 2.5D даёт тот же
|
||||
viewBox редактора… Возврат восстанавливает камеру Просмотра.» AC2 покрывает
|
||||
(а) косвенно (пол не сдвигается на той же сцене). Ситуация (в) — отдельный,
|
||||
явно упомянутый в контракте код-путь (`_warmAdoptViewport`,
|
||||
`src/houseplan-card.ts:3292-3340`, ветка `!sameProjection && vp.logicalCenter`
|
||||
на строке 3325) — не имеет ни своего AC, ни строки в «Плане автотестов», ни
|
||||
явной ссылки на существующий тест. Проверено: `grep` по `test/` и `demo/` на
|
||||
`_warmAdoptViewport`, `warmViewportState`, `vp.projection`, `activeLabsIso` —
|
||||
ноль совпадений, сценарий сегодня вообще не тестируется, а после правки
|
||||
именно в нём легче всего оставить старую (наивную) формулу переноса зума и не
|
||||
заметить регресс — этот путь сложнее двух остальных (восстановление после
|
||||
ремаунта Lovelace, а не прямое пользовательское действие).
|
||||
|
||||
- **Чем краснеет:** пусто — ни в AC, ни в «Плане автотестов» нет ни unit-, ни
|
||||
smoke-проверки, которая упадёт, если `_warmAdoptViewport` продолжит
|
||||
переносить зум без пересчёта `z' = z·fit'.w/fit.w` при разной проекции.
|
||||
- **Как чинится в скоупе:** добавить в контракт либо отдельный AC (unit- или
|
||||
smoke-тест на восстановление тёплого памятника, сохранённого в другой
|
||||
проекции, с проверкой итогового viewBox по формуле К6), либо явно
|
||||
распространить формулировку AC5 на этот путь и назвать конкретный тест в
|
||||
«Плане автотестов». Без этого AC9 («живой путь… для пола нигде не осталось
|
||||
cos 20°») не подтверждает, что и путь восстановления памятника избавлен от
|
||||
старой формулы — только вход в редактор и сохранение настройки.
|
||||
|
||||
Находка в скоупе задачи (сам К6 её называет), правится в этой же итерации ТЗ —
|
||||
отдельный issue не заводится (§12).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обязательные разделы §7.1 присутствуют полностью, без пропусков.
|
||||
- Открытый технический вопрос из промежуточного авторского комментария
|
||||
(«в каких координатах декор не меняется — экран или план») закрыт финальной
|
||||
редакцией К2/К1: пол тождественен, поэтому вопрос снят самим «третьим
|
||||
путём» — эскалации владельцу не требуется, вопрос решён по существу
|
||||
(§7.1: технический спор решает ревьюер, а не откладывает на владельца).
|
||||
- К1, К3, К7 и связка К4↔#649-плитка проверены чтением реального кода
|
||||
(`iso-projection.ts`, `iso-openings.ts`, `iso-walls.ts`, `iso-tiles.ts`,
|
||||
`houseplan-card.ts`, `houseplan-editor-runtime.ts`, `device-presentation.ts`)
|
||||
— расхождений с текущим состоянием не найдено, числа (`H=84`,
|
||||
`sin20°≈0.342`, `28.73`, `172` байта запаса бюджета) совпадают с константами
|
||||
и тестами в дереве.
|
||||
- Owner-decision по варианту «Б» (значки на высоту стен, заход на дальние
|
||||
грани принят) подкреплён реальным замером на `buildIsoOverlayRenderScene` и
|
||||
силуэтах стен структурной сцены — не голословное решение.
|
||||
- Не-скоуп корректно отделяет удаление кода поиска #651 (отдельный tech-debt
|
||||
issue, уже объявлено для стадии реализации) и не трогает редакторы/
|
||||
`houseplan-space-card`/плоский Просмотр.
|
||||
- Бюджет стартового графа назван явно (AC10, гейт #438) с фактическим запасом
|
||||
172 Б и обязательством ленивых чанков — не оставлено как невязка.
|
||||
- i18n и модель данных корректно объявлены «без изменений», подтверждено по
|
||||
файлам.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Реализацию — её нет (этап `S4-spec-review`, код появляется только после
|
||||
`S5-ready`, правило №1 AGENTS.md).
|
||||
- Не прогонял `npx tsc --noEmit` / `npm test` / `npm run build` — на этом
|
||||
этапе нет диффа кода, гейты неприменимы (ставятся на code-review).
|
||||
- Не проверял golden/скриншоты — они появятся только с реализацией (AC8).
|
||||
- Не проверял мутанты в `scripts/mutation-registry.mjs` — задача явно
|
||||
откладывает перепривязку якорей без прогона (owner rule #709), это решение
|
||||
ТЗ, а не пробел ревью.
|
||||
- Не проверял `docs/USER-GUIDE.ru.md`/`.md` и `CHANGELOG*` тексты — они ещё не
|
||||
написаны (release-артефакты идут с реализацией); ТЗ верно резервирует под
|
||||
них место.
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Жёлтый.** Один Medium-пробел в скоупе (M1): один из трёх явно
|
||||
перечисленных в К6 путей переноса камеры между проекциями не имеет ни AC, ни
|
||||
теста, хотя код-путь для него существует и уже сегодня не покрыт тестами.
|
||||
High-находок нет, ТЗ технически последовательно, разделы полны, догадок,
|
||||
выданных за факт, не найдено. Возврат автору — добавить AC/тест на
|
||||
восстановление тёплого памятника, сохранённого в другой проекции, либо явно
|
||||
расширить область действия AC5 на этот путь.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `683c9250b95a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `4e114c6e9445be19c037c6d01d995e8175bfe45d`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 4e114c6e9445
|
||||
```
|
||||
- Тело issue: `ba038b3a982f24f1a070cb2f4395db54759463317b0e7eeaa002ee1f21f55438`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user