docs: review document for #122

Issue: #122
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-13 21:04:32 +00:00
parent 76ce755742
commit 4c73e2ccdb
+241
View File
@@ -0,0 +1,241 @@
# SPEC-REVIEW-122-r1
- **Issue:** https://github.com/Matysh/houseplan-card/issues/122
- **ТЗ под ревью:** `docs/specs/122-isometric-stage2.md` (коммит `76ce755`,
ветка `issue/122-isometric-stage2`)
- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
- **Трек:** обычный (не `small`) — аналитика владельца оценила сложность и
риск 9/10, поверхностей больше одной (рендер/геометрия/UX/perf); лёгкий трек
корректно не применён, `docs/specs/<NN>-*.md` присутствует
- **Цикл:** r1/4 (полный трек, лимит 4)
## Скоуп ревью
Проверялось соответствие ТЗ:
- `docs/SCOPE.md` — попадание в Core user jobs (J1/J2/J3), узкое
owner-approved исключение из фотореалистичного/3D-запрета, инвариант lock;
- `PROCESS.md` §2.4/§2.5 (DoR), §7.1 (обязательные разделы ТЗ), §12
(запреты), §3 (правила 1, 2, 6, 9);
- `AGENTS.md` — классы файлов (только `docs/**` в этом коммите), ветка,
трейлеры;
- нормативному предшественнику `docs/specs/089-isometric-view-stage1.md` и
`docs/adr/089-isometric-stage1-renderer.md` — что Stage 2 не переписывает
принятые решения Stage 1 задним числом;
- каноническим документам затронутой подсистемы: `docs/ISOMETRIC.md`,
`docs/WALL-THICKNESS.md`, `docs/LIGHT.md`, `docs/TOUCH-SUPPORT.md`,
`docs/UX-MODES.md`;
- `docs/USER-GUIDE.ru.md` — что видимого пользователю текста не появляется и
документ не должен меняться;
- фактическому состоянию кода (`src/render/opening-symbol.ts`, `src/logic.ts`,
`src/labs.ts`, `demo/golden/matrix.mjs`) — на предмет того, что технические
утверждения ТЗ не являются непроверенной догадкой;
- треду issue #122 целиком, включая аналитику S2, вопросы владельца Q0–Q6 и
их принятие, а также связанному issue #124 (performance-блокер).
## Как проверялось
1. Прочитан весь тред issue #122: аналитика (ценность 7/10, P2, сложность
9/10), пакет вопросов владельцу Q0–Q6 с default-вариантами, принятие всех
default'ов владельцем 2026-08-13, хендофф автора «ТЗ готово к ревью».
Подтверждено, что ни один вопрос не был техническим, замаскированным под
продуктовый: Q0 (приоритет/ценность), Q1 (объём видимого сравнения
Stage1/Stage2 — второй флаг или нет), Q2 (что задают референсы: материал
или замена содержимого), Q3 (визуальное представление проёмов), Q4 (новый
декоративный свет — да/нет), Q5 (floor edge/тени), Q6 (деградация) — все
про то, что видит пользователь, и про объём этого issue.
2. Сверены обязательные разделы ТЗ (§7.1 `PROCESS.md`) — таблица ниже.
3. Прочитан `docs/specs/089-isometric-view-stage1.md` целиком и
`docs/ISOMETRIC.md` — подтверждено, что Stage 2 ссылается на реальные
решения Stage 1 (`wallBodiesGeometry()`, единая проекция, fingerprint без
`_cfgEpoch`/HA state, LRU-кэш на 8 сцен, latched flat fallback,
`projectedFrame()`, Labs-грамматика `iso`/`since 1.62.0`/`expires 1.65.0`)
и не выдаёт их за собственное изобретение и не переписывает Stage 1 задним
числом (ADR-89 не трогается, заводится новый `docs/adr/122-*.md`).
4. Прочитан `src/labs.ts` (`since: '1.62.0'`, `expires: '1.65.0'`) и
`package.json` (`version: 1.63.0`) — утверждение ТЗ «expiry не расширяется,
`1.65.0-beta.1` уже мёртв» технически корректно и согласуется с описанным в
`docs/ISOMETRIC.md` числовым сравнением ядра версии.
5. Прочитан `src/render/opening-symbol.ts` — подтверждены буквально: gate
открывается ровно на 10° (`10 * amount`, `docs/specs/122:213-215` против
`opening-symbol.ts:97`), window — двухстворчатый casement
(`rotate(${-90*amount})`/`rotate(${90*amount})`, `opening-symbol.ts:83-89`
против §6.5 «two-leaf/casement»). Это не догадка автора, а точное описание
существующего кода.
6. Прочитан `openingAmount()` в `src/logic.ts:316-323` — подтверждено
поведение «no contact → door/gate open (1), window closed (0)», которое ТЗ
называет «static-open default when no contact exists» (§6.5, Door/Gate).
7. Прочитан `docs/LIGHT.md` и `docs/WALL-THICKNESS.md` — подтверждено, что
«один регион света на источник», «wallBodiesGeometry как единственный
источник тела стены», «exterior silhouette из union центральных линий» —
реальные, а не придуманные инварианты, на которые ТЗ ссылается в §6.3, §6.7,
§7.
8. Прочитан `docs/TOUCH-SUPPORT.md` целиком — формулировки ТЗ §6.8
(«pointer/focus/ARIA inert», «View и kiosk остаются полностью
поддерживаемыми», «touch-only failure — product defect») дословно
отражают контракт, а не изобретены заново.
9. Прочитан `docs/USER-GUIDE.ru.md` — поиском подтверждено отсутствие каких-
либо упоминаний изометрии/объёмного вида; утверждение ТЗ §9 «документ
остаётся молчащим про скрытый Iso» верно на текущий момент.
10. Проверено существование golden-сцены `isometric-no-borders-dark` в
`demo/golden/matrix.mjs:23` — AC15/§12.3 ссылаются на реальный, а не
вымышленный сценарий.
11. Прочитан issue #124 (`gh issue view 124`) — подтверждено: воспроизводимый
performance-регресс `viewToggleMs.median 192.8мс` vs лимит `131.7мс` на
exact-SHA профиле `large-house-isometric-v1`, статус `S1-new` (ещё не
проанализирован). ТЗ §10 корректно описывает его как DoR-блокер
реализации (не ревью), с двумя явными путями снятия (закрытие #124 или
отдельное явное решение владельца) — ни то, ни другое не подменяет и не
ослабляет существующий бюджет.
12. Проверено `docs/specs/README.md:80` — строка на #122 добавлена в том же
коммите, ссылка issue ↔ ТЗ двусторонняя.
13. Проверены трейлеры коммита `76ce755`: `Issue: #122`, `User-Visible: no`;
`git show --stat` подтверждает изменение только `docs/specs/122-*.md` и
`docs/specs/README.md` — класс C, продуктовый код (класс A) не менялся,
правило №1 `AGENTS.md`/`PROCESS.md` соблюдено на этапе `spec`.
## Обязательные разделы (§7.1 PROCESS.md)
| Раздел | Есть | Комментарий |
|---|---|---|
| Сценарий (персона/поверхность/момент) | ✅ | §1 |
| Что человек увидит до/после (без терминов реализации) | ✅ | §1, одной фразой |
| Проблема | ✅ | §2 |
| Скоуп / не-скоуп | ✅ | §4 / §5 |
| Контракт поведения | ✅ | §6 (8 подразделов) |
| UX | ✅ | §9 (явно: нет нового UI/i18n) |
| Модель данных и миграция | ✅ | §8 |
| i18n | ✅ | §9 (пусто, обосновано) |
| AC1…ACn с доказательством | ✅ | §11, 16 штук, каждый с типом |
| План автотестов | ✅ | §12 (unit/smoke/golden/performance/backend) |
| Риски | ✅ | §14, 12 строк с вероятностью/impact/mitigation |
| Откат | ✅ | §15 |
| Release-артефакты | ✅ | §13 |
Все обязательные разделы присутствуют и содержательны. Дополнительно
присутствуют продуктовые под-разделы §7.1 (персона/поверхность/момент внутри
§1) и явный блок §16 «принятые технические предположения — можно менять без
пересмотра продукта», корректно отделённый от решений владельца §3.
## Находки
Находок уровня **High** и **Medium** нет.
### Low-1 — отсутствует буквальная декларация «Touch editor: …»
**Файл:** `docs/specs/122-isometric-stage2.md` (весь документ)
`docs/TOUCH-SUPPORT.md` («Documentation rule») требует от новых спецификаций
редакторных фич явно указывать одно из: `Touch editor: supported` / `best
effort / intentionally degraded` / `not exposed`. Предшественник, Stage 1
(`docs/specs/089-isometric-view-stage1.md`, §7 «Touch editor: не exposed»),
эту декларацию давал буквально, хотя ситуация идентична — редакторы Stage 2
не касается вовсе (§6.1 «Editors and houseplan-space-card stay Flat»). Stage 2
такой буквальной строки не содержит, хотя по содержанию §6.1/§6.8 поведение
однозначно совпадает с «not exposed» — реальной неоднозначности для
разработчика или ревьюера кода нет.
**Решение ревьюера:** Low, не блокирует. Формально Stage 2 не вводит новую
редакторную фичу (правило `TOUCH-SUPPORT.md` адресовано именно им), а
содержательно контракт «редакторы не меняются» зафиксирован дважды (§6.1,
§9). Оставляю на усмотрение автора — можно добавить строку `Touch editor: not
exposed` при следующей правке для единообразия с Stage 1, можно снять этой
записью.
### Low-2 — три AC используют тип доказательства вне буквального перечня §2.5
**Файл:** `docs/specs/122-isometric-stage2.md:445-457` (AC13, AC15, AC16)
DoR (`PROCESS.md` §2.5) перечисляет типы доказательства как `unit` / `backend`
/ `smoke` / `golden` / «ревью кода». AC13 помечен только `(performance)`,
AC15 — `(golden + documentation review)`, AC16 — `(typecheck + unit + build)`.
Буквально `performance`, `typecheck`, `build` и `documentation review` в этот
перечень не входят. По существу все три доказуемы существующими механизмами:
`performance_smoke`/Full Performance и `typecheck`/`build` — реальные
release-blocking гейты `PROCESS.md` §8, а «documentation review» — часть
обычного код-ревью (§2.7). Тот же паттерн уже был отмечен как Low-1 в
`SPEC-REVIEW-123-r1` и оставлен автору без блокировки цикла.
**Решение ревьюера:** Low, не блокирует. Переформулировка в термины §2.5 (например,
«ревью кода со ссылкой на `performance_smoke`/exact-SHA Full Performance» и
«ревью кода со ссылкой на зелёный `typecheck`/`test`/`build`») повысила бы
буквальную трассируемость, но не меняет, чем на практике будет доказан
критерий. Оставляю на усмотрение автора или снимаю этой записью.
## Что проверено и корректно
- Соответствие `docs/SCOPE.md`: задача закрывает J1/J2/J3, остаётся внутри
узкого owner-approved исключения (2.5D, не фотореализм/не свободная камера/
не interior editor), не трогает единственную санкционированную поверхность
actuation замков (`docs/SCOPE.md` «The lock invariant»).
- Все вопросы владельцу (Q0–Q6) — продуктовые (видимое поведение, объём
видимых изменений), ни один технический вопрос не был переадресован
владельцу; все ответы получены и внесены в ТЗ без расширения скоупа.
- ТЗ явно наследует, а не переопределяет Stage 1: проекция, `wallBodiesGeometry`,
fingerprint без `_cfgEpoch`/HA-состояния, LRU-кэш 8 сцен, latched flat
fallback, Labs-грамматика `iso`/`1.62.0`/`1.65.0` — все повторно
использованы корректно, ни одно принятое решение Stage 1 не переписывается
задним числом (историческая ADR-89 остаётся неприкосновенной, заводится
новая `docs/adr/122-isometric-stage2-composition.md`).
- Технические утверждения о существующем коде (10° gate, two-leaf window,
static-open default при отсутствии contact, один регион света на источник,
`wallBodiesGeometry` как единственный источник геометрии стен) подтверждены
чтением реального кода, а не приняты на слово автора.
- Скоуп/не-скоуп (§4/§5) корректно отсекает публичный rollout, второй флаг
`iso2`, свободную камеру/#82, новые material-настройки, объёмные редакторы и
`houseplan-space-card`, схемные поля высоты/подоконника, WebGL/Three.js — все
типичные места, где скоуп мог бы незаметно расшириться, явно закрыты.
- Зависимость от #124 обработана корректно: не как продуктовое решение
Q0–Q6 (не переоткрывает их), а как отдельный DoR-блокер реализации (§10) с
двумя явными путями снятия; ревью ТЗ по правилам `PROCESS.md` §10 разрешено
вести, пока #124 открыт.
- i18n/UX (§9): подтверждено отсутствием новых строк, README/CHANGELOG/
USER-GUIDE не обещают публичную функцию — соответствует `User-Visible: no`
контракту для всей будущей реализации.
- Модель данных и совместимость (§8): подтверждено отсутствием новых
схемных полей, ключей `localStorage` версий и compatibility-записей;
откат (§15) не требует миграции и работает немедленно через
`?hp-labs=-iso`/`off`, как и в Stage 1.
- AC1–AC16 однозначны, у каждого указан тип доказательства (за вычетом Low-2)
и план автотестов (§12) даёт конкретный, воспроизводимый по коду маршрут;
§12.1 явно требует, чтобы каждый unit-мутант «умел падать» (удаление
внешнего edge, сдвиг leaf с jamb, включение HA-состояния в cache key,
дублирование floor symbol, лишний light layer).
- Не найдено ни одного продуктового утверждения о поведении, которое не
следует ни из канонических документов, ни из решений владельца Q0–Q6, ни
из чтения существующего кода, и при этом не помечено как предположение;
раздел §16 корректно отделяет свободно изменяемые технические детали
(именование модулей, точные проценты высоты панелей, число SVG-корней) от
зафиксированных владельцем решений.
- Реестр `docs/specs/README.md` обновлён тем же коммитом, коммит несёт
корректные трейлеры (`Issue: #122`, `User-Visible: no`) и не содержит
изменений класса A — правило №1 соблюдено на этапе `spec`.
## Чего не проверял
- Не проверял реализуемость конкретной геометрической декомпозиции
(`iso-openings.ts`/`iso-scene.ts`, точные проценты высоты панелей, число
SVG-defs) — по правилам ТЗ (§16) это свободно изменяемое техническое
предположение автора кода, не предмет ревью ТЗ.
- Не проверял реальный performance-профиль `large-house-isometric-v1` и не
прогонял #124 — на этапе `spec` это не требуется; факт открытости и
статус #124 (`S1-new`) проверен через `gh issue view`, не исполнением
бенчмарка.
- Не запускал никаких автотестов, golden или смоков — на этапе `spec` это не
требуется; существование референсных артефактов (`isometric-no-borders-dark`
в `demo/golden/matrix.mjs`) проверено чтением файла, не исполнением.
- Не проверял содержимое `docs/adr/089-isometric-stage1-renderer.md` построчно
на полное покрытие всех констант камеры — доверился ссылке ТЗ и
`docs/ISOMETRIC.md` как согласованному пересказу; расхождений при чтении
обоих источников не обнаружено.
- Не оценивал субъективную эстетическую адекватность референсов владельца
(issue #89, комментарий с изображениями) — это продуктовое решение,
принятое владельцем через Q2 и не подлежащее пересмотру ревьюером ТЗ.
## Вердикт
Зелёный. High: 0, Medium: 0. Две находки Low (отсутствие буквальной
декларации `Touch editor: …` и три AC с типом доказательства вне буквального
перечня §2.5) — не блокируют, оставлены автору на усмотрение с записью в этом
документе; при желании их можно поправить в этом же цикле без возврата в
`S3-spec`.