mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 21:01:21 +00:00
committed by
Sergey Matyunin
parent
15e047949e
commit
27ba3f2ee0
@@ -1,11 +1,12 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1042, issue: 367. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1043, issue: 367. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| #646 | [CODE-REVIEW-646-r1.md](CODE-REVIEW-646-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #645 | [SPEC-REVIEW-645-r1.md](SPEC-REVIEW-645-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 2 | Отсутствует обязательная строка Touch editor: … из docs/TOUCH-SUPPORT.md → «Documentati…; ТЗ не учитывает существующую зависимость live-подписей Resize от позиции кнопки «Настро…; / Low | `docs/TOUCH-SUPPORT.md` `docs/reviews/SPEC-REVIEW-449-r1.md` `docs/specs/359-furniture-placement-preview.md` `docs/specs/449-double-fit-all.md` `src/houseplan-editor-runtime.ts` `src/houseplan-card.ts` `src/resize-labels.ts` `docs/process/REVIEWER.md` |
|
||||
| #645 | [SPEC-REVIEW-645-r2.md](SPEC-REVIEW-645-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #643 | [CODE-REVIEW-643-r1.md](CODE-REVIEW-643-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #642 | [SPEC-REVIEW-642-r1.md](SPEC-REVIEW-642-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #642 | [CODE-REVIEW-642-r1.md](CODE-REVIEW-642-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
|
||||
@@ -0,0 +1,214 @@
|
||||
# SPEC-REVIEW-645-r2
|
||||
|
||||
Issue: #645 — «Plan editor: временно перемещать кнопку "Настройки комнаты",
|
||||
если она перекрывает элементы» (bug/polish, P2, полный трек — лёгкий трек
|
||||
аналитика отклонила: нарушен критерий §5 «нет нового UX-контракта», плюс
|
||||
задача трогает touch-контракт редактора).
|
||||
Заход: r2 · блокирующих циклов израсходовано 1 из 4.
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Предмет r2 — дельта к тексту ТЗ между r1 (жёлтый, M1+M2) и текущей редакцией
|
||||
тела issue #645, а не текст целиком (`PROCESS.md` §2.10). Продуктовый код не
|
||||
менялся: `git log --oneline origin/dev..HEAD` содержит только
|
||||
`76fc405b docs: review document for #645` (публикация документа r1), и
|
||||
`git diff --stat origin/dev...HEAD` трогает исключительно
|
||||
`docs/reviews/INDEX.md` и `docs/reviews/SPEC-REVIEW-645-r1.md`. Ни одного
|
||||
файла класса A/B нет — это подтверждает и хендофф-комментарий автора
|
||||
(«продуктовый код не менялся»). Диапазон `origin/dev...HEAD` для этого этапа
|
||||
пуст по‑прежнему, разбирается только текст.
|
||||
|
||||
Материал: тело issue #645 на момент чтения (`updated_at`
|
||||
`2026-09-24T18:34:09Z`, сразу после итогового комментария автора), метки
|
||||
`bug`, `P2`, `polish`, `S4-spec-review`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Найден вердикт и документ предыдущего раунда:
|
||||
`docs/reviews/SPEC-REVIEW-645-r1.md` (жёлтый, High 0, Medium 2 — M1, M2),
|
||||
зафиксированный в дереве на `76fc405b` вместе с обновлением
|
||||
`docs/reviews/INDEX.md`.
|
||||
2. Дельта объявлена по комментариям автора (`gh api
|
||||
repos/Matysh/houseplan-card/issues/645/comments`): два новых комментария
|
||||
«Исправления по SPEC-REVIEW-645-r1» и «Сделано: …», оба датированы
|
||||
`2026-09-24T18:34:0{6,7}Z`, и `updated_at` тела issue `18:34:09Z` —
|
||||
редакция тела произошла сразу после них, других правок текста между r1 и
|
||||
r2 не было.
|
||||
3. Прочитано тело issue #645 целиком заново (раздел `## ТЗ`, 258 строк) —
|
||||
не выборочно, чтобы отличить реальную правку от заявления автора «в тексте
|
||||
исправлено».
|
||||
4. По каждой находке r1 показано, какой конкретный фрагмент текущего текста
|
||||
её закрывает — см. «Закрытие раунда r1» ниже. Для M1 сверено дословное
|
||||
совпадение добавленной строки с одним из трёх допустимых значений
|
||||
`docs/TOUCH-SUPPORT.md:221–223` (`Documentation rule`). Для M2 повторно
|
||||
прочитан код, на который опирается находка, чтобы подтвердить, что он не
|
||||
менялся и предложенная формулировка контракта действительно закрывает
|
||||
именно эту связку:
|
||||
- `src/houseplan-editor-runtime.ts:3427` — `gearCenter:
|
||||
poleOfInaccessibility(poly)` внутри `_rszEdgeLabels`, без условия на
|
||||
временную позицию — код идентичен цитате в SPEC-REVIEW-645-r1 (та же
|
||||
строка, тот же безусловный вызов).
|
||||
- `src/resize-labels.ts:25` (`ResizeAreaPlacementInput.gearCenter`) и
|
||||
`src/resize-labels.ts:97,113–116` (`gearScreen`/`overlaps` против
|
||||
`gearCenter`) — сигнатура и использование параметра не изменились;
|
||||
значит контракт п.15 (единый эффективный центр для рендера кнопки и
|
||||
Resize-подписи) действительно закрывает разрыв, а не описывает уже
|
||||
исправленный код.
|
||||
5. Проверено, что новый AC11 и новый пункт «Затронутые модули»
|
||||
(`src/resize-labels.ts`) взаимно согласованы с планом автотестов (п.5 —
|
||||
«переместить кнопку… начать Resize стены… доказать, что подпись обходит
|
||||
фактическую капсулу») и с разделом «Риски» (новый пункт «live-подпись
|
||||
Resize может обходить старый автоматический центр… если render и Resize
|
||||
используют разные resolver» → явно назван закрытым AC11).
|
||||
6. Проверено, что делта не переоткрывает ранее закрытые части r1: контракт
|
||||
п.1–14, AC1–AC10, «Затронутые модули» (кроме добавленной строки),
|
||||
«Не-скоуп», «Модель данных», i18n, «Производительность», «Откат»,
|
||||
«Release-артефакты» текстуально не изменились относительно того, что
|
||||
цитирует и разбирает `SPEC-REVIEW-645-r1.md` (построчное сравнение с
|
||||
цитатами в разделе «Что проверено» этого документа).
|
||||
7. Проверено на новую двусмысленность: не создаёт ли добавленный п.15
|
||||
противоречия с п.11 (сброс временной позиции при изменении геометрии).
|
||||
П.11 регулирует **финальную** (закоммиченную) геометрию комнаты; п.15
|
||||
явно говорит про **preview**-геометрию активного Resize «в этом же
|
||||
кадре» — разные моменты жизненного цикла, конфликта нет: если Resize
|
||||
отменяется без коммита, актуальная геометрия комнаты не меняется и
|
||||
валидность временной точки пересчитывается по-прежнему через п.11.
|
||||
8. Перечитан `docs/TOUCH-SUPPORT.md` целиком ещё раз (`Documentation rule`,
|
||||
строки 214–226) — требуемый формат не изменился со времени r1.
|
||||
|
||||
Этап spec: тяжёлые/browser-гейты не запускались — код по-прежнему не
|
||||
написан (см. п.1 «Скоуп ревью»), прогон `tsc`/`test`/`build`/
|
||||
`check-docs.mjs` на этом этапе бессмыслен, диапазон класса A/B пуст.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка (r1) | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M1** — нет обязательной строки `Touch editor: …` из `docs/TOUCH-SUPPORT.md` | Добавлена строка `**Touch editor: best effort / intentionally degraded.**` в начало раздела `### UX и доступность`, дословно совпадает с одним из трёх допустимых значений | Тело issue #645, раздел «UX и доступность», первая строка; `docs/TOUCH-SUPPORT.md:222` — точное совпадение формулировки |
|
||||
| **M2** — Resize-подписи уклоняются от старого автоматического центра, а не от фактической (перетащенной) позиции кнопки; риск воспроизвести исходный дефект в паре «подпись ↔ кнопка» | Новый пункт контракта 15 требует единого эффективного центра для рендера кнопки и `_rszEdgeLabels`/`placeResizeAreaLabel`, явно запрещает обход «старого» `poleOfInaccessibility(poly)`, определяет поведение при недопустимой preview-точке; `src/resize-labels.ts` добавлен в «Затронутые модули»; добавлен **AC11** с доказательством `unit + browser smoke`; в «Риски» — явный пункт про расхождение resolver, закрытый AC11 | Тело issue #645: контракт «15. Согласованность с Resize»; «Затронутые модули», строка про `src/resize-labels.ts`; «Критерии приёмки», AC11; «Риски», предпоследний пункт; «Принято предположительно», пункт про «место общего resolver эффективного центра» |
|
||||
|
||||
Обе находки r1 закрыты текстом ТЗ; новых пробелов при проверке закрытия не
|
||||
обнаружено (см. «Как проверялось», пп. 4–7).
|
||||
|
||||
## Унаследовано из r1 (без повторной проверки)
|
||||
|
||||
Ниже — то, что дельта не задевает и что принимается по документу
|
||||
`docs/reviews/SPEC-REVIEW-645-r1.md` (материал: тело issue #645 на момент
|
||||
r1, код на `2b3959e55bcb139b6ad872a312ad212de36f44db`, вердикт жёлтый,
|
||||
High 0, Medium 2):
|
||||
|
||||
- Обоснованность полного трека (нарушенный критерий §5) и отсутствие
|
||||
дубликатов — из «Аналитики» автора, не менялось.
|
||||
- Полнота обязательных разделов `PROCESS.md` §7.1 (кроме добавленной строки
|
||||
Touch editor и пункта 15 — они дельта и перепроверены выше).
|
||||
- Точность фактических заявлений ТЗ о существующем коде: `_renderRoomGear()`,
|
||||
`poleOfInaccessibility(room.poly)`, `.rlgearbtn`/`pointer-events: auto`
|
||||
(`src/houseplan-editor-runtime.ts:10276–10300`), порог 3 CSS px в
|
||||
`_pointerMoveNow` (`src/houseplan-card.ts:7031`), существующий паттерн
|
||||
`_suppressClick`, наличие `pointInPolygon` в `src/logic.ts`, терминология
|
||||
`room.settings_title`/`room.settings_short` (`src/i18n/ru.json:765,800`,
|
||||
`docs/USER-GUIDE.ru.md:1621`) — код с r1 не менялся (см. «Скоуп ревью»),
|
||||
переоснований для пересмотра нет.
|
||||
- Покрытие контракта п.1–14 критериями AC1–AC10 построчно (таблица в r1,
|
||||
раздел «Что проверено и корректно») — тексты пунктов 1–14 и AC1–AC10 не
|
||||
менялись.
|
||||
- Оценка модели данных/миграции/i18n/производительности/отката/
|
||||
release-артефактов как полных и корректных — тексты этих разделов (кроме
|
||||
«Затронутые модули», куда добавлена одна строка про `resize-labels.ts`) не
|
||||
менялись.
|
||||
- Отсутствие открытых продуктовых вопросов владельцу.
|
||||
- Low-находка «не поднимаю отдельно» про `lostpointercapture` vs штатный
|
||||
`pointerup` (раздел «Чего не проверял» r1) — техническая, отнесена к
|
||||
«Принято предположительно», статус не изменился.
|
||||
|
||||
## Находки
|
||||
|
||||
Находок нет — ни High, ни Medium, ни Low. Обе Medium-находки r1 закрыты (см.
|
||||
таблицу выше), новых пробелов делта не создала.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **M1 закрыта корректно**: формулировка дословно совпадает с допустимым
|
||||
значением `docs/TOUCH-SUPPORT.md`, расположена в предметно верном месте
|
||||
(открывает «UX и доступность», рядом с остальными touch-оговорками),
|
||||
что и было предложено r1 как один из вариантов.
|
||||
- **M2 закрыта содержательно, не формальной отпиской**: выбран первый из
|
||||
двух вариантов, предложенных r1 («явно включить в скоуп»), с добавлением
|
||||
AC, правкой «Затронутые модули» и явным описанием поведения на границе
|
||||
(недопустимая preview-точка → синхронный откат кнопки и подписи на
|
||||
автоматический центр). Формулировка учитывает разницу между
|
||||
финальной и preview-геометрией (см. «Как проверялось», п.7) — типичная
|
||||
ошибка «просто добавили AC, не подумав о моменте применения» не
|
||||
воспроизведена.
|
||||
- **Новый AC11 однозначен и несёт способ доказательства** (`unit
|
||||
placeResizeAreaLabel/resolver + browser smoke`), согласован с пунктом 5
|
||||
плана автотестов, который был обновлён синхронно с добавлением AC11.
|
||||
- **Делта не тронула ничего вне заявленного двух-пунктового исправления**:
|
||||
сверка текста контракта 1–14, AC1–AC10, «Не-скоуп», i18n,
|
||||
«Производительность», «Откат», «Release-артефакты» с цитатами
|
||||
`SPEC-REVIEW-645-r1.md` не выявила расхождений.
|
||||
- **Продуктовый код действительно не менялся** между r1 и r2 —
|
||||
`git diff --stat origin/dev...HEAD` содержит только два файла
|
||||
документации (сама публикация r1), что делает «унаследовано без
|
||||
перепроверки» по коду формально обоснованным, а не предположением.
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- **Реализацию** — её по-прежнему нет; этап spec, код не написан.
|
||||
- **`npx tsc --noEmit` / `npm test` / `npm run build` /
|
||||
`node scripts/check-docs.mjs`** — не гонял: диапазон класса A/B пуст,
|
||||
прогон гейтов на этапе ревью ТЗ бессмыслен (§2.4 их не требует), это же
|
||||
было верно для r1.
|
||||
- **Browser smoke / golden / инварианты модели / performance** — код,
|
||||
который они бы проверяли, ещё не существует; план автотестов ТЗ (включая
|
||||
обновлённый п.5 про Resize-сценарий) оценён на реалистичность чтением, не
|
||||
исполнением.
|
||||
- **Полный повторный аудит всех разделов ТЗ «с нуля»** — сознательно не
|
||||
делал: `PROCESS.md` §2.10 предписывает объём по дельте, когда дельта
|
||||
локальна (здесь — да: два точечных добавления, подтверждённых
|
||||
комментариями автора и `updated_at`); неизменные разделы приняты из
|
||||
документа r1 с указанием SHA материала (раздел «Унаследовано из r1»).
|
||||
- **`docs/CANVAS.md`, `docs/WALL-THICKNESS.md`, `docs/LIGHT.md`,
|
||||
`docs/SUN.md`** — не читал, как и в r1: не канонические документы для
|
||||
этой предметной области; `resize-labels.ts` затрагивает геометрию Resize,
|
||||
но не эти подсистемы напрямую (нет собственной канонической страницы,
|
||||
кроме уже прочитанного `docs/TOUCH-SUPPORT.md`/`docs/UX-MODES.md`).
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Issue: #645, метки на момент ревью: `bug`, `P2`, `polish`,
|
||||
`S4-spec-review`.
|
||||
- ТЗ: раздел `## ТЗ` тела issue #645, редакция после правок r1
|
||||
(`updated_at`: `2026-09-24T18:34:09Z`).
|
||||
- Комментарии, учтённые дополнительно к r1: «Исправления по
|
||||
SPEC-REVIEW-645-r1» (`2026-09-24T18:34:06Z`), «Сделано: внесены обе
|
||||
Medium-правки r1 в ТЗ…» (`2026-09-24T18:34:07Z`).
|
||||
- Рабочее дерево репозитория на момент чтения кода: `76fc405b2c6c566c8654019c373fa9fb4e878a18`
|
||||
(HEAD, справочно — для проверки, что фактические заявления ТЗ о текущем
|
||||
коде и находка M2 по-прежнему опираются на неизменившийся код; продуктовый
|
||||
код между r1 и r2 не менялся).
|
||||
- Материал предыдущего раунда: `docs/reviews/SPEC-REVIEW-645-r1.md`,
|
||||
вердикт жёлтый, High 0, Medium 2 (M1, M2), код на
|
||||
`2b3959e55bcb139b6ad872a312ad212de36f44db`.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0
|
||||
|
||||
Обе Medium-находки r1 закрыты содержательно, без формальных отписок и без
|
||||
новых пробелов (см. «Закрытие раунда r1»). Делта не переоткрывает ранее
|
||||
принятые разделы. ТЗ готово к DoR/переходу в `S5-ready`.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/645-room-settings-button-drag`, коммит `76fc405b2c6c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `ff8460bad9ebe36cd0bb9ebfe7eb75688a327997`
|
||||
```
|
||||
git log --all --format='%H %T' | grep ff8460bad9eb
|
||||
```
|
||||
- Тело issue: `1cfefdd93db1946a31ecfd2bc15198fd49ae0e9031bd1b6b9751e50258aa0a46`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user