mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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-находка (формулировка триггера проёмов у`́же реального условия кода)
|
||||
не блокирует и не требует правки. Готово к разработке.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: подготовлено вручную ревьюером -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `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 (не блокирует)
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `1fb84755c22b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `b5414fcc6281268a81bae04b9905f7199afa6242`
|
||||
```
|
||||
git log --all --format='%H %T' | grep b5414fcc6281
|
||||
```
|
||||
- Тело issue: `9d969d777f9787e5a50d02b2e675ad1448e945a27b47899d2d37c445a612f30e`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user