Files
houseplan-card/docs/reviews/CODE-REVIEW-427-r1.md
Codex d4dd027b0a build: prepare v1.71.0-beta.2 candidate
Issue: #426
Issue: #427
Issue: #428
Issue: #431
Issue: #432
Issue: #434
User-Visible: no
2026-09-03 15:23:40 +03:00

17 KiB
Raw Permalink Blame History

CODE-REVIEW — issue #427 · заход r1

SHA материала: f71de899750dc0a59e176ad0554a09b34927b25f (HEAD, детач от origin/issue/427-decor-large-image-downscale-action) База сравнения: origin/dev = 4cabcbe828e0ec7349414cab4626d43f78d95f88 Трек: trivial (короткий трек, ТЗ в теле issue, ревью — комментарий; код-ревью как обычно) Заход: r1 · блокирующих циклов израсходовано 0 из 2

Скоуп

Из аудита беты обнаружено, что renderBackdropGuard для decor-изображений (allowOriginal=false, файл источника >2 МиБ) гасил весь блок действий диалога-предупреждения вместо одной кнопки «Оставить оригинал». Пользователь не мог добавить крупное изображение в декор ни оригиналом (запрещено намеренно), ни уменьшенной копией (должно быть разрешено) — оставалась только «Отмена».

AC (issue body, короткий трек):

  1. Для decor-raster >2 МиБ с probe.kind !== "hard" guard показывает «Отмена» и «Загрузить уменьшенную копию», но не «Оставить оригинал»; кнопка активна, пока не идёт операция.
  2. Клик «Загрузить уменьшенную копию» использует существующий downscale → decor-asset upload, не грузит исходник, закрывает guard после успеха; hard остаётся только с «Отмена»; подложка сохраняет обе кнопки.
  3. Targeted production-bundle smoke краснеет на старом условии и различает decor >2 МиБ / hard / обычную подложку; EN/RU User Guide и оба changelog обновлены.

