Files
houseplan-card/docs/reviews/SPEC-REVIEW-39-r1.md
2026-08-29 06:14:09 +00:00

20 KiB
Raw Permalink Blame History

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, SHA 156be645
  • Трек: полный (владелец, 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 нет — ревью касается только текста ТЗ.

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

  1. docs/SCOPE.md — сценарий укладывается в надёжность onboarding-загрузки плана (соседствует с J4), сама задача уже протриажена владельцем как valid tech-debt (комментарий от 2026-08-14), продуктового конфликта со стандартными правилами (never-delete-a-file, lock invariant) нет.
  2. AGENTS.md, PROCESS.md §2.3–2.5, §7.1 — состав обязательных разделов ТЗ и критерии DoR.
  3. Issue #39: тело + оба комментария (аналитика 2026-08-14, актуализация 2026-08-29). Вопросов к владельцу автор не поднимал («Вопросы: нет»).
  4. docs/USER-GUIDE.ru.md — сверил терминологию: «подложка», «Редактор подложки», диалоги hp-dialog — используются в ТЗ корректно, не изобретены.
  5. docs/CANVAS.md → обнаружил ссылку на docs/BACKDROP.md (канонический документ калибровки/размещения подложки на канвасе, не входит в список в промпте, но напрямую относится к подсистеме). Прочитан: описывает plan_x/y, scale, rotation — геометрию подложки, а не upload/decode. Задача этой геометрии не касается («Вне скоупа» ТЗ подтверждает), конфликта нет.
  6. Сверка технических якорей ТЗ с текущим кодом (раздел «не додумывается — выносится» требует, чтобы заявленные факты о коде были фактами, а не догадкой):
    • _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» — это не догадка, а верно прочитанный существующий код. Вывод: технические анкеры ТЗ точны, автор не выдаёт предположение за факт там, где дело касается существующего кода.
  7. Сверка ревизии 1 → 2 (git show 156be645^:docs/specs/039-large-backdrops.md vs текущая) — чтобы не приписать ревизии 2 разрыв, унаследованный от ревизии 1. Структурные пробелы (см. находки ниже) присутствовали уже в ревизии 1 — не новый регресс, но это первый ревью документа, поэтому они предъявляются сейчас.
  8. i18n: подтвердил, что проект поддерживает en/ru/de (src/i18n/{en,ru,de}.*), так что «Тексты en/ru/de» в ТЗ — не опечатка и не лишний язык.
  9. 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 → в задаче