diff --git a/docs/reviews/SPEC-REVIEW-509-r2.md b/docs/reviews/SPEC-REVIEW-509-r2.md new file mode 100644 index 00000000..df3d40d9 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-509-r2.md @@ -0,0 +1,197 @@ +# SPEC-REVIEW-509-r2 + +Issue: [#509](https://github.com/Matysh/houseplan-card/issues/509) — «Сводная +панель: при первом показе подвисание и «Source unavailable» вместо значений +до загрузки metrics-чанка». +Этап: spec (PROCESS.md §2.4). Заход: r2. Блокирующих циклов израсходовано +1 из 4 (потрачен жёлтым вердиктом r1; зелёный цикла не образует, #227). + +Материал: тело issue #509, раздел `## ТЗ`, редакция после комментария автора +`2026-09-10T08:07:55Z` («ТЗ r2 — обе находки закрыты»). Предыдущий заход — +[SPEC-REVIEW-509-r1.md](https://github.com/Matysh/houseplan-card/blob/dev/docs/reviews/SPEC-REVIEW-509-r1.md), +материал того раунда: ветка `dev` `d204bf5d7737`, дерево `cf36bd3b2a3141c427a40f9e10aac607faf02a1f`, +тело issue `sha256 8d514ac196bbc453d5b25b352421dc27f9925b8668a89e44c00971f304f2c3e4`, +вердикт `yellow · High 0 · Medium 2`. + +## 1. Скоуп ревью + +Заход r2, не первый — разбор по дельте (PROCESS.md §2.10). Дельта не +затрагивает ребейз (кода по задаче ещё нет вовсе), не меняет контракт целиком +и не задевает новую подсистему — она ровно закрывает три находки r1. Разбор +поэтому сокращён до дельты плюс всего, до чего эта дельта дотягивается. + +**Объявление дельты** (сверка текущего тела с текстом, разобранным в r1): + +- в преамбулу `## ТЗ` добавлено предложение с явным критерием §5, который + задача не проходит; +- в конец ТЗ добавлен раздел «Принятые технические предположения» (4 пункта); +- контракт п.4 переформулирован: вместо «микрозадача/`requestIdleCallback` с + фолбэком на таймер» — точный механизм `setTimeout(0)` → `requestAnimationFrame` + → `setTimeout(0)` со ссылкой на новый раздел допущений и на новый AC8; +- добавлены **AC8** (бюджет времени от скелета до значения) и **AC9** + (прежнее значение при инвалидации мемо, без скелета); +- строка таблицы «Чем краснеет» для защиты «прежнее значение при инвалидации» + теперь указывает на AC9 (в r1 эта строка на AC не ссылалась — сама защита + не была доказана ни одним AC). + +Остальной текст (Сценарий, Что человек увидит, Не-скоуп, AC1–AC7, Риски, +Откат, Release-артефакты, персона, i18n) не менялся — эти AC и утверждения +наследуются из r1 без повторной проверки (см. §4 ниже). + +## 2. Как проверялось + +- Текущее тело issue #509 получено `gh issue view 509 --json body,comments,labels,state`. +- Полностью прочитан `docs/reviews/SPEC-REVIEW-509-r1.md`, включая блок + «Материал раунда». +- Прочитан комментарий-закрытие автора (`2026-09-10T08:07:55Z`). +- Каждая из трёх находок r1 (Medium-1 из трёх пунктов, Medium-2, Low-1) + сверена построчно с новым текстом — не принята на слово по заявлению автора + «обе находки закрыты» (см. таблицу §3): по каждому пункту процитирована + конкретная строка ТЗ, которая его закрывает, а не факт наличия нового + раздела как такового. +- Проверено, что новый раздел «Принятые технические предположения» оформлен + в формате, который требует PROCESS.md §7.1 («принято предположительно, + поменять свободно»), а не как скрытая догадка, поданная за решение. +- Проверено `docs/USER-GUIDE.ru.md` (раздел «Сводная панель», строки + 255–294) — новый визуальный термин «скелет»/пульсирующий прямоугольник не + вводит нового пользовательского текста и не расходится с зафиксированной + терминологией: раздел вообще не описывает переходные состояния загрузки, + конфликта нет; заявление ТЗ «новых i18n-ключей нет» по-прежнему верно + (текст не добавляется, только визуальный плейсхолдер). +- Подтверждено, что кода по задаче всё ещё не существует: `git ls-remote + --heads origin 'issue/509-*'` — пусто; `HEAD` = `57f38fed` («docs: review + document for #509», т.е. коммит публикации r1, не код). Гейты + `typecheck`/`test`/`build`/смоки на этом этапе неприменимы — как и в r1. +- Подтверждено, что метки issue не изменились: `bug`, `S4-spec-review`, без + `small` — полный трек по-прежнему корректен и теперь явно обоснован в + тексте (закрытие Low-1). + +## 3. Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **Medium-1**, п.1 — механизм переноса расчёта вне кадра назван как альтернатива («микрозадача/`requestIdleCallback`»), а не решение; ни один AC не ограничивал время до появления значения | Механизм назван однозначно: `setTimeout(0)` → `requestAnimationFrame` → `setTimeout(0)`; `requestIdleCallback` явно отвергнут с причиной (откладывается на неопределённое время под нагрузкой); время ограничено новым AC | Раздел «Принятые технические предположения», п.1 «Момент расчёта…»; Контракт п.4 (переформулирован); **AC8** | +| **Medium-1**, п.3 — форма хранения «прежнего значения» при инвалидации мемо не названа | Названа: «stale-while-revalidate» — мемо не сбрасывается при инвалидации, старое значение остаётся на экране до завершения фонового пересчёта | Раздел «Принятые технические предположения», п.2 «Stale-while-revalidate»; согласуется с новым **AC9** | +| **Medium-1**, п.2 — способ технической инъекции счётчика вызовов для AC3 не назван ни как решение, ни как «на усмотрение реализации» | **Не закрыт текстом ТЗ.** Формулировка AC3 не изменилась («унит со счётчиком вызовов через инъекцию»). Разобран отдельно ниже как Low-2 и снят мной с записью — см. §5 | — | +| **Medium-2** — защита «при инвалидации мемо панель показывает прежнее значение, а не скелет» не доказана ни одним AC1–AC7 | Новый **AC9** прямо проверяет два состояния: мемо устарело → `ready` со старым значением + запланированный пересчёт; мемо нет → `pending`. Мутация «мемо сбрасывается в скелет» теперь имеет названного свидетеля | **AC9**; таблица «Чем краснеет», строка «прежнее значение при инвалидации» → «unit (AC9)» | +| **Low-1** — критерий §5, который задача не проходит, не назван явно при выборе полного трека | Назван дословно: «Критерий §5, который задача не проходит: «нет влияния на производительность и бюджеты» — здесь оно и есть предмет задачи» | Преамбула `## ТЗ`, сразу после заголовка | + +## 4. Унаследовано из r1 + +Без повторной проверки принимается всё, чего дельта r2 не касается — со +ссылкой на [SPEC-REVIEW-509-r1.md](https://github.com/Matysh/houseplan-card/blob/dev/docs/reviews/SPEC-REVIEW-509-r1.md), +материал: `dev` `d204bf5d7737` / дерево `cf36bd3b2a3141c427a40f9e10aac607faf02a1f` +/ тело issue `sha256 8d514ac196bbc453d5b25b352421dc27f9925b8668a89e44c00971f304f2c3e4`: + +- фактические утверждения ТЗ о текущем коде, перепроверенные там чтением + построчно: `LoadedSummaryPanelRuntime.value()` (`summary-panel-runtime-loaded.ts:715-720`), + `metrics()` (`:687-712`), `totalCleanFloorAreaM2` (`summary-panel-metrics.ts:59-84`), + `innerContourForRoom` (`wall-thickness.ts:2750-2779`), приём общей геометрии + карточкой через `_innerRoomContour`/`_wallUnionGeometry()` + (`houseplan-card.ts:9219-9240`), существование и подключение + `SummaryPanelPresentation` (#505, закрыт и смёржен), реальность i18n-ключа + `summary.unavailable` на 4 языках; +- соответствие персоны и поверхности `docs/SCOPE.md` (Household members, + View/kiosk, J1) и отсутствие конфликта со SCOPE; +- присутствие обязательных разделов §7.1 по существу (таблица r1, §2); +- техническая реализуемость AC3 в части передачи общей геометрии ( + `innerContourForRoom` уже принимает shared-параметры как опциональные — + инженерная перестановка вызовов, а не гипотеза); +- отсутствие иных незаявленных вопросов владельцу — единственный вопрос был + закрыт дефолтом («вариант A») ещё до r1; +- оценка рисков (флейк порога AC4, единичные кадры «недоступности» для + entity-источников) и отката (правка локальна для `summary-panel-*`) — + дельта их не касается. + +Эти пункты не перепроверялись повторно, так как дельта r2 не вносит правок в +описываемый ими код или текст. + +## 5. Находки (r2) + +### Low-2 (снимается записью) — способ инъекции счётчика вызовов для AC3 остался неназванным + +AC3 по-прежнему формулирует доказательство как «унит со счётчиком вызовов +через инъекцию», не уточняя механизм, хотя `wallBodiesGeometry` и +`multiWallNodesForGeometry` — обычные экспортируемые функции ES-модуля, а не +параметры конструктора или поля объекта (это же отмечено в Medium-1 r1, +п.2). Формально пункт 2 находки r1 не закрыт: раздел «Принятые технические +предположения» решает пункты 1 и 3, но не пункт 2. + +**Почему я снимаю это, а не возвращаю жёлтым.** Способ подсчёта вызовов — +чистая стратегия теста, а не решение, которое видит пользователь или которое +меняет контракт поведения хотя бы одного AC; PROCESS.md §7.1 прямо относит +«стратегию тестов» к тому, что «агенты решают сами». Технически это +стандартный приём (`vi.spyOn` на неймспейс модуля `wall-thickness.ts` при +сохранении остальной реализации, либо `vi.mock` с `importOriginal` и +частичной заменой двух экспортов) — он не требует нового паттерна в +продуктовом коде и не был поставлен под сомнение как нереализуемый ни в r1, +ни сейчас. Формальный пробел реален, но не блокирует ни имплементируемость, +ни проверяемость AC3 по существу, поэтому не стоит третьего цикла ревью +ради одной фразы в способе доказательства. Автору стоит при реализации явно +выбрать один из вариантов (`vi.spyOn` на неймспейс либо обёртку с DI) — +запись здесь фиксирует это как открытый, но не блокирующий пункт. + +## 6. Что проверено и корректно + +- Medium-1 (пп.1, 3) и Medium-2 закрыты по существу: не косметическим + дополнением текста, а конкретными новыми AC (AC8, AC9), встроенными и в + контракт (п.4 ссылается на AC8 и на раздел допущений), и в таблицу «Чем + краснеет» — а не оставлены висеть отдельно от AC, как предупреждал + прецедент Medium-1 r1. +- Low-1 закрыт дословной формулировкой критерия §5, без домысливания. +- Новый раздел «Принятые технические предположения» оформлен ровно в формате + PROCESS.md §7.1 — «принято предположительно, менять свободно, ревьюер + вправе оспорить» — а не как факт о будущем коде, поданный без пометки. +- Ширина скелета (≈4.5em) — единственная новая деталь в разделе допущений, + которая формально видна пользователю (ширина плейсхолдера), но это не + продуктовый вопрос по смыслу §7.1 (не «что человек видит и делает» на + уровне сценария, а стилевая деталь уже согласованного визуального + решения), и она прямо помечена как оспоримое предположение — эскалации + владельцу не требует. +- Никакой новой догадки, поданной как факт вне пометки, в дельте не найдено. +- Терминология не расходится с `docs/USER-GUIDE.ru.md` — раздел «Сводная + панель» не документирует переходные состояния, конфликтовать нечему; новых + i18n-ключей действительно нет. +- Метки issue (`bug`, `S4-spec-review`, полный трек) согласованы с текстом. +- Мелкая редакционная деталь: `AC8` в списке идёт до `AC7` (правка вставила + новые пункты перед последним исходным) — это не влияет на уникальность + номеров и не порождает двусмысленности, поэтому не считаю это находкой. + +## 7. Чего не проверял + +- Не перепроверял чтением код `value()`, `metrics()`, `totalCleanFloorAreaM2`, + `_innerRoomContour`, `SummaryPanelPresentation`, ключ `summary.unavailable` + — дельта r2 не меняет ни одного утверждения об этих местах кода, инвентарь + r1 наследуется (§4). +- Не прогонял `typecheck`/`test`/`build`/смоки/`golden`/`performance_smoke` — + кода по задаче нет вообще (ветка `issue/509-*` не создана), эти гейты + относятся к этапу `code`, а не `spec`. +- Не проверял синтаксическую реализуемость `setTimeout(0)` → + `requestAnimationFrame` → `setTimeout(0)` в конкретном классе рантайма — + это стандартные браузерные API, а конкретная реализация ещё не написана. +- Не оценивал качество будущих мутационных гардов таблицы «Чем краснеет» + сверх того, что они названы и привязаны к AC — код появится при + реализации. +- Не проверял механизм подсчёта `sha256` тела issue в блоке якорей + документа — инфраструктура конвейера, не предмет ревью ТЗ. + +## 8. Вердикт + +High: 0. Medium: 0 (обе находки r1 закрыты по существу новыми AC/разделом). +Low: 1 (Low-2 из этого раунда — снят записью в §5; Low-1 r1 закрыт автором). + +Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0 + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `57f38fedbaed` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `614aeecffc19c8ec89e25e908dafb13556a0f286` + ``` + git log --all --format='%H %T' | grep 614aeecffc19 + ``` +- Тело issue: `6224b491f7bef6e6962692c3c901bf5121432fd1e0244740514a420aff96ba36` +- Вердикт конвейера: `green` · High 0