mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -1,9 +1,10 @@
|
|||||||
# Индекс ревью
|
# Индекс ревью
|
||||||
|
|
||||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1064, issue: 375. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1065, issue: 376. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||||
|
|
||||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||||
|---|---|---|---|---:|---:|---|---|
|
|---|---|---|---|---:|---:|---|---|
|
||||||
|
| #651 | [SPEC-REVIEW-651-r1.md](SPEC-REVIEW-651-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | устаревшая формулировка «экспериментальный 2.5D-вид» противоречит текущему статусу функции | `CHANGELOG.md` `CHANGELOG.ru.md` `docs/ISOMETRIC.md` `docs/USER-GUIDE.ru.md` `docs/STATUS.md` `USER-GUIDE.ru.md` |
|
||||||
| #650 | [CODE-REVIEW-650-r1.md](CODE-REVIEW-650-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
| #650 | [CODE-REVIEW-650-r1.md](CODE-REVIEW-650-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||||
| #649 | [SPEC-REVIEW-649-r1.md](SPEC-REVIEW-649-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | в скоупе задачи (возвращается автору); принято ревьюером с записью, правки не требует | `lab.js` `houseplan-card.ts` |
|
| #649 | [SPEC-REVIEW-649-r1.md](SPEC-REVIEW-649-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | в скоупе задачи (возвращается автору); принято ревьюером с записью, правки не требует | `lab.js` `houseplan-card.ts` |
|
||||||
| #649 | [SPEC-REVIEW-649-r2.md](SPEC-REVIEW-649-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
| #649 | [SPEC-REVIEW-649-r2.md](SPEC-REVIEW-649-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||||
|
|||||||
@@ -0,0 +1,90 @@
|
|||||||
|
# SPEC-REVIEW-651-r1
|
||||||
|
|
||||||
|
**Issue:** [#651](https://github.com/Matysh/houseplan-card/issues/651) — «2.5D: значки устройств смещаются при зуме и меняют взаимное выравнивание относительно 2D»
|
||||||
|
**Этап:** spec (ревью ТЗ, PROCESS.md §2.4)
|
||||||
|
**Заход:** r1 · блокирующих циклов израсходовано (после этого вердикта): 1 из 4
|
||||||
|
**Материал раунда:** тело issue #651, раздел `## ТЗ r1` (снято на момент ревью, 2026-09-26); связанные issue #644 (эпик-предшественник, closed), #570/#583/#585 (Stage 4); канонические документы `docs/SCOPE.md`, `docs/ISOMETRIC.md`, `docs/USER-GUIDE.ru.md`; код `src/iso-overlays.ts`, `src/iso-scene-render.ts`, тесты `test/iso-overlays.test.mjs`, `test/iso-scene-render.test.mjs`, perf-фикстуры `demo/performance/budgets-large-house-isometric.json`, `demo/performance/isometric-stage3-dense-fixture.mjs`.
|
||||||
|
|
||||||
|
## Вердикт
|
||||||
|
|
||||||
|
**Жёлтый.** High: 0 · Medium: 1 (в скоупе, возвращается автору) · Low: 0.
|
||||||
|
|
||||||
|
Единственная находка — устаревшая терминология «экспериментальный 2.5D-вид» и вытекающая из неё инструкция по changelog, которые противоречат canonical-документам после Stage 6 (#649): 2.5D — публичная настройка, не альфа-функция. Остальная часть ТЗ — обязательные разделы §7.1, однозначность AC, способ доказательства каждого AC, явный блок технических предположений — выполнена корректно и не требует правок.
|
||||||
|
|
||||||
|
## Скоуп проверки
|
||||||
|
|
||||||
|
Проверено соответствие ТЗ:
|
||||||
|
1. `docs/SCOPE.md` — какую строку Core user jobs закрывает задача;
|
||||||
|
2. обязательным разделам ТЗ по PROCESS.md §7.1 (сценарий, что человек увидит, проблема, скоуп/не-скоуп, поведенческий контракт, UX, данные/миграция, i18n, AC1…ACn с доказательством, план автотестов, риски, откат, release-артефакты);
|
||||||
|
3. однозначности и проверяемости каждого AC;
|
||||||
|
4. терминологии — против `docs/USER-GUIDE.ru.md` и `docs/ISOMETRIC.md` (обязательное чтение по инструкции ревью для видимого поведения);
|
||||||
|
5. фактической продуктовой истории вопроса — прочитан эпик #644 целиком (включая Q1–Q10 и ответы дизайнера), чтобы понять, откуда в ТЗ #651 взято решение о групповом сдвиге, пределе 48 CSS px и запрете уменьшения, и не противоречит ли ТЗ #651 более раннему явному ответу по деградации;
|
||||||
|
6. существования артефактов, на которые ссылаются AC и план автотестов (файлы тестов, perf-фикстуры, golden-набор), — чтобы отличить проверяемое ТЗ от ссылок на несуществующий код.
|
||||||
|
|
||||||
|
## Как проверялось
|
||||||
|
|
||||||
|
- Тело issue #651 получено через `gh issue view 651 --json body` (MCP `get_issue`/`get_issue_comments` были недоступны без разрешения пользователя в этой сессии — использован `gh` CLI как эквивалентный путь получения того же публичного API).
|
||||||
|
- Прочитаны `docs/SCOPE.md`, `docs/process/REVIEWER.md`, `docs/ISOMETRIC.md` целиком, фрагмент `docs/USER-GUIDE.ru.md` (раздел про объёмный вид плана, строки 336–346), `docs/STATUS.md` (строка про 2.5D View).
|
||||||
|
- Прочитан целиком эпик #644 (тело + все комментарии, включая ответы дизайнера `singlmolt-prog`, association `NONE`, — оценка авторства подтверждена через `gh api .../issues/644/comments --jq '.[] | {user, association}'`).
|
||||||
|
- `grep`/`glob` по репозиторию: подтверждено существование `resolveIsoOverlayPlacement`, `resolveIsoOverlayCollisions`, `buildIsoOverlayRenderScene` в `src/iso-overlays.ts`/`src/iso-scene-render.ts`; подтверждено существование perf-фикстур `large-house-isometric` и `isometric-stage3-dense`, упомянутых в AC8; прочитан фрагмент `test/iso-scene-render.test.mjs` (тест `#473 W3`), чтобы понять характер существующих ожиданий по зуму, на которые ссылается «Аналитика».
|
||||||
|
- Код продукта не менялся, гейты (`tsc`/`test`/`build`) не запускались — на этапе `spec` они не относятся к предмету ревью (ревью ТЗ, не диффа).
|
||||||
|
|
||||||
|
## Находки
|
||||||
|
|
||||||
|
### Medium (в скоупе) — устаревшая формулировка «экспериментальный 2.5D-вид» противоречит текущему статусу функции
|
||||||
|
|
||||||
|
**Файл:** тело issue #651, раздел `## ТЗ r1`, п. 1 «Пользовательский сценарий» и п. 13 «Release-артефакты».
|
||||||
|
|
||||||
|
**Воспроизведение:**
|
||||||
|
- п. 1 сценария: «Пользователь смотрит готовый план в `View` или `kiosk`… включает **экспериментальный 2.5D-вид**…»
|
||||||
|
- п. 13 release-артефактов: «`CHANGELOG.md` и `CHANGELOG.ru.md`: исправление дрейфа/распада раскладки значков в 2.5D (`User-Visible: yes`). **Не раскрывать способ включения экспериментальных функций.**»
|
||||||
|
|
||||||
|
Это прямо расходится с canonical-документами, которые инструкция ревью требует читать для видимого поведения:
|
||||||
|
- `docs/ISOMETRIC.md:7-8`: «Since Stage 6 (#649) the 2.5D View is a public mode» и далее (`docs/ISOMETRIC.md:10-28`, раздел «Activation»): «One installation-wide setting, `settings.volumetric_view: boolean`, switched in **General settings › Display › Show the plan in 2.5D**… There is no toggle on the card and no alpha entry: `iso` is gone from `LABS_FLAGS`… The `hp_alpha` switch itself remains as a mechanism without experiments.»
|
||||||
|
- `docs/USER-GUIDE.ru.md:338-346`: «Объёмный вид плана включает администратор один раз: **Общие настройки → Отображение → Объёмный вид плана (2.5D)**…» — раздел написан для конечного пользователя, без единого упоминания «экспериментальный» или «альфа».
|
||||||
|
- `docs/STATUS.md:42-43`: релиз-кандидат beta.3 явно описан как «It makes 2.5D a public opt-in setting (#649)», «#649 Stage 6 makes it public… replaces the alpha entry, the header toggle and the phone-menu item».
|
||||||
|
|
||||||
|
**Почему это находка, а не стилистика.** Формулировка «экспериментальный» — не безобидная деталь сценария: из неё прямо выведена инструкция для release-артефакта («не раскрывать способ включения экспериментальных функций»). Скрывать нечего: активация — документированная публичная настройка в общем разделе UI, уже описанная в `USER-GUIDE.ru.md` открытым текстом с названием пункта меню. Если разработчик буквально выполнит инструкцию п. 13, changelog получится обеднённым или обфусцированным без причины — вместо обычной практики называть публичную настройку по имени, как делают другие записи changelog про этот же режим (см. `docs/STATUS.md:42`, где #649 описан открыто). Формулировка также немного искажает картину аудитории бага для читателя ТЗ: это не редкий альфа-путь, а обычный публичный тумблер, доступный любому администратору.
|
||||||
|
|
||||||
|
**Чем закрывается.** Заменить «экспериментальный 2.5D-вид» на терминологию `USER-GUIDE.ru.md` («объёмный вид плана (2.5D)» / «Показать план в 2.5D» — как в интерфейсе), убрать из п. 13 инструкцию «не раскрывать способ включения» (либо переформулировать в нейтральное «changelog не описывает внутренний контракт наложений/кэша», если цель была не про активацию, а про технический контракт — сейчас не читается однозначно).
|
||||||
|
|
||||||
|
**Класс:** Medium, в скоупе задачи — правится прямо в теле ТЗ, отдельный issue не заводится (#202).
|
||||||
|
|
||||||
|
## Что проверено и корректно
|
||||||
|
|
||||||
|
- **Обязательные разделы §7.1** — все присутствуют: сценарий (п.1, с «до → после одной фразой»), проблема (вводный `## Проблема`/`## Наблюдаемое поведение`), скоуп/не-скоуп (п.3–4), поведенческий контракт (п.5.1–5.5), UX (п.6), данные/миграция (п.7), локализация (п.8), AC1–AC9 с доказательством (п.9), план автотестов (п.10), риски (п.11), откат (п.12), release-артефакты (п.13). Дополнительно указаны затрагиваемые модули (п.14) и явный блок технических предположений (п.15) — точно то, что требует PROCESS.md §7.1 для «размытых мест».
|
||||||
|
- **Соответствие docs/SCOPE.md.** Задача чинит J1/J2/J3 в рамках уже принятого исключения #89 («deterministic 2.5D presentation of the existing canonical plan… same geometry»); новой модели, камеры, настройки или редактора не вводит. Согласуется с «Не входит» (п.4 ТЗ), явно исключающим новую настройку/схему/ручное позиционирование.
|
||||||
|
- **Однозначность и доказуемость AC1–AC9.** Каждый AC называет конкретный тестовый файл/тип проверки (`test/iso-overlays.test.mjs`, `test/iso-scene-render.test.mjs`, browser smoke, Linux golden, exact-SHA perf) и наблюдаемый критерий (компоненты групп, единый вектор, инвариантность к zoom/pan, предел 48 px, совпадение visual/hit root, отсутствие побочных изменений). Ни один AC не сформулирован как самоцель реализации («сделать функцию X») — все формулируют наблюдаемый результат.
|
||||||
|
- **Продуктовая история вопроса подтверждена по первоисточнику.** Эпик #644 (тело + комментарии) прочитан целиком: групповой сдвиг (Q1), предел 48 CSS px и запрет уменьшения (Q5, первая часть) — были явно приняты автором задачи (дизайнером) как продуктовое решение и корректно перенесены в ТЗ #651 (п.5.3.6: «Принятое в #644 решение о групповом сдвиге, пределе 48 px и запрете уменьшения сохраняется»).
|
||||||
|
- **Пересмотр деградации (§5.3.5–6) — рассмотрен отдельно и принят.** #644 Q5 ранее фиксировал другую деградацию при переполнении 48 px («делится на одиночные значки»); ТЗ #651 заменяет это на «группа остаётся жёсткой, допускается остаточное наложение», обосновывая более поздним явным требованием исходного бага («если несколько значков в 2D находились на одной оси… они также остаются на одной оси… повторные зум и переключение не должны накапливать смещение» — это формулировка из раздела «Ожидаемое поведение» самого issue, а не собственная догадка автора ТЗ). Разбор: это пограничный случай «что считать приемлемой деградацией» (категория, которая по §7.1 обычно эскалируется владельцу), но здесь он (а) прямо выводится из уже сформулированного в issue требования, а не придуман автором ТЗ, (б) явно раскрыт в трёх местах согласованно — поведенческий контракт (5.3.5–6), UX (п.6) и блок технических предположений (п.15.5) с пометкой «ревьюер вправе оспорить», а не спрятан как решённый факт. Технический спор автора ТЗ и ревьюера по такого рода пункту по PROCESS.md §7.1 решается вердиктом ревьюера — вывод: обоснование достаточно, менять не требую.
|
||||||
|
- **Отсутствие открытых продуктовых вопросов проверено, а не принято на слово.** Кроме разобранного выше пункта о деградации, других мест, где догадка выдаётся за факт, не найдено.
|
||||||
|
- **Артефакты, на которые ссылаются AC и план автотестов, существуют.** `resolveIsoOverlayPlacement`, `resolveIsoOverlayCollisions`, `buildIsoOverlayRenderScene` — реальные экспорты `src/iso-overlays.ts`/`src/iso-scene-render.ts`; perf-бюджеты `large-house-isometric` (`demo/performance/budgets-large-house-isometric.json`) и `isometric-stage3-dense` (`demo/performance/isometric-stage3-dense-fixture.mjs`, `demo/performance/budgets-isometric-stage3-dense.json`) существуют и совпадают с именами в AC8 — это не выдуманные ссылки. `demo/smoke_isometric_overlay_stability.mjs` из плана автотестов (п.10.3) пока не существует, но ТЗ прямо говорит «добавить/расширить», это не находка.
|
||||||
|
- **Отсутствие схемы/миграции (п.7)** соответствует действительности: изменение — исключительно runtime-presentation слой (`iso-overlays.ts`/`iso-scene-render.ts`), схема конфигурации не затрагивается.
|
||||||
|
- **i18n (п.8)** — новых строк нет, что верно для чисто геометрического фикса без новых UI-элементов.
|
||||||
|
- **Откат (п.12)** и **golden-протокол (AC7)** сформулированы в соответствии с процессом: новый baseline — только из reviewed Linux artifact, не из author-коммита.
|
||||||
|
|
||||||
|
## Чего не проверял
|
||||||
|
|
||||||
|
- Не запускал `tsc`/`npm test`/`npm run build` — на этапе `spec` предмет ревью текст ТЗ, а не код; кода изменений ещё нет (ветка `issue/651-iso-device-layout-stability` создана от `origin/dev`, но не содержит правок реализации на момент ревью).
|
||||||
|
- Не проверял руками поведение в браузере (нет реализации, нечего запускать) — это работа код-ревью следующего этапа.
|
||||||
|
- Не читал `test/iso-overlays.test.mjs` построчно целиком (только структуру и заголовки, плюс точечно `test/iso-scene-render.test.mjs`) — на этапе ТЗ этого достаточно, чтобы убедиться, что упомянутые в АС файлы и функции существуют; полный аудит тестов на «умеет падать» — задача код-ревью (§2.7).
|
||||||
|
- Не проверял вторую часть ответа на Q5 в #644 («уменьшение группы котельной до 86% было костылём лаборатории, в продукт не переносится») на предмет отдельных скрытых противоречий за пределами прямого предмета #651 — вне скоупа этой задачи.
|
||||||
|
- Полные наборы golden/perf/smoke не прогонял — на этом этапе кода для прогона ещё нет; это часть п.10 «План автотестов» и войдёт в работу при реализации (§2.6) и последующее код-ревью (§2.7).
|
||||||
|
|
||||||
|
## Рекомендация автору
|
||||||
|
|
||||||
|
Одна точечная правка текста ТЗ (терминология + инструкция п.13) — не требует нового цикла анализа по существу контракта. После правки формулировки задача готова к статусу «Готово к разработке» без дополнительного захода по существу поведенческого контракта.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||||
|
|
||||||
|
## Материал раунда
|
||||||
|
|
||||||
|
- Ветка: `issue/651-iso-device-layout-stability`, коммит `ec2b8c15220f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||||
|
- Дерево материала: `a6750c5fd9b9b80f3a4f0f7b1642a5148c938b6f`
|
||||||
|
```
|
||||||
|
git log --all --format='%H %T' | grep a6750c5fd9b9
|
||||||
|
```
|
||||||
|
- Тело issue: `ea257bbcd4d329123e5400e69da0f9fb7460dfd29bcd0ddd2871e5e0a81b9e97`
|
||||||
|
- Вердикт конвейера: `yellow` · High 0
|
||||||
Reference in New Issue
Block a user