diff --git a/docs/reviews/CODE-REVIEW-384-r1.md b/docs/reviews/CODE-REVIEW-384-r1.md new file mode 100644 index 00000000..6b70c3b7 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-384-r1.md @@ -0,0 +1,80 @@ +# CODE-REVIEW-384-r1 + +Issue: #384 · «hidden architecture no longer widens the `fit: house` frame» +Заход: r1 · трек `small` (лимит циклов код-ревью — 2) · блокирующих циклов израсходовано 0/2 +Материал: `git diff origin/dev...HEAD`, диапазон `origin/dev..HEAD` (2 коммита) +SHA на момент вердикта: `4f28bf4f7f1699a7fb094ab05c72348c9a66acb2` (проверено `git rev-parse HEAD` непосредственно перед выводом) +Ветка: `issue/384-fit-house-hidden-walls` + +Первый раунд код-ревью — раздел «объём по дельте» (§2.10) не применяется, разбор полный. + +## Скоуп + +Коммиты: +- `16133394` `fix: hidden architecture no longer widens the fit: house frame (#384)` — `User-Visible: yes`, `Issue: #384`. Меняет `src/space-render.ts`, расширяет `demo/smoke_space_card.mjs`, добавляет мутант в `scripts/mutation-gate.mjs`, правит `docs/CANVAS.md`, `docs/USER-GUIDE.md`, `docs/USER-GUIDE.ru.md`, `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`. +- `4f28bf4f` `build: refresh bundle trees for #384` — `User-Visible: no`, класс D (пересборка бандла + скриншоты документации). + +ТЗ — лёгкий трек, тело issue (редакция 2 после SPEC-REVIEW-384-r1/r2, оба ревью ТЗ уже пройдены: r1 жёлтый M1 закрыт, r2 зелёный). AC1–AC4 из тела issue проверялись против этого диффа. + +## Как проверялось + +Прогнано лично на SHA `4f28bf4f` (зелёного Validate на этом SHA не найдено, поэтому дешёвые и относящиеся к делу гейты прогнаны самостоятельно): + +| Гейт | Команда | Результат | +|---|---|---| +| Typecheck | `npx tsc --noEmit` | чисто, без вывода | +| Юнит | `npm test` | `# tests 1585 / pass 1584 / fail 0 / skipped 1` — совпадает с заявленным в хендоффе | +| Build + сверка бандла | `npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | `MATCH`; дополнительно `npm run bundle:sync` — `git status`/`git diff --stat` после пересборки пуст, дерево уже синхронно | +| Бюджет | `npm run bundle:budget` | initial View 277 978 Б / 300 000 (запас 22 022 Б) — совпадает с заявленным «+14 Б» (277 951 в хендоффе), расхождение в пределах шума хешей чанков | +| `any` на новых строках | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | «Новых any нет» (8 добавленных строк в 1 файле) | +| Доки-отпечаток | `node scripts/check-docs.mjs` (diff трогает `src/**` → обязателен) | «Documentation checks passed (7 files, 10 external links)» | +| Выбор смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «НЕОПРЕДЕЛЁННОСТЬ» — символ на изменённых строках не совпал ни с одним смоком автоматически (в диффе нет нового именованного символа проекта — правка сидит внутри уже существующей функции `renderSpaceStatic`). Решение по строкам ниже принято вручную, не автоматом | +| Целевой смок (AC1/AC2/AC3) | `node demo/smoke_space_card.mjs` | `OK space-card: ...` — прогнан, зелёный; `hiddenTwinFrames.hidden.w = 752` против `hiddenTwinFrames.shown.w = 300938` (см. ниже) | +| Смежный смок (Glow/viewBox) | `node demo/smoke_glow_blending.mjs` | `{"ok":true,"blend":"screen","pools":60,"staticParity":true,"staticPools":60}` — совпадает с заявленным «parity 60/60»; прогнан, т.к. риск-раздел ТЗ утверждает независимость Glow от структуры кадра, а Glow рисуется в том же viewBox, который эта задача меняет | +| Мутационный гейт (целевой мутант) | `node scripts/mutation-gate.mjs --id=fit-house-hidden-walls-vote` | `ok чистый прогон: node demo/smoke_space_card.mjs` → `ok fit-house-hidden-walls-vote: тест покраснел, как обязан` → `поймано 1 из 1` | +| Процесс-гейт | `node scripts/process-gate.mjs` | «гейт пройден, предупреждений 1» (п.3: ТЗ для класса A не найдено файлом — ожидаемо для метки `small`, ТЗ в теле issue) | + +Не прогонялись и почему: +- **`golden:verify`** — диф меняет видимый кадр (`viewBox`), но проверено `grep -n "fit:" demo/golden/matrix.mjs`: ни один golden-сценарий не задаёт `fit: 'house'` вовсе (только `showBorders:false` в изометрических сценариях без `fit`, которые используют другой, независимый путь кадрирования — см. «Один голос — один источник» ниже). Полный прогон golden не покрыл бы изменённую ветку и не доказал бы ничего сверх уже прогнанных смоков; прогон предрелизный (PROCESS §8) в любом случае. +- **`python -m pytest tests_backend`** — диф не трогает `custom_components/**/*.py`. +- **`npm run invariants -- --config ...`** — диф не меняет модель геометрии (рёбра комнат, записи толщины, `layout`, `marker.space`, `open_spans`); меняется только то, что уже посчитанная геометрия отдаёт в подсчёт кадра. `npm test` (зелёный) уже гоняет инварианты на всех моделях проекта. +- **Полный набор `demo/smoke_*.mjs` (205 файлов)** — задача — один модуль, один цикл кадрирования; `smoke-select.mjs` не нашёл автоматической связи («НЕОПРЕДЕЛЁННОСТЬ»), а `smoke_wall_junctions` (похожее по названию, но нерелевантное — прецедент #234) сюда не подходит по смыслу: там про геометрию стыков, здесь — про то, участвует ли уже готовая геометрия в подсчёте bbox. Прогнаны только два смока, чью связь с диффом я установил по чтению кода (прямое совпадение — `smoke_space_card` сам расширен этим коммитом; зарегистрированная связь — `smoke_glow_blending` из-за общего viewBox). +- **Performance-профили** — не названы в AC, диф не задевает горячий путь (условие плюс пропуск цикла, не новый проход по геометрии). + +## Находки + +Нет находок уровня High или Medium — ни в скоупе, ни вне его. + +### Дополнительная проверка (сделана ревьюером сверх AC, результат — отрицательный) + +При чтении обнаружил, что параллельная (не структурная) «content»-рамка `fit: content`/по умолчанию тоже безусловно включает `extras`/`zeroWalls.lines` в список `placed` (`src/space-render.ts:355-365`) — то есть на первый взгляд тот же класс дефекта («невидимая архитектура голосует в кадре») мог существовать и для дефолтного режима, вне скоупа #384. Эмпирически проверил отдельным сценарием (твин-карточки без `fit`, с тем же удалённым столбом, `show_borders` true/false): рамки совпали побайтово (`{x:-50,y:50,w:1100,h:900}` в обоих случаях). Причина — `contentFrame` использует `core` (статистическое отбрасывание выбросов, §4.1 CANVAS.md), а не «сохранить каждый структурный элемент», как `fit: house`; удалённый столбец в обоих случаях отбрасывается как выброс независимо от видимости. Это не дефект — отдельного issue не заводил. + +## Что проверено и корректно (AC1–AC4) + +- **AC1** (далеко выступающая стена/колонна-extra + `show_borders:false` + `fit:house` → кадр по комнатам): доказано автотестом — новый блок `hiddenTwinFrames` в `demo/smoke_space_card.mjs` клонирует снапшот конфига, добавляет `wall_columns` с дальним столбцом и разнится только `show_borders`; ассерт `hidden.x+hidden.w < shown.x+shown.w - 100` прошёл (`752` против `300938` по ширине на живом прогоне). **Тест умеет падать** — подтверждено мутационным гейтом (`--id=fit-house-hidden-walls-vote`, патч снимает `if (disp.showBorders)` перед циклом `extras` → смок красный, поймано 1/1). +- **AC2** (`show_borders:true` → кадр байт-в-байт как раньше): доказано и чтением, и исполнением. По коду: все три новых гварда — `if (disp.showBorders) for (...)` — при `showBorders === true` строго эквивалентны прежнему безусловному циклу (условие не меняет тело, только пропускает его при `false`); `needsCanonicalWallGeometry` новой формы `walls.length || (extras.length && disp.showBorders)` при `showBorders === true` даёт то же логическое значение, что старая `... || (extras.length && (disp.showBorders || fit === 'house'))` — обе истинны на этой ветке ИЛИ. Исполнением: существующие ассерты `tightFrame`, `tightNoTitleFrame`, `containment ±0.51px`, `tightPaintedEnvelope` в том же прогоне `smoke_space_card.mjs` не изменились и остались зелёными (значения `tightFrame` и `tightNoTitleFrame` идентичны в выводе прогона). +- **AC3** (`show_borders:false` + видимые проёмы → проёмы всё ещё голосуют): **проверено чтением, не исполнением**. Гвард проёмов (`if (!disp.hideOpenings) for (const resolved of resolvedHosted)`, :453) диффом не тронут; в обоих твинах `hideOpenings` не установлен (по умолчанию `false`), то есть проёмы формально видимы и участвуют в обоих сценариях смока, но тест не изолирует это отдельным ассертом (например, не сравнивает `hidden`-кадр с эталонным «только комнаты+проёмы»). Symmetричность с рендер-гвардом проёмов подтверждена и в docs (CANVAS.md, USER-GUIDE) — формулировки согласованы с кодом. +- **AC4** (полный гейт зелёный, бюджет ≈0): подтверждено таблицей выше; бюджет 277 978/300 000 Б (запас 22 022 Б, дельта от заявленных автором 277 951 в пределах хеш-шума). + +### Асимметрия проверки трёх симметричных гвардов (Low, не блокирует, не правится) + +Диф добавляет один и тот же гвард `if (disp.showBorders)` в трёх местах: тела стен из `canonicalWallGeometry.components`, `extras`, `zeroWalls.lines`. Мутационный тест и новый твин-смок доказывают исполнением только гвард `extras` (демо-фикстура `smoke_space_card.mjs` никогда не задаёт `spCfg.walls` — проверено отдельным прогоном `houseplan/config/get` на пространстве `f1`: `wallsLen: 0` до патчей смока), поэтому гварды `canonicalWallGeometry.components` и `zeroWalls.lines` доказаны **только чтением**: та же механическая конструкция `if (disp.showBorders) for (...)`, тело циклов не изменено, риск опечатки симметричен и низок. AC1 это покрывает буквой («стена ИЛИ колонна-extra»), так что это не невыполненный AC, а точечный пробел мутационного покрытия. Снимаю без правки: расширение демо-фикстуры настоящими `walls`/нулевыми стенами ради этого — по объёму сравнимо с отдельной задачей, не стоит того здесь. + +## Один голос — один источник + +Диф не добавляет и не меняет ни одной пользовательской величины (нет новых чисел/подписей/превью) — правило неприменимо. Затронутая величина — `viewBox` (геометрический кадр), а не отображаемое число; `test/single-source-numbers.test.mjs` входит в зелёный `npm test` и не задет по существу. + +## Трейлеры и трассируемость + +- `16133394`: `Issue: #384`, `User-Visible: yes` — оба changelog (`docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`) правлены в этом же коммите, запись под `## Unreleased`, ссылка на #384 есть в обоих. +- `4f28bf4f`: `Issue: #384`, `User-Visible: no` — только класс D (бандл + скриншоты документации), корректно. +- Ветка `issue/384-fit-house-hidden-walls` соответствует `issue/-slug`. +- `process-gate.mjs` — зелёный (1 некритичное предупреждение, ожидаемое для лёгкого трека). + +## Документация + +`docs/CANVAS.md:232-236`, `docs/USER-GUIDE.md:798-803`, `docs/USER-GUIDE.ru.md:1546-1552` — все три обновлены оговоркой «участвуют только пока видимы (`show_borders`)», согласовано с кодом и друг с другом; это ровно то, что требовало M1 из SPEC-REVIEW-384-r1. `check-docs.mjs` подтверждает отпечаток скриншотов (7 файлов) в порядке. + +## Вердикт + +Зелёный. High: 0, Medium: 0. Единственная заметка — Low по асимметрии мутационного покрытия трёх симметричных гвардов — снята решением ревьюера без правки (см. выше), в документ занесена с обоснованием.