docs: review document for #534

Issue: #534
User-Visible: no

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

@@ -0,0 +1,259 @@
+# SPEC-REVIEW-534-r2
+
+## Скоуп
+
+Issue #534, этап `spec` (ревью ТЗ, PROCESS.md §2.4), заход r2, до этого
+раунда израсходован 1/4 блокирующих цикла (r1 — жёлтый, Medium в скоупе).
+ТЗ живёт в теле issue (решение владельца #517).
+
+Материал этапа spec — тело issue, не код. Между r1 и r2 продуктовый код не
+менялся: `git diff 41ae5e99..HEAD -- . ':!docs/reviews'` пуст, рабочая
+копия на `e40d3f18` содержит только коммит с документом
+`SPEC-REVIEW-534-r1.md`. Единственная дельта раунда — правка тела issue
+владельцем (комментарий-вердикт r1 и последующий комментарий Matysh от
+2026-09-11T18:58:53Z). Хэш нормализованного тела изменился
+(`sha256:18961b89…` в r2 против `sha256:98e12a0f…`, зафиксированного в
+материале r1) — дельта подтверждена, а не предполагается.
+
+## Как проверялось
+
+1. Прочитан вердикт r1 (комментарий `IC_…`, 2026-09-11T18:53:19Z) и
+   документ `docs/reviews/SPEC-REVIEW-534-r1.md` на коммите `41ae5e99`
+   (дерево материала `e87f2fb0f5a9…`, зафиксировано в блоке «Материал
+   раунда» того документа).
+2. Дельта тела issue объявлена (см. «Скоуп») и разобрана построчно:
+   старые формулировки К2/К3/«Принятые предположения»/AC5/«Риски»
+   реконструированы по цитатам r1-документа и сопоставлены с текущим
+   текстом (`gh issue view 534 --json body -q '.body'`, сохранено в
+   `/tmp/current_body.md`).
+3. Технические ссылки новой редакции проверены чтением репозитория на
+   материале ревью (SHA `e40d3f18`, код не менялся с r1):
+   - `src/houseplan-card.ts:11787` (`repeat(devs, (d) => d.id, …)`) и
+     `:12996` (`repeat(items, (o) => o.id, …)`) — оба списка сегодня без
+     `keyed`, ссылки К2 «оба списка… `keyed(…, repeat(…))`» описывают
+     код, которого ещё нет, корректно как целевое состояние;
+   - `scripts/mutation-gate.mjs:7434-7456` — оба существующих мутанта
+     (`openings-rendered-without-keys`, `device-markers-rendered-without-keys`)
+     сделаны точным `find`/`replace` по буквальной строке с `repeat(…)`;
+     `applyPatches` (`mutation-gate.mjs:8910-8916`) бросает исключение,
+     если якорь не найден ровно один раз — промах анкера не тихий, гейт
+     красится сборкой мутанта, а не пропускает проверку молча;
+   - `src/styles/plan.styles.ts:531-549` — комментарий-ловушка #525 уже
+     сегодня объясняет `.op-leaf { transition: transform 0.6s ease }` и
+     `.op-arc { transition: stroke-dashoffset 0.6s ease }` как причину,
+     по которой список проёмов **уже с #525** рендерится через `repeat`,
+     наравне с маркерами устройств — тем же абзацем, который К2/К3 этого
+     раунда обязывают дополнить;
+   - `src/houseplan-card.ts:8226-8260` (`_openingsR`) — при
+     `!resolution.resolved` в режиме `plan` добавляется orphan-запись
+     (`:8242-8243`), а вне `plan` эта запись выпадает из списка
+     (`:8244`, `flatMap` возвращает `[]`) — состав `items` меняется от
+     переключения режима Plan/View **без смены пространства**, длина
+     массива и позиции последующих элементов сдвигаются;
+   - `src/houseplan-card.ts:12987-12996` (`_renderOpenings`) — рендерит
+     `items` из `_openingsR` тем же `repeat`, что и обсуждается в К2/К3.
+
+Гейты не гонялись: этап spec, продуктовый код не менялся (§2.4, §8) —
+как и в r1.
+
+## Закрытие раунда r1
+
+| находка r1 | чем закрыта | где это видно |
+|---|---|---|
+| Medium — допущение К2 «внутри пространства позиционное переиспользование маркеров безопасно» опровергнуто чтением (`devices.styles.ts:213`, живые переходы) | Формулировка К2 переписана: оба списка теперь `keyed(space.id, repeat(items, id, …))` — внутренний `repeat` **сохраняется**, а не убирается. Добавлен явный контракт К3 «почему пер-элементный ключ обязан остаться» с тем же техническим обоснованием, которое привёл ревьюер (переходы `.device-shell-frame`, `--device-shell-stroke` по состоянию). Допущение №2 переписано из утверждения о безопасности в утверждение о цене (диф `repeat` идёт по быстрому пути, измерено 905,9 vs 903,7 мс) — корректность больше не зависит от истинности этого допущения, потому что `repeat` защищает по ключу независимо от того, совпадают ли ключи по порядку. Добавлен новый AC2а — свидетель именно на сценарий «состав списка маркеров меняется без смены пространства». Риск 2 переписан под новый сценарий и ссылается на К3/AC2а/мутант. | Тело issue: К2 (строка 48), К3 (50), AC2а (60), Риск 2 (90), допущение №2 (96). Выполнено способом 1 из трёх предложенных ревьюером r1 (сохранить пер-элементную защиту), усилено способами 2 (допущение переформулировано честно) и 3 (добавлен AC-свидетель) — все три сразу, не одно на выбор. |
+| Low — «проёмов единицы» (К3 в старой нумерации) неточно | Число исправлено на «порядка тридцати на этаж» | Допущение №3 (строка 97): «Проёмов на `large-house` порядка тридцати на этаж, не «единицы»» |
+
+Обе находки r1 закрыты текстом ТЗ, а не заявлением в комментарии — проверено построчным чтением текущего тела issue, а не принято на слово.
+
+## Унаследовано из r1
+
+Без повторной проверки принято из `docs/reviews/SPEC-REVIEW-534-r1.md`
+(документ и SHA `41ae5e99868db27e3bfde9a40426c0a636c374bd`, дерево
+материала `e87f2fb0f5a9…`), потому что дельта r2 этих участков текста и
+кода не касается:
+
+- Продуктовая рамка §7.1 (сценарий, «что человек увидит» до/после,
+  выбор полного трека по явно названному критерию §5, i18n/миграция/
+  бэкенд/touch как явные «нет» вместо молчания) — текст сценария в r2 не
+  менялся;
+- AC1 (существующий смок остаётся зелёным без правок) — смок и его
+  проверки не менялись, вывод r1 держится;
+- AC3/AC4 (численный ориентир 910 мс и полный перф-гейт из девяти
+  профилей) — числа и формулировка не менялись между раундами;
+- AC6 (golden) и базовая часть AC7 (записи в оба changelog,
+  `User-Visible: yes`) — не менялись;
+- Существование директивы `keyed` в зависимости `lit`, точность ссылок
+  на номера строк `houseplan-card.ts:11787/12996` и на существующие
+  мутанты в `scripts/mutation-gate.mjs` — репозиторий на этих участках
+  не менялся с r1, перепроверка кода не требовалась (я всё же прочитал
+  эти строки заново в рамках проверки новой находки ниже, они совпадают
+  с тем, что зафиксировал r1).
+
+## Разбор AC (только то, чего касается дельта)
+
+- **AC2а** (новая) — формулировка однозначна, способ доказательства
+  назван (`smoke`, вероятно расширение того же
+  `demo/smoke_space_switch_transitions.mjs`, который назван в
+  «Затронутые файлы»), критерий прохождения проверяем по идентичности
+  узла, как и AC2. Двусмысленности нет. **Но её область — только
+  маркеры устройств**, см. находку ниже.
+- **AC5** — обновлена под новую форму кода (два мутанта: снимающий
+  внешний `keyed` и снимающий внутренний `repeat`). Явно называет
+  переанкеровку `device-markers-rendered-without-keys`. **Не называет**
+  `openings-rendered-without-keys`, хотя его якорь (`mutation-
+  gate.mjs:7441`, буквальная строка с `repeat(items, (o) => o.id, …)`)
+  тоже перестанет совпадать один-в-один после того, как К2 обернёт
+  список проёмов в `keyed(…)` — та же механика, что и для маркеров.
+  Практически это не тихий пропуск (`applyPatches` бросает исключение
+  на несовпавшем якоре, PROCESS §8/находка выше), но ТЗ не проговаривает
+  требование явно, оставляя его исполнителю угадывать.
+- **AC7** — ссылка на «комментарий К3» согласована с новой нумерацией
+  контракта (проверено: К3 в новом тексте — это статья про
+  пер-элементный ключ, а не про число проёмов, как в r1). «Затронутые
+  файлы» требуют дополнить комментарий-ловушку **в обоих** файлах стилей
+  (`plan.styles.ts` и `devices.styles.ts`) — см. находку ниже про то, что
+  сам контракт К3 обосновывает это только для одного из двух.
+
+## Находки
+
+### Medium — К2 распространяет `keyed+repeat` на список проёмов «для единообразия», но обоснование (К3) и свидетель (AC2а) написаны только для маркеров устройств
+
+**Что меняет К2.** «Оба списка — маркеры устройств и проёмы — рендерятся
+как `keyed(<идентификатор пространства>, repeat(items, (item) =>
+item.id, …))». Это новое требование к списку проёмов: до этого раунда
+(и в r1, и сегодня в коде) проёмы рендерятся голым
+`repeat(items, (o) => o.id, …)` без внешнего `keyed`. Причина
+унификации — допущение №3: «проёмов… не заслуживают отдельного режима,
+поэтому оба списка приводятся к одной форме» — обоснование
+единообразием, а не функциональной необходимостью.
+
+**К3 и AC2а покрывают только один из двух списков.** К3 («почему
+пер-элементный ключ обязан остаться») целиком про маркеры: «Состав
+списка маркеров меняется без смены пространства: призраки в редакторе
+устройств (`showGhosts`) и появление устройства по живому синку
+конфига… на `.device-shell-frame`… живы переходы… Убрать внутренний
+ключ — значит вернуть тот же класс дефекта». AC2а — тоже буквально «свои
+детель К3… **состав списка маркеров** меняется… ни один существующий
+**маркер** не меняет свой DOM-узел». Ни К3, ни AC2а не упоминают проёмы.
+
+**Тот же риск для проёмов подтверждён чтением, а не гипотетичен.**
+
+1. Проёмы уже сегодня несут живые переходы того же типа, что маркеры, и
+   это задокументировано прямо в комментарии-ловушке #525
+   (`src/styles/plan.styles.ts:531-542`, тот самый абзац, который К2/К3
+   в этом раунде требуют дополнить): `.op-leaf { transition: transform
+   0.6s ease }`, `.op-arc { transition: stroke-dashoffset 0.6s ease }`.
+   Комментарий прямо называет обе причины рядом: «the openings and the
+   device markers… with this issue [#525]» — то есть исходная задача
+   #525 уже трактовала оба списка как один класс риска, и текущее ТЗ
+   (К2) следует этой логике для перфоманса, но не переносит её в
+   контракт корректности К3.
+2. У проёмов есть собственный, отличный от маркеров, но настоящий
+   триггер изменения состава списка **внутри одного пространства**:
+   `_openingsR` (`src/houseplan-card.ts:8226-8260`) добавляет
+   orphan-запись, только когда `this._mode === 'plan'`
+   (`:8241-8243`), и убирает её, когда режим не `plan`
+   (`:8244`, `flatMap` возвращает `[]` для нерешённого хоста). То есть
+   переключение между режимами Plan/View **в одном и том же
+   пространстве** меняет длину и, соответственно, позиции элементов
+   `items`, который затем рендерится тем же `repeat` в
+   `_renderOpenings` (`:12987-12996`) — прямой аналог `showGhosts` для
+   маркеров, только с другим триггером.
+
+**Почему это Medium, а не Low.** Если `repeat` для проёмов оставлен «по
+инерции единообразия» без записанной причины и без свидетеля, следующий
+читатель — ровно тот сценарий, которого боится сама К3 («иначе следующий
+читатель уберёт «лишнюю» обёртку») — с равным основанием сочтёт его
+избыточным именно для проёмов, потому что ни контракт, ни тест этого не
+запрещают. Регрессия при этом ловится AC1/AC2 только частично: они
+проверяют переключение *между* пространствами, а не смену режима
+*внутри* одного. Находка того же класса и в той же строке контракта
+(К2/К3), которую r1 уже поднимал для маркеров — только в r2 асимметрия
+переехала на второй список, который сама эта правка впервые
+подвела под общий режим.
+
+**Что нужно для DoR (решает автор):**
+1. распространить К3 и AC2а на оба списка одним общим утверждением
+   (единый триггер формулировки — «состав списка меняется без смены
+   пространства» верно и для orphan-переключения Plan/View) и завести
+   AC2б либо расширить AC2а на проёмы конкретным сценарием (например,
+   вход/выход из режима Plan при наличии проёма с нерешённым хостом); либо
+2. явно обосновать, почему для проёмов регресс невозможен либо не имеет
+   значения (например, если `.op-leaf`/`.op-arc` не проигрывают
+   транзишн в застывшем Plan-виде, или если orphan-переключение всегда
+   сопровождается прочим ре-рендером, который сбрасывает состояние) — и
+   принять это как записанный риск, а не молчаливое умолчание;
+3. как минимум явно назвать в AC5 переанкеровку
+   `openings-rendered-without-keys` наравне с `device-markers-
+   rendered-without-keys` — это не закрывает риск (1)/(2), но убирает
+   как минимум неполноту самого AC5 относительно кода, который сам же
+   К2 меняет.
+
+**Решение ревьюера:** блокирует переход в `S5-ready` без правки одним из
+способов 1 или 2 выше (способ 3 сам по себе недостаточен — устраняет
+только неполноту AC5, но не закрывает риск отсутствия контракта/AC для
+проёмов). Medium в скоупе задачи — К2 в этом самом раунде впервые
+распространила изменение на список проёмов, поэтому решение относится к
+предмету этой правки, а не к соседнему поведению; отдельный issue не
+заводится (§2.4, §2.7, #202).
+
+## Что проверено и корректно
+
+- Обе находки r1 (Medium и Low) закрыты текстом ТЗ, проверено построчным
+  чтением, а не заявлением автора — таблица «Закрытие раунда r1» выше.
+- Допущение №2 больше не путает конкретный фикс (#524, `box-shadow`) с
+  общим случаем: теперь это утверждение о производительности, а не о
+  безопасности, и корректность К3 от его истинности не зависит — `repeat`
+  защищает по ключу независимо от совпадения порядка.
+- AC2а — новый, недвусмысленный AC с указанным способом доказательства.
+- Нумерация контракта (К1–К5) внутренне согласована: перекрёстные ссылки
+  AC7→К3, AC5→AC2а, Риск 2→К3/AC2а/АС5 указывают на верные, актуальные
+  пункты нового текста.
+- Технические ссылки К2 (директива `keyed`, номера строк 11787/12996,
+  существующие мутанты) точны — перепроверены на неизменном с r1 коде.
+- Число проёмов («порядка тридцати на этаж») теперь соответствует
+  фикстуре `large-house` (было подтверждено ещё в r1).
+
+## Чего не проверял
+
+- Не запускал код и гейты — этап spec, продуктовый код не менялся с r1
+  (§2.4, §8), как и в прошлом раунде.
+- Не проверял, действительно ли `.op-leaf`/`.op-arc` физически
+  проигрывают переход в статичном Plan-виде при переключении режима
+  (например, если элемент в момент переключения не виден или
+  transition подавлен другим правилом) — находка выше опирается на
+  чтение CSS и `_openingsR`, а не на измерение в браузере; это ровно тот
+  вопрос, который DoR-требование (пункт 2 находки) просит закрыть
+  автору чтением/измерением, если он выберет этот путь вместо пункта 1.
+- Не проверял частоту реального использования Plan/View-переключения с
+  ровно одним orphan-проёмом «в проде» — как и в r1 по аналогичному
+  вопросу для маркеров, для решения находки (правка контракта либо
+  обоснованный отказ) это не требуется: сценарий не исключён кодом и не
+  покрыт тестом, этого достаточно, чтобы вернуть на уточнение.
+
+## Вердикт
+
+Жёлтый. Один Medium в скоупе задачи: К2 в этом раунде распространила
+`keyed(space.id, repeat(…))` на список проёмов «для единообразия», но
+контракт К3 и его свидетель AC2а обосновывают и проверяют сохранение
+внутреннего ключа только для маркеров устройств, хотя тот же класс риска
+у проёмов подтверждён чтением (комментарий-ловушка #525 уже описывает их
+как один класс, а `_openingsR` меняет состав списка при переключении
+Plan/View внутри одного пространства). High нет. Обе находки r1 закрыты
+текстом, проверено построчно. Возврат автору на правку одним из двух
+названных способов (расширить К3/AC2а на проёмы, либо обосновать и
+записать отказ как риск).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `e40d3f18d2fd` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `d42689f768439e92dac62df2f8055b5f2ed7d1db`
+  ```
+  git log --all --format='%H %T' | grep d42689f76843
+  ```
+- Тело issue: `e9c78bcaf2429aff824dd80820d9b17d1a73c4b49c557df613d54a59d5590907`
+- Вердикт конвейера: `yellow` · High 0