docs: review document for #396

Issue: #396
User-Visible: no

docs/reviews/SPEC-REVIEW-396-r2.md

@@ -0,0 +1,197 @@
+# SPEC-REVIEW-396-r2
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/396
+- **ТЗ под ревью:** `docs/specs/396-camera-transition-fixes.md`, коммит `d5fd5926faab12ec0883458b1d4a932ea9344bb3` (HEAD, `docs: #396 spec revision 2 per SPEC-REVIEW-396-r1`)
+- **Этап:** spec (PROCESS.md §2.4)
+- **Трек:** обычный (без метки `small`/`trivial`)
+- **Заход:** r2 · блокирующих циклов ревью ТЗ израсходовано 1 из 4 (до этого вердикта)
+
+## Скоуп ревью
+
+Раунд r1 вернул красный вердикт с одной High-находкой: раздел «(1) B1» ТЗ
+утверждал, что переход отменяют семь конкретных мест
+(`:1267,:1401,:1551,:4188,:4216,:6089,:6206`), а фактическая сверка показала
+десять мест плюс отдельный `.dispose()` на disconnect — при этом ни настоящий
+`_stagePointerDown` (`:6387`, буквальный сценарий issue), ни настоящий
+`_zoomAt` (`:6283`) в семёрке не фигурировали, а раздел «Скоуп» не называл
+`_stagePointerDown` затрагиваемой функцией. Владелец ответил ревизией 2:
+заменил список семи чисел таблицей из одиннадцати мест с классификацией
+«пользовательская/структурная», добавил `_stagePointerDown` в «Скоуп»,
+переформулировал AC1/AC2.
+
+Это ревью — по дельте (PROCESS.md §2.9): единственный правленый раздел —
+«(1) B1» плюс «Скоуп/не-скоуп» плюс AC1/AC2 (см. диф ниже). B2, M2, AC3–AC7,
+UX, модель данных, i18n, риски, откат, release-артефакты дельту не задевают и
+наследуются из r1 без повторной проверки. Причина полного нового прочтения
+таблицы вместо точечной сверки одной строки: сама таблица — это ровно тот
+артефакт, для которого r1 объявил высокий риск фактической ошибки, и
+инструкция раунда прямо требует не доверять заявлению автора на слово, а
+сверять «строку кода или текста».
+
+```
+git diff 4e0a30a78161a4ff59679454d1f8b5c471e10008..HEAD -- docs/specs/396-camera-transition-fixes.md
+```
+(коммит `4e0a30a7` — HEAD ветки на момент вердикта r1).
+
+Продуктового кода в ветке по-прежнему нет: `git diff --stat 4e0a30a7..HEAD`
+показывает только `docs/reviews/SPEC-REVIEW-396-r1.md` (публикация прошлого
+вердикта) и `docs/specs/396-camera-transition-fixes.md`. `src/**` не тронут.
+
+## Как проверялось
+
+1. Прочитан весь диф правленых разделов ТЗ (раздел «(1) B1», «Скоуп/не-скоуп»,
+   AC1, AC2, строка «Ревизия»).
+2. Каждая строка новой таблицы одиннадцати мест сверена с фактическим кодом на
+   HEAD той же командой, что и в r1:
+   `grep -n "_cancelCameraTransition(false)\|_cancelCameraTransition(true)\|_cameraTransition\.\(cancel\|dispose\)" src/houseplan-card.ts`
+   — 14 строк (13 точек вызова + строка-определение самого `_cancelCameraTransition`
+   на `:1242`, которая не точка вызова и в таблицу не входит).
+3. Каждая из 13 точек вызова прочитана в контексте содержащей функции:
+   `:1159` (`_onMotionChange`), `:1267` (`_startCameraTransition`, no-op
+   ветка), `:1401` (`_cancelModeTransition`), `:1551` (`_commitSpace`),
+   `:2363` (`_pageVisibility`), `:2745` (`disconnectedCallback` →
+   `.dispose()` напрямую), `:4188` и `:4216` (обе — внутри
+   `_adoptStructuralResponses`, `:4167-4224`), `:6089` (`_applyView`),
+   `:6206` (`_refitView`), `:6283` (`_zoomAt`), `:6360` (`_restoreZoom`),
+   `:6387` (`_stagePointerDown`).
+4. Отдельно перепроверены оба вызывающих контекста `_zoomAt` (`:6517` —
+   pinch-move, `:6897` — touch multitouch move): оба вызывают `_saveZoom()`
+   сразу после `_zoomAt(...)` — заявление таблицы «правки не требует» верно.
+5. Сверено, что `_stagePointerDown` (`:6387`) действительно первой строкой
+   вызывает `_cancelCameraTransition(false)` и не содержит гарантированного
+   `_saveZoom()` на пути простого клика — это подтверждает и контракт AC1, и
+   то, что High-1 из r1 закрыт по существу (строка теперь в «Скоуп» и в AC1).
+6. Гейты (`typecheck`/`test`/`build`) не прогонялись — продуктового кода нет
+   (только doc-коммиты), это вне скоупа этапа spec (PROCESS.md §2.4/§8), как
+   и в r1.
+
+## Находки
+
+### Medium-1 — таблица приписывает строки `:4188`/`:4216` несуществующей функции `_load`
+
+**Файл:** `docs/specs/396-camera-transition-fixes.md`, раздел «(1) B1»,
+строка таблицы `| \`:4188\`, \`:4216\` \`_load\` | adoption конфига и layout | структурная |`.
+
+Фактическая проверка: в `src/houseplan-card.ts` идентификатора `_load` не
+существует (`grep -n "\b_load\b" src/houseplan-card.ts` — пусто). Обе строки
+`:4188` и `:4216` лежат внутри одной и той же функции
+`_adoptStructuralResponses` (`:4167-4224` — сигнатура на `:4167`, config-adopt
+ветка на `:4188`, layout-adopt ветка на `:4216`), которую вызывают
+`_loadFromServer` (`:4247`, вызов на `:4284`) и второй caller на `:4460`
+(config-only reload). Ни один из этих реальных идентификаторов не совпадает
+с `_load`: ближайшие по написанию — `_loadFromServer`, `_loadOk`, `_loading`,
+`_loadTries`, `_loadRetryTimer` — все разные функции/поля.
+
+Это ровно тот класс дефекта, который r1 квалифицировал как High (таблица
+называет несуществующее/неверное имя функции, хотя визуально оформлена как
+проверенный факт — «сверено `grep`'ом… каждое прочитано»), и он **новый**:
+в тексте r1 (и в собственном ответе ревьюера, и в разборе владельца) эти же
+строки корректно названы `_adoptStructuralResponses` — ревизия 2, поправляя
+High-1, сама внесла эту неточность взамен верного имени.
+
+**Почему Medium, а не High.** У этой строки нет требуемой правки кода — класс
+«структурная», и правка по контракту B1 нужна только в `_stagePointerDown`
+(`:6387`), которая в «Скоуп» названа явно и корректно. Ложное имя `_load` не
+рискует тем, что разработчик пропустит обязательную правку (в отличие от
+High-1 r1, где отсутствие `_stagePointerDown` в списке грозило оставить
+буквальный сценарий issue незачиненным) — оно рискует ввести в заблуждение
+читателя, сверяющего полноту классификации (найдёт `_loadFromServer`, не
+найдёт `_load`, потратит время на поиск несуществующего имени), и подрывает
+доверие к самому заявлению «каждое прочитано» в этой же таблице.
+
+**Что нужно поправить.** Заменить `_load` на `_adoptStructuralResponses` в
+строке таблицы для `:4188`/`:4216`.
+
+### Low-1 (waived) — вторая функция в строке `:1159`/`:2363` не названа
+
+Строка таблицы `| \`:1159\` \`_onMotionChange\`, \`:2363\` | ... |` называет
+функцию только для первого числа; `:2363` фактически лежит в `_pageVisibility`
+(`:2357-2363`, поле-стрелка класса), но таблица этого не пишет. В отличие от
+Medium-1 здесь нет ложного утверждения — просто пропуск имени, классификация
+(«`cancel(true)`, цель коммитится, уже сохраняется через `settled`») верна для
+обеих строк. Не блокирует; можно поправить попутно при правке Medium-1, а
+можно оставить — снимаю без требования правки.
+
+## Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где это видно |
+|---|---|---|
+| High-1: список из семи чисел не совпадает с реальными вызовами; `_stagePointerDown` (`:6387`) и `_zoomAt` (`:6283`) отсутствуют в списке; «Скоуп» не называет `_stagePointerDown` | Список заменён таблицей из одиннадцати мест (все 13 точек вызова, включая `.dispose()` на `:2745`), с явной классификацией «пользовательская/структурная»; `_stagePointerDown` добавлен в «Скоуп» жирным и назван «буквальный сценарий issue» с явной пометкой «сохранять»; `_zoomAt` явно помечен «правки не требует» с обоснованием | `docs/specs/396-camera-transition-fixes.md`, раздел «(1) B1» (новая таблица) и раздел «Скоуп / не-скоуп» (`**\`_stagePointerDown\`**`); проверено построчной сверкой с кодом в этом раунде (см. «Как проверялось» п.2–5) — таблица теперь фактически верна за единственным исключением Medium-1 (имя `_load` вместо `_adoptStructuralResponses` для `:4188`/`:4216`) |
+| Риск «поймает только код-ревью, лишний цикл» | Снят тем же исправлением: `_stagePointerDown` теперь явно в скоупе и в AC1, реализация не сможет случайно пропустить это место, читая раздел «Скоуп» как границу работы | AC1: «После пользовательской отмены перехода (`_stagePointerDown` — касание плана...) сохранённый зум пространства равен показанному» |
+
+## Унаследовано из r1
+
+Не проверялось повторно в этом раунде — дельта их не касается, приняты по
+документу `docs/reviews/SPEC-REVIEW-396-r1.md` на коммите `4e0a30a78161a4ff59679454d1f8b5c471e10008`:
+
+- **B2 (якорь от отстающего кадра)** — контракт, математическое обоснование
+  (`_cameraTargetAt` строит якорь от `_cameraState()`, presented, независимо
+  от `target`), правка §10 `docs/specs/082-smooth-zoom.md` (AC6) — раздел
+  «(2) B2» ТЗ не входит в диф r1→r2, не перечитывался.
+- **M2 (feather не заморожен)** — цитата `resolveGlowFeather(...,
+  !this._pinchStart && !this._panStart)` (`houseplan-card.ts:10845-10847`)
+  сверена дословно в r1; в этом раунде дополнительно подтверждено, что строка
+  не сдвинулась (см. «Как проверялось», п. проверки M2 в контексте
+  неизменного дифа) — раздел «(3) M2» ТЗ тоже вне дифа r1→r2.
+- **AC3–AC7** — формулировки не менялись (вне дифа), контракты и способы
+  доказательства признаны верными в r1.
+- **Структура ТЗ / обязательные разделы PROCESS.md §7.1** — все на месте,
+  проверено в r1 (сценарий, до/после, скоуп, UX, модель данных, i18n, план
+  автотестов, риски, откат, release-артефакты).
+- **Правка §10/§13 `docs/specs/082-smooth-zoom.md` (AC6)** — обоснованность
+  и то, что она не требует продуктового решения владельца, подтверждены в r1.
+- **Трек (обычный, не `small`)** — обоснован в r1 и не оспаривается.
+- **Открытых продуктовых вопросов владельцу нет** — подтверждено в r1;
+  Medium-1 этого раунда — тоже чисто технический вопрос (имя функции в
+  таблице), не продуктовый, решается вердиктом, а не владельцем.
+- **Мутанты названы конкретно** и подтверждают правило «тест умеет падать» —
+  раздел «Мутанты» вне дифа r1→r2.
+
+## Что проверено и корректно (в этом раунде, по дельте)
+
+- Новая таблица из одиннадцати мест фактически верна для 12 из 13 точек
+  вызова (все, кроме `:4188`/`:4216`) — построчно сверено с кодом на HEAD.
+- `_stagePointerDown` корректно классифицирован как единственное
+  пользовательское место, требующее правки, и корректно добавлен в «Скоуп».
+- `_zoomAt` корректно исключён из мест, требующих правки: оба его реальных
+  вызывающих контекста (`:6517`, `:6897`) действительно вызывают `_saveZoom()`
+  сразу после — проверено чтением обоих мест.
+- AC1 и AC2 переформулированы в соответствии с новой таблицей, без изменения
+  сути контракта B1/B2/M2 из r1; AC2 корректно охватывает `_restoreZoom` как
+  структурное место, которое ничего не пишет в `localStorage` (сам
+  `_restoreZoom` и вызываемый им `_applyView` не содержат `_saveZoom()`).
+- Раздел «Ревизия» корректно ссылается на SPEC-REVIEW-396-r1 и называет
+  High-1 явно — соответствует ожиданию «показать, чем именно закрыта находка
+  предыдущего раунда» текстом, а не только заявлением.
+
+## Чего не проверял
+
+- Разделы «(2) B2», «(3) M2», UX, модель данных/миграция, i18n, план
+  автотестов, риски, откат, release-артефакты — не входят в диф r1→r2,
+  унаследованы из r1 (см. раздел выше), повторно не перечитывались построчно
+  против кода в этом раунде.
+- Гейты `typecheck`/`test`/`build`/`check-docs`/`invariants`/смоки/
+  `golden:verify` — неприменимы: продуктового кода в ветке нет (только
+  doc-коммиты), как и в r1.
+- Содержимое внешнего аудита `AUDIT-2026-08-31-v1700beta1.md` — по-прежнему
+  не в репозитории, не перепроверялось (не входит в дельту этого раунда).
+- `docs/specs/082-smooth-zoom.md` целиком — не перечитывался повторно в этом
+  раунде, так как правки §10/§13 вне дифа r1→r2; принято на веру из r1.
+
+## Вердикт
+
+Жёлтый. High: 0. Medium: 1 (Medium-1 — строка таблицы раздела «(1) B1»
+приписывает `:4188`/`:4216` несуществующей функции `_load` вместо
+фактической `_adoptStructuralResponses`; в скоупе этого же раунда, чинится в
+нём же, без отдельного issue). Low: 1, waived (пропуск имени функции для
+`:2363` — не блокирует, можно поправить попутно).
+
+High-1 из r1 закрыт по существу: `_stagePointerDown` (буквальный сценарий
+issue) теперь явно в «Скоуп» и в AC1, `_zoomAt` корректно исключён из мест,
+требующих правки, таблица одиннадцати мест фактически верна за единственным
+исключением Medium-1. После правки Medium-1 (заменить `_load` на
+`_adoptStructuralResponses` в одной строке таблицы) — заход по дельте
+(PROCESS.md §2.9): проверить только эту строку, всё остальное наследуется без
+повторной проверки.
+
+**Вердикт: жёлтый · заход r2 · блокирующих циклов 2/4 · High: 0 · Medium: 1 → в задаче · Документ: docs/reviews/SPEC-REVIEW-396-r2.md**