diff --git a/docs/reviews/SPEC-REVIEW-39-r3.md b/docs/reviews/SPEC-REVIEW-39-r3.md new file mode 100644 index 00000000..32d94f59 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-39-r3.md @@ -0,0 +1,173 @@ +# SPEC-REVIEW-39-r3 + +- Issue: https://github.com/Matysh/houseplan-card/issues/39 — «[HP-UX-09] большие подложки» +- Этап: ТЗ на ревью (PROCESS.md §2.4) +- Заход: r3 · блокирующих циклов израсходовано 2/4 (зелёные вердикты цикл не образуют, #227) +- ТЗ: `docs/specs/039-large-backdrops.md`, ревизия 4, SHA `9431a5ce` +- Предыдущий вердикт: жёлтый, r2, SHA `f787de99` (`docs/reviews/SPEC-REVIEW-39-r2.md`) — 0 High, 2 Medium (r2-M1, r2-M2) +- Трек: полный (владелец, 2026-08-15: «P3, polish/tech-debt, обычный трек») — без изменений + +## Скоуп проверки (дельта r2→r3) + +Дельта локальна и полностью укладывается в границы одной правки одного файла: +`git diff f787de99..9431a5ce -- docs/specs/039-large-backdrops.md` — 22 +добавленных / 7 удалённых строк, ничего больше (`git diff --stat f787de99..9431a5ce` +показывает только этот файл плюс публикацию `docs/reviews/SPEC-REVIEW-39-r2.md`, +которую я не оцениваю как предмет ревью — это артефакт прошлого раунда). +Единственный коммит в диапазоне — `9431a5ce docs: #39 spec revision 4 per +SPEC-REVIEW-39-r2` (`Issue: #39`, `User-Visible: no`). Ребейза на ушедший вперёд +`dev` не было (`f787de99` — прямой родитель по линии этого файла). Контракт +поведения не менялся: ревизия 4 отвечает точечно на r2-M1 и r2-M2, новых AC не +добавляла, новой подсистемы не задела. Объём дельты не сопоставим с исходной +задачей — веду по дельте согласно PROCESS.md §2.9/§2.10, полный разбор с нуля +не требуется. + +## Закрытие раунда r2 + +| Находка r2 | Чем закрыта | Где это видно | +|---|---|---| +| **r2-M1** — AC4б без названного способа доказательства: не назван триггер decode-fail/таймаута в смоке, не проверяется ключ тоста и сброс инпута, ни один мутант не ловит регресс «автофолбэк на оригинал» | Раздел «Тесты и мутанты» получил абзац «Hard фаза 2 в смоке»: `window.createImageBitmap` подменяется на (а) reject — путь decode-fail, (б) вечно висящий Promise — путь таймаута; названы 4 ассерта (тост содержит `_t('backdrop.downscale_failed')`, `.value` сброшен, `_spaceDialog.planFile === null`, повторный выбор открывает диалог заново) плюс счётчик обращений к upload/planFile со старым оригиналом; в реестр мутантов добавлен пункт (5) — подмена catch фазы 2 продолжением старого пути ловится смоком по этому счётчику | `docs/specs/039-large-backdrops.md:176–186` | +| **r2-M2** — «Release-артефакты» не называет security даже отрицательно, хотя вводится ручной парсер заголовков от пользователя | Добавлен пункт «security (явное решение)»: парсер читает только фиксированные оффсеты/длины уже полученного `ArrayBuffer`, ни одно поле файла не становится размером аллокации, `w×h` — арифметика с защитой от переполнения через `Number`-границы и отказом `unknown` на неправдоподобных значениях, любые исключения → `unknown`; юнит-таблица получает fuzz-набор враждебных заголовков (нулевые/гигантские длины чанков, обрезанный VP8X, SOF без длины); SVG-путь парсером не тронут | `docs/specs/039-large-backdrops.md:214–221` | + +Обе находки r2 закрыты полностью, а не частично: r2-M1 требовал по своей +формулировке «правки» назвать способ форсировать decode-fail/таймаут — «мок +`window.createImageBitmap`… на усмотрение автора, но названный» — ревизия 4 +называет ровно этот способ и для decode-fail, и для таймаута, плюс закрывает +все перечисленные в r2 пробелы (тост, сброс инпута, мутант на автофолбэк). +r2-M2 требовал одну строку с явным решением по security — ревизия 4 даёт +больше запрошенного минимума (перечисляет конкретные защитные инварианты +парсера и связывает их с юнит-фикстурами), но это не выход за скоуп: те же +фикстуры уже были обещаны разделом «Тесты и мутанты» ревизии 3, здесь их +только называют явно под заголовком security. + +## Унаследовано из r2 (и транзитивно из r1) + +Без повторной проверки — дельта их не касается: + +- Технические анкеры кода (`_pickPlanFile`, ручной base64-цикл, aspect через + ``, `MAX_FILE_BYTES`, квота, транзакция staging-до-save) — сверены + построчно в r1 (SHA `156be645`), текст этих мест дельта r2→r3 не меняла. +- Бенчмарк-матрица (4–165 МП, headless Chromium) и константы + `src/backdrop-probe.ts` (`WARN_DECODED_BYTES`, `HARD_DIMENSION`, + `DOWNSCALE_TARGET_PX`) — не менялись со ревизии 2/3. +- AC1–AC3, AC5–AC9, AC4/AC4б по своей формулировке (не по способу + доказательства) — не менялись в этой дельте, признаны проверяемыми в r1/r2. +- Раздел «Сценарий» и «До/После» (закрытие M1 из r1) и полный список i18n-ключей + en/ru (закрытие M2 из r1) — не менялись, проверены в r2 (`SPEC-REVIEW-39-r2.md`, + SHA `f787de99`). +- Тост как технически достижимый механизм из `_pickPlanFile` → + `this.host._showToast` — сверено в r2 по коду `houseplan-editor-runtime.ts:8252` + и `houseplan-card.ts:2495`; дельта r3 не меняла ни вызывающий код, ни + описание тоста, только добавила новый ключ в уже существующий канал. +- `docs/SCOPE.md` J4 и standing rules (never-delete-a-file, lock invariant) — + нерелевантны/не задеты, сверено в r1 и r2, дельта r3 текст сценария и скоупа + не трогала. +- Терминология («подложка», диалоги `hp-dialog`) соответствует + `docs/USER-GUIDE.ru.md` — сверено в r1, новых терминов в дельте r3 нет. +- Low-находки r2 («чего не проверял»): возможное несоответствие + `backdrop.unknown_title`/`large_title` и наличие ли у `hp-dialog` готового + сценария с тремя кнопками действия — оставлены открытыми решением + ревьюера r2 (не блокируют DoR), дельта r3 их не касалась, повторно не + поднимаю. + +## Как проверялось в этом раунде + +1. Нашёл вердикт r2 и его SHA в опубликованном документе + `docs/reviews/SPEC-REVIEW-39-r2.md:6` (`f787de99`, назван явно) и в + комментарии issue от 2026-08-29T06:22:04Z — совпадают. +2. `git diff f787de99..9431a5ce -- docs/specs/039-large-backdrops.md` + построчно (полный вывод см. выше в «Скоуп проверки»). +3. `git diff --stat f787de99..9431a5ce` — подтвердил, что дельта не тронула + ничего вне `docs/specs/039-large-backdrops.md` (второй файл в диапазоне — + публикация прошлого документа ревью, не предмет этого раунда). +4. Прочитал итоговый файл целиком (232 строки), не только diff-хунки — сверил + новый абзац «Hard фаза 2 в смоке» и мутант (5) против UX-контракта фазы 2 + (`:99–104`) и AC4б (`:157–159`): триггер, тост, сброс инпута и «нет + автофолбэка» совпадают дословно с тем, что описано в UX и AC. +5. Сверил новый security-абзац (`:214–221`) с разделом «Диагностика: заголовки, + не decode» (`:75–85`) и «Риски» (`:194–197`, «Зоопарк заголовков») — + противоречий нет, детали (фиксированные оффсеты, отказ в `unknown`) + согласуются, а не дублируют другим числом. +6. `gh issue view 39 --json comments` — прочитал все 6 комментариев целиком, + включая комментарий владельца о ревизии 4 (совпадает с diff, новых устных + решений или продуктовых вопросов не содержит). +7. `docs/specs/README.md` (список обязательных release-артефактов) — сверил, + что после правки перечень «changelog / docs / i18n / golden / performance / + security» закрыт по всем пунктам буквально, а не молчанием. +8. Проверил метки issue (`gh issue view 39 --json labels`): `S4-spec-review`, + `P3`, `polish`, `tech-debt` — трек по-прежнему полный, файл ТЗ обязателен + (не `small`), что и наблюдается. + +## Гейты + +Diff — только `docs/specs/039-large-backdrops.md` (плюс публикация прошлого +review-документа), 0 файлов в `src/**`/`custom_components/**`/`test/**`/ +`demo/**`. Как в r1 и r2: + +| Гейт | Применимо? | Причина | +|---|---|---| +| `npx tsc --noEmit` / `npm test` / `npm run build` | нет | диапазон не содержит кода | +| `node scripts/check-docs.mjs` | нет | `src/**` не тронут | +| `npm run invariants` | нет | геометрия/`layout`/толщина стен не затронуты | +| browser-смоки, `golden:verify`, `pytest tests_backend` | нет | реализации по-прежнему нет; смоки/юниты из ТЗ (`test/backdrop-probe.test.mjs`, `demo/smoke_backdrop_guard.mjs`) не существуют | + +Намеренно ничего не прогонял: предмет этапа `spec` — текст ТЗ, а не код, и +прогон гейтов над docs-only диапазоном не проверил бы ничего по существу. + +## Находки + +Нет. High: 0. Medium: 0. Обе находки r2 закрыты полностью (см. таблицу выше), +новых Medium/High дельта не вносит. + +## Что проверено и корректно + +- r2-M1 закрыта: способ форсирования decode-fail (`reject`) и таймаута + (вечно висящий `Promise`) назван явно через мок `window.createImageBitmap`; + все 4 ранее не названных ассерта (ключ тоста, сброс инпута, чистый staging, + повторный выбор) теперь в тексте; мутант (5) ловит именно ту регрессию, + ради которой AC4б вводился («молча грузить оригинал, от которого + отказались, — нечестно»). +- r2-M2 закрыта: security-решение явное, содержательное (fixed-offset parsing, + без file-controlled allocation size, fail-closed в `unknown`, fuzz-таблица) + и не противоречит уже описанной диагностике и рискам. +- Новый текст согласован с остальным документом: UX-контракт фазы 2 + (`:99–104`), AC4б (`:157–159`) и обновлённые «Тесты и мутанты» (`:176–186`) + описывают одно и то же поведение одними и теми же терминами (тост, + сброс инпута, staging), расхождений в формулировках нет. +- «Откат», «Вне скоупа», i18n-список и «Риски» дельтой не менялись и остаются + согласованными с новым текстом. +- Владельцу вопросов не задавалось ни в этом, ни в предыдущих раундах — обе + находки r2 были техническими и решены автором ТЗ в своей компетенции. +- DoR §2.5 по совокупности трёх ревизий (r1→r4) закрыт буквально: сценарий и + «до/после» есть, AC1–AC9(+4б) пронумерованы с указанием способа + доказательства, i18n-ключи en/ru перечислены явно, риски и release-артефакты + (включая security) названы, откат — один revert без миграции конфига. + +## Чего не проверял + +- Реальное поведение в браузере — реализации по-прежнему нет, `test/backdrop-probe.test.mjs` + и `demo/smoke_backdrop_guard.mjs` не существуют; их «умение падать» — + предмет код-ревью (AC4б, мутант 5 в частности). +- Точна ли оценка «10 с таймаут» будет действительно проверяться смоком без + реального 10-секундного ожидания в CI: текст называет механизм (мок + глобала на вечно висящий `Promise`), но не говорит, укорачивается ли сам + таймер реализации под тестовым флагом. Это не было отдельно + сформулировано как обязательное требование в «Правке» r2-M1 (там был + предложен выбор «мок глобала **или** ускоренный таймер — на усмотрение + автора, но названный»), и названный вариант формально закрывает находку. + Оставляю Low, не блокирует: это решение исполнителя о скорости прогона + смока, не о корректности AC4б, и он вправе сам выбрать — ускорить таймер + под тестовым флагом или мириться с 10-секундным ожиданием в смоке. +- Low-находки, унаследованные из r2 (`backdrop.unknown_title` vs переиспользование + `large_title`; поддержка `hp-dialog` трёх кнопок действия) — не переоценивал, + дельта их не касалась, решение реализации остаётся открытым по тем же + причинам, что в r2. +- Точность чисел бенчмарка (4–165 МП) — не пересчитывал, как и в r1/r2. + +## Вердикт + +0 High, 0 Medium. Обе Medium-находки r2 (r2-M1, r2-M2) закрыты полностью и +без остатка, новых блокирующих находок дельта r2→r3 не вносит. ТЗ (ревизия 4) +удовлетворяет §7.1 и DoR §2.5 по совокупности всех четырёх ревизий. + +**Вердикт: зелёный · заход r3 · блокирующих циклов 2/4 · High: 0 · Medium: 0**