From d86b613cefa5f541c1f9ef82d73d092d4d045562 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 19:11:13 +0000 Subject: [PATCH] docs: review document for #585 Issue: #585 User-Visible: no --- docs/reviews/SPEC-REVIEW-585-r1.md | 197 +++++++++++++++++++++++++++++ 1 file changed, 197 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-585-r1.md diff --git a/docs/reviews/SPEC-REVIEW-585-r1.md b/docs/reviews/SPEC-REVIEW-585-r1.md new file mode 100644 index 00000000..73cca766 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-585-r1.md @@ -0,0 +1,197 @@ +# SPEC-REVIEW-585-r1 + +**Issue:** [#585](https://github.com/Matysh/houseplan-card/issues/585) — узкая разрешимая щель между узлами решётки 4 px теряется при разрешении коллизий поднятых 2.5D-маркеров. +**Этап:** ТЗ на ревью (`S4-spec-review`), заход r1, полный трек (не `small`: сложность/риск 8/10, влияние на производительность). +**Ревьюер:** независимая сессия, без контекста реализации (в реализации кода ещё не участвовал никто — `S6-in-progress` не проходился). +**Вердикт: зелёный.** + +--- + +## 1. Скоуп ревью + +Тело issue #585 целиком: репорт дефекта («Что не так» / «Чем доказано» / «Почему не +починено в бете» / «Как чинить по-настоящему» / «Приёмка»), раздел `## ТЗ` (16 +пронумерованных подразделов по PROCESS.md §7.1) и единственный комментарий — +`S2-analysis` от 2026-09-19, фиксирующий полный трек с названным нарушенным +критерием §5. + +Задача — исправление ранее одобренного исключения из `docs/SCOPE.md` (2.5D, +issue #89): скрытый experimental-вид за `hp_alpha`, сохранённая модель/камера/ +пользовательский контракт не трогаются, меняется только взаимное размещение +поднятых маркеров устройств и замков проёмов внутри уже принятого бюджета 48 CSS +px / 4 CSS px. + +## 2. Как проверялось + +Прочитано в порядке, предписанном заданием: + +1. `docs/SCOPE.md` — мандат 2.5D как узкое одобренное исключение (#89), инвариант + блокировки замков (не задет — задача не касается действий). +2. `AGENTS.md`, `PROCESS.md` (главы 1–14 целиком) — классы файлов, лёгкий трек и + критерий его отказа, §7.1 обязательные разделы ТЗ, §2.4/§2.10 формат ревью и + правило деления по дельте (не применялось: это r1, других документов по #585 + в `docs/reviews/` и в истории git нет — проверено `find` и + `git log --all --oneline | grep 585`, единственный релевантный коммит + `9683a590` — временное расширение допусков перед бетой, не фикс). +3. Тело issue #585 и его единственный комментарий (аналитика). +4. `docs/USER-GUIDE.ru.md` — не требуется: фича скрыта за `hp_alpha`, не входит + в пользовательский интерфейс, ТЗ явно фиксирует `User-Visible: no`. +5. `docs/ISOMETRIC.md` целиком (Stage 1/2/4) — проверка того, что числа и + формулировки ТЗ (48 CSS px, 4 CSS px, камера `rotDeg=0 tiltDeg=20`, + исключение room labels из взаимной коллизии, отсутствие тетера/подложки) + совпадают с зафиксированным контрактом, а не изобретены заново. +6. `src/iso-overlays.ts` целиком — сверка утверждений ТЗ с действующей + реализацией `resolveIsoOverlayPlacement`/`resolveIsoOverlayCollisions`: + стабильный порядок (`nudgeDistanceCss` → `kind\0id`), текущий грубый шаг + 4 px (`ISO_OVERLAY_GROUP_COARSE_STEP_CSS_PX`, коммит `699ab471`/#583), + существующий broad phase по ячейкам (`nearby`, `wallsNear`), деградированный + fallback с минимальным штрафом (`penalty`), исключение room labels + (`IsoOverlayCollisionKind = Exclude`). +7. `demo/performance/budgets-large-house-isometric.json`, + `demo/performance/budgets-isometric-stage3-dense.json` — текущие значения + `noiseAllowanceMs` (450/150/120) сверены с числами ТЗ §10 (что именно + временное и что возвращается к 150/60/75). +8. `test/performance-budget.test.mjs` (строки 260–290) — тест + `'изометрический допуск жеста покрывает ровно принятый шаг #583, не больше + (#585)'` уже сегодня фиксирует эти же временные числа и явно называет #585 + как задачу, которая их отменит. ТЗ описывает уже существующий, а не + придуманный контракт. +9. `demo/fixtures/large-house.mjs:33` — `name: \`Room ${floor + 1}.${index + + 1}\`` подтверждает, что «Room 2.5» из АС2 — реальное имя комнаты фикстуры + `large` (этаж 2, комната 5), а не опечатка/догадка. +10. `demo/golden/matrix.mjs:284` и `demo/golden/baselines/` — сцена + `isometric-large-warm-remount-dark` (fixture `large`, space `perf-floor-2`, + mode `view`) существует и baseline-файл на месте. +11. `demo/performance/*.json` / `scripts` — имена профилей + `large-house-isometric-v1` и `isometric-stage3-dense-v1` совпадают с полем + `profile` обоих budget-файлов, значит АС6 ссылается на реальные профили. + +Гейты не запускались: этап ТЗ не подразумевает исполнение кода (§2.4), материал +для проверки — текст, а не диапазон коммитов. Кода по #585 в ветке ещё нет +(`git log --all` не находит коммитов реализации #585 после `9683a590`). + +## 3. Находки + +Нет ни одной High- или Medium-находки. + +**Low (снята с записью, не блокирует).** Раздел «1. Сценарий» не называет +персону `docs/SCOPE.md` по имени (например, «home admin»), хотя PROCESS.md §7.1 +явно требует называть персону. Ситуация описана однозначно — включение +скрытого `hp_alpha` через URL/hash-параметр физически доступно только тому, кто +осознанно лезет в query/hash грамматику продукта, то есть тому же «home admin», +единственной персоне со сценариями настройки/тестирования по SCOPE.md; у +«household members» и «guests/kiosk» нет пути включить эксперимент. Отсутствие +буквального слова не создаёт риска неверного толкования и не влияет ни на один +AC — снимаю без правки текста. + +## 4. Обязательные разделы §7.1 — проверка полноты + +Все обязательные разделы присутствуют и пронумерованы в теле issue: +сценарий (1) · что человек увидит до/после (2) · проблема (3) · скоуп (4) и +не-скоуп (5) · контракт поведения (6) · UX (7) · модель данных и миграция (8) · +i18n (9) · производительность (10, сверх минимума §7.1, обоснованно для этой +задачи) · критерии приёмки AC1…AC8 с доказательством (11) · план автотестов +(12) · затронутые файлы (13) · риски и откат (14) · release-артефакты (15) · +принятые предположения (16). Ничего из требуемого не пропущено. + +Раздел 2 («До/после») — одна фраза без терминов реализации, как требуется: +«в Room 2.5 два маркера визуально совмещаются» → «оба маркера видны раздельно». + +## 5. Однозначность AC и способ доказательства + +| AC | Формулировка проверяема? | Доказательство названо? | +|---|---|---| +| AC1 | Да — числовые пороги (≤48 px сдвиг, ≥4 px зазор, внутри комнаты, без пересечения стены) | unit, узкий геометрический свидетель | +| AC2 | Да — конкретная сцена и комната названы и существуют в фикстуре/матрице голденов (проверено, п.2.9–2.10 выше) | golden + unit (двойной оракул, без golden как единственного судьи — прямое требование #435-класса) | +| AC3 | Да — перечислены все существующие контракты (порядок/перестановка, радиус, room/hole/wall safety, исключение room labels, fit-mode skip, degraded fallback) | unit | +| AC4 | Ограниченность поиска сформулирована качественно («существенно меньше полного диска», «нет площадного перебора 1 px»), но абсолютный запрет — полного скана диска — задан точно и проверяем по коду | unit + ревью кода (названо явно, не спрятано за «прочитано глазами») | +| AC5 | Да — «меняется только объявленная сцена» с явным условием на исключения | golden | +| AC6 | Да — точные числа допусков (150/60/75), неизменность hardMax/ratio, точный SHA CI | Linux CI (perf-профили названы по имени) | +| AC7 | Да — команды названы буквально | `npm run typecheck`, `npm run test:unit`, `npm run build` | +| AC8 | Да — три конкретных документа | ревью кода | + +Единственная качественная формулировка (AC4, «существенно меньше») не создаёт +находки: жёсткая, машинно проверяемая часть контракта («не содержит площадного +перебора 1 px по диску», п.6.5–6.8 контракта поведения) уже количественная и её +достаточно для отказа реализации, вернувшейся к старому алгоритму под новым +именем. Точный коэффициент сокращения — деталь, которую пользователь не +наблюдает, и её по PROCESS.md §7.1 решают агенты, а не владелец. + +## 6. Проверка «догадка выдана за факт» + +Каждое утверждение о существующем поведении в ТЗ (числа 48/4 CSS px, порядок +сортировки, отсутствие тетера/подложки, исключение room labels, деградированный +fallback, текущие временные допуски 450/150/120, профили performance, имя +проблемной сцены и комнаты) сверено с действующим кодом, тестами, budget-json +или `docs/ISOMETRIC.md` (см. §2 выше) — совпадает буквально, ничего не +изобретено задним числом. + +Единственное явно новое техническое решение — сам способ поиска (граничный +перебор вместо площадного/решётчатого скана, п.6.5–6.8 контракта) — помечено в +разделе 16 «Принятые предположения» как техническое решение автора, подлежащее +пересмотру ревьюером кода, а не как факт о существующей системе. Это ровно тот +случай, когда решать не пользователю: способ поиска пользователем не +наблюдается, наблюдается только результат (АС1–АС5). + +Открытых продуктовых вопросов автор не поднимал и не должен: сценарий, +объём видимых изменений (только внутри уже скрытого эксперимента, обычный вид +не трогается) и деградация (существующий контракт degraded, не новый) уже +зафиксированы предыдущими задачами (#570, #583) и не требуют решения владельца +заново. + +## 7. Соответствие DoR (§2.5) на выходе из ревью + +- ТЗ существует в теле issue, ссылка issue↔ТЗ на месте (это и есть issue) — ✅; +- AC1…AC8 пронумерованы, у каждого назван способ доказательства — ✅ (см. §5); +- затронутые файлы и модули перечислены (§13 ТЗ) — ✅; +- i18n: явно «нет новых строк/ключей» — ✅; +- миграция/compatibility: явно «не меняются», согласуется с + `docs/CONFIG-COMPATIBILITY.md` (алгоритм работает только с производной + экранной геометрией кадра, схему не трогает) — ✅; +- производительность: названа количественно, включая обратный откат временных + допусков и неизменность hard-потолков — ✅; +- touch: явно сохранён существующий контракт («Фокус, hover, click/tap и + HA-действия остаются привязаны к визуально смещённому маркеру по + существующему контракту», §7 ТЗ) — влияния нет, названо — ✅; +- release-артефакты: явно `User-Visible: no`, никакого changelog — ✅; +- откат: атомарный откат алгоритма + baseline + budget allowances — ✅; +- открытых продуктовых вопросов нет (см. §6 выше) — ✅. + +Все пункты DoR выполнены — задача готова к переходу в `S5-ready`. + +## 8. Что не проверялось (и почему это не требуется на этом этапе) + +- Работоспособность самого граничного алгоритма (это предмет кода, а не ТЗ); + на этапе спецификации проверяется формулируемость и проверяемость контракта, + а не его реализация. +- Реальный прогон `npm run typecheck` / `npm test` / `npm run build` — код по + #585 в ветке не появился, гонять гейты не на чём. +- Правильность цифр в таблицах «Чем доказано» / «Почему не починено в бете» — + это диагностика самого автора, не входящая в проверяемый контракт ТЗ (АС не + утверждают ни одно из измеренных там чисел); её будет проверять код-ревью на + реальном прогоне Linux CI по АС6. +- Полный список из 12 изометрических голденов и byte-diff у остальных 11 — вне + ТЗ-ревью, входит в АС5 и проверяется код-ревью по итогам съёмки. + +--- + +**Материал раунда:** тело issue #585 на момент разбора (id issue: 585, +единственный комментарий `S2-analysis` от 2026-09-19T19:03:26Z). Других +документов ревью или коммитов реализации по #585 в репозитории нет — +`git log --all --oneline | grep 585` находит только `9683a590` (предшествующее +временное расширение допусков, не относится к этой ТЗ-редакции). Раздел +«Унаследовано из r0» не пишется: это первый заход. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `c38382b7318a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `a34fbb3a3bd1828db2c7fb65a42e43f9ae921195` + ``` + git log --all --format='%H %T' | grep a34fbb3a3bd1 + ``` +- Тело issue: `7e85ec65b07414bb734b5f19ffb10510ee60b3af4fdb52ed429df29d2593ebc8` +- Вердикт конвейера: `green` · High 0