docs: review document for #525

Issue: #525
User-Visible: no

docs/reviews/SPEC-REVIEW-525-r1.md

@@ -0,0 +1,235 @@
+# SPEC-REVIEW-525-r1
+
+## Скоуп
+
+Issue #525 (`bug`, `P2`, полный трек — критерии §5 «нет влияния на
+производительность» и «одна поверхность» нарушены, названо явно в
+аналитике). ТЗ живёт в теле issue под заголовком `## ТЗ` (решение
+владельца #517). Материал ревью — тело issue #525 и два комментария
+(S2-аналитика, «ТЗ готово»); issue открыт, метка `S4-spec-review`.
+
+Предмет: узел `.op-leaf`/`.op-arc` (створка и дуга проёма) и
+`.device-shell-frame` (маркер устройства) переигрывают чужой CSS-переход
+при переключении пространства, потому что списки проёмов и устройств
+рисуются `Array.map` вместо `repeat()` с ключом — Lit переиспользует DOM-узел
+по позиции, а не по идентичности. Решение: ключи `repeat(items, o => o.id)`
+для проёмов и `repeat(devs, d => d.id)` для маркеров устройств; новый смок
+`smoke_space_switch_transitions.mjs`; два мутанта в
+`scripts/mutation-gate.mjs`; перф-гейт на `large-house-interaction`.
+
+## Как проверялось
+
+Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (включая §7.1,
+§8, §10, §12), тело issue #525 и оба комментария. Дополнительно, раз ТЗ
+опирается на конкретные строки кода и числа, а не только на формулировки,
+проверено чтением репозитория на SHA `19e421b3` (это же SHA автор называет
+как базу замера):
+
+- `src/houseplan-card.ts:12996` (`_renderOpenings`, `items.map` без `repeat`)
+  и `:11787` (`devs.map` — список маркеров устройств, тоже без `repeat`) —
+  оба подтверждены чтением, ссылки в ТЗ точны;
+- `src/styles/plan.styles.ts:523-529` — переходы `.op-leaf`/`.op-arc`
+  подтверждены;
+- `src/glow-scene.ts:581,612` — прецедент `repeat(input.spots, spot => spot.key, …)`
+  подтверждён;
+- `src/live-editor.ts:305` — функция `paintDevice`, на которую ссылается
+  контракт (идентичность узла по `data-id`), существует;
+- `space.rooms.map` на `:9416`, `:11794`, `:11798` и декор на `:8763` —
+  подтверждены, ключей действительно нет;
+- `src/summary-panel-i18n.ts` — ключ `summary.problem.duplicate_id`
+  подтверждает существование валидации на дублирующиеся id, на которую ТЗ
+  ссылается в разделе допущений;
+- `demo/performance/budgets-large-house-interaction.json` и
+  `demo/performance/evaluate.mjs` — пересчитаны все пять чисел AC6
+  (`spaceSwitchMs` 769.35, `switchCycleMs` 1696.28, `firstStableRenderMs`
+  3000 — упирается в `hardMaxMs`, `modelReadyMs` 944.97,
+  `longTask.maxSingleMs` 910) по формуле гейта
+  `max(база×(1+maxRegressionRatio), база+noiseAllowanceMs)`, зажатой
+  `hardMaxMs`. Все пять сошлись с точностью до сотых — числа посчитаны, а не
+  подобраны;
+- `scripts/mutation-gate.mjs` — реестр мутантов существующего формата (664
+  штуки), добавление двух новых (`openings-rendered-without-keys`,
+  `device-markers-rendered-without-keys`) технически укладывается в
+  существующий механизм find/replace + смок;
+- `demo/smoke_*.mjs` (239 файлов на эту дату) — прецедент
+  `element.getAnimations({subtree: true})` уже используется
+  (`demo/smoke_motion_sense.mjs:87`), то есть требование «поэлементный обход
+  с обходом вложенных shadow root» реализуемо без велосипеда — это деталь
+  реализации, а не предмет ревью ТЗ;
+- `docs/CHANGELOG.ru.md` / `docs/CHANGELOG.md` — секция «Не выпущено» /
+  «Unreleased» существует, формат черновика записи совпадает с реальным
+  стилем файла;
+- `docs/UX-MODES.md`, `docs/CANVAS.md` — не содержат положений, которым
+  контракт ТЗ противоречил бы.
+
+Гейты не гонялись: на этапе ТЗ нет кода для проверки, продуктовый код не
+менялся. Дешёвые гейты (`typecheck`/`test`/`build`) к этому ревью
+неприменимы.
+
+## Продуктовая рамка
+
+Персона названа верно — домочадцы/гости в Просмотре, `docs/SCOPE.md`
+прямо называет View mode продуктом для этих двух персон. Поверхность —
+переключение вкладки пространства. Сценарий закрывает J1/J2 (карточка
+показывает состояние проёмов — и не должна показывать ложное событие).
+«До/после» сформулировано одной фразой без терминов реализации, как того
+требует §7.1. Возражений нет.
+
+## Разбор AC
+
+Все шесть AC пронумерованы, у каждого указан способ доказательства и (сверх
+формального минимума спецификации — это требование кодревью, §2.7, но
+здесь уже сделано заранее) отдельная колонка «чем краснеет» с названным
+мутантом или сломанным инвариантом. Это не даёт ревьюеру ТЗ поводов для
+формальных претензий к самой таблице.
+
+- **AC1/AC2** — конкретны, привязаны к точным CSS-классам и свойствам,
+  проверяются одним новым смоком с именованной фикстурой. Мутанты
+  (`openings-rendered-without-keys`, `device-markers-rendered-without-keys`)
+  бьют ровно по описанному контракту (откат `repeat` → `map`).
+- **AC3** — сформулирован как инвариант («допустимое множество классов»), а
+  не как перечень запрещённых. Это сильнее позиционного запрета: новая
+  легитимная анимация, добавленная в будущем на кадре переключения,
+  обязана быть вписана в список руками, иначе смок красный. Автор сам
+  вынес это как спорный пункт (п.2 в комментарии «ТЗ готово») — согласен с
+  выбором инварианта: перечень «что нельзя» тут слабее и не поймал бы
+  новый класс дефекта так, как поймал прецедент #234 из PROCESS.md.
+- **AC4** — защищает от обратной регрессии (глушение настоящей анимации),
+  чем краснеет — названо прямо (удаление `transition` из `.op-leaf`).
+- **AC5** — короткий, но по существу: без него смок рискует повторить
+  ловушку `document.getAnimations()` из аналитики (#521 упомянут как
+  прецедент того же класса ложного свидетеля). Формулировка «в смоке нет
+  `document.getAnimations()`» — проверяемый текстовый инвариант, не
+  оценочное суждение.
+- **AC6** — единственный AC с численным порогом; пороги не выдуманы, они
+  считаются существующим гейтом `benchmark:compare` относительно базового
+  SHA. Числа в ТЗ (раздел «Цена вариантов») пересчитаны и совпадают
+  (см. «Как проверялось»).
+
+Двусмысленности, которая заставила бы гадать при реализации, не нашёл —
+ни один AC не оставляет открытым вопрос «а что именно считается
+прохождением».
+
+## Разбор допущений
+
+Раздел «Принято предположительно» корректно отделяет то, что решает
+владелец (объём видимого изменения — J1/J2 остаются с известным
+остаточным дефектом на комнатах/декоре, если он вообще проявится) от
+того, что решает автор (какой ключ использовать, как разрешать дубликаты).
+Оба пункта снабжены обоснованием, не голым «сделаю так»:
+
+- **Комнаты/декор вне скоупа** — обосновано: та же болезнь есть
+  (`space.rooms.map` без ключа, подтверждено чтением), но у неё нет
+  измеренной цены и нет наблюдаемого проявления в собственной фикстуре
+  задачи; риск компенсирован инвариантом AC3 (новая анимация на кадре
+  переключения красит смок, а не проходит тихо). Это ровно та ситуация из
+  §7.1, где технический вопрос («где провести границу правки») решён
+  автором с объяснением и не эскалирован владельцу — граница проведена не
+  по ощущению, а по тому, что можно измерить в этой задаче.
+- **Вариант 2 постановки (глушение переходов) отклонён** — обоснование
+  учитывает конкретное следствие в живом коде (`paintDevice` ищет узел по
+  `data-id`, глушение оставило бы узлу чужую идентичность на кадр) — не
+  вкусовщина.
+- **Уникальность ключей** — подтверждена существующей валидацией
+  (`summary.problem.duplicate_id`), это не голословное «наверное, там
+  где-то проверяется».
+
+Ни одного места, где догадка выдана за факт без пометки, не нашёл.
+
+## Находки
+
+### Low — не проговорены явные «нет» по четырём пунктам DoR
+
+ТЗ не содержит отдельных строк по i18n, модели данных/миграции и влиянию
+на touch, а также не выделяет UX отдельным разделом (§7.1 перечисляет их
+как обязательные разделы, §2.5 требует явного «нет», а не молчания).
+По существу для всех четырёх пунктов ответ очевиден и следует из
+остального текста: `repeat()` вместо `map()` не меняет DOM-атрибуты,
+которые видит пользователь и editor-код, значит нет новых строк i18n, нет
+миграции конфига (раздел «Откат» прямо говорит «данные, конфиг и
+публичные контракты не затронуты» — это неявно закрывает и модель
+данных), и нет причин, по которым touch-путь отличался бы от desktop —
+оба используют один и тот же рендер списка. Двусмысленности при
+реализации это не создаёт, поэтому не поднимаю до Medium — но перед
+`S5-ready` стоит дописать четыре короткие строки, чтобы чек-лист DoR
+(§2.5) закрывался явно, а не по умолчанию.
+
+**Решение ревьюера:** не блокирует, снимается с этой записью; если автор
+дополнит текст при следующей правке — хорошо, отдельного цикла ради этого
+не открываю.
+
+### Low — формулировка changelog называет только «открытие»
+
+Черновик записи («двери и окна больше не проигрывают чужую анимацию
+открытия» / «no longer replays a neighbouring door's opening animation»)
+называет только направление «открытие», хотя симптом в шапке issue прямо
+описывает и открытие, и закрытие («ни один датчик не срабатывал» — а
+дверь при переключении может «доехать» в обе стороны в зависимости от
+состояний контактов в двух пространствах). В проекте слово «opening»
+уже занято как существительное — название проёма (двери/окна/ворот)
+— в этом же черновике оно использовано ещё и как «действие открытия»,
+что при последующем переводе может прочитаться двусмысленно.
+Предлагаю на этапе реализации сменить формулировку на нейтральную к
+направлению, например «больше не проигрывают чужую анимацию» / «no
+longer replays a neighbouring door's animation» — без потери смысла и без
+двух значений одного слова в одном предложении.
+
+**Решение ревьюера:** не блокирует, косметика текста релиза; снимается с
+этой записью, право последнего слова у автора при коммите changelog.
+
+## Что проверено и корректно
+
+- Обязательные продуктовые разделы §7.1 (сценарий, что человек увидит до и
+  после) присутствуют и однозначны.
+- Контракт поведения, AC1–AC6 с доказательством и «чем краснеет» —
+  присутствуют, каждый проверяем, ни один не оставляет открытого вопроса.
+- Технические утверждения о коде (номера строк, наличие прецедента,
+  наличие функций, наличие валидации дублей) — все подтверждены чтением
+  репозитория на материале ревью, ни одно не оказалось догадкой.
+- Численные пороги AC6 пересчитаны независимо и совпадают с текстом ТЗ.
+- Риски названы предметно (цена ключей, golden-порядок узлов, идентичность
+  в живом пути, потеря настоящей анимации), у каждого есть привязка к AC
+  или гейту, который его ловит.
+- Откат — один revert, без миграции данных.
+- Release-артефакты — обе строки changelog заявлены, формат соответствует
+  реальной структуре файлов.
+- Допущения отделены от решений, каждое обосновано, ни владельцу не
+  задано вопросов, которые можно было решить технически — соответствует
+  правилу «владельцу только продуктовые вопросы».
+- Классификация трека (полный, а не light) обоснована явно названными
+  нарушенными критериями §5 — соответствует требованию «обычный трек без
+  названного критерия не обоснование».
+
+## Чего не проверял
+
+- Не запускал никакой код и ни один гейт — на этапе ревью ТЗ кода ещё нет,
+  проверять нечего; это не пропуск, а свойство стадии.
+- Не проверял golden-эталоны и реальный визуальный результат — они
+  появятся только с реализацией; риск назван в самом ТЗ и его проверка
+  на кандидате явно перенесена в CI автором.
+- Не проверял поведение на реальном Chromium (полусекундный переход,
+  фактическое переиспользование узла) — доверился измерениям аналитики
+  (S2-комментарий), они внутренне согласованы (числа `transform`,
+  `stroke-dashoffset`, факт переиспользования узла) и не противоречат
+  устройству кода, которое я прочитал (переходы в `plan.styles.ts`
+  существуют ровно на тех классах, о которых идёт речь).
+
+## Вердикт
+
+Зелёный. Обе находки — Low, ни одна не создаёт двусмысленности при
+реализации и не требует решения владельца; обе сняты этой записью, без
+возврата в `S3-spec`.
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `19e421b3cfe2` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `6fcc8b1b8af112dc50a57a7823e133142b1c8911`
+  ```
+  git log --all --format='%H %T' | grep 6fcc8b1b8af1
+  ```
+- Тело issue: `989e544d33b374bf0801e75697a88ddd7a979a3acf116ac70c5fb605ba79ffb4`
+- Вердикт конвейера: `green` · High 0