Files
houseplan-card/docs/reviews/SPEC-REVIEW-39-r2.md
T
2026-08-29 06:22:11 +00:00

22 KiB
Raw Blame History

SPEC-REVIEW-39-r2

  • Issue: https://github.com/Matysh/houseplan-card/issues/39 — «[HP-UX-09] большие подложки»
  • Этап: ТЗ на ревью (PROCESS.md §2.4)
  • Заход: r2 · блокирующих циклов израсходовано 1/4
  • ТЗ: docs/specs/039-large-backdrops.md, ревизия 3, SHA f787de99
  • Предыдущий вердикт: жёлтый, r1, SHA 156be645 (docs/reviews/SPEC-REVIEW-39-r1.md) — 0 High, 4 Medium (M1–M4)
  • Трек: полный (владелец, 2026-08-15: «P3, polish/tech-debt, обычный трек») — без изменений с r1

Скоуп проверки (дельта r1→r2)

Дельта локальна и полностью укладывается в границы одной правки одного файла: git diff 156be645..HEAD -- docs/specs/039-large-backdrops.md — 93 добавленных/ изменённых строки в docs/specs/039-large-backdrops.md, ничего больше (commit f787de99, Issue: #39, User-Visible: no). Никакого ребейза на ушедший вперёд dev (156be645 — прямой родитель по линии этого файла, между r1 и r2 в dev были только чужие docs/build-коммиты, не трогавшие эту подсистему). Контракт поведения не менялся с r0→r1: ревизия 3 отвечает точечно на M1–M4, новых AC не добавляла (кроме переименования AC4→AC4+AC4б, что и есть предмет M4). Полный разбор с нуля не требуется — веду по дельте согласно PROCESS.md §2.9/§2.10.

Закрытие раунда r1

Находка r1 Чем закрыта Где это видно
M1 — нет разделов «Сценарий»/«Что человек увидит» Добавлен раздел «Сценарий»: персона (владелец дома), поверхность (диалог пространства, «Файл»), момент (выбор скана плана 150–600 DPI); отдельно «До»/«После» одной фразой без терминов реализации docs/specs/039-large-backdrops.md:13–26
M2 — i18n без перечня ключей Добавлен раздел «i18n» с 7 новыми ключами backdrop.*, каждый с en+ru текстом; de оговорён как перевод тех же ключей docs/specs/039-large-backdrops.md:112–136
M3 — нет разделов «Риски»/«Release-артефакты» Оба раздела добавлены: «Риски» — 4 пункта со смягчением каждого; «Release-артефакты» — changelog RU+EN, docs/BACKDROP.md, USER-GUIDE, docs/TESTING.md, i18n, golden, performance docs/specs/039-large-backdrops.md:181–206 — закрыта частично, см. r2-M2 ниже
M4 — AC4 объединял два разных момента hard без UI-контракта hard явно разделён на «фазу 1» (синхронная, >16384 по заголовку, диалог too_large_*, только «Отмена») и «фазу 2» (асинхронная, decode-fail/таймаут 10 с после выбора «Уменьшенной копии» — тост downscale_failed, staging чист, инпут сброшен, явно нет автофолбэка на оригинал); AC разделён на 4 и 4б docs/specs/039-large-backdrops.md:94–104 (UX), :155–159 (AC) — закрыта частично, см. r2-M1 ниже

M1 и M2 закрыты полностью. M3 и M4 закрыты в части, ради которой их подняли (структура/UI-контракт), но правка потянула за собой недоделанный хвост в соседнем обязательном разделе — см. находки ниже.

Унаследовано из r1

Без повторной проверки (дельта их не касается) приняты выводы docs/reviews/SPEC-REVIEW-39-r1.md на SHA 156be645:

  • Технические анкеры кода (_pickPlanFile base64-цикл, aspect via <img>, MAX_FILE_BYTES, check_quota, транзакция staging-до-save) — сверены построчно в r1, дельта их текст не меняла.
  • Бенчмарк-матрица (4–165 МП, headless Chromium) и константы src/backdrop-probe.ts — не менялись, «честная оговорка» про недоступный reference-планшет присутствует так же.
  • AC1–AC3, AC5–AC9 (кроме переименования AC4) — не менялись, признаны проверяемыми в r1.
  • Терминология («подложка», диалоги hp-dialog) соответствует docs/USER-GUIDE.ru.md — сверено в r1, в дельте новых терминов, кроме уже проверенных ниже i18n-ключей, нет.
  • docs/BACKDROP.md (геометрия/калибровка) не конфликтует — задача его не касается.
  • Standing rules docs/SCOPE.md (never-delete-a-file, lock invariant) — нерелевантны, не задеты.
  • docs/SCOPE.md J4 («from zero to a working plan… image/PDF/draw») — задача по-прежнему укладывается в надёжность onboarding-загрузки плана, сама задача уже протриажена владельцем.

Как проверялось в этом раунде

  1. Прочитал вердикт r1 и его SHA в комментарии issue #39 (156be645, явно назван автором ревью — не пришлось восстанавливать по догадке).
  2. git diff 156be645..HEAD -- docs/specs/039-large-backdrops.md — построчно, без сокращений (см. вывод выше).
  3. Прочитал итоговый файл целиком (217 строк) — не только diff-хунки, чтобы увидеть места, где новый текст должен был согласоваться со старым (i18n ↔ UX, AC4б ↔ «Тесты и мутанты») и не согласовался.
  4. Сверил новые i18n-ключи с существующей toast-инфраструктурой: this.host._showToast уже вызывается ровно в _pickPlanFile (src/houseplan-editor-runtime.ts:8252, toast.plan_formats) — тост из этого же метода технически достижим, это не новый недоступный механизм (комментарий про «only the View card owns toast infrastructure», houseplan-card.ts:2495, относится к locale-failure по #354 и не запрещает тосты из HouseplanEditorRuntime, который вызывает this.host._showToast в этом самом методе на соседней строке).
  5. docs/specs/README.md:14–21 — канонический список обязательных release-артефактов, включая условие «release/performance/security artifacts, если они входят в acceptance gate» — сверил текущий раздел «Release-артефакты» против этого списка пункт за пунктом.
  6. Перечитал «Тесты и мутанты» (не менялся в дельте) против новых AC4/AC4б, которые дельта менял — согласованность способа доказательства с новым текстом AC.
  7. gh issue view 39 --comments — тело + все 4 комментария, включая комментарий владельца о ревизии 3 (совпадает с diff, новых устных решений не содержит).
  8. docs/SCOPE.md J4 — не переоценивал заново (см. «Унаследовано»), только убедился, что новый текст сценария не расширяет скоуп задачи за пределы onboarding-загрузки.

Гейты

Diff — только docs/specs/039-large-backdrops.md, 0 файлов в src/**/ custom_components/**. Как и в r1:

Гейт Применимо? Причина
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 нет реализации по-прежнему нет, смоки из ТЗ не существуют

Ничего не прогонял намеренно — тот же аргумент, что в r1: docs-only коммит, прогон гейтов не проверит предмет ревью этапа spec.

Находки

Обе находки — Medium, в скоупе задачи (правятся автором ТЗ в этом же issue, без отдельного issue). High-находок нет.

r2-M1 — AC4б добавлена, но «Тесты и мутанты» не обновлён под неё: способ доказательства не назван

M4 (r1) требовал явного UI-контракта для второй фазы hard — он появился (тост, сброс инпута, чистый staging, явный отказ от автофолбэка). Но раздел «Тесты и мутанты» (:169–179) остался буквально тем же текстом, что был до разделения AC4 на AC4/AC4б: смок описан одной строкой «hard-путь» без уточнения, какую из двух физически разных фаз она проверяет (синхронная классификация по заголовку до клика vs асинхронный decode-fail/таймаут 10 с после клика «Уменьшенная копия»). Мутант (3) «hard понижен до warn — смок красный» относится по формулировке только к фазе 1 (классификация по заголовку). Ни один пункт не называет способ проверки для:

  • собственно триггера фазы 2 (как смок вообще заставит createImageBitmap упасть или не уложиться в 10 с — мок глобала, форсированный reject, укороченный таймер под тестами? ничего не сказано, а 10 секунд реального ожидания в смоке — это сам по себе вопрос, который автор ТЗ обязан решить, а не тест-автор кода);
  • показа тоста именно с ключом backdrop.downscale_failed;
  • сброса поля выбора файла (упомянуто в UX как факт, но не как утверждение, которое тест обязан проверить);
  • главной новой гарантии AC4б — отсутствия автофолбэка на оригинал (текст прямо называет это осознанным решением: «молча грузить то, от чего пользователь только что отказался, нечестно» — но ни один мутант реестра не ловит регресс «фолбэк на оригинал появился»).

Это ровно тот случай, когда правка по одному замечанию (M4) способна оставить AC непроверяемым, если её не протянуть до соседнего обязательного раздела — раздел «план автотестов» из §7.1 обязателен наравне с «Сценарием» и «Release-артефактами», и для AC4б он сейчас пуст по существу (общая фраза «hard-путь» её не покрывает, потому что фаза 2 — не то же самое действие, что фаза 1, ни по триггеру, ни по результату на экране).

Как воспроизвести: прочитать :169–179 («Тесты и мутанты») рядом с :94–104 (UX) и :155–159 (AC4/AC4б) — ни слова про decode-fail/таймаут, тост, сброс инпута или мутант на «автофолбэк вернулся».

Правка: одна-две строки в «Тесты и мутанты»: назвать способ форсировать decode-fail/таймаут в смоке (мок window.createImageBitmap, ускоренный таймер под флагом теста — на усмотрение автора, но названный), добавить проверку ключа тоста и сброса инпута в описание смока, добавить пятый пункт в реестр мутантов («автофолбэк на оригинал при decode-fail — тест/смок красный»).

r2-M2 — «Release-артефакты» закрывает performance, но не называет security даже отрицательно

M3 (r1) указывал на отсутствие «ни одного пункта» performance/security. Ревизия 3 добавила явный пункт performance (demo/benchmark_backdrop_decode.mjs, вне перф-гейта CI), но пункта security нет вовсе — ни утвердительного, ни отрицательного. docs/specs/README.md:21 требует называть «release/performance/security artifacts, если они входят в acceptance gate» — условие снимает обязательность, только если её снятие сформулировано, а не просто отсутствует. Задача вводит новый парсер заголовков произвольных PNG/JPEG/WebP-файлов, пришедших от пользователя (backdrop-probe.ts, ручной разбор офсетов IHDR/SOF/VP8x) — именно тот код, для которого вопрос «а не упадёт ли парсер на специально испорченном заголовке» стандартно задаётся explicitly, а не подразумевается. Раздел «Риски» касается близкой темы («зоопарк заголовков» → unknown, не throw), но это довод про корректность, не про security-обзор конкретно.

Не берусь сказать, нужен ли здесь полноценный security-гейт (решение владельца/автора, не моё) — но ТЗ обязано явно сказать «не входит в gate, потому что…», а не промолчать, иначе DoR §2.5 «release-артефакты перечислены» не закрывается буквально: перечень сейчас неполон по своему же шаблону (перечислены changelog/docs/golden/i18n/performance, но не security).

Как воспроизвести: grep docs/specs/039-large-backdrops.md на security — ноль совпадений; docs/specs/README.md:21 требует явного решения по этому пункту.

Правка: одна строка в «Release-артефакты», например: «security: не входит в gate — parser читает только фиксированные смещения в границах уже считанного Uint8Array, без сети/eval, покрыт юнит-таблицей битых заголовков (см. „Риски“)» — или, если автор считает иначе, назвать нужный артефакт.

Что проверено и корректно

  • M1, M2 закрыты полностью и без остатка (см. таблицу выше).
  • Новый раздел «Сценарий» отвечает на оба вопроса §7.1 одной фразой каждый, без терминов реализации («вкладка замирает», а не «OOM»/«heap»).
  • Новые i18n-ключи (кроме одного отступления, см. «Чего не проверял») дают согласованные en/ru пары, используют существующую переменную-интерполяцию в духе остального проекта ({w}, {h}, {fileMb}).
  • Тост как механизм для фазы 2 технически достижим из того же метода, что уже вызывает тост сегодня (_pickPlanFile → this.host._showToast) — не выдуманный недоступный UI-примитив.
  • «Риски» — 4 пункта, каждый со смягчением, включая явную честную оговорку про недоступный reference-планшет (унаследована из r1, не потеряна при переносе в новый раздел).
  • «Откат» и «Вне скоупа» не менялись, остаются согласованными с новым текстом (ни сценарий, ни i18n, ни risks/release-артефакты не расширяют скоуп).
  • Владельцу вопросов не задано — обе новые Medium-находки (r2-M1, r2-M2) решаются автором ТЗ в рамках его компетенции, продуктового решения владельца не требуют.

Чего не проверял

  • Реальное поведение в браузере — реализации по-прежнему нет.
  • Название backdrop.unknown_title — в i18n-разделе для unknown-состояния назван только backdrop.unknown_body (:119–122), без отдельного title-ключа; текст предполагает переиспользование backdrop.large_title («Large image») для диалога о файле с нечитаемыми размерами, что смыслово немного не совпадает («не факт, что файл большой — просто не прочитались размеры»). Не поднимаю это до Medium: реализация вправе переиспользовать существующий заголовок или добавить свой — оба варианта не меняют AC6 и не блокируют DoR, это чисто техническое решение в духе «assumed, change freely» из самого М2. Отмечаю как Low, не блокирует; автор может добавить строку либо оставить как есть.
  • Возможность hp-dialog показывать 3 кнопки действия одновременно (нужно для warn-диалога: «Уменьшенную копию»/«Оставить оригинал»/«Отмена») — не нашёл готового прецедента с тремя действиями в текущем houseplan-card.ts за разумное время поиска; это UI-компонентный вопрос кода, а не текста ТЗ, и не был поднят ни в r1, ни ревизией 3 — не расширяю разбор на компонент, которого дельта не касается.
  • Точность чисел бенчмарка (4–165 МП) — не пересчитывал, как и в r1: не предмет этого этапа.
  • test/backdrop-probe.test.mjs, demo/smoke_backdrop_guard.mjs — не существуют, появятся в реализации, их «умение падать» — предмет код-ревью.

Вердикт

0 High, 2 Medium (обе в скоупе, возвращаются автору для правки ТЗ в этом же раунде, без отдельных issue). M1 и M2 из r1 закрыты полностью; M3 и M4 закрыты в своей продуктовой/UI части, но каждая потянула недоделанный хвост в соседнем обязательном разделе процесса (план автотестов для новой AC4б; security-строка в release-артефактах) — оба хвоста мелкие (одна-две строки), но DoR §2.5 буквально не закрыт, пока они не добавлены.

Вердикт: жёлтый · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 2 → в задаче