From a9a6d37bbafb66d245a1e06d2ae314a05b8c41d9 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:14:21 +0000 Subject: [PATCH] docs: review document for #534 Issue: #534 User-Visible: no --- docs/reviews/SPEC-REVIEW-534-r3.md | 218 +++++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-534-r3.md diff --git a/docs/reviews/SPEC-REVIEW-534-r3.md b/docs/reviews/SPEC-REVIEW-534-r3.md new file mode 100644 index 00000000..612b1da4 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-534-r3.md @@ -0,0 +1,218 @@ +# SPEC-REVIEW-534-r3 + +## Скоуп + +Issue #534 (`bug`, `P1`, полный трек — критерий §5 «нет влияния на +производительность» явно назван нарушенным в самой ТЗ). Этап `spec` +(ревью ТЗ, PROCESS.md §2.4), заход **r3**, до этого раунда израсходовано +**2/4** блокирующих циклов (r1 — жёлтый/Medium, r2 — жёлтый/Medium; оба в +скоупе, оба вернули задачу на правку). Лимит цикла для полного трека — 4 +(§2.4), не 2 (2 — это лимит именно лёгкого трека). + +Предмет спецификации не изменился с r1: регресс #525 (перевод списков +маркеров и проёмов на `repeat(items, (x) => x.id, …)`) добавил лишний диф +по непересекающимся ключам при смене пространства на `large-house`/`plan- +snap`; решение — обернуть оба списка во внешний `keyed(space.id, …)`, +сохранив внутренний `repeat` как защиту от переупорядочивания внутри +пространства. + +## Как проверялось (объём по дельте, §2.9/§2.10) + +Материал предыдущего раунда объявлен в блоке «Материал раунда» документа +`docs/reviews/SPEC-REVIEW-534-r2.md`: коммит `e40d3f18d2fd`, дерево +`d42689f768439e92dac62df2f8055b5f2ed7d1db`, тело issue +`sha256:e9c78bcaf2429aff824dd80820d9b17d1a73c4b49c557df613d54a59d5590907`. + +1. Текущее тело issue снято `gh issue view 534 --json body -q '.body'` → + `/tmp/current_body.md`, посчитан хеш: + `sha256:18c0ed92ea4d832fcb8d3d4aeedf02d05af2e4c174d0f7e0b099d4b14e31ff8a` + — отличается от хеша r2, дельта в теле issue подтверждена, а не + предполагается. +2. Дельта — правка тела issue владельцем, описанная в комментарии + «Ответ на ревью ТЗ r2 — заход 3» (2026-09-11T19:09:03Z): К3 расширен на + оба списка (маркеры и проёмы), AC2а расширен второй половиной для + проёмов, AC5 явно называет переанкеровку обоих мутантов, Риск 2 + называет оба триггера. Сверено построчно с текущим телом issue — все + перечисленные правки на месте (К3 — строки 50–53 тела, AC2а — 65–69, + AC5 — 72, Риск 2 — 99). +3. Продуктовый код на материале ревью (рабочая копия на `1fb84755`, тем + же SHA, что был у r1/r2 — код с r1 не менялся, это ревью ТЗ) прочитан + заново там, где дельта делает новые фактические утверждения: + - `src/houseplan-card.ts:8226-8258` (`_openingsR`) — подтверждено + построчно: `this._mode === 'plan' ? [{...fallback, orphanReason}] : + []`. К3/AC2а формулируют это как «Plan ↔ View»; в коде условие шире + — `plan` против любого из **трёх** остальных режимов (`view`, + `devices`, `decor`, `houseplan-card.ts:990`). «Plan ↔ View» — + корректный частный случай общего триггера, демонстрирует и + проверяет тот же механизм (изменение состава `items` без смены + `space.id`); не искажает ни один AC и не расширяет риск — см. Low + ниже; + - `src/styles/plan.styles.ts:531-542` — комментарий-ловушка #525 + прочитан целиком: один блок на оба списка, сегодня называет + `.op-leaf`/`.op-arc` (проёмы) и `.device-shell-frame, box-shadow` + (маркеры, устаревшая причина — box-shadow снят #524). К3/АС7 + корректно требуют его переписать под новые триггеры и актуальные + переходы; + - `src/styles/devices.styles.ts:213` — `.device-shell-frame { + transition: border-color .15s, opacity .2s; }` — переходы живы, + как и утверждает К3 (маркеры); + - `src/houseplan-card.ts:11787` (`repeat(devs, …)`) и `:12996` + (`repeat(items, …)`) — оба списка сегодня без внешнего `keyed`, + совпадает с описанием целевого состояния К2; + - `node_modules/lit/directives/keyed.js` — директива по-прежнему + доступна в установленной зависимости. + +Гейты не гонялись — этап spec, продуктовый код не менялся с r1 (§2.4, +§8), как и в r1/r2. + +## Закрытие раунда r2 + +| находка r2 | чем закрыта | где это видно в текущем теле issue | +|---|---|---| +| Medium — К2 распространила `keyed(space.id, repeat(…))` на проёмы «для единообразия», но контракт К3 и свидетель AC2а обосновывали и проверяли сохранение внутреннего ключа только для маркеров устройств; тот же риск для проёмов подтверждён чтением (`_openingsR`, комментарий-ловушка #525) | К3 переписан заголовком «Почему пер-элементный ключ обязан остаться — **в обоих списках**» и содержит отдельный абзац «Проёмы» с тем же обоснованием, что привёл ревьюер: `_openingsR` включает orphan-запись только в режиме Plan, `.op-leaf`/`.op-arc` несут переходы `transform 0.6s`/`stroke-dashoffset`. AC2а расширен второй половиной сценария «проёмы: переключение Plan ↔ View при наличии проёма с нерешённым хостом». AC5 явно называет переанкеровку **обоих** существующих мутантов — `device-markers-rendered-without-keys` **и** `openings-rendered-without-keys`. Риск 2 переписан: «Триггеров два и они разные: для маркеров… для проёмов…», ссылается на «обе половины AC2а». | Тело issue: К3 (строки 50–53), AC2а (65–69), AC5 (72), Риск 2 (99). Выполнены сразу способ 1 (расширить К3/AC2а) и способ 3 (явно назвать оба мутанта в AC5) из трёх, предложенных r2 — не выбран один вариант, закрыты оба одновременно. | + +Находка закрыта текстом ТЗ, проверено построчным чтением текущего тела +issue, а не заявлением автора в комментарии. + +## Унаследовано из r1 и r2 + +Без повторной проверки принято из `docs/reviews/SPEC-REVIEW-534-r1.md` +(SHA `41ae5e99868db27e3bfde9a40426c0a636c374bd`) и +`docs/reviews/SPEC-REVIEW-534-r2.md` (SHA `e40d3f18d2fdb1bc5f2f9f6dec969c490cddde1f`), +поскольку дельта r3 этих участков текста не касается: + +- Продуктовая рамка §7.1 (сценарий, «что человек увидит» до/после, выбор + полного трека по явно названному критерию §5, i18n/миграция/бэкенд/ + touch как явные «нет») — не менялась с r1. +- К1 («узел не переживает смену пространства») и К4/К5 (бюджет, вне + объёма) — не менялись с r1. +- AC1 (существующий смок остаётся зелёным без правок), AC3/AC4 + (численный ориентир 910 мс, полный перф-гейт на 9 профилях), AC6 + (golden), базовая часть AC7 (записи в оба changelog, `User-Visible: + yes`) — числа и формулировки не менялись между r1/r2/r3. +- Существование директивы `keyed` в зависимости `lit`, точность ссылок + на номера строк `houseplan-card.ts:11787/12996` и на существующие + мутанты в `scripts/mutation-gate.mjs:7434-7456` — репозиторий на этих + участках не менялся с r1 (перепроверено заново в этом раунде наравне с + новыми утверждениями К3, см. «Как проверялось» — совпадает с r1/r2). +- Допущение №1 (`keyed` гарантированно выбрасывает поддерево) и №2 + (внутренний `repeat` идёт по быстрому пути внутри пространства, + измерено 905,9 vs 903,7 мс) — не менялись с r2. + +## Разбор AC (только то, чего касается дельта r3) + +- **К3** — теперь однозначно покрывает оба списка отдельными абзацами с + названным триггером, named CSS-классом и переходом для каждого. + Двусмысленности, кто из двух списков защищён, а кто нет, больше нет. +- **AC2а** — обе половины (маркеры/проёмы) сформулированы параллельно, + способ доказательства общий (`smoke`), критерий прохождения проверяем + по идентичности узла и отсутствию анимации для каждого списка отдельно. + Единственное найденное уточнение — сценарий проёмов назван «Plan ↔ + View», хотя код переключает orphan-запись по условию «`plan` vs любой + из трёх остальных режимов»; см. Low ниже, не блокирует. +- **AC5** — оба существующих мутанта названы по имени, требование + переанкеровки для обоих явное, не подразумеваемое. Неполнота, + найденная в r2 (`openings-rendered-without-keys` не назван), закрыта. +- **AC7** — «комментарий К3 стоит рядом с анимациями» (множественное + число) теперь согласовано с К3, описывающим анимации для обоих + списков; «Затронутые файлы» верно называют оба файла стилей + (`plan.styles.ts`, где сегодня живёт единственный комментарий-ловушка + на оба списка, и `devices.styles.ts`, рядом с самим + `.device-shell-frame`) — технический вопрос «один комментарий или два» + ТЗ не обязано решать (не наблюдаемо пользователем, решает автор по + §7.1 «всё, чего пользователь не наблюдает»). + +Остальные AC (AC1, AC3, AC4, AC6) дельта не задевает — наследуются из +r1/r2 без повторной проверки. + +## Находки + +Блокирующих (High/Medium) находок нет. + +### Low — АС2а/К3 называют триггер проёмов «Plan ↔ View», код переключает шире + +`_openingsR` (`src/houseplan-card.ts:8241-8244`) убирает orphan-запись +при **любом** режиме, отличном от `plan` (`view`, `devices`, `decor`), не +только при `view`. «Plan ↔ View» — корректный и достаточный частный +случай для смока: механизм (изменение состава `items` без смены +`space.id`) один и тот же независимо от того, куда именно переключились +из `plan`. Ни один AC от более узкой формулировки не становится +неверным или непроверяемым — переключение именно в `view` даёт тот же +эффект, что и в `devices`/`decor`. + +**Решение ревьюера:** не блокирует. Формулировка достаточна для +однозначного и проверяемого AC; если исполнитель захочет расширить смок +на все три режима — это усиление сверх ТЗ, не требование ревью. + +## Что проверено и корректно + +- Единственная Medium-находка r2 закрыта текстом ТЗ, проверено построчным + чтением текущего тела issue (не заявлением автора) — таблица «Закрытие + раунда r2» выше. +- Технические факты нового текста К3 (переходы `.op-leaf`/`.op-arc`, + условие `_openingsR` на `this._mode === 'plan'`, живые переходы + `.device-shell-frame`) подтверждены прямым чтением исходников на + материале ревью, а не приняты на слово. +- AC2а, AC5, Риск 2, «Затронутые файлы» внутренне согласованы между + собой и с новым текстом К3 — перекрёстные ссылки указывают на верные + актуальные пункты. +- Нумерация контракта (К1–К5) и AC (включая AC2а) не разъехалась между + раундами. +- Бюджет циклов не превышен: это второй жёлтый цикл из четырёх + возможных для полного трека (§2.4), лимит 4, использовано было бы 2 — + но этот раунд, если зелёный, бюджет не тратит (§4, #227). + +## Чего не проверял + +- Не запускал код и гейты — этап spec, продуктовый код не менялся с r1 + (§2.4, §8). +- Не проверял в браузере, действительно ли переключение `plan → devices` + или `plan → decor` (а не только `plan → view`) физически воспроизводит + тот же дефект — риск в Low-находке выше опирается на чтение условия + `_openingsR`, а не на измерение; для вывода «AC2а достаточен» это не + требуется, поскольку механизм (состав `items` меняется без смены + `space.id`) один и тот же для всех трёх направлений. +- Не проверял частоту реальных сценариев `showGhosts`/лайв-синка/ + Plan-Х-переключения «в проде» количественно — не требуется для этого + раунда, вопрос был закрыт ещё в r1/r2 (сценарий кодом не исключён и не + покрыт тестом — этого достаточно, чтобы Medium r2 был обоснованным, а + его закрытие текстом ТЗ — достаточным). + +## Вердикт + +**Зелёный.** Обе предыдущие Medium-находки (r1 — допущение о безопасности +внутрипространственного переиспользования маркеров, r2 — асимметрия +контракта К3/AC2а между маркерами и проёмами) закрыты текстом ТЗ, +проверено построчным чтением, а не заявлением автора. High нет. Одна +Low-находка (формулировка триггера проёмов у`́же реального условия кода) +не блокирует и не требует правки. Готово к разработке. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `1fb84755c22ba1fe990a96936399b579f0835a20` (рабочая + копия на этом SHA; код с r1 не менялся, это ревью ТЗ). +- Тело issue (нормализовано `gh issue view 534 --json body -q '.body'`): + `sha256:18c0ed92ea4d832fcb8d3d4aeedf02d05af2e4c174d0f7e0b099d4b14e31ff8a` +- Предыдущий материал: r1 — коммит `41ae5e99868db27e3bfde9a40426c0a636c374bd`, + тело `sha256:98e12a0f7bec0c050802b1caee9c139db3bbc7b7c1169744fb1ccd567e35babd`; + r2 — коммит `e40d3f18d2fdb1bc5f2f9f6dec969c490cddde1f`, + тело `sha256:e9c78bcaf2429aff824dd80820d9b17d1a73c4b49c557df613d54a59d5590907`. +- Вердикт: `green` · High 0 · Medium 0 · Low 1 (не блокирует) + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `1fb84755c22b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `b5414fcc6281268a81bae04b9905f7199afa6242` + ``` + git log --all --format='%H %T' | grep b5414fcc6281 + ``` +- Тело issue: `9d969d777f9787e5a50d02b2e675ad1448e945a27b47899d2d37c445a612f30e` +- Вердикт конвейера: `green` · High 0