mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
claude[bot]
@@ -0,0 +1,193 @@
|
||||
# Ревью ТЗ — issue #20, заход r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/20
|
||||
- **ТЗ:** `docs/specs/020-glow-open-door-spill.md` (коммит `962b1d0d`, ветка
|
||||
`issue/20-glow-open-door-spill`, единственный коммит поверх `origin/dev`)
|
||||
- **Трек:** обычный (владелец явно снял `small` 2026-08-15 и подтвердил
|
||||
«лёгкий трек: нет» повторно 2026-08-28 — меняется горячий geometry/cache
|
||||
path и требуется performance proof)
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
|
||||
- **Вердикт:** зелёный · High: 0 · Medium: 0
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Проверялось ТЗ `docs/specs/020-glow-open-door-spill.md` как артефакт этапа
|
||||
`S4-spec-review`: обязательные разделы §7.1 PROCESS.md, однозначность и
|
||||
доказуемость AC1…AC10, соответствие `docs/SCOPE.md`, канонической модели
|
||||
`docs/LIGHT.md` и терминологии `docs/USER-GUIDE.ru.md`, а также независимая
|
||||
проверка каждого фактического утверждения ТЗ о текущем коде (не пересказ
|
||||
автора) — ТЗ описывает конкретные функции, кэш-ключи и геометрические типы,
|
||||
и первый вопрос ревью к такому тексту: существуют ли они там, где сказано, и
|
||||
делают ли то, что сказано. Продуктового кода по #20 нет (ветка содержит
|
||||
только правку ТЗ), поэтому гейты §8 к этому заходу неприменимы.
|
||||
|
||||
История issue: тело 2026-08-09 описывало старую (уже не существующую) модель
|
||||
интервалов-occluder; комментарий владельца 2026-08-11 актуализировал
|
||||
механику под пост-#71 модель полигона видимости; комментарий 2026-08-15
|
||||
зафиксировал P3/feature/обычный трек и закрыл продуктовые вопросы («Вопросы:
|
||||
нет — поведение проёма без contact entity и fallback уже определены»);
|
||||
комментарий 2026-08-28 подтвердил взятие в работу и текущую редакцию ТЗ.
|
||||
Предыдущего цикла ревью ТЗ на этом issue не было — это первый заход, раздел
|
||||
«Унаследовано из r0» не применяется.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§2.3–2.10, §7.1,
|
||||
§7.2) целиком.
|
||||
2. Прочитано тело issue #20 и все четыре комментария владельца.
|
||||
3. Прочитан файл ТЗ `020-glow-open-door-spill.md` целиком (224 строки).
|
||||
4. Прочитан канонический `docs/LIGHT.md` целиком — источник истины для
|
||||
модели света, на которую ТЗ явно ссылается.
|
||||
5. Проверена терминология в `docs/USER-GUIDE.ru.md` (`Glow`, ворота как
|
||||
аналог двери, «стены не пропускают Glow») — используемые в ТЗ слова не
|
||||
изобретены, они те же, что видит пользователь в интерфейсе.
|
||||
6. Независимо сверены с кодом все технические утверждения ТЗ, которые могли
|
||||
бы оказаться недоказанной догадкой, выданной за факт:
|
||||
- `_lightBarriers()` (`src/houseplan-card.ts:10261-10392`) — подтверждена
|
||||
фильтрация `passages` через `isInteriorLightOpeningType` (allowlist
|
||||
`door | gate | passage`), проверка пола по обе стороны (`onFloor`),
|
||||
разделение контурных вырезов (`roomPassages`/`cuts`) и партиций
|
||||
(`_partitionOpeningCuts` → `PartitionOpeningCut`), кэш-ключ
|
||||
`contentFingerprint([raw, cellCm, gridPitch])` — сейчас **не** содержит
|
||||
состояние проёмов, что и есть повод для задачи;
|
||||
- `openingAmount()` (`src/logic.ts:316-324`) — подтверждён текущий
|
||||
бинарный контракт (0/1, fallback `1` для двери/ворот без датчика,
|
||||
`unavailable`/`unknown` не меняются `invert`) — ровно то, что таблица
|
||||
ТЗ §"Контракт поведения / 1" описывает как сохраняемый инвариант для
|
||||
известного/fallback случая; расширение под `current_position` cover —
|
||||
заявленная новая работа, не выдаётся за существующую;
|
||||
- `_openingAmt()`/`OpeningVisibleSpec.amount` (`src/houseplan-card.ts:
|
||||
12303-12309`, `src/render/opening-symbol.ts:8-61`) — подтверждено, что
|
||||
символ проёма уже использует единое дробное `amount` в `[0,1]`
|
||||
(`Math.max(0, Math.min(1, spec.amount))`), то есть претензия ТЗ «один
|
||||
коэффициент для символа и Glow» технически реализуема без переделки
|
||||
символа — расширяется только источник числа;
|
||||
- `PartitionOpeningCut` (`src/physical-geometry.ts:145-186`) — подтверждён
|
||||
генерический сегмент `{hostId, a, b, depth}`, не привязанный к полной
|
||||
длине проёма — масштабирование `a`/`b` вокруг центра для частичного
|
||||
выреза, которое требует ТЗ, архитектурно естественно;
|
||||
- `_roomWallOpeningInputs()` (`src/houseplan-card.ts:7494-7505`,
|
||||
`geometryRoomOpeningInputs`) отдаёт `{x, y, angle, length}` на проём —
|
||||
подтверждено, что вырез строится из `length` вокруг центра
|
||||
(`houseplan-card.ts:10310-10314`), то есть `length * amount` с тем же
|
||||
центром/осью — не выдумка, а прямое следствие существующей формулы;
|
||||
- `_contactCandidates()` (`src/houseplan-editor-runtime.ts:6379-6394`) —
|
||||
подтверждено, что `cover`-домен уже допустим как `opening.contact` для
|
||||
дверных device_class (`door`, `garage_door`, `garage`, `opening`,
|
||||
`window`) — «доступный cover» в контракте ТЗ не новая возможность
|
||||
привязки, а новое использование уже существующей;
|
||||
- `lruWrite(this._lightBarrierPool, cacheKey, entry, 8)` и
|
||||
`lruWrite(this._glowClipCache, clipKey, geometry, 256)`
|
||||
(`houseplan-card.ts:10389, 10506`) — подтверждены названные в ТЗ
|
||||
границы LRU;
|
||||
- `demo/smoke_glow.mjs`, `demo/fixtures/large-house.mjs`,
|
||||
`demo/benchmark_glow.mjs` — подтверждено существование
|
||||
large-house-фикстуры и Glow-бенчмарка, на которые опирается AC9,
|
||||
ничего вымышленного.
|
||||
7. Проверено соответствие `docs/SCOPE.md`: владелец уже классифицировал
|
||||
задачу как «пространственная правдивость света» (комментарий 2026-08-15)
|
||||
— это часть J1 («живой пространственный обзор») и общей заявки миссии
|
||||
«live, tappable map»; ревью не переоткрывает уже принятое владельцем
|
||||
scope-решение.
|
||||
8. Проверены незакрытые продуктовые вопросы: их нет — владелец прямо
|
||||
подтвердил в комментарии 2026-08-15, что поведение без contact entity и
|
||||
fallback уже определено; в ТЗ нет ни одного утверждения о видимом
|
||||
поведении, помеченного как решённое, но не подтверждённого ни одним
|
||||
документом или существующим кодом.
|
||||
9. Код не запускался, гейты §8 не прогонялись — на этапе ревью ТЗ
|
||||
продуктового кода нет (см. «Скоуп ревью» выше).
|
||||
|
||||
## Проверка §7.1 — обязательные разделы
|
||||
|
||||
Все присутствуют и содержательны: сценарий (персона/поверхность/момент) ·
|
||||
что человек увидит до/после (одна фраза, без терминов реализации, кроме
|
||||
слова «Glow», которое само является интерфейсным термином из
|
||||
USER-GUIDE.ru.md, а не внутренним) · проблема · скоуп и не-скоуп · контракт
|
||||
поведения · UX · модель данных и миграция · i18n · AC1…AC10 с указанным
|
||||
способом доказательства (unit/smoke/golden/CI-perf) · план автотестов ·
|
||||
риски и меры · откат · release-артефакты · блок «принято предположительно».
|
||||
|
||||
Блок «принято предположительно» (§218-223 ТЗ) содержит только технические
|
||||
решения (форма и файл чистого aperture-хелпера, сериализация signature,
|
||||
механика частичного `PartitionOpeningCut`, разбиение smoke/golden файлов) —
|
||||
ни одно из них не является продуктовым вопросом, спрятанным под видом
|
||||
допущения.
|
||||
|
||||
## Проверка AC
|
||||
|
||||
Все десять AC пронумерованы, формулируют конкретное наблюдаемое поведение
|
||||
(или измеримый контракт кэша/perf) и называют способ доказательства:
|
||||
|
||||
- AC1, AC2 — closed/open переход, unit геометрии + browser smoke с
|
||||
переключением состояния fake `hass`;
|
||||
- AC3 — частичное открытие через `cover.current_position`, unit + smoke по
|
||||
пикселям;
|
||||
- AC4 — матрица `invert` на известных/fallback состояниях, unit;
|
||||
- AC5 — классификатор типов (door/gate/passage прозрачны, window/наружная
|
||||
дверь/неизвестный тип непрозрачны), unit + smoke;
|
||||
- AC6 — паритет контурной стены и независимой перегородки, geometry unit +
|
||||
smoke fixture;
|
||||
- AC7 — fail-dark источника внутри закрытого проёма, source-guard
|
||||
unit/smoke;
|
||||
- AC8 — контракт кэша (переиспользование при посторонней сигнатуре, новый
|
||||
ключ при смене amount, bounded LRU), unit;
|
||||
- AC9 — perf-бюджет на large-house fixture, exact-SHA CI performance smoke
|
||||
перед бетой (согласовано с AGENTS.md: performance/golden — предрелизный
|
||||
гейт, а не гейт этой стадии);
|
||||
- AC10 — golden-сцена closed/open/50%, reviewed Linux golden-артефакт.
|
||||
|
||||
Все десять привязаны к реально существующим точкам расширения кода (см.
|
||||
п.6 выше), ни один не описывает несуществующий API как готовый.
|
||||
|
||||
## Находки
|
||||
|
||||
Не найдено ни одной High- или Medium-находки. ТЗ не содержит утверждений о
|
||||
поведении, которых нет ни в `docs/LIGHT.md`, ни в текущем коде и которые не
|
||||
помечены как допущение; открытых продуктовых вопросов нет — они закрыты
|
||||
владельцем заранее.
|
||||
|
||||
### L1 (Low, снимается без правки) — влияние на touch не названо отдельной строкой
|
||||
|
||||
DoR-чеклист (`PROCESS.md` §2.5) требует явно названного влияния на touch по
|
||||
`docs/TOUCH-SUPPORT.md`. В самом ТЗ такой отдельной строки нет. Снимаю без
|
||||
правки: раздел «УХ» ТЗ прямо утверждает «новых контролов и сообщений нет» —
|
||||
изменение целиком в рендеринге Glow в View/kiosk, без нового
|
||||
взаимодействия, а `docs/TOUCH-SUPPORT.md` требует паритета именно для
|
||||
интерактивных путей View, которых здесь не появляется. Рендер одинаков на
|
||||
любом указывающем устройстве; отдельно фиксировать «влияние: нет» было бы
|
||||
формальностью, а не содержательной проверкой. Если автор всё же добавит эту
|
||||
строку явно в ТЗ — это не будет считаться повторным циклом ревью.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- все причинно-следственные утверждения ТЗ о текущей модели (`_lightBarriers`,
|
||||
`openingAmount`, `PartitionOpeningCut`, LRU-границы, cover-контакты,
|
||||
large-house-фикстура) подтверждены чтением кода, а не поверены на слово;
|
||||
- контракт таблицы «Один коэффициент открытия» не противоречит текущему
|
||||
поведению `openingAmount()` для известных/fallback случаев — расширяет его,
|
||||
не переопределяя уже проверенный инвариант;
|
||||
- не-скоуп корректно исключает наружный свет, tunnel fill, автоматическое
|
||||
управление дверью и смену конфигурационной схемы — ни один пункт не
|
||||
расширяет заявленный скоуп «мимоходом»;
|
||||
- release-артефакты называют актуализацию `docs/LIGHT.md`, что необходимо:
|
||||
канонический документ сейчас не описывает частичную апертуру и обязан
|
||||
получить это описание в том же изменении;
|
||||
- открытых продуктовых вопросов нет, что подтверждено собственным
|
||||
комментарием владельца, а не заявлением автора ТЗ.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `npm run typecheck`/`npm test`/`npm run build` — продуктового
|
||||
кода в ветке нет, применять их не к чему на этой стадии.
|
||||
- Не проверял производительность фактическим прогоном `benchmark_glow.mjs` —
|
||||
AC9 сам относит это доказательство к предрелизному exact-SHA CI, а не к
|
||||
этому циклу.
|
||||
- Не проверял golden — сцена ещё не существует (AC10, план автотестов п.4),
|
||||
проверять нечего до реализации.
|
||||
- Не оценивал точную формулу сериализации `openingStateSignature` и место
|
||||
файла aperture-хелпера — оба явно вынесены в блок «принято
|
||||
предположительно, поменять свободно» и не являются предметом этого ревью.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. ТЗ готово к разработке.
|
||||
Reference in New Issue
Block a user