diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index bfa5bfcb..8f0ca92a 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -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 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-713-r1.md b/docs/reviews/SPEC-REVIEW-713-r1.md new file mode 100644 index 00000000..48db654f --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-713-r1.md @@ -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 на этот путь. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `683c9250b95a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `4e114c6e9445be19c037c6d01d995e8175bfe45d` + ``` + git log --all --format='%H %T' | grep 4e114c6e9445 + ``` +- Тело issue: `ba038b3a982f24f1a070cb2f4395db54759463317b0e7eeaa002ee1f21f55438` +- Вердикт конвейера: `yellow` · High 0