20 KiB
SPEC-REVIEW-39-r1
- Issue: https://github.com/Matysh/houseplan-card/issues/39 — «[HP-UX-09] большие подложки»
- Этап: ТЗ на ревью (PROCESS.md §2.4)
- Заход: r1 · блокирующих циклов израсходовано 0/4
- ТЗ:
docs/specs/039-large-backdrops.md, ревизия 2, SHA156be645 - Трек: полный (владелец, 2026-08-15: «P3, polish/tech-debt, обычный трек»)
- Предыдущего вердикта по этому ТЗ нет — это первый проход ревью, раздел «Унаследовано из r0» не применяется, разбор полный по §2.10 (не второй цикл).
Скоуп проверки
Диагностика разрешения/decoded-памяти PNG/JPEG/WebW до какой-либо тяжёлой
аллокации, warn/hard-диалог, клиентский downscale до 4096px, замена ручного
base64-цикла на FileReader. Один файл изменён в этом коммите:
docs/specs/039-large-backdrops.md (165 строк, Class C). Продуктового/кода
diff нет — ревью касается только текста ТЗ.
Как проверялось
docs/SCOPE.md— сценарий укладывается в надёжность onboarding-загрузки плана (соседствует с J4), сама задача уже протриажена владельцем как valid tech-debt (комментарий от 2026-08-14), продуктового конфликта со стандартными правилами (never-delete-a-file, lock invariant) нет.AGENTS.md,PROCESS.md§2.3–2.5, §7.1 — состав обязательных разделов ТЗ и критерии DoR.- Issue #39: тело + оба комментария (аналитика 2026-08-14, актуализация 2026-08-29). Вопросов к владельцу автор не поднимал («Вопросы: нет»).
docs/USER-GUIDE.ru.md— сверил терминологию: «подложка», «Редактор подложки», диалогиhp-dialog— используются в ТЗ корректно, не изобретены.docs/CANVAS.md→ обнаружил ссылку наdocs/BACKDROP.md(канонический документ калибровки/размещения подложки на канвасе, не входит в список в промпте, но напрямую относится к подсистеме). Прочитан: описываетplan_x/y, scale, rotation — геометрию подложки, а не upload/decode. Задача этой геометрии не касается («Вне скоупа» ТЗ подтверждает), конфликта нет.- Сверка технических якорей ТЗ с текущим кодом (раздел «не додумывается —
выносится» требует, чтобы заявленные факты о коде были фактами, а не
догадкой):
_pickPlanFile(src/houseplan-editor-runtime.ts:8243) — ручной base64-циклString.fromCharCodeподтверждён построчно (:8253-8255); разрешение действительно не проверяется; aspect действительно берётся через<img>.onload(:8261-8266) — то есть полный decode уже происходит на этом шаге. Совпадает с текстом ТЗ дословно.MAX_FILE_BYTES = 50 * 1024 * 1024подтверждён вcustom_components/houseplan/validation.py:24, использование вhttp_api.py:252— совпадает с «50 МБ» в ТЗ.check_quotaвcustom_components/houseplan/plans.py:216— квота подтверждена, комментарий рядом («Files are never removed for getting old») согласуется с standing rule SCOPE.md «never delete a user's file».- Транзакционный комментарий в
_saveSpaceDialog(src/houseplan-editor-runtime.ts:~8375-8390, «Upload BEFORE touching the config...») подтверждает заявление ТЗ «staging не трогает конфиг до успешного save» — это не догадка, а верно прочитанный существующий код. Вывод: технические анкеры ТЗ точны, автор не выдаёт предположение за факт там, где дело касается существующего кода.
- Сверка ревизии 1 → 2 (
git show 156be645^:docs/specs/039-large-backdrops.mdvs текущая) — чтобы не приписать ревизии 2 разрыв, унаследованный от ревизии 1. Структурные пробелы (см. находки ниже) присутствовали уже в ревизии 1 — не новый регресс, но это первый ревью документа, поэтому они предъявляются сейчас. - i18n: подтвердил, что проект поддерживает en/ru/de (
src/i18n/{en,ru,de}.*), так что «Тексты en/ru/de» в ТЗ — не опечатка и не лишний язык. docs/TOUCH-SUPPORT.md—_pickPlanFileживёt только в Plan editor (houseplan-editor-runtime.ts), редакторы — desktop-first/best-effort, View/kiosk (блокирующая часть touch-контракта) это изменение не затрагивает. Конфликта нет, но ТЗ не проговаривает это явно (см. находки).
Гейты
Диапазон изменений — только docs/specs/039-large-backdrops.md (Class C,
0 файлов в src/**/custom_components/**). Код не менялся, поэтому:
| Гейт | Применимо? | Причина |
|---|---|---|
npx tsc --noEmit / npm test / npm run build |
нет | diff не содержит кода; сверять бандл не с чем |
node scripts/check-docs.mjs |
нет | триггерится изменениями src/**, их нет |
npm run invariants |
нет | геометрия/layout/толщина стен не затронуты |
browser-смоки, golden:verify, pytest tests_backend |
нет | нет реализации, смоки из ТЗ (demo/smoke_backdrop_guard.mjs) ещё не существуют — они появятся в коде, не в этом коммите |
Ничего не прогонял намеренно — прогон дорогих или дешёвых гейтов на
docs-only коммите ничего не проверяет по существу и не относится к предмету
ревью этапа spec.
Находки
Все находки — Medium, в скоупе задачи (правятся автором ТЗ в этом же issue, без отдельного issue). High-находок нет: ТЗ реализуемо и в основном проверяемо, ни одна находка не требует решения владельца.
M1 — нет обязательных продуктовых разделов §7.1 «сценарий» и «что человек увидит»
PROCESS.md §7.1: «Два первых раздела — продуктовые... Сценарий: какая персона, на какой поверхности, в какой момент. Что человек увидит: одной фразой, без терминов реализации. ТЗ, которое не может ответить на эти два вопроса, описывает работу, а не изменение продукта.»
Текущий документ начинается сразу с «Цель» → «Текущее состояние (анкеры кода)» — реализационный язык с первой содержательной строки. Персона (Home admin), поверхность (Plan editor, диалог создания/замены пространства) и момент (выбор файла подложки при создании/редактировании floor plan) нигде не названы явно, хотя ответ на изучение (см. «Как проверялось», п.6, 9) есть — что и делает пробел исправимым без обращения к владельцу.
Как воспроизвести: прочитать docs/specs/039-large-backdrops.md — раздел
«Сценарий» и фраза «что человек увидит до/после» без терминов реализации
отсутствуют физически.
Правка: добавить два первых раздела по шаблону §7.1.
M2 — i18n-раздел не перечисляет ключи, DoR §2.5 не закрыть
§7.1 требует раздел «i18n» в самом ТЗ; DoR §2.5 требует «ключи en + ru перечислены» как отдельный обязательный пункт. Текущий текст — одна строка внутри UX: «Тексты en/ru/de; диалог на мобильной ширине без горизонтального скролла» — ни одного имени ключа.
Три новых пользовательских текста как минимум (warn-заголовок/тело, кнопка «Загрузить уменьшенную копию», кнопка «Оставить оригинал», hard-текст с советом уменьшить файл) не имеют предполагаемых ключей — это чисто техническое решение (§7.1: «Всё, чего пользователь не наблюдает [в смысле имени ключа], агенты решают сами»), но раздел должен содержать список, не быть пустым.
Как воспроизвести: grep docs/specs/039-large-backdrops.md на \.title\|\.body\|key —
ни одного предложенного ключа.
Правка: перечислить en+ru ключи (можно предположительно, «assumed, change freely») — это разблокирует DoR-пункт.
M3 — нет разделов «Риски» и «Release-артефакты», обязательных по §7.1
§7.1 перечисляет обязательные разделы: «...план автотестов · риски ·
откат · release-артефакты». В документе есть «Откат» и «Тесты и
мутанты», но нет ни выделенного раздела «Риски» (хотя по существу риск
называн — «честная оговорка» про недоступный reference-планшет, — он не
собран в заявленный по процессу раздел и не сопровождён явной пометкой
«принято предположительно, поменять свободно», как требует §7.1 в конце
документа), ни раздела «Release-артефакты» (changelog RU+EN, документация,
golden/скриншоты, performance/security — ни один пункт не назван, хотя
User-Visible: yes почти наверняка потребуется на новый UX-диалог).
Как воспроизвести: grep заголовков ^## в файле — «Риски» и
«Release-артефакты» отсутствуют.
Правка: добавить оба раздела; риски можно перенести из «Честной
оговорки», Release-артефакты — минимум подтвердить User-Visible: yes и оба
changelog.
M4 — AC4 объединяет два разных момента наступления hard без единого UI-контракта
probeBackdrop классифицирует hard синхронно по заголовку (сторона
16384px) — до какого-либо клика пользователя, поэтому в этом случае Cancel-only диалог логично показывается сразу вместо
safe/warn. Но АС4 также включает «decode-ошибка или таймаут 10 с на этапе уменьшения» — это происходит ПОСЛЕ того, как пользователь уже увиделwarn-диалог и нажал «Загрузить уменьшенную копию» (шаг из UX-раздела, «Уменьшение:createImageBitmap... OffscreenCanvas... aspect сохраняется»). В этот момент пользователь уже закрыл первый диалог и, вероятно, видит какое-то состояние busy/loading.
ТЗ не говорит, что именно пользователь увидит при отказе на этом втором шаге: тот же Cancel-only диалог переоткрывается, показывается инлайн-ошибка поверх текущего состояния, или тост — сказано только «текущая подложка и staging не изменились» (что описывает данные, а не экран). Это ровно тот класс вопроса, который §7.1 called «продуктовый» («что человек видит или делает» в пограничном случае) и который автор обязан решить явно (не обязательно спрашивая владельца — решение внутри компетенции автора, но оно должно быть записано, а не подразумеваться).
Без этого AC4 не полностью проверяем одним однозначным тестом: тест-автор должен будет сам придумать, какой UI ожидать после ошибки на втором шаге, и разные реализации/тесты разойдутся.
Как воспроизвести: перечитать AC4 и раздел UX «hard» — оба говорят про
данные (staging/config не меняются), ни один не описывает экран после
runtime-отказа downscale, случившегося уже после клика в warn-диалоге.
Правка: одна фраза, например «повторно показывается тот же hard-диалог поверх прерванного действия» или «инлайн-ошибка в уже открытом warn-диалоге, без его закрытия» — и синхронизировать с AC4.
Что проверено и корректно
- Технические анкеры кода (base64-цикл, aspect via
<img>, MAX_FILE_BYTES, quota, staging-до-save транзакция) — точны, сверены построчно с текущимdev, не догадка. - Бенчмарк-матрица (4–165 МП) внутренне согласована, источник (headless
Chromium,
createImageBitmap+downscale) назван, ограничение (нет reference wall-tablet) явно оговорено, а не скрыто; направление ошибки консервативно (раньше предупреждаем, а не позже) — это разумная инженерная позиция, а не выданная за факт догадка. - Константы собраны в одном модуле (
src/backdrop-probe.ts) с явной целью «полевая рекалибровка = правка одного файла» — соответствует духу «assumed, change freely». - AC1–AC3, AC5–AC9 — каждый привязан к способу доказательства (unit/smoke/
ревью) и формулирует наблюдаемое поведение однозначно; для AC1 и AC9 в
тексте прямо продумано, как тест проверит «до-decode» инвариант (spy на
window.createImageBitmap, отсутствие canvas в юните) — «тест умеет падать» проверяемо уже на этапе спеки через реестр мутантов (4 пункта, каждый называет, что именно ломается). - Терминология («подложка», диалоги) соответствует
docs/USER-GUIDE.ru.md, не изобретена. - Не найдено нарушений standing rules SCOPE.md (never-delete-a-file, lock invariant — оба нерелевантны этой задаче и не задеты).
docs/BACKDROP.md(геометрия/калибровка подложки) не конфликтует — задача его не касается, что подтверждено разделом «Вне скоупа».- Раздел «Вне скоупа» и «Откат» по существу закрывают «модель данных и миграция» (эндпоинты/схема не меняются) — содержательно ОК, только не под отдельным заголовком (см. M3, куда это включено).
Чего не проверял
- Реальное поведение в браузере — реализации ещё нет, диалог/downscale не существуют в коде; нечего запускать.
demo/benchmark_backdrop_decode.mjs— упомянут как будущий файл, в репозитории отсутствует (git ls-filesне находит), не мог сверить числа бенчмарка независимо — доверился as-is как research-отчёту автора, корректность самих чисел проверке на этом этапе не подлежит (это войдёт в реализацию).- Не оценивал производительность реального downscale на low-end wall-tablet устройстве — сам автор честно называет это недоступным; ревью не может закрыть то, что недоступно и автору.
- Не проверял
test/backdrop-probe.test.mjsиdemo/smoke_backdrop_guard.mjs— они не существуют, появятся в реализации; их «умение падать» — предмет код-ревью, не этого этапа.
Вердикт
0 High, 4 Medium (все в скоупе задачи, возвращаются автору для правки ТЗ, без отдельных issue). Технический фундамент ТЗ хороший (анкеры кода точны, бенчмарк честен, AC в основном проверяемы), но структурная неполнота против чек-листа §7.1 (M1–M3) и одна реальная развилка поведения без явного решения (M4) не позволяют закрыть DoR §2.5 как есть.
Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 4 → в задаче