From 95175a761613cfe1db355981bcb3f48b5344be5c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 01:20:43 +0000 Subject: [PATCH] docs: review document for #525 Issue: #525 User-Visible: no --- docs/reviews/SPEC-REVIEW-525-r1.md | 235 +++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-525-r1.md diff --git a/docs/reviews/SPEC-REVIEW-525-r1.md b/docs/reviews/SPEC-REVIEW-525-r1.md new file mode 100644 index 00000000..e258b951 --- /dev/null +++ b/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`. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `19e421b3cfe2` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `6fcc8b1b8af112dc50a57a7823e133142b1c8911` + ``` + git log --all --format='%H %T' | grep 6fcc8b1b8af1 + ``` +- Тело issue: `989e544d33b374bf0801e75697a88ddd7a979a3acf116ac70c5fb605ba79ffb4` +- Вердикт конвейера: `green` · High 0