diff --git a/docs/reviews/CODE-REVIEW-525-r1.md b/docs/reviews/CODE-REVIEW-525-r1.md new file mode 100644 index 00000000..0f021fb9 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-525-r1.md @@ -0,0 +1,164 @@ +# CODE-REVIEW-525-r1 + +Issue: #525 · Заход r1 · блокирующих циклов израсходовано 0 из 4 + +Материал ревью: `ab296302d179ff7a3bd047591db35ff67c92a683` (рабочая копия на нём). +Ветка приведена к `dev` конвейером до ревью (1 коммит dev поверх, `6bb34720` → +`ab296302` — это перебазирование тех же трёх коммитов, SHA сменились). Согласно +§7.2, после ребейза это другой код — разбор ниже полный, а не по дельте. + +## Скоуп + +Три коммита: + +| SHA | Класс | Что | +|---|---|---| +| `9edef041` | A+B+C, `User-Visible: yes` | `_renderOpenings` и список маркеров переведены на `repeat(…, (x) => x.id, …)`; комментарий-ловушка в `plan.styles.ts`; новый смок `demo/smoke_space_switch_transitions.mjs`; два мутанта в `mutation-gate.mjs`; тест-линт AC5 в `test/smoke-harness-contract.test.mjs`; потолок `bundle-budget.mjs` 300 400 → 300 700; оба changelog; пересобранный бандл | +| `8d5bade6` | C | обновлён `docs/images/screenshots.json.sourceFingerprint` (фингерпринт по `src/**`, картинки не менялись) | +| `ab296302` | B | запись `_renderDevice`/`_renderOpenings` → `smoke_space_switch_transitions.mjs` в `scripts/smoke-links.mjs` | + +Продуктовый диф минимален: `src/houseplan-card.ts` — две строки (`map` → `repeat` +в обоих списках), `src/styles/plan.styles.ts` — только комментарий. Совпадает с +ТЗ («Чего задача не трогает»): i18n, миграция конфига, touch, бэкенд и +публичные API диф не задевает — подтверждено чтением всего диффа, посторонних +изменений нет. + +## Как проверялось + +Дешёвые гейты Validate на этом SHA уже зелёные (ссылка в задаче ревьюеру), +поэтому `tsc`/`test`/`build` заново не гонял. Прогнал сам: + +| Гейт | Команда | Результат | +|---|---|---| +| Смок-свидетель | `node demo/smoke_space_switch_transitions.mjs` | OK, все 12 проверок `true`, `unexpectedAnimations: []` | +| Мутант AC1 | `node scripts/mutation-gate.mjs --check` (id `openings-rendered-without-keys`) | `ok` — гейт сам собирает бандл в отдельном воркчисте, патчит `repeat`→`map`, смок красный, воркчист убран | +| Мутант AC2 | тот же прогон (id `device-markers-rendered-without-keys`) | `ok`, аналогично | +| Тест-линт AC5 | `node --test test/smoke-harness-contract.test.mjs` | 8/8 pass, включая новый `свидетель переходов при переключении меряет поэлементно (#525)` | +| Бюджет бандла | `node scripts/bundle-budget.mjs` | `initial View: 300059 B gzip (потолок 300700 ±2000, budget 301066, headroom 1007)` — совпадает с текстом коммита до байта | +| Документация | `node scripts/check-docs.mjs` | `Documentation checks passed (7 files, 12 external links)` | +| smoke-select по диффу | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Зарегистрированная связь»: `smoke_space_switch_transitions.mjs` ← `_renderDevice` — до третьего коммита была бы «НЕОПРЕДЕЛЁННОСТЬ», запись это чинит | +| Golden | `npm run golden:verify` | зелёный, 80/80 кадров `passed`, включая все `junction-*`, `openings-*`, `device-*`, `lighting-*` — риск переупорядочивания DOM из ТЗ не реализовался | +| Смежные смоки редактора | `node demo/smoke_device_position_history.mjs`, `node demo/smoke_drag_bounds.mjs` | оба OK (риск «идентичность в живом пути» из ТЗ) | +| Perf (AC6) | `npm run benchmark:large-house-interaction` (без базы) | завершился штатно, порядок величин (`spaceSwitch` max 636 мс, `switchCycle` max 623/total 1211 мс за 3 наблюдения) правдоподобен относительно заявленных медиан 524.8/1293.9 мс | + +Не прогонял сам: `benchmark:compare` против базы `19e421b3` (нужен отдельный +воркчист на другом SHA для сборки базового отчёта — дорого при уже дважды +независимо сошедшихся числах, см. AC6 ниже), `npx tsc --noEmit`/`npm +test`/`npm run build` (взяты с зелёного Validate на этом SHA), HA-бэкенд +(`custom_components/**` не тронут), полный набор смоков (диф не задевает +ничего вне `_renderOpenings`/`_renderDevice`/стилей переходов). + +## AC · чем доказан · чем краснеет (проверено, не только заявлено) + +| # | AC | Доказательство | Чем краснеет | Проверил сам | +|---|---|---|---|---| +| AC1 | нет перехода на `.op-leaf`/`.op-arc` после переключения | `demo/smoke_space_switch_transitions.mjs` → `noOpeningTransitionOnSwitch` | мутант `openings-rendered-without-keys` | да, смок зелёный + мутант красный на отдельном прогоне | +| AC2 | нет перехода `box-shadow` на `.device-shell-frame` | тот же смок → `noMarkerTransitionOnSwitch` | мутант `device-markers-rendered-without-keys` | да, аналогично | +| AC3 | бегут только законные анимации после переключения | `onlyTheSwitchItselfAnimates` + `stillQuietOneFrameLater`, обход всего теневого дерева | любое новое переиспользование — красный список `unexpectedAnimations` | да, поле пустое на прогоне | +| AC4 | настоящая смена состояния двери внутри пространства по-прежнему анимируется | `aRealDoorOpeningStillAnimates` | удаление перехода — смок красный (не проверял мутантом отдельно, но логика симметрична AC1: тот же переход, инверсия проверки) | смок зелёный, чтением подтверждена симметрия проверки | +| AC5 | свидетель меряет поэлементно, не по документу | `test/smoke-harness-contract.test.mjs`, новый кейс | возврат `document.getAnimations()` — тест красный | да, 8/8 pass, код смока подтверждает поэлементный обход `shadowRoot` | +| AC6 | перф в пределах лимитов | таблица в хендоффе, лимиты пересчитаны ревьюером ТЗ независимо (SPEC-REVIEW-525-r1) | превышение лимита — job красный | частично: сам гейт не гонял A/B против базы, полагаюсь на сходящиеся независимые числа (см. «чего не проверял») | + +AC4 не имеет отдельного мутанта в реестре — это не находка: симметрия +проверки («переход должен идти» вместо «перехода не должно быть») делает +ложноположительный смок маловероятным, а сам факт `aRealDoorOpeningStillAnimates: true` +воспроизведён прогоном. + +## Находки + +Находок нет. Три отступления от буквы ТЗ, названные автором в хендоффе, +рассмотрены и приняты: + +1. **Комментарий-ловушка не в `houseplan-card.ts`, а в `plan.styles.ts`.** + Причина объективна и проверена: файл стоит ровно на потолке + `core-file-budget` (13649 строк при лимите 13650 — перепроверил `wc -l` и + тестовый лимит в `test/core-file-budget.test.mjs:26`). Перенос комментария + к самим переходам логичнее — это точка, которую откроет автор следующего + анимируемого свойства. +2. **`repeat` вместо `keyed`** — цена проверена автором (+241 Б против +373 Б), + решение соответствует контракту ТЗ буквально («Ключ проёма — `o.id`», + упоминание `repeat` явно). +3. **Запись в `scripts/smoke-links.mjs`** вне первой редакции ТЗ, но устраняет + реальный пробел («НЕОПРЕДЕЛЁННОСТЬ» → «зарегистрированная связь», проверено + прогоном `smoke-select` до и по смыслу описано в AGENTS.md как штатный + инструмент). Класс B, тот же issue — правомерно. + +Риски ТЗ («цена ключей», «golden», «идентичность в живом пути», «потеря +настоящей анимации») все закрыты проверкой, а не только словом автора: цена +ключей подтверждена гейтом бюджета бандла с точным числом, golden зелёный на +80 кадрах, `drag_bounds`/`device_position_history` зелёные, AC4 зелёный. + +## Что проверено и корректно + +- Диф продуктового кода ограничен двумя строками (`map` → `repeat`) плюс + стилевой комментарий — соответствует заявленному объёму и ТЗ. +- Единственные точки рендера списков проёмов (`_renderOpenings`, вызывается из + одного места, `:11741`) и маркеров (`:11787`) — оба переведены на `repeat`; + других мест, рисующих `.op-leaf`/`.op-arc`/`.device-shell-frame` через + списочный `map`, в `src/*.ts` нет (проверено `grep`). `src/space-render.ts` — + отдельный статический рендерер (`space-card.ts`), строит HTML-строку без + инкрементального Lit-diffing и под контракт AC1–AC3 не подпадает; ТЗ его и + не называет. +- Трейлеры: все три коммита несут `Issue: #525`; `9edef041` — `User-Visible: + yes` с правками обоих changelog в этом же коммите (проверено `git show + --stat`); `8d5bade6` и `ab296302` — `User-Visible: no`, что верно (docs- и + gate-правки без видимого поведения). +- Терминология changelog («пространства», «двери и окна», «створка») совпадает + с `docs/USER-GUIDE.ru.md`. +- Тело issue после зелёного ревью ТЗ менялось (добавлен абзац «чего не + трогает» и переписан черновик changelog) — автор сам назвал это в + комментарии до перехода в `S6`, повторно свежий текст сверен построчно с + реализацией, расхождений нет. +- `docs/images/screenshots.json`: все 11 `imageSha256` в диффе не изменились + (проверено `git diff` — строки без `+`/`-`), меняется только + `sourceFingerprint`/`sourceSha256` — соответствует правилу «фингерпринт + считается по всему `src/**`». + +## Чего не проверял и почему + +- `npx tsc --noEmit`, `npm test`, `npm run build` — сверены с зелёным Validate + на точном SHA `ab296302` (см. ссылку в постановке задачи ревьюеру). +- `npm run benchmark:compare` против базы `19e421b3` — не воспроизводил A/B: + потребовал бы отдельного воркчиста на другом коммите и полного цикла из 7 + образцов на обеих версиях. Сам факт «работает без ошибок» и правдоподобие + порядка величин проверил (см. таблицу выше); числа AC6 дополнительно + сходятся с независимым пересчётом лимитов ревьюером ТЗ (`SPEC-REVIEW-525-r1.md`, + формула `max(база×(1+ratio), база+noise)` с `hardMaxMs`, совпадение до + сотых). Если это признать недостаточным — единственный пункт, который стоит + перепроверить отдельным прогоном. +- `python -m pytest tests_backend` — `custom_components/**/*.py` не тронут. +- Полный набор `demo/smoke_*.mjs` (240 файлов) — не запускал: диф ограничен + двумя рендерерами и стилевым комментарием, `smoke-select` дал ровно одну + «зарегистрированную связь», смежные риски (редактор, golden) проверены + прицельно. +- Инварианты модели (`npm run invariants`) — диф не трогает геометрию, толщину + стен, `layout`, `marker.space` или `open_spans`; это чисто слой отрисовки + (Lit-директива), гейт не применим. + +## Материал раунда + +- SHA ветки: `ab296302d179ff7a3bd047591db35ff67c92a683` +- Дерево материала: рабочая копия на этом SHA, детальный `HEAD` detached +- Диапазон: `origin/dev..HEAD` = `9edef041`, `8d5bade6`, `ab296302` +- ТЗ: тело issue #525, раздел `## ТЗ`, зелёное ревью + `docs/reviews/SPEC-REVIEW-525-r1.md` + +## Вердикт + +Зелёный. AC1–AC6 доказаны (AC6 — с оговоркой, см. «чего не проверял»), защитные +AC1/AC2/AC5 подтверждены собственным прогоном мутантов и тестов, риски ТЗ +закрыты проверяемо, находок High/Medium нет. + +--- + + + +## Материал раунда + +- Ветка: `issue/525-space-switch-transitions`, коммит `ab296302d179` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `2b749e6ac7af5d0cb523647be6fbf0bccce999ad` + ``` + git log --all --format='%H %T' | grep 2b749e6ac7af + ``` +- Тело issue: `786c461d9a358bfeb155d707363704ca5568415678b3d326d4240bed4ec75908` +- Вердикт конвейера: `green` · High 0