mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,241 @@
|
|||||||
|
# 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 → в задаче**
|
||||||
Reference in New Issue
Block a user