mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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.**
|
||||
Reference in New Issue
Block a user