From e19f5b0a0ecc33466df9577d651dc7661284f8d8 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:31:11 +0000 Subject: [PATCH] docs: review document for #713 Issue: #713 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-713-r2.md | 223 +++++++++++++++++++++++++++++ 2 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-713-r2.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 8f0ca92a..2545af6c 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,10 +1,11 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 195, issue: 94. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 196, 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` | +| #713 | [SPEC-REVIEW-713-r2.md](SPEC-REVIEW-713-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — | | #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-r2.md b/docs/reviews/SPEC-REVIEW-713-r2.md new file mode 100644 index 00000000..34c50b91 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-713-r2.md @@ -0,0 +1,223 @@ +# SPEC-REVIEW-713-r2 + +**Issue:** #713 · «Плоские предметы декора и разный сдвиг устройств при переключении в 2.5D» +**Этап:** spec (PROCESS.md §2.4) · **Трек:** `ask` · **Заход:** r2 · блокирующих циклов израсходовано 1/4 до этого раунда + +## Материал раунда + +- Ветка `dev`, коммит `90d38b168508cf2069b11e1f325ae68d9848c500` (дерево + `a372ab54a48802298add970ceb07056b09c25a72`), рабочая копия уже на нём. Код + продукта в этом раунде не менялся — этап `spec`, `S4-spec-review`. +- Тело issue #713, раздел `## ТЗ`, текущая редакция (после правки по r1). +- Комментарий автора `2026-09-30T11:24:41Z`: перечень правок по + `SPEC-REVIEW-713-r1.md` — «M1: К6 переписан… AC5 расширен на (а), новый + AC11 на (в) и холодный старт… Дополнительно: К8 — кадр больше не + резервирует 48 px под сдвиги #651, AC9 дополнен». +- Документ предыдущего раунда: `docs/reviews/SPEC-REVIEW-713-r1.md` + (вердикт жёлтый, Medium 1 — M1, High 0). +- Код прочитан только там, где дельта делает утверждение ТЗ проверяемым: + `src/houseplan-card.ts:3292-3340` (`_warmAdoptViewport`), + `src/iso-scene-render.ts:683-732` (`resolveIsoOverlayFitEnvelope`), + `src/iso-overlays.ts:13` (`ISO_OVERLAY_MAX_NUDGE_CSS_PX`), `demo/smoke_warm_remount.mjs` + (существующий паттерн ремаунта тёплой карточки, на который ссылается новый + план автотестов). Никакой продуктовый код в рамках этого ревью не менялся. + +## Дельта против r1 + +`git diff` неприменим к телу issue; дельта — точечная правка текста ТЗ, +подтверждённая построчно: сравнение цитат из `SPEC-REVIEW-713-r1.md` +(старые формулировки К6/AC5/AC9, процитированные в находке M1) с текущим +текстом issue. Изменения ограничены: + +- К6 — добавлен явный список «где действует» с под-пунктами (а)/(б)/(в) и + отдельно расписан путь (в) — восстановление тёплого памятника из другой + проекции — с формулой пересчёта zoom и поведением при незагруженном + 2.5D-рантайме; +- АС5 — расширен явной формулировкой для (а): «сохранение настройки при + zoom ≠ 1 и сдвинутой камере оставляет viewBox прежним… а zoom равен + `fit'.w / view.w` новой проекции — в обе стороны»; +- новый АС11 — покрывает (в) и холодный старт с включённым 2.5D без + памятника; +- К8 и АС9 — добавлено утверждение «кадр больше не резервирует 48 CSS px + под сдвиги #651»; +- план автотестов — строка про `demo/smoke_iso_flat_parity.mjs` расширена: + «AC2–AC5 и AC11 (ремаунт через `warmBoot` с конфигурацией, сменённой на + сервере), со свидетелем на старом коде». + +Остальные разделы (Сценарий, До/После, Проблема, Скоуп/Не-скоуп, К1–К5, К7, +К9, UX, модель данных, i18n, риски, откат, release-артефакты, «Принятые +предположения») текстуально не изменились — сверено построчно с цитатами и +пересказом r1-документа, расхождений не найдено. Разбор по ним — в разделе +«Унаследовано из r1» ниже, повторно не проверяю. + +Дельта локальна (правка одного контрактного пункта и двух AC по итогам +одной находки прошлого раунда) — полный разбор задачи не требуется, +проверяю только то, что дельта задевает: закрытие M1 и отсутствие новых +противоречий в изменённом тексте. + +## Как проверялось + +### Закрытие M1 + +M1 (r1): К6 называл три пути переноса камеры между проекциями, а +доказательство (AC5) покрывало только два — (б) полностью, (а) косвенно +через AC2; путь (в), восстановление тёплого памятника из другой проекции +(`_warmAdoptViewport`, реальный код `src/houseplan-card.ts:3292-3340`), не +имел ни AC, ни строки в плане автотестов. + +Проверено, что правка закрывает находку по существу, а не только по форме: + +1. **Текст контракта.** К6 теперь явно перечисляет (а)/(б)/(в) с формулой + `z' = fit'.w / view.w` и уточняет поведение (в) при незагруженном + 2.5D-рантайме («пересчёт выполняется при его загрузке, без сдвига + картинки») — то есть явно относится и к отложенной загрузке, не только к + немедленному случаю. +2. **AC.** Новый AC11 текстуально соответствует К6(в): «тёплый памятник, + сохранённый в плоском виде при zoom ≠ 1 и сдвинутой камере, после + включения 2.5D на сервере и ремаунта карточки восстанавливается с тем же + viewBox… и zoom `fit'.w / view.w`… в том числе когда рантайм 2.5D + догружается после ремаунта» — это дословно путь (в) плюс явный + ленивый подслучай. Отдельным предложением в AC11 закрыт и «холодный + старт без памятника» (тоже часть К6), который в r1 вообще не был + поименован отдельно. +3. **Способ доказательства назван, а не декларирован.** План автотестов + указывает конкретный файл `demo/smoke_iso_flat_parity.mjs` и конкретный + механизм — «ремаунт через `warmBoot` с конфигурацией, сменённой на + сервере». Проверено, что это не выдуманный ярлык: паттерн «снять со + страницы старый ``, создать новый с тем же + `localStorage`, дождаться warm-адопта» уже существует и рабочий — + `demo/smoke_warm_remount.mjs` (снятие/пересоздание карточки, проверка + `_warmSlot`, `_zoom` после ремаунта). Новый смок может переиспользовать + этот же паттерн, добавив только смену `settings.volumetric_view` между + `c1.remove()` и созданием `c2` — реалистичный, не гипотетический план. +4. **Код-путь (в) действительно ведёт к найденному дефекту сегодня** — + значит AC11 не вакуумен, будущий тест «умеет упасть». Прочитан + `src/houseplan-card.ts:3320-3329`: + ``` + this._zoom = vp.zoom; + ... + if (!sameProjection && vp.logicalCenter) { + const center = projection === 'iso' ? projectPlanPoint(...) : [...]; + this._applyView(vp.zoom, center[0], center[1]); + } + ``` + При смене проекции (`!sameProjection`) код пересчитывает только центр, + но переиспользует **старый** `vp.zoom` без формулы `fit'.w / view.w` — + ровно то, что К6(в)/AC11 требуют исправить. На сегодняшнем коде AC11 + действительно красный, а не тавтология. +5. **AC9 и К8** («кадр больше не резервирует 48 px под сдвиги #651») — + проверено чтением `src/iso-scene-render.ts:696-732` + (`resolveIsoOverlayFitEnvelope`) и `src/iso-overlays.ts:13` + (`export const ISO_OVERLAY_MAX_NUDGE_CSS_PX = 48;`): `pad = + ISO_OVERLAY_MAX_NUDGE_CSS_PX * scale` на строке 710 — это и есть + резерв 48 CSS px под кластерный сдвиг #651, который ТЗ называет. + Утверждение К8 технически точное, а не догадка: код, который перестаёт + быть нужным при отключении поиска #651 в живом пути, назван верно и по + имени, и по значению константы. + +Вывод: M1 закрыта по существу — недостающий путь (в) получил свой AC с +конкретной формулой, конкретный тест назван и его механизм реалистичен, +затронутый код-путь подтверждён чтением и сегодня действительно ведёт себя +не так, как того требует новый контракт (тест не может оказаться +тавтологией). + +## Находки + +Новых Medium/High в дельте не найдено. + +**L1 (Low, снимается с записью).** AC2 и AC3 явно помечены «свидетель +красный на старом коде», AC11 — нет, хотя по смыслу и по факту (см. п.4 +выше) он тоже красный на текущем коде. Формально план автотестов покрывает +это одной фразой для группы AC2–AC5 и AC11 разом («со свидетелем на старом +коде»), так что пробела в доказательстве нет — только асимметрия +форматирования внутри самого AC11. Снимаю без возврата автору: проверено +чтением кода (п.4 выше), что тест действительно будет падать на нынешнем +`_warmAdoptViewport`, и план автотестов эту гарантию уже даёт на уровне +файла. + +## Что проверено и корректно + +- M1 закрыта: путь (в) К6 получил AC11 с формулой и явным ленивым + подслучаем, план автотестов называет конкретный файл и переиспользуемый + паттерн ремаунта. +- Новое утверждение К8/AC9 («кадр не резервирует 48 px») подтверждено + чтением `resolveIsoOverlayFitEnvelope` и константы + `ISO_OVERLAY_MAX_NUDGE_CSS_PX = 48` — не догадка. +- Код-путь `_warmAdoptViewport` (строки 3292–3340, в частности ветка + `!sameProjection` на 3325) подтверждает, что AC11 не тавтологичен: + сегодня zoom при смене проекции не пересчитывается по формуле К6. +- Остальной текст ТЗ (не задетый дельтой) не изменился со времени r1 — + сверено построчно с цитатами r1-документа. +- Открытых вопросов владельцу не осталось; технических споров, вынесенных + на владельца, в дельте нет. + +## Чего не проверял + +- Реализацию — её нет (этап `S4-spec-review`, правило №1 AGENTS.md). +- `npx tsc --noEmit` / `npm test` / `npm run build` — не прогонял: кода нет, + `node_modules` не установлен (зависимости и Chromium на этапе spec не + ставятся, #696), гейты неприменимы к этому этапу. +- `node scripts/smoke-select.mjs`, `golden:verify`, `pytest tests_backend`, + `npm run invariants` — неприменимы: нет диффа продуктового кода, который + можно было бы прогнать через эти гейты. +- Мутанты `scripts/mutation-registry.mjs` — задача сама откладывает + перепривязку якорей без прогона (owner rule #709); это решение ТЗ, а не + пробел ревью. +- Разделы, не задетые дельтой (К1–К5, К7, К9, Скоуп/Не-скоуп, риски, откат, + release-артефакты, производительность/бюджеты) — не проверял заново; + унаследованы из r1 (см. ниже). + +## Закрытие раунда r1 + +| Находка | Чем закрыта | Где это видно | +|---|---|---| +| M1 — К6(в) («тёплый памятник из другой проекции») не имел AC и строки в плане автотестов | К6 переписан с явным списком (а)/(б)/(в) и формулой `z' = fit'.w / view.w`; добавлен AC11, покрывающий (в) и холодный старт; план автотестов явно называет AC11 в строке про `demo/smoke_iso_flat_parity.mjs` | Тело issue #713, разделы «Контракт поведения → К6» и «Критерии приёмки → AC11», «План автотестов»; проверено чтением `src/houseplan-card.ts:3320-3329`, что путь действительно ведёт себя иначе, чем требует новый К6 | + +## Унаследовано из r1 + +Без повторной проверки в этом раунде принято (документ и материал того +раунда — `docs/reviews/SPEC-REVIEW-713-r1.md`, материал: `dev`@`683c9250b95a`, +дерево `4e114c6e9445be19c037c6d01d995e8175bfe45d`): + +- Обязательные разделы §7.1 присутствуют полностью. +- К1 (`isoPlaneMatrix`, тождество пола, `H·sin 20°`) — сверено чтением + `src/iso-projection.ts`. +- К7 (ключ глубины проёма `s·y + z`) и независимость очереди стен от этой + формулы — сверено чтением `src/iso-openings.ts:376-379`, + `src/iso-walls.ts:107,147,154,206`. +- К4 vs подъём плитки #649 (`0.075·D` не зависит от `marker.size`) — сверено + чтением `src/iso-tiles.ts`, `styles/iso-tiles.styles.ts`. +- К6(а)/(б) и код `_convertProjectionView`/`_syncVolumetricSetting`/ + `_warmAdoptViewport` как реальные, а не гипотетические код-пути — сверено + чтением `src/houseplan-card.ts:2331-2358, 3292-3340`. +- `gs.volumetric_view_hint` в `en.json`/`ru.json` действительно уже обещает + «Decor and the flat plan stay unchanged» — сверено чтением i18n-файлов. +- Owner-решение «вариант Б» подкреплено реальным замером на + `buildIsoOverlayRenderScene`, не голословно. +- Бюджет стартового графа (AC10, #438) назван явно, запас 172 Б. +- i18n и модель данных корректно объявлены «без изменений». + +## Вердикт + +**Зелёный.** M1 закрыта по существу: путь К6(в) получил формулу, AC11 и +названный тест с реалистичным механизмом (переиспользуемый паттерн +`demo/smoke_warm_remount.mjs`), а чтением кода подтверждено, что этот AC не +тавтологичен — `_warmAdoptViewport` сегодня действительно не пересчитывает +zoom по новой формуле при смене проекции. Новое утверждение К8/AC9 про +резерв 48 px проверено и точно совпадает с кодом. High — 0, Medium — 0 (в +задаче нет открытых Medium); один Low снят с записью, не требует возврата +автору. ТЗ готово к `S5-ready`. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `90d38b168508` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `a372ab54a48802298add970ceb07056b09c25a72` + ``` + git log --all --format='%H %T' | grep a372ab54a488 + ``` +- Тело issue: `2305e229564d42ff52bc6d6672c579214fcd49ec80df28e2f111ea726179e7ab` +- Вердикт конвейера: `green` · High 0