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

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.

---

Материал раунда

• Ветка: dev, коммит 19e421b3cfe2 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 6fcc8b1b8af112dc50a57a7823e133142b1c8911

  git log --all --format='%H %T' | grep 6fcc8b1b8af1
  

• Тело issue: 989e544d33b374bf0801e75697a88ddd7a979a3acf116ac70c5fb605ba79ffb4
• Вердикт конвейера: green · High 0