diff --git a/docs/reviews/SPEC-REVIEW-160-r1.md b/docs/reviews/SPEC-REVIEW-160-r1.md new file mode 100644 index 00000000..afbbb01e --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-160-r1.md @@ -0,0 +1,212 @@ +# SPEC-REVIEW — issue #160 · заход r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/160 +- **Этап:** spec (PROCESS.md §2.4) +- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 +- **Материал:** тело issue #160 + 6 комментариев (аналитика 2026-08-15, + вопросы владельцу, ответы дизайнера 2026-08-16, повторная актуализация + 2026-08-31, актуализированная аналитика 2026-09-05, хендофф ТЗ), файл + `docs/specs/160-isometric-stage3.md` на коммите `d82be48d` + (`docs(issue): specify isometric stage 3`), `docs/specs/README.md` того же + коммита. +- **Ветка:** `issue/160-isometric-stage3`, HEAD = `d82be48d`. +- **Трек:** полный (владелец прямо отверг `small`/`trivial` в аналитике + 2026-08-15 и подтвердил 2026-09-05: нарушены критерии сложности ≤3, одной + поверхности, отсутствия нового UX-контракта и отсутствия влияния на + performance/touch). ТЗ в `docs/specs/`, отдельного файла в теле issue — + корректно для полного трека. + +## Скоуп ревью + +Проверялись: обязательные разделы ТЗ по PROCESS.md §7.1, однозначность и +доказуемость каждого AC, отсутствие догадок, выданных за решения, соответствие +`docs/SCOPE.md` (какую строку Core user jobs закрывает), соответствие +трейлеров коммита и корректность обновления `docs/specs/README.md`. +Реализации нет — код-уровневые гейты (typecheck/test/build/golden/smoke/ +performance) к этапу spec не относятся и не запускались. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.4/§7.1/§7.2/§4/§5. +2. Прочитано тело issue #160 и все 6 комментариев по порядку, включая полные + тексты Q1–Q8 и ответов дизайнера/владельца. +3. Прочитан файл ТЗ `docs/specs/160-isometric-stage3.md` целиком (867 строк). +4. Факты о «текущем состоянии», заявленные в ТЗ как решённые (не как догадки), + сверены с реальным кодом и документами, а не приняты на слово: + - `ISO_CAMERA` (`rotDeg:0, tiltDeg:20, xyScale:1, zScale:1`), + `ISO_WALL_HEIGHT=64` — `src/iso-projection.ts:22-29` — совпадает с §0/D1. + - `LABS_FLAGS`/`LabsFlag` в `src/labs.ts` не содержит `since`/`expires` — + подтверждает заявление ТЗ, что срок действия `iso`-флага снят #448. + - `ALPHA_STORAGE_KEY = 'houseplan_card_alpha_v1'` (`src/labs.ts:9`) и + `LS_VIEW = 'houseplan_card_view_v1'` (`src/houseplan-card.ts:499`, + `src/houseplan-editor-runtime.ts:612`) — совпадают с §9. + - `docs/ISOMETRIC.md` уже фиксирует безсрочный `hp_alpha`, тот же projector + для SVG/HTML и `projectedFrame()` — совпадает с §7.1/§0. + - LRU cap 8 (`lruWrite(this._isoGeometryCache, source.key, value, 8)`, + `src/houseplan-card.ts:6129`) — совпадает с §7.2/AC12. + - `openingAmount()` существует как общая функция (`src/logic.ts:316`, + используется в `houseplan-card.ts`, `space-render.ts`, `glow-scene.ts`) — + совпадает с D5/AC6 про «единственный live amount». + - `renderZigbeeTopologyOverlay`/`hp-zigbee-topology-overlay.ts:169-174` + читает позицию маркера из живого DOM (`marker.style.left/top`, + `getBoundingClientRect()` на `.dev[data-id]`) — подтверждает точное + техническое утверждение D2, что топология обязана продолжать брать + фактические DOM-позиции, а не проецировать floor point заново; это не + догадка, а описание существующего кода. + - `demo/smoke_isometric_contract.mjs`, `demo/smoke_isometric_live_touch.mjs`, + `demo/performance/budgets-large-house-isometric.json` — существуют, + совпадают с §12.2/§14/§10. + - Stage 2 ТЗ (`docs/specs/122-isometric-stage2.md`) действительно уже + описывает ambient/contact/leaf shadows и matte materials — «что есть + сейчас» в аналитике 2026-08-15 не преувеличивает состояние Stage 2. + - #124, #168, #448 закрыты (проверено `gh issue view`) — совпадает с + заявлениями актуализации 2026-09-05. + - `docs/SCOPE.md`: узкое 2.5D-исключение действительно одобрено для #89 и + покрывает «ту же J1/J2/J3-состояние, действия и геометрию, без второй + модели и свободной камеры» — Stage 3 (§0 ТЗ) укладывается в эту рамку + буквально, не расширяя её. +5. Проверены трейлеры коммита `d82be48d` (`Issue: #160`, `User-Visible: no`) — + корректно для документации, не входящей в DoD видимого поведения (функция + скрыта, changelog не трогается — соответствует §15 ТЗ). +6. Проверено обновление индекса `docs/specs/README.md` — строка `#160` + добавлена, дата актуальности сдвинута — корректно. +7. #122 и #89 — оба `CLOSED`, подтверждая, что предыдущие стадии того же + узкого исключения уже прошли процесс без отдельного обновления + `docs/SCOPE.md` под каждую стадию — значит отсутствие правки SCOPE.md в + этом ТЗ не является новым отклонением от прецедента. + +## Проверка §7.1 (обязательные разделы) + +| Раздел §7.1 | Где в ТЗ | +|---|---| +| Сценарий (персона/поверхность/момент) | §1 — продвинутый пользователь/тестировщик с `hp_alpha=1`, View/kiosk | +| Что человек увидит до/после | §2, одной фразой без терминов реализации | +| Проблема | §3 | +| Скоуп и не-скоуп | §5, §6 | +| Контракт поведения | §4 (D1–D8), §7 | +| UX | §8 | +| Модель данных и миграция | §9 (явный отрицательный контракт) | +| i18n | §9 (явно: новых ключей нет) | +| AC1…ACn с доказательством | §11, AC1–AC15, у каждого поле **Evidence** | +| План автотестов | §12 | +| Риски | §17 | +| Откат | §18 | +| Release-артефакты | §15 | + +Все обязательные разделы присутствуют и содержательны, ни один не является +переформулировкой issue без добавленной информации. + +## Проверка «какую строку Core user jobs это закрывает» + +`docs/SCOPE.md` резервирует для #89 «узкое 2.5D-исключение… то же состояние +J1/J2/J3, действия и геометрия, без второй модели дома и свободной камеры». +ТЗ §0 и аналитика 2026-09-05 явно называют это основанием и не расширяют +исключение: камера остаётся одной фиксированной ортографической матрицей +(D1), никакая новая модель дома, свободная камера, WebGL/3D-мебель не +вводится (§6). Это соответствует правилу SCOPE.md буквально, а не по +касательной. + +## Проверка на догадку, выданную за решение + +Каждое числовое/поведенческое решение D1–D8 (`rotDeg=+4°`, `tiltDeg=20°`, +raised-list из D2, 44×44 px, `visualOffset`, отсутствие glass/handles у +проёмов, привязка теней к фиксированному light vector, продление жизни через +`hp_alpha` вместо `expires`) прослежено до конкретного ответа дизайнера +(комментарий +https://github.com/Matysh/houseplan-card/issues/160#issuecomment-5316071365) +и/или прямого решения владельца в аналитике 2026-09-05 — не изобретено +автором ТЗ. Единственные значения, не зафиксированные владельцем буквально +(`visualOffset`, safety gap, nudge cap — конкретные числа 4/4/48), correctно +вынесены в §19 «Принятые технические предположения» с пометкой «не новые +продуктовые вопросы, можно уточнять реализацией/ADR» — это именно тот +паттерн, который требует PROCESS.md §7.1, а не скрытая догадка. + +## Находки + +### Low — «Workflow boundary» (§16) ссылается на устное решение владельца, не отражённое в issue + +В §16 ТЗ и в последнем комментарии автора («ТЗ готово к независимому +ревью») утверждается: «По прямой команде владельца после реализации нельзя +инициировать код-ревью… остановиться на `S6-in-progress`». Ни в теле issue, +ни в одном из 6 комментариев такого поручения нет — это не продуктовый вопрос +из тех, что решает владелец по PROCESS.md §7.1 (что видит/делает человек, +какой объём видимых изменений в issue), и не спор автора с ревьюером, +разрешаемый вердиктом. Это самостоятельное процессное утверждение, +непроверяемое по доступному следу. + +**Почему не High/Medium:** раздел явно называет себя паузой, а не отменой +код-ревью («Это пауза перед код-ревью, а не его отмена»), и не меняет ни один +AC, скоуп или технический контракт — код-ревью всё равно остаётся +обязательным (AGENTS.md: «Code review is never skipped on either track») +перед `S8`/бетой. Ложность этого утверждения не создаёт дефекта в продукте, +только временную путаницу в маршруте после `S6`. + +**Как закрыть:** низкая серьёзность позволяет либо снять с запиской, либо +поправить. Рекомендация — при выходе в `S6-in-progress` продублировать это +поручение отдельным issue-комментарием от лица владельца (или сослаться на +внешний канал, если оно дано вне GitHub), чтобы следующий агент видел +основание в трассируемом виде, а не в теле ТЗ. Блокирующим для зелёного +вердикта не является. + +Других находок (High/Medium, в скоупе или вне скоупа) не выявлено. + +## Что проверено и корректно + +- Полнота обязательных разделов §7.1 — да, см. таблицу выше. +- Однозначность и доказуемость всех 15 AC — у каждого явный Evidence, + формулировки не оставляют открытого «как понять, что сделано». +- Соответствие 13 пунктов Scope (§5) и AC (§11) — все пункты scope покрыты + хотя бы одним AC (сверено построчно). +- Отсутствие непомеченных догадок — все нормативные числа/решения D1–D8 + прослежены до комментариев с ответами дизайнера/владельца; единственные + неподтверждённые владельцем числа явно помечены как предположения в §19. +- Технические факты о «текущем состоянии» (камера, LRU cap, storage keys, + Labs-контракт, поведение zigbee-topology-overlay, состояние #124/#168/#448) + подтверждены чтением кода/issues, а не приняты на слово автора. +- Трейлеры коммита `d82be48d` (`Issue: #160`, `User-Visible: no`) корректны + для документационного изменения, не входящего в DoD видимого поведения. +- Обновление `docs/specs/README.md` (индекс, дата) корректно. +- Негативный контракт данных/i18n/security (§9, §13, AC13) явный и + проверяемый чтением на этапе код-ревью. +- Раздел «откат» (§18) и «риски» (§17) присутствуют и специфичны для задачи, + не шаблонны. + +## Чего не проверял (и почему не нужно на этом этапе) + +- `npx tsc --noEmit`, `npm test`, `npm run build`, `npm run bundle:sync`, + golden/smoke/performance — код ещё не написан, класс A/B-файлы не менялись + этим коммитом (менялся только `docs/specs/**`, класс C); гейты реализации + относятся к этапу code review, не к spec review. +- `node scripts/check-docs.mjs` — не требуется: diff не касается `src/**`. +- `npm run invariants` — не требуется: diff не касается геометрии/ссылок в + коде, только текст ТЗ. +- Golden/browser smoke, названные в плане автотестов ТЗ (§12), — это план на + будущую реализацию, а не факт; они не могут быть прогнаны сейчас и не + являются частью этого ревью. +- Не проверялась реализация Stage 1/2 построчно за пределами тех фактов, + которые ТЗ Stage 3 использует как «текущее состояние» (см. раздел «Как + проверялось») — этого достаточно, чтобы подтвердить отсутствие догадок в + Stage 3. + +## Вердикт + +Зелёный. ТЗ полное, все обязательные разделы на месте, каждый AC однозначен +и снабжён способом доказательства, все нормативные решения прослежены до +ответов дизайнера/владельца, а не изобретены. Единственная находка — Low, +не блокирует, снимается с запиской выше. + +--- + + + +## Материал раунда + +- Ветка: `issue/160-isometric-stage3`, коммит `d82be48dfbe7` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c224cbd2cee2cf653280a13a6e372148c60e26c1` + ``` + git log --all --format='%H %T' | grep c224cbd2cee2 + ``` +- ТЗ `docs/specs/160-isometric-stage3.md`, блоб `837f05545baf1a85ae03a1452d887827f4351a22` + ``` + git log --all --find-object=837f05545baf1a85ae03a1452d887827f4351a22 -- docs/specs/160-isometric-stage3.md + ```