mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 11:49:16 +00:00
claude[bot]
@@ -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**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/427-decor-large-image-downscale-action`, коммит `f71de899750d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `cbca87d1f7a12944d5ada9892d1115f1d1b764ec`
|
||||
```
|
||||
git log --all --format='%H %T' | grep cbca87d1f7a1
|
||||
```
|
||||
Reference in New Issue
Block a user