Files
houseplan-card/docs/reviews/SPEC-REVIEW-534-r2.md
2026-09-11 19:07:13 +00:00

24 KiB
Raw Permalink Blame History

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а на проёмы, либо обосновать и записать отказ как риск).


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

  • Ветка: dev, коммит e40d3f18d2fd — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: d42689f768439e92dac62df2f8055b5f2ed7d1db
    git log --all --format='%H %T' | grep d42689f76843
    
  • Тело issue: e9c78bcaf2429aff824dd80820d9b17d1a73c4b49c557df613d54a59d5590907
  • Вердикт конвейера: yellow · High 0