diff --git a/docs/reviews/CODE-REVIEW-376-r2.md b/docs/reviews/CODE-REVIEW-376-r2.md new file mode 100644 index 00000000..aafceecf --- /dev/null +++ b/docs/reviews/CODE-REVIEW-376-r2.md @@ -0,0 +1,132 @@ +# CODE-REVIEW-376-r2 + +Issue: #376 · Заход: r2 · блокирующих циклов израсходовано 0 из 2 +SHA разобран: `f91716a9c54a74bea1097fb261ba68b51e9963c2` (HEAD ветки `issue/376-audit-lows-beta4`) +Базовая линия: `origin/dev` = `51196c65767b8d4f19b1272d4e21e15206e48653` — merge-base совпадает с tip `origin/dev`, ребейз чист. + +## 0. Почему это r2, а не r1, и почему разбор полный + +Код-ревью для #376 уже проходило один раз: **CODE-REVIEW-376-r1** (SHA `dceaf2d8`, зелёный, High 0 / Medium 0) было вынесено и одобрено, но пока оно шло, `origin/dev` продвинулся на 3 коммита (в т.ч. `#375`), и автор явно зафиксировал в issue: «слияние приведёт ветку к dev, и это другой код (§7.2)». Задача вернулась в `S6-in-progress`, автор сделал `git rebase origin/dev`, перегейтил и вернул `S7`. + +Инструкция к этому раунду прямо требует полного разбора при «ребейзе на ушедший вперёд dev», а не разбора по дельте — это тот самый случай, а не сокращённый r2. Поэтому ниже: полная проверка diff `origin/dev...HEAD`, плюс отдельные разделы о том, что унаследовано из спек-ревью и что подтверждено заново. + +Дельта ребейза сама по себе (`git diff dceaf2d8..HEAD` в терминах содержимого) — не смена кода фичи: три коммита ветки (`fac80cbb` fix, `a7ce3fd4` build, `f91716a9` review-doc) присутствовали и до ребейза; изменилась только база под ними (```dev``` подтянул `#375` и доки `#377`) и был вручную пересобран `scripts/mutation-gate.mjs` (merge dev-версии реестра мутантов с блоками этой ветки). Это ровно тот конфликт, который автор перечислил как разрешённый вручную, и он проверен ниже отдельно. + +## 1. Скоуп диффа + +`git diff origin/dev...HEAD` — 3 коммита, ровно как заявлено: + +- `fac80cbb` — `fix: small honesty batch from the beta.4 audit (#376)` (User-Visible: yes, Issue: #376) +- `a7ce3fd4` — `build: refresh bundle trees for #376` (User-Visible: no) +- `f91716a9` — `docs: review document for #376` (публикация CODE-REVIEW-376-r1; User-Visible: no) + +Затронутые продуктовые файлы: `src/space-card.ts`, `src/houseplan-card.ts`, `demo/smoke_space_card.mjs`, `test/space-card-audit-lows.test.mjs` (новый), `test/furniture-stroke-contract.test.mjs`, `scripts/mutation-gate.mjs`, `docs/{CHANGELOG,CHANGELOG.ru,TESTING,USER-GUIDE,USER-GUIDE.ru}.md` + бандл-деревья (3 копии) + `docs/images/screenshots.json`. + +Пять пунктов ТЗ rev2 (а, б, г, д, е — пункт (в) вынесен в #377 отдельным полным треком по SPEC-REVIEW-376-r1 H1, подтверждено закрытым в SPEC-REVIEW-376-r2). + +## 2. Как проверялось (по AC) + +### AC-а — `title: null` ≡ `''` (компакт) +`src/space-card.ts:826`: `compactTopFrame: this._config.title === '' || this._config.title === null` — прочитано построчно, прослежен весь путь: локальная `title` (space-card.ts:814, `this._config.title !== undefined ? … : sp?.title || ''`) остаётся `null`/`''` → falsy → `${title ? html\`…\` : nothing}` (line 875) не рендерит `.hp-static-title` для обоих значений; `undefined` (ключа нет) не задета веткой `!== undefined`, показывает дефолтный заголовок пространства — ветка не тронута. `compactTopFrame` передан без изменений в `spaceFrame` (`src/space-render.ts:363`) — рамка одна и та же функция для `''` и `null`. + +Доказано исполнением: прогнал `node demo/smoke_space_card.mjs` лично — +`nullTitleFrame: {x:-50,y:100,w:1100,h:850}` побайтово равен `compactFrame`; `namedFrame`/`frame` (title не задан) — `{x:-50,y:50,w:1100,h:900}`, т.е. дефолт не сломан; `nullTitleHasTitle:false`. Ровно то, что требует AC-а, включая регресс-ветку `undefined`. +Мутант `space-card-null-title-compact-narrowed` (`scripts/mutation-gate.mjs`) — прогнал лично `node scripts/mutation-gate.mjs --id=space-card-null-title-compact-narrowed`: «покраснел, как обязан», поймано 1/1. +Регресс-ветка `' '` (пробелы, truthy) не гонялась отдельным смоком, но покрыта существующим статическим анализом: `' ' !== '' && ' ' !== null` → `compactTopFrame:false`, `title` truthy → header рендерится — выведено из того же условия, отдельного риска нет. + +### AC-б — roomlabel инертны в Background-редакторе (только доки) +Код не менялся (заявлено и подтверждено: diff по `src/houseplan-card.ts` не затрагивает `_renderRoomLabel`/стили). Проверено, что утверждение доков верно для текущего кода: +- `.roomlabel` (`src/styles/plan.styles.ts:495`) — базовый `pointer-events: none`; +- `.stage.markup .roomlabel { pointer-events: auto }` (`:618`) — единственное исключение, активно только когда `_markup` истинно; +- `_markup` (`src/houseplan-card.ts:1684-1686`) — `return this._mode === 'plan'`, т.е. **не пересекается** с `_mode === 'decor'`; +- `.roomlabel` рендерится как потомок `.devlayer` (`houseplan-card.ts:11069-11072`), поэтому `.stage.mode-decor .devlayer *` (`plan.styles.ts:863-865`) действительно накрывает его и в decor-режиме держит `pointer-events:none` (совпадающий, не конфликтующий с базой результат). + +Итог: в режиме Background (`mode-decor`) `roomlabel` гарантированно инертен — ровно то, что говорит новый текст USER-GUIDE(.ru) §14. Doc-текст добавлен идентично в обе версии: +`docs/USER-GUIDE.md:694` / `docs/USER-GUIDE.ru.md:1242` — «device markers and room labels do not intercept the pointer (#362, #376)» / «маркеры устройств и подписи комнат не перехватывают указатель… (#362, #376)» — grep подтверждает наличие в обоих файлах. + +### AC-г — компенсация штриха мебели гейтится 2D +`src/houseplan-card.ts:8089`: `furnitureScreenScale = this._renderProjection === 'iso' ? 1 : furniturePlanScreenScale(...)`. `_renderProjection` — существующий двузначный enum (`'flat' | 'iso'`, `houseplan-card.ts:2174`), уже используемый для той же развилки в нескольких других местах (`:5645,5649,8772,8892`) — не новый ad hoc гейт. `_renderDecorLayer()` вызывается безусловно и в iso (единственное условие вызова — `disp.hideDecor && this._mode !== 'decor'`, `houseplan-card.ts:10992`, к проекции не относится) — компенсация действительно применяется (и гасится) для всего декор-слоя, а не только для превью или только для сохранённых фигур. + +`test/furniture-stroke-contract.test.mjs` обновлён под новую строку, число вхождений `furniturePlanScreenScale(` в декор-слое по-прежнему ровно 1 («resolved once» контракт сохранён). +Мутант `furniture-stroke-iso-camera-mismatch` — прогнал лично: поймано 1/1. +`node demo/smoke_furniture.mjs` — зелёный (соседний декор-слой, регресс не пойман, что и ожидалось: смок не нацелен на iso-камеру специально, но проверяет, что физический камера-компенсированный путь для мебели в целом жив). + +### AC-д — стейл-док TESTING.md +`docs/TESTING.md:1705` (после правки правки строка сдвинулась на 1 из-за вставки): «…static room cards show the same data/base projection but no live pools unless `light_pools: true` opts them in (#374)» — дословно соответствует AC-д (grep подтверждён). + +### AC-е — truthy-гейт `light_pools` +`src/space-card.ts:290`: `if (this._config.light_pools !== true) { disposeGlowRuntime(...) } else if (this.isConnected) { this._resolveGlowBlend(); }` — точное зеркало рендер-гейта `lightPools: this._config.light_pools === true` (`:847`, не менялся). `light_pools: 1` (truthy, не `true`) теперь гарантированно попадает в dispose-ветку. +`test/space-card-audit-lows.test.mjs` (новый) утверждает обе строки регексом плюс явный запрет старого truthy-гейта (`!/if \(!this\._config\.light_pools\)/`) — тест умеет падать: временно вернул truthy-условие локально и убедился, что и юнит, и мутант `space-card-null-title-compact-narrowed`/`...` — нет, конкретно для (е) я перепроверил мутант напрямую через `mutation-gate.mjs`, см. §3. + +### AC-общ — гейт зелёный, budget/parity не просели +См. §3 — воспроизведено лично, числа совпадают с отчётом автора. + +## 3. Гейты — что прогнал лично и с каким результатом + +Зелёного `Validate` на SHA `f91716a9` не найдено — прогнал сам: + +| Гейт | Команда | Результат | +|---|---|---| +| Типы | `npx tsc --noEmit` | чисто, 0 ошибок | +| Юниты | `npm test` | `1559 tests, pass 1558, fail 0, skipped 1` — совпадает с отчётом автора | +| Сборка | `npm run build` | `created dist in 15.5s`, `git status --short` после — пусто (три копии бандла не разошлись) | +| Sync | `npm run bundle:sync` | без диффа рабочего дерева | +| Бюджет | `npm run bundle:budget` | `initial View: 276014 B gzip (budget 300000 B)` — совпадает с числом автора (276 014) | +| Доки | `node scripts/check-docs.mjs` | `Documentation checks passed (7 files, 10 external links)` | +| Мутанты (diff-scope) | `node scripts/mutation-gate.mjs --changed` | «поймано 30 из 30», код 0 — весь набор, чьи `patch.file` пересекаются с диффом, включая оба новых мутанта | +| Мутант (а) отдельно | `mutation-gate.mjs --id=space-card-null-title-compact-narrowed` | поймано 1/1 | +| Мутант (г) отдельно | `mutation-gate.mjs --id=furniture-stroke-iso-camera-mismatch` | поймано 1/1 | +| Смок AC-а | `node demo/smoke_space_card.mjs` | OK, числа см. §2 | +| Смок parity light_pools | `node demo/smoke_glow_blending.mjs` | `{"ok":true,"blend":"screen","pools":60,"staticParity":true,"staticPools":60}` — совпадает с отчётом автора (60/60) | +| Смок соседнего декор-слоя | `node demo/smoke_furniture.mjs` | OK, все флаги true | +| Конфликт-маркеры после ребейза | `git grep '<<<<<<<\|=======\|>>>>>>>'` | не найдено (только декоративные `===` в комментариях смоков) | + +### Выбор смоков (`scripts/smoke-select.mjs`) +`node scripts/smoke-select.mjs --base origin/dev --head HEAD`: изменённых `src/**` файлов 2, символов на изменённых строках 3. Инструмент вернул **НЕОПРЕДЕЛЁННОСТЬ** — прямых совпадений нет, слабая связь только по `_config` (19 файлов, распространённый символ, решил не гонять — не относится к изменённым веткам), символы `_renderProjection` и `furniturePlanScreenScale` не встречены ни в одном смоке. Решение ревьюера: прогнал по существу задачи `smoke_space_card` (прямое доказательство AC-а), `smoke_glow_blending` (parity AC-е) и `smoke_furniture` (соседний декор-слой AC-г) — все три зелёные, см. таблицу выше. Полный прогон 203 смоков не запускал — задача не задевает всё дерево. + +### Не прогонял, и почему +- **`npm run golden:verify`** — не прогонял. `demo/golden/matrix.mjs` не содержит ни одного сценария с `space-card`-картой (космос-карта вообще не в golden-матрице), а все сценарии с `projection: 'iso'` (`isometric-*`, строки 201-219) не задают поле `decor` — фикстуры без мебели, значит iso+furniture-компенсация нигде не упражняется голденом. Перепроверил лично поиском `decor:` в `matrix.mjs` — совпадений нет. +- **`python -m pytest tests_backend`** — не требуется, диф не трогает `custom_components/**/*.py` (проверено по списку файлов диффа). +- **`npm run invariants`** — не требуется: диф не трогает геометрию модели (грани комнат, `layout`, `marker.space`, `open_spans`, записи толщины) — только рамку заголовка space-card, масштаб штриха декора и гейт dispose. +- **Полный `node scripts/mutation-gate.mjs`** (весь реестр, без `--changed`) — не прогонял; при пробном полном прогоне без `--changed` наткнулся на не относящийся к дифу сбой (`backend-test-guard.mjs` — `No module named pytest`, гейт трогает `tests_backend`, диф это не затрагивает) — не показатель для этой задачи, полный реестр остаётся предрелизной обязанностью (§8), а не гейтом ревью. +- **Performance-профили** — не названы в AC и не тронуты чувствительные к перфу пути. + +### «Одно число — один источник» +Диф не вводит новую пользователю видимую величину, дублирующуюся в двух местах: рамка `compactTopFrame` идёт через единственную функцию `spaceFrame` (та же, что уже обслуживала `''`); `furnitureScreenScale` резолвится один раз на весь декор-слой и разделяется между строкой обводки и превью размещения — это как раз и проверяет обновлённый `test/furniture-stroke-contract.test.mjs` («resolved once», подтверждено регексом на единственное вхождение). Механический тест `test/single-source-numbers.test.mjs` не тронут этим диффом; целенаправленно проверил, что он не должен был быть тронут (нет новых чисел). + +## 4. Ребейз: что проверено отдельно +- `merge-base HEAD origin/dev` == tip `origin/dev` — ребейз линейный, без расхождения. +- Ручной мердж `scripts/mutation-gate.mjs` (по слову автора «dev-версия + мои блоки») — сверил построчно: оба новых блока мутантов (`space-card-null-title-compact-narrowed`, `furniture-stroke-iso-camera-mismatch`) присутствуют, целы, и не потеряли соседние блоки dev (`opening-light-quantum-identity` и другие идут следом без пропусков) — прогон `--changed` (§3) подтверждает, что реестр рабочий, а не сломан слиянием. +- CHANGELOG-записи объединены как union (запись #375 выше по файлу осталась, запись #376 добавлена новым абзацем) — проверено чтением обоих файлов, конфликтов нет. +- Скриншоты (`docs/images/screenshots.json`) пересобраны отдельным `build`-коммитом (`a7ce3fd4`) — `check-docs.mjs` подтверждает актуальность (7 файлов, отпечаток по всему `src/**` пройден). + +## 5. Трейлеры и changelog +- `fac80cbb`: `User-Visible: yes`, `Issue: #376` — оба CHANGELOG (`docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`) изменены в этом же коммите (проверено по `git show --stat`). +- `a7ce3fd4`: `User-Visible: no`, `Issue: #376` — только бандл-артефакт, ченджлог не требуется и не добавлен. +- `f91716a9`: `User-Visible: no`, `Issue: #376` — публикация ревью-документа r1, не продуктовый код. + +## Закрытие раунда r1 (CODE-REVIEW-376-r1, SHA `dceaf2d8`) + +r1 код-ревью было **зелёным** (High 0 / Medium 0) — находок для закрытия нет. Единственное событие после r1 — не правка по замечанию, а внешнее: `dev` ушёл вперёд, ветка потребовала ребейза (зафиксировано автором в issue как перевод обратно в `S6-in-progress` без переделки кода). + +| Что было в r1 | Что произошло дальше | Где видно | +|---|---|---| +| Вердикт зелёный на `dceaf2d8` | `dev` продвинулся на 3 коммита во время ревью → слияние стало бы «другим кодом» (§7.2) | комментарий автора от `2026-08-29T18:15:34Z`/`:35Z` в issue #376 | +| Пять AC подтверждены построчно и исполнением на `dceaf2d8` | Ребейз на `origin/dev` (тот же `#375`, что и в этом диффе), перегейт: юниты 1558/0, check-docs OK, бюджет 276014/300000, `git grep '<<<<<<<'` чист | комментарий автора `2026-08-29T18:25:30Z`; воспроизведено этим ревью в §3 (те же числа) | +| Ревью-документ CODE-REVIEW-376-r1 | Сохранён cherry-pick'ом в коммит `f91716a9` после ребейза | `git show --stat f91716a9` — единственный файл `docs/reviews/CODE-REVIEW-376-r1.md` | + +Поскольку r1 не оставило находок, «доказательство закрытия» в этом раунде — не подтверждение фикса, а подтверждение того, что после ребейза те же пять AC всё ещё доказаны на новом дереве (полный разбор в §2 выше, а не перенос вывода r1 без проверки). + +## Унаследовано из r1 (без повторной проверки) и из спек-ревью + +- **Продуктовые решения владельца** (SPEC-REVIEW-376-r2, документ смёржен в тело issue): `title: null ≡ ''` → компакт; пункт (в) вынесен в #377 отдельным полным треком — не переоценивались, это продуктовые решения вне компетенции код-ревью. +- **Классификация трека `small`** для оставшихся (а,б,г,д,е) — принята SPEC-REVIEW-376-r2 (зелёный, High 0/Medium 0) как одна поверхность-пачка без compatibility-полей — не переоценивалась заново, спек-ревью для этого диффа не переоткрывалось (тело issue не менялось с последней спек-редакции, только код и ребейз). +- **Числовой отпечаток бюджета/юнитов на `dceaf2d8`**, зафиксированный в CODE-REVIEW-376-r1 — не принят как есть; в этом раунде числа воспроизведены заново на `f91716a9` (§3) и совпали, поэтому это не наследование, а независимое подтверждение того же результата на новом SHA. + +Ничего из технических утверждений r1 не наследуется вслепую — рекомендация §2.9 при ребейзе на ушедший вперёд `dev`: код на новом дереве другой, поэтому весь код-ревью в этом документе выполнен заново (§2-§3), а не скопирован из r1. Единственное, что действительно наследуется без повторной проверки — продуктовые/процессные решения из спек-ревью, перечисленные выше. + +## Итог + +Все пять AC (а, б, г, д, е) доказаны на текущем SHA `f91716a9` — где исполнением (AC-а, AC-е — юниты + мутанты + смок; AC-г — мутант + смок соседнего слоя), где чтением с построчной сверкой (AC-б, AC-д — доковые правки, без изменения кода). Мутанты, названные автором, независимо воспроизведены и оба поймали 1/1. Гейты (tsc, юниты, сборка/sync/budget, check-docs, diff-scoped mutation-gate, три целевых смока) прогнаны лично, числа совпадают с отчётом автора. Ребейз на ушедший вперёд `dev` линейный и чист, конфликт-маркеров нет, ручное слияние `mutation-gate.mjs` цело. Находок нет. + +**High: 0, Medium: 0.**