Files
houseplan-card/docs/reviews/SPEC-REVIEW-375-r1.md
2026-08-29 17:43:19 +00:00

18 KiB
Raw Permalink Blame History

SPEC-REVIEW-375-r1

Issue: #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 — по объёму это не полный разбор дельты, а проверка единственного добавленного раздела.