From c3eb225c8f91071f6c5c19d7f92529ecfbcf49a2 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 07:20:27 +0000 Subject: [PATCH] docs: review document for #427 Issue: #427 User-Visible: no --- docs/reviews/CODE-REVIEW-427-r1.md | 197 +++++++++++++++++++++++++++++ 1 file changed, 197 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-427-r1.md diff --git a/docs/reviews/CODE-REVIEW-427-r1.md b/docs/reviews/CODE-REVIEW-427-r1.md new file mode 100644 index 00000000..abd628f3 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-427-r1.md @@ -0,0 +1,197 @@ +# 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 + ```