docs: review document for #534

Issue: #534
User-Visible: no

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

@@ -0,0 +1,246 @@
+# SPEC-REVIEW-534-r1
+
+## Скоуп
+
+Issue #534 (`bug`, `P1`, полный трек — критерий §5 «нет влияния на
+производительность» явно назван нарушенным в самой ТЗ). ТЗ живёт в теле
+issue под заголовком `## ТЗ` (решение владельца #517). Issue открыт, метка
+`S4-spec-review`, заход r1, бюджет циклов 0/4 не тронут.
+
+Предмет: линейка v1.74.0 добавила 7 длинных задач P95 на профилях
+`large-house` и `plan-snap` — регресс внесён #525 (перевод списка маркеров
+устройств на `repeat(devs, (d) => d.id, …)`, чтобы Lit не переиспользовал
+узел позиционно и не проигрывал переход на неслучившемся событии). На 200
+маркерах `repeat` строит и обходит карту ключей на каждый рендер, а при
+смене пространства ключи не пересекаются вовсе — диф оплачивается напрасно.
+Решение ТЗ: обернуть слой маркеров в `keyed(space.id, …)`
+(`lit/directives/keyed.js`) вокруг обычного `map()`, оставить проёмы на
+`repeat` (там элементов на порядок меньше).
+
+## Как проверялось
+
+Прочитаны `docs/SCOPE.md`, `PROCESS.md` (§1–§10, включая §7.1, §2.4, §2.10,
+§5, §8), тело issue #534 целиком и единственный комментарий (аналитика
+Codex, S2, с бисектом и измерением на пробе). Дополнительно, раз ТЗ
+опирается на конкретные строки, числа и поведение существующих механизмов,
+прочитан репозиторий на материале ревью (SHA `41ae5e99`, см. блок якорей):
+
+- `src/houseplan-card.ts:11787` (`repeat(devs, (d) => d.id, …)`, слой
+  маркеров) и `:12996` (`repeat(items, (o) => o.id, …)`, слой проёмов) —
+  оба подтверждены чтением, ссылки в ТЗ точны;
+- `node_modules/lit/directives/keyed.js` — директива существует в составе
+  уже установленной зависимости `lit`, ссылка К2 на неё не голословна;
+- `scripts/mutation-gate.mjs:7440` (`openings-rendered-without-keys`) и
+  `:7446` (`device-markers-rendered-without-keys`) — оба существующих
+  мутанта нацелены ровно на строки, которые правка меняет; find/replace
+  `device-markers-rendered-without-keys` перестанет применяться после
+  правки (текст `${repeat(devs, …)}` исчезнет) — ТЗ (AC5) корректно
+  предвидит необходимость его переанкеровки;
+- `demo/smoke_space_switch_transitions.mjs` — прочитан целиком. Подтверждено:
+  AC1/AC2 действительно можно закрыть существующим смоком без правок —
+  проверка `noMarkerNodeReusedForAnotherDevice` (строки 124–130) судит по
+  идентичности узла (`entry.node.isConnected && entry.node.dataset.id !==
+  entry.id`), а не по побочному признаку, как и требует AC2;
+- `src/styles/plan.styles.ts:531–542` — комментарий-ловушка #525
+  существует, называет `.device-shell-frame` (`box-shadow`) как причину
+  для маркеров; ТЗ в разделе «Затронутые файлы» верно указывает, что этот
+  комментарий нужно дополнить (К3);
+- `src/styles/devices.styles.ts:196-217` — проверена природа переходов на
+  маркере (детали см. в находке ниже);
+- `demo/fixtures/large-house.mjs:10-11` — `DEVICE_COUNT = 200`,
+  `OPENING_COUNT = 100` (на 3 этажа, т.е. ~33 на пространство) — цифры ТЗ о
+  200 маркерах точны; слово «единицы» про проёмы (К3) — преувеличение
+  (реально несколько десятков на этаж), но не влияет ни на один AC;
+- `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` — записи #524/#525 в разделе
+  «Unreleased»/«Не выпущено» существуют, формат черновика (AC7) совпадает
+  со стилем файла;
+- `test/device-marker-polish-contract.test.mjs`, `test/space-order.test.mjs`,
+  `test/device-face.test.mjs` — ни один не проверяет позиционную
+  стабильность маркера при изменении состава списка внутри одного
+  пространства (относится к находке ниже).
+
+Гейты не гонялись: на этапе ревью ТЗ продуктовый код не менялся, дешёвые
+гейты (`typecheck`/`test`/`build`) к этому этапу неприменимы (§2.4, §8).
+
+## Продуктовая рамка
+
+Сценарий и «что человек увидит» сформулированы одной фразой без терминов
+реализации, как требует §7.1: «каждое переключение дороже примерно на
+80 мс главного потока… карточка перестаёт отвечать на ввод чуть дольше,
+чем должна» → «переключение возвращается к цене, которая была до #525».
+Персона — домочадцы/гости на большом плане, `docs/SCOPE.md` называет View
+mode продуктом для этих персон; регресс блокирует именно это. Полный трек
+обоснован явно названным нарушенным критерием §5 («нет влияния на
+производительность»), а не молчаливым выбором — соответствует требованию
+2026-08-27. i18n/миграция/бэкенд и touch закрыты явным «ничего» /
+«тот же путь» — ТЗ учло Low-находку из SPEC-REVIEW-525-r1 (там эти пункты
+пришлось поднимать отдельно), здесь они на месте с самого начала.
+
+## Разбор AC
+
+- **AC1** — «остаётся зелёным без правок»: подтверждено чтением, существующий
+  смок уже проверяет то, что требуется, и правка К2/К3 его не задевает
+  структурно (тест не завязан на `repeat` изнутри, только на видимое
+  поведение DOM/CSS).
+- **AC2** — свидетель по идентичности узла уже существует (см. «Как
+  проверялось»), формулировка ТЗ точно описывает то, что тест делает.
+- **AC3** — численный ориентир (910 мс) подтверждён измерением автора на
+  `main` + патч (903,7 мс), не выдуман.
+- **AC4** — внешний гейт (Full Performance), критерий однозначен (девять
+  профилей, включая оба проблемных).
+- **AC5** — корректно предвидит необходимость правки мутанта
+  (подтверждено выше — старый find/replace перестанет матчиться).
+- **AC6/AC7** — golden и ревью кода, стандартные, без двусмысленности.
+
+Двусмысленности, которая заставила бы гадать «что считается прохождением»,
+в текстах AC не нашёл.
+
+## Находки
+
+### Medium — ключевое техническое допущение К2/«Принято предположительно» №2 неверно, и это открывает непроверенный регресс той же природы, что чинит вся задача
+
+**Утверждение ТЗ:** «Внутри одного пространства позиционное переиспользование
+маркеров безопасно: после #524 на маркере не осталось переходов, которые
+могла бы запустить смена значений.»
+
+**Проверено чтением и опровергнуто:** `src/styles/devices.styles.ts:213`
+несёт `.device-shell-frame { … transition: border-color .15s, opacity .2s; }`
+— оба перехода живы. #524 убрал только переход `box-shadow` (см. комментарий
+на той же строке: «no box-shadow here: cqw-sized, restarts on every
+container resize (#524)»), а не «переходы» вообще — допущение спутало
+конкретный фикс с общим случаем. `--device-shell-stroke` (цвет рамки)
+переопределяется по состоянию устройства (`devices.styles.ts:296,312,322,421`
+— alert-цвета) — то есть у двух РАЗНЫХ устройств в одном списке этот цвет
+типично отличается.
+
+**Почему это ломает контракт К2.** К2 предписывает конкретную реализацию:
+`keyed(space.id, …)` вокруг **обычного `map()`** для слоя маркеров — то есть
+внутри одного пространства элементы списка теряют пер-элементный ключ,
+который сейчас даёт `repeat(devs, (d) => d.id, …)`. Если состав `devs`
+меняется без смены `space.id` — а он меняется: `showGhosts` (`houseplan-
+card.ts:11307`, `this._mode === 'devices' && this._showAll`) переключает
+видимость части устройств в редакторе устройств, а само появление нового
+устройства в пространстве от лайв-синка конфигурации (J6, «new-device
+flag») тоже меняет состав `devs` без переключения пространства — Lit
+переиспользует DOM-узлы **по позиции**, не по устройству. Маркер, стоявший
+на позиции N, может получить данные другого устройства с другим
+`--device-shell-stroke`/`opacity`, и уже живой `transition: border-color
+.15s, opacity .2s` эти узла честно проиграет — ровно тот класс дефекта
+(«анимация события, которого не было»), который и #525, и эта самая задача
+называют главным риском (Риск 1 в ТЗ), только для другого триггера: не
+переключение пространства, а изменение состава списка внутри пространства.
+
+**Почему это не поймано ни одним AC.** `demo/smoke_space_switch_transitions.mjs`
+(AC1/AC2) тестирует только переключение между двумя пространствами; ни один
+существующий тест (`test/device-marker-polish-contract.test.mjs`,
+`test/space-order.test.mjs`, `test/device-face.test.mjs`) не проверяет
+переход, вызванный изменением состава списка маркеров внутри одного
+пространства. Риски ТЗ (1–3) эту ситуацию не называют.
+
+**Почему это в скоупе.** К2 — это ровно тот код, который правит задача;
+регрессия, если она проявится, будет внесена этим же коммитом, а не
+существует независимо от него сегодня (сейчас защиту даёт per-item ключ
+`repeat`, который К2 явно убирает).
+
+**Что нужно для DoR (любое из трёх, решает автор):**
+1. исправить допущение и сохранить пер-элементную защиту внутри
+   пространства — например, `keyed(space.id, () => repeat(devs, (d) =>
+   d.id, …))`: смена `space.id` по-прежнему выбрасывает поддерево целиком
+   без дорогого сравнения с 200 чужими ключами (устраняет весь измеренный
+   регресс), а `repeat` внутри защищает от переупорядочивания в пределах
+   одного пространства, как и сегодня; либо
+2. показать конкретным измерением/рассуждением, что состав `devs` в
+   реальных сценариях (не только в фикстуре бенчмарка) не меняется без
+   смены `space.id` так, чтобы порядок сдвигался — и явно принять остаточный
+   риск в разделе «Риски», а не в «Принятые предположения» как решённый
+   факт; либо
+3. добавить AC и свидетеля на сценарий «состав списка маркеров меняется
+   внутри пространства» по образцу уже существующего свидетеля #525/#528.
+
+**Решение ревьюера:** блокирует переход в `S5-ready` без правки одним из
+трёх способов выше — Medium в скоупе задачи, чинится в этом же issue
+(§2.4, §2.7 #202), отдельный issue не заводится.
+
+### Low — «их единицы» (К3) неточно описывает число проёмов
+
+К3 обосновывает, почему проёмы остаются на `repeat`: «их единицы, диф ничего
+не стоит». По фикстуре `large-house` (`DEVICE_COUNT=200`,
+`OPENING_COUNT=100` на 3 этажа) на пространство приходится порядка
+30 проёмов — не «единицы», хотя и на порядок меньше 200 маркеров, так что
+общий вывод (репит на проёмах дешевле) не меняется и ни один AC от этого не
+зависит.
+
+**Решение ревьюера:** не блокирует, снимается этой записью; можно поправить
+формулировку заодно с остальной правкой К2, отдельного цикла не открываю.
+
+## Что проверено и корректно
+
+- Обязательные продуктовые разделы §7.1 (сценарий, что человек увидит до и
+  после) присутствуют и однозначны.
+- Полный трек обоснован явно названным нарушенным критерием §5.
+- i18n, миграция/бэкенд, touch — явные «нет»/«тот же путь», а не молчание
+  (что в #525 пришлось поднимать Low-находкой — здесь уже учтено).
+- AC1–AC7 пронумерованы, у каждого указан способ доказательства; численные
+  пороги AC3 (910 мс) и AC4 (девять профилей) проверяемы и не выдуманы —
+  число AC3 сверено с измерением автора на пробе.
+- AC5 корректно предвидит поломку существующего мутанта новой формой кода
+  и требует его переанкеровки — не тихое исчезновение защиты.
+- Технические ссылки (номера строк, наличие директивы `keyed` в
+  зависимостях, наличие мутантов, содержимое свидетеля) подтверждены
+  чтением репозитория, ни одна не оказалась пустой.
+- Откат — один revert, без миграции данных, корректно.
+- Риски 1 и 3 предметны и привязаны к AC/гейтам, которые их ловят.
+- Release-артефакты (changelog RU+EN, `User-Visible: yes`) названы верно.
+
+## Чего не проверял
+
+- Не запускал никакого кода и ни одного гейта — на этапе ревью ТЗ
+  продуктовый код не менялся, дешёвые гейты (`typecheck`/`test`/`build`)
+  неприменимы к этой стадии (§2.4, §8).
+- Не проверял golden-эталоны и реальные тайминги на Chromium/раннере —
+  появятся только с реализацией; численные ориентиры ТЗ (903,7 мс, 910 мс)
+  доверены измерению автора (внутренне согласованы с бисектом в
+  комментарии) и будут окончательно подтверждены Full Performance по AC4.
+- Не проверял частоту реальных сценариев `showGhosts`/лайв-добавления
+  устройства «в проде» количественно — риск в находке выше обоснован кодом
+  (условие фильтра, наличие живых CSS-переходов, наличие state-зависимого
+  цвета рамки), а не измерением частоты; для итогового решения (пункт 1–3
+  находки) этого достаточно, эмпирическая частота не меняет того, что
+  сценарий кодом не исключён и не покрыт тестом.
+
+## Вердикт
+
+Жёлтый. Один Medium в скоупе задачи (допущение К2 о безопасности
+внутрипространственного позиционного переиспользования маркеров неверно и
+открывает непроверенный регресс той же природы, которую чинит вся задача);
+High нет. Возврат автору на правку одним из трёх названных способов;
+одна Low-находка о формулировке снята этой записью.
+
+---
+
+<!-- material-anchors: подготовлено вручную ревьюером, конвейер §416 не запускался в этом окружении -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `41ae5e99868db27e3bfde9a40426c0a636c374bd`.
+- Дерево материала: `e87f2fb0f5a99184cd9795a5e59155b96b3ac2f6`
+  ```
+  git log --all --format='%H %T' | grep e87f2fb0f5a9
+  ```
+- Тело issue (нормализовано `gh issue view 534 --json body -q '.body'`):
+  `sha256:98e12a0f7bec0c050802b1caee9c139db3bbc7b7c1169744fb1ccd567e35babd`
+- Вердикт: `yellow` · High 0 · Medium 1 (в скоупе)
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `41ae5e99868d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `e87f2fb0f5a99184cd9795a5e59155b96b3ac2f6`
+  ```
+  git log --all --format='%H %T' | grep e87f2fb0f5a9
+  ```
+- Тело issue: `3ba57688eafe3310c9e3d0157758ea519c3b89d980958e504d2c2b54b3a43cb3`
+- Вердикт конвейера: `yellow` · High 0