mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 04:09:17 +00:00
@@ -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<object, …>()`, ключ `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<string, StaticLightBarrierEntry>`, без 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` — по объёму это не полный разбор дельты, а проверка
|
||||
единственного добавленного раздела.
|
||||
Reference in New Issue
Block a user