diff --git a/docs/reviews/SPEC-REVIEW-375-r1.md b/docs/reviews/SPEC-REVIEW-375-r1.md new file mode 100644 index 00000000..e9469fd0 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-375-r1.md @@ -0,0 +1,206 @@ +# SPEC-REVIEW-375-r1 + +Issue: [#375](https://github.com/Matysh/houseplan-card/issues/375) — Glow в space-card: static-путь потерял кэш-иерархию полной карты +Этап: spec (PROCESS.md §2.4) · Трек: `small` (лёгкий) · Заход: r1 · блокирующих циклов израсходовано 0 из 2 +ТЗ: тело issue #375 (лёгкий трек — файл в `docs/specs/` не создаётся) + +## Скоуп ревью + +ТЗ описывает четыре точечных фикса кэш-иерархии Glow в static-пути +`houseplan-space-card`, введённом #374 (`0dfc7424`): + +- **К1 (V6a)** — `src/glow-scene.ts:177`: убрать `[...input.devices]`, чтобы + `resolvedLightSources` снова попадал в `RESOLVED_LIGHT_CACHE` (`WeakMap` по + идентичности массива, `src/devices.ts:426`). Регресс задевает **и полную + карту**, не только static. +- **К2 (V6b)** — `cachedStaticWallGeometry` (`src/space-render.ts`) должен + прикреплять `sourceFingerprint` тем же способом, что и `_wallUnionGeometry` + полной карты (`src/houseplan-card.ts:8458`), чтобы условие recut-ветки в + `src/glow-scene.ts:319` когда-либо становилось true. +- **К3 (V6c)** — `cachedStaticLightBarriers` (`src/space-render.ts:113`) + переводится с одиночной записи на LRU-8 по fingerprint, паритет с + `_lightBarrierPool` полной карты (`src/houseplan-card.ts:1815,10267,10296`). +- **К4 (V6d, Low)** — блок `enabledClip` (`src/space-render.ts:654-671`) + получает кэш с bbox-префильтром extras, паритет с `_cleanFloor` + (`src/houseplan-card.ts:9383-9401`). + +Поверхности — `src/glow-scene.ts`, `src/space-render.ts`, юниты +glow-scene/space-render, перф-контракты space-glow. UI, i18n, конфиг-схема не +задеты. Аналитика (§2.2) выполнена владельцем в том же issue, трек `small` +выставлен и обоснован ("одна связная подсистема, продуктовых решений нет, +видимых изменений UI нет"). + +## Как проверялось + +Ручного тестирования на этапе spec нет; проверка — сверка каждого утверждения +ТЗ с текущим деревом `dev` (`4eede5bc`), построчно по названным файлам и +номерам строк, плюс сверка ссылок на тесты/бюджеты/issue на существование. + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (жизненный + цикл, §5/§5.1 лёгкий трек, §7.1 обязательные разделы, §4 лимит циклов, + формат вердикта). +2. Прочитано тело issue #375 и комментарий аналитики целиком. +3. Прочитан канонический `docs/LIGHT.md` (раздел «Caching», «Which surfaces + render pools») — модель кэш-иерархии, которую ТЗ восстанавливает для + static-пути, соответствует канону. +4. **К1**: `src/glow-scene.ts:177` — подтверждён `[...input.devices]`. + `src/devices.ts:426,602-606,693-696` — подтверждён `RESOLVED_LIGHT_CACHE = + new WeakMap()`, ключ `devices as object` — то есть кэш + действительно промахивается на новом массиве каждый вызов. Регрессия + полной карты (не только static) подтверждена: обе карты проходят через + `resolveGlowCandidates`. +5. **К2**: `src/glow-scene.ts:317-321` — подтверждено условие `recut = + input.sharedWallGeometry && sharedFingerprint === revision.geometryFingerprint + ? recutWallBodiesGeometry(...) : null`. `src/houseplan-card.ts:8455-8461` — + подтверждено, что полная карта прикрепляет `sourceFingerprint` через + `Object.defineProperty(value, 'sourceFingerprint', { value: + contentFingerprint([this._curSpaceCfg, this._cellCm, this._gridPitch]), + enumerable: false })`. `src/space-render.ts:501-510` — подтверждено, что + static-билдер `cachedStaticWallGeometry` строит `wallBodiesUnionPath(...)` + и возвращает его **без** этого свойства → `sharedFingerprint` всегда + `undefined`, recut-ветка в static действительно недостижима. Сверена и + тройка: `resolveLightBarrierRevision({ rawSpaceConfig: spCfg, … })` в + `src/space-render.ts:557-558` использует ту же переменную `spCfg` + (`src/space-render.ts:271`), какую ТЗ предлагает передать в + `contentFingerprint` для `sourceFingerprint` — фингерпринты действительно + совпадут по содержимому, когда геометрия не менялась. Технической ошибки в + предложенном дизайне К2 не найдено. +6. **К3**: `src/space-render.ts:113-129` — подтверждена одна запись на + `spaceId` (`Map`, без LRU). Полная карта: + `src/houseplan-card.ts:1815` (`_lightBarrierPool = new Map(...)`), + `:10267` (`lruRead`), `:10296` (`lruWrite(..., 8)`) — LRU-8 подтверждена. + Отдельно проверена оговорка про `cachedStaticPhysicalBodiesCache` + (`src/space-render.ts:95-110`, ключ `physicalFingerprint` на + `src/space-render.ts:285-294`: `rooms, partitions, columns, cellCm, + hostedOpenings` — состояние contact-сенсора туда не входит) — довод + ревьюеру принят, ping-pong действительно невозможен на этом кэше, К3 + корректно его не трогает. +7. **К4**: `src/space-render.ts:654-671` — подтверждено отсутствие кэша: + `innerContourForRoom` + `floorMinusBodies` + `islandsOf` пересчитываются на + каждый вызов рендера, если не все комнаты room обладают glow. Bbox-префильтр + полной карты подтверждён в `src/houseplan-card.ts:9383-9393` (`_cleanFloor`). +8. Сверены все ссылки на тесты/бюджеты плана автотестов: `test/glow-scene. + test.mjs` существует; `test/space-render.test.mjs` — новый файл (нормально, + раскладка файлов — решение исполнителя); `test/source-fingerprint.test.mjs` + — существующий паттерн проверки `sourceFingerprint`/recut для полной карты, + на который ссылается AC2; `demo/smoke_glow_blending.mjs`, + `demo/smoke_space_card.mjs` существуют; `cacheGrowth.glowClip` — реальный + ключ бюджета (`demo/performance/budgets-large-space-card-{default,glow}.json`, + `budgets-space-glow-smoke.json`); профили `space-default`/`space-glow` — + реальные записи матрицы `full-performance` (`test/performance-workflow. + test.mjs:38-39`, `.github/workflows/performance.yml`). +9. Проверена ссылка `#376д` (§6, отложенная стейл-документация) — issue #376 + существует, тема совпадает («Пачка Low из аудита beta.4 …, стейл-док … + (а–е)»). Не фабрикация. +10. Прогонов гейтов не делал — на этапе spec-review это не требуется (нет + кода для тестирования, только текст ТЗ). Отчёт по code-review дальше + ответит на вопрос «работает ли», когда код появится. + +## Находки + +### [Medium, в скоупе] Отсутствует обязательный раздел «Откат» + +Шаблон лёгкого трека (`AGENTS.md`, §5 `PROCESS.md`): «ТЗ пишется в теле issue +по шаблону: **проблема · контракт · AC1…ACn с доказательством · откат**». +DoR-чеклист (`PROCESS.md` §2.5) отдельно требует: «**откат**: как выключить +или вернуть назад (флаг Labs, обратная миграция)» и помечает пункт как +обязательный («если хоть один пункт не выполнен — статус не «Готово к +разработке»»). + +В ТЗ #375 такого раздела нет вообще — ни явного заголовка, ни фразы про +откат в §5 «Риски» или где-либо ещё. Прецедент в этом же репозитории +показывает, что это активно проверяемый пункт даже для тривиальных +кэш-правок: `SPEC-REVIEW-366-r1` (тоже `small`, тоже правка кэш-ключа) +содержит «Откат описан верно и достаточен: один revert, кэш +самоинвалидируется», `SPEC-REVIEW-363-r1` — «откат, release-артефакты — все +присутствуют в теле issue». + +Задача не меняет конфиг, не вводит Labs-флаг и не имеет миграции — поэтому +ответ тривиален («обычный revert коммита, откатывать нечего кроме кода»), но +он должен быть написан явно, а не додуман ревьюером за автора. Без этого +раздела issue формально не может уйти в `S5-ready` даже при зелёном ревью. + +**Воспроизведение**: тело issue #375, раздел «ТЗ» — заголовки 1–7 +(Сценарий … Принятые предположения); ни один не содержит слова «откат». + +### [Low, не блокирует] DoR-пункты «миграция/compatibility» и «touch» не проговорены явно + +`PROCESS.md` §2.5 требует по каждому явно закрыть «миграция и +compatibility-поля решены по `docs/CONFIG-COMPATIBILITY.md`» и «влияние на +touch по `docs/TOUCH-SUPPORT.md`» — при отсутствии влияния ожидается явное +«нет», а не молчание. В ТЗ есть явное «i18n: не задето», но для +миграции/compatibility и touch аналогичной фразы нет. Учитывая, что диапазон +правки — исключительно `glow-scene.ts`/`space-render.ts` (внутренняя +геометрия и кэш, без UI/жестов/конфиг-схемы), ответ в обоих случаях +очевидно «нет», и это не самостоятельный риск. Достаточно добавить по одной +строке вместе с фиксом раздела «Откат» — отдельного цикла ревью это не стоит +и решением ревьюера не блокирует зелёный вердикт после исправления Medium. + +### [Low, снято ревьюером] Заявленная сложность 4/10 против порога `small` ≤3 + +Комментарий-аналитика (§2.2) сам называет «Сложность/риск: 4/10», что выше +порога `small` («сложность и риск ≤ 3», `PROCESS.md` §5), но не называет это +как нарушенный критерий — просто утверждает «критерии §5 проходит». +Формально это внутреннее противоречие того же комментария. Снимаю без +требования правки: комментарий подписан владельцем (`authorAssociation: +OWNER`), назначение статусной метки — по процессу и есть то самое «явное +решение владельца» (`AGENTS.md`, «Присвоение метки и есть то самое явное +решение»), а по факту объём — четыре точечных фикса в двух смежных модулях с +готовым прецедентом реализации (полная карта), что ближе к 2–3, чем к 4. +Фиксирую для прозрачности, действия не требую. + +## Что проверено и корректно + +- Все четыре диагноза (а–d, здесь К1–К4) построчно подтверждены чтением + текущего кода `dev` — ни один не является догадкой, выданной за факт; + каждая ссылка на файл/строку/тест/бюджет существует и означает то, что + написано в ТЗ. +- AC1–AC4 пронумерованы, проверяемы, у каждого указан способ доказательства + (юнит) и явно оговорено требование «доказательство должно падать» при + откате фикса — это то самое «тест умеет падать», которое требует код-ревью + дальше. +- AC5 корректно ссылается на существующие smoke/perf-артефакты + (`smoke_glow_blending`, `smoke_space_card`, `cacheGrowth.glowClip`, + `space-default`/`space-glow`). +- AC6 (мутанты м1/м2) сформулирован конкретно и привязан к AC1/AC3, что + снимает риск теста, который «не умеет падать» — задел на код-ревью. +- К2 технически корректен: `rawSpaceConfig` (`spCfg`) для static совпадает + по роли с `_curSpaceCfg` полной карты, фингерпринты совпадут по + содержимому при неизменной геометрии — recut-ветка станет достижимой. +- К3 корректно исключает `cachedStaticPhysicalBodiesCache` из скоупа — + причина (чисто геометрический ключ, без состояния contact-сенсора) + подтверждена чтением, ping-pong на этом кэше физически невозможен. +- «Принятые предположения» (§7) присутствуют отдельным блоком, как требует + §7.1 — ёмкость LRU 8 и включение К4 в этот же issue названы явно и + открыты для оспаривания. +- Продуктовая рамка: задача не расширяет и не сужает SCOPE.md — это + восстановление уже принятой (в #374) кэш-модели для существующей опции, + видимых изменений нет, что и заявлено в «Сценарий»/«Контракт». +- Ссылка на внешний аудит (`AUDIT-2026-08-29-beta169-4.md`) не находится в + репозитории (ожидаемо — рабочий документ вне дерева), но это не влияет на + вывод: каждое утверждение перепроверено независимо чтением кода, а не + взято на веру из аудита. + +## Чего не проверял + +- Не прогонял `tsc`/`test`/`build` — на этапе spec-review кода ещё нет, + прогон бессмыслен; за это отвечает код-ревью. +- Не проверял численные замеры производительности из «Сути» (0.11 → 0.92 + мс/вызов, ×8 на 200 устройствах) — это иллюстрация регресса, а не AC; + фактическую цифру перепроверит код-ревью через `demo/benchmark_glow.mjs`. +- Не оценивал реализуемость LRU-структуры К3/К4 «изнутри» (точная форма + данных, WeakMap-паттерн) — это техническое решение, явно оставленное + исполнителю (`PROCESS.md` §7.1: «всё, чего пользователь не наблюдает, + агенты решают сами»). + +## Вывод + +High: 0. Medium: 1, в скоупе задачи (отсутствует раздел «Откат») — чинится +в этом же issue без нового цикла-кандидата на отдельный документ. Low: 2, +оба сняты решением ревьюера с записью выше (один — быстрая правка вместе с +Medium, второй — без требуемого действия). + +**Вердикт: жёлтый.** Автор добавляет раздел «Откат» (и, кстати, две строки +про миграцию/touch) в тело issue, после чего можно повторно ставить +`S4-spec-review` — по объёму это не полный разбор дельты, а проверка +единственного добавленного раздела.