Продуктовая рамка (docs/SCOPE.md): decor-изображения — существующая принятая функциональность редактора (спецификация #51, docs/specs/051-custom-decor-images.md), в аудите excess-functionality не отмечена к удалению. Это точечный регрессионный фикс уже обещанного сценария UX, а не новая функция — скоуп-вопросов не возникает.

Как проверялось

Дельта равна всей задаче (2 коммита от dev), r1 — разбор полный, разделы «Унаследовано из r0» и «Закрытие предыдущего раунда» не нужны.

Прочитано:

  • docs/SCOPE.md, AGENTS.md, PROCESS.md (§1, §2.7, §2.10, §5.1, §7.2, §8, §12).
  • Тело issue #427 и три комментария (аналитика, занятие, хендофф).
  • git diff origin/dev...HEAD полностью (--stat + построчно по каждому текстовому файлу).
  • src/backdrop-pick.ts целиком (не только диф) — единственный тронутый продуктовый файл.
  • Все три вызова renderBackdropGuard (houseplan-editor-runtime.ts ×2, houseplan-onboarding-runtime.ts ×1) — проверено, что подложка/онбординг используют allowOriginal по умолчанию true и фиксом не задеты.
  • src/decor-image-editor.ts (uploadFromInput, upload) — подтверждена привязка guardAboveBytes = 2 МиБ и replaceSelection по умолчанию false.
  • demo/smoke_backdrop_guard.mjs целиком, включая новый блок #427 и checkAll/check в demo/serve.mjs (семантика: каждый ключ out.* должен быть true, иначе смок падает).
  • docs/USER-GUIDE.md, docs/USER-GUIDE.ru.md, оба CHANGELOG*.md.
  • Минифицированный бандл-чанк backdrop-pick-*.js в custom_components/… и dist/… — визуально подтверждено, что новый хэш файла и его контент расходятся со старым (не stale copy).

Гейты

Гейт Статус Как подтверждено
npx tsc --noEmit (typecheck) не прогонял повторно зелёный в Validate на этом же SHA (см. ниже)
npm test (юниты + мутанты) не прогонял повторно зелёный в Validate на этом же SHA
npm run build + bundle:sync (3 копии бандла) не прогонял повторно зелёный в Validate на этом же SHA
node scripts/check-docs.mjs (фингерпринт скриншотов) не прогонял повторно коммит f71de899 — отдельный docs-коммит именно под это; зелёный в том же Validate-прогоне
Полный браузерный смок-набор (3 шарда) не прогонял повторно все 3 шарда зелёные в том же прогоне (включает smoke_backdrop_guard)
golden:verify не прогонял повторно job «Golden-кадры против принятых эталонов» зелёный в том же прогоне
performance_smoke не прогонял повторно job «Перф-смок: бюджет времени кадра» зелёный в том же прогоне
python -m pytest tests_backend -q не требуется diff не трогает custom_components/**/*.py; job Бэкенд в прогоне — skipped (путь-фильтр, ожидаемо)
npm run invariants -- --config … не требуется diff не трогает геометрию (рёбра комнат, толщину стен, layout, marker.space, open_spans) — только UI-кнопки диалога загрузки
scripts/smoke-select.mjs --base 4cabcbe8 --head f71de899 прогнан вывод: «НЕОПРЕДЕЛЁННОСТЬ» (0 символов на изменённых строках инлайн-разметки шаблона lit). Не разрешение ничего не прогонять — но полный набор всё равно уже прогнан в CI (все 3 шарда), так что находка инструмента полностью перекрыта фактическим прогоном.

Проверка, что Validate действительно на этом SHA и действительно зелёный: gh run view 33726518377 → headSha: f71de899…, conclusion: success; разбивка по job: типы/юниты/бандл — success, три шарда смоков — success, golden — success, перф-смок — success, hassfest/HACS/backend — skipped (путь-фильтр, ожидаемо для чисто фронтенд-диффа).

Одно число — один источник. Диф не добавляет и не меняет ни одной пользовательской величины (МиБ файла, целевые размеры уменьшенной копии) — это существующие вычисления probe/downscaleDimensions, тронута только видимость двух кнопок. Пункт неприменим к этой правке.

Находки

Нет. High/Medium/Low не обнаружено.

Что проверено и корректно

  1. Корень бага устранён именно так, как описан. Было: hard || !allowOriginal ? null : <оба варианта> — гасило весь блок кнопок. Стало: hard ? null : <"Отмена" всегда рендерится вне условия> + allowOriginal ? <"Оставить оригинал"> : null перед кнопкой уменьшения, которая теперь рендерится безусловно внутри !hard-ветки (src/backdrop-pick.ts:241-247). Соответствует AC1/AC2 дословно.
  2. hard-случай не тронут: как и раньше, при probe.kind === 'hard' рендерится только «Отмена» — независимо от allowOriginal. Прочитано в коде, не выполнением.
  3. Подложка (backdrop) не затронута: оба вызова renderBackdropGuard для plan-file (houseplan-editor-runtime.ts:8565, houseplan-onboarding-runtime.ts:148) не передают allowOriginal → действует дефолт true → обе кнопки остаются, как и до фикса.
  4. allowOriginal для decor по-прежнему file.size <= 2 МиБ (houseplan-editor-runtime.ts:8563) — граница не менялась, поменялась только реакция диалога на false.
  5. AC3 (test): новый блок в demo/smoke_backdrop_guard.mjs:113-156 целенаправленно бьёт именно в починенную ветку — decorBigFile собран так, чтобы probe.kind остался warn (то же изображение 6200×6200, что и в существующем AC2-кейсе этого же файла), а file.size превысил 2 МиБ через аппендж «мусорных» байт после JPEG EOI (комментарий в смоке объясняет, почему Chromium декодирует такой файл штатно). Это разводит ровно три состояния AC1: decor >2 МиБ/warn, decor hard, обычная подложка — третье покрыто уже существующими более ранними секциями того же файла (строки 65-111), которые фикс не трогает и которые заведомо продолжают проходить (обе кнопки для подложки, дефолт allowOriginal=true не менялся). Тест умеет падать: при откате src/backdrop-pick.ts к состоянию до фикса условие hard || !allowOriginal ? null : … гасит оба варианта → decorButtons.length было бы 1 (только «Отмена»), а out.decorOversizeOffersReducedWithoutOriginal требует length === 2 → checkAll роняет смок. Проверено чтением логики checkAll/check (demo/serve.mjs:66-77: каждый ключ результата обязан быть true, иначе попадает в _failures и процесс завершается с ненулевым кодом через finish()). Дополнительно проверено чтением: очистка моков (_uploadDecorImage восстановлен, _backdropGuard и _decorAssetGuardReplace сброшены через close()-колбэк decor-ветки _renderBackdropGuard) не оставляет состояния, которое могло бы исказить последующие секции того же смока (alpha-ветка, AC4 и далее) — фактически это подтверждено тем, что все три браузерных шарда CI на этом SHA зелёные.
  6. replaceSelection в смоке === false соответствует вызову _decorImageUpload(ev) без replaceSelection (по умолчанию false, houseplan-editor-runtime.ts:5160) — не «замена выделения», а обычная загрузка в палитру. Согласовано с кодом decor-image-editor.ts:135-169.
  7. Документация (AC3, текстовая часть): docs/USER-GUIDE.md и .ru.md теперь явно разводят «лимит 2 МиБ относится к сохранённому canonical-файлу» и «диалог предлагает уменьшенную копию, оригинал недоступен» — точная формулировка, снимающая расхождение, зафиксированное в самом issue («USER-GUIDE описан по факту, а не по ТЗ»). Термины взяты из существующего текста руководства, не изобретены.
  8. Трейлеры и changelog: b87e99f9 несёт Issue: #427 + User-Visible: yes и правит оба docs/CHANGELOG*.md в том же коммите — соответствует правилу. f71de899 (User-Visible: no, docs-only фингерпринт) корректно классифицирован как невидимая пользователю правка.
  9. Класс изменений: только src/backdrop-pick.ts — класс A (issue обязателен, есть); demo/smoke_backdrop_guard.mjs — класс B (может переиспользовать issue задачи — переиспользует); docs/**, CHANGELOG* — класс C; dist/**, custom_components/houseplan/frontend/** — класс D, синхронно пересобраны (подтверждено зелёным job «синхрон бандла» на этом SHA, плюс визуальная проверка изменённого чанка backdrop-pick-*.js). Нарушений границ классов нет.
  10. Инвариант локов/декора (docs/SCOPE.md) — правка не затрагивает пути актуации (resolveToggleIntent, isControllable, _cardToggle); decor- изображения не являются security-таргетом. Неприменимо, но проверено чтением diff на предмет случайного расширения actuation-поверхности — такого нет.

Чего не проверял

  • Ручной запуск npm run typecheck / npm test / npm run build / node scripts/check-docs.mjs / полного смок-набора / golden:verify / performance_smoke — не требовалось: зелёный Validate уже подтверждён на точном SHA f71de899 (gh run view 33726518377), включая все три шарда смоков, golden и перф-смок.
  • python -m pytest tests_backend -q — diff не трогает custom_components/**/*.py.
  • npm run invariants — diff не трогает геометрию/толщину/layout/ marker.space/open_spans.
  • Ручное открытие приложения в браузере (визуальная проверка диалога глазами) — ревью не включает ручное тестирование по регламенту; вместо этого AC доказаны чтением кода + падающим-по-конструкции автотестом, плюс независимое подтверждение зелёным CI-прогоном браузерных смоков на этом же SHA.
  • Общий пробел покрытия upload/delete decor-изображений юнит-тестами (tsconfig.test.json не включает backdrop-pick.ts/decor-image-editor.ts) — сам issue называет его отдельно закрытым в #433; вне скоупа #427, новый issue не требуется (уже есть).

Вердикт

Все три AC доказаны: код читаемо соответствует ожидаемому поведению, целевой smoke добавлен, показан падающим на до-фиксовом условии, и зелёным в реальном CI-прогоне на итоговом SHA; документация и оба changelog обновлены в соответствующих коммитах с корректными трейлерами. Находок нет.

Вердикт: зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0


Материал раунда

  • Ветка: issue/427-decor-large-image-downscale-action, коммит f71de899750d — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: cbca87d1f7a12944d5ada9892d1115f1d1b764ec
    git log --all --format='%H %T' | grep cbca87d1f7a1