Issue: #426 Issue: #427 Issue: #428 Issue: #431 Issue: #432 Issue: #434 User-Visible: no
17 KiB
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, короткий трек):
- Для decor-raster >2 МиБ с
probe.kind !== "hard"guard показывает «Отмена» и «Загрузить уменьшенную копию», но не «Оставить оригинал»; кнопка активна, пока не идёт операция. - Клик «Загрузить уменьшенную копию» использует существующий
downscale → decor-asset upload, не грузит исходник, закрывает guard после
успеха;
hardостаётся только с «Отмена»; подложка сохраняет обе кнопки. - 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 не обнаружено.
Что проверено и корректно
- Корень бага устранён именно так, как описан. Было:
hard || !allowOriginal ? null : <оба варианта>— гасило весь блок кнопок. Стало:hard ? null : <"Отмена" всегда рендерится вне условия>+allowOriginal ? <"Оставить оригинал"> : nullперед кнопкой уменьшения, которая теперь рендерится безусловно внутри!hard-ветки (src/backdrop-pick.ts:241-247). Соответствует AC1/AC2 дословно. hard-случай не тронут: как и раньше, приprobe.kind === 'hard'рендерится только «Отмена» — независимо отallowOriginal. Прочитано в коде, не выполнением.- Подложка (backdrop) не затронута: оба вызова
renderBackdropGuardдля plan-file (houseplan-editor-runtime.ts:8565,houseplan-onboarding-runtime.ts:148) не передаютallowOriginal→ действует дефолтtrue→ обе кнопки остаются, как и до фикса. allowOriginalдля decor по-прежнемуfile.size <= 2 МиБ(houseplan-editor-runtime.ts:8563) — граница не менялась, поменялась только реакция диалога наfalse.- 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, decorhard, обычная подложка — третье покрыто уже существующими более ранними секциями того же файла (строки 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 зелёные. replaceSelectionв смоке=== falseсоответствует вызову_decorImageUpload(ev)безreplaceSelection(по умолчаниюfalse,houseplan-editor-runtime.ts:5160) — не «замена выделения», а обычная загрузка в палитру. Согласовано с кодомdecor-image-editor.ts:135-169.- Документация (AC3, текстовая часть):
docs/USER-GUIDE.mdи.ru.mdтеперь явно разводят «лимит 2 МиБ относится к сохранённому canonical-файлу» и «диалог предлагает уменьшенную копию, оригинал недоступен» — точная формулировка, снимающая расхождение, зафиксированное в самом issue («USER-GUIDE описан по факту, а не по ТЗ»). Термины взяты из существующего текста руководства, не изобретены. - Трейлеры и changelog:
b87e99f9несётIssue: #427+User-Visible: yesи правит обаdocs/CHANGELOG*.mdв том же коммите — соответствует правилу.f71de899(User-Visible: no, docs-only фингерпринт) корректно классифицирован как невидимая пользователю правка. - Класс изменений: только
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). Нарушений границ классов нет. - Инвариант локов/декора (
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 уже подтверждён на точном SHAf71de899(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/deletedecor-изображений юнит-тестами (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— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
cbca87d1f7a12944d5ada9892d1115f1d1b764ecgit log --all --format='%H %T' | grep cbca87d1f7a1