mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,198 @@
|
||||
# SPEC-REVIEW-461-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/461
|
||||
- **ТЗ:** `docs/specs/461-wall-draw-click-performance.md`
|
||||
- **Материал:** SHA `1ef21037a8c32ad203a9cad05c801420223db8b6`, ветка
|
||||
`issue/461-wall-draw-click-performance`, коммит `docs: specify wall draw
|
||||
click performance` (`docs/specs/461-wall-draw-click-performance.md` +
|
||||
`docs/specs/README.md`, `Issue: #461 · User-Visible: no`)
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
|
||||
- **Вердикт: зелёный**
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Этап spec (PROCESS.md §2.4). Задача полного трека (S4-spec-review), ТЗ живёт
|
||||
файлом в `docs/specs/`. Продуктовый код не менялся — коммит только
|
||||
документационный, code review этот раунд не задевает.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитал issue #461 целиком: тело (с редакцией измерений) и все четыре
|
||||
комментария — первую (ошибочную) фикстуру, перемер на плане дачи,
|
||||
живой замер в браузере владельца и аналитику.
|
||||
2. Прочитал `docs/SCOPE.md` — сверил заявленную привязку к J4/J6.
|
||||
3. Прочитал `PROCESS.md` §2.2–2.4, §3, §4, §7.1–7.2 — обязательные разделы,
|
||||
формат AC, порог продуктовых вопросов, лимит циклов.
|
||||
4. Прочитал ТЗ целиком (473 строки, все 18 разделов).
|
||||
5. Сверил каждую техническую предпосылку ТЗ с actual `dev`, а не поверил на
|
||||
слово — потому что ТЗ цитирует конкретные номера строк, сигнатуры функций
|
||||
и существующий прецедент (#451/resize), а догадка, выданная за факт, —
|
||||
отдельный класс замечания:
|
||||
- `_persistActiveDraftSegment` (2777), `_commitPhysicalGeometry` (2128),
|
||||
`_checkSpacePhysicalGeometry` (2143, 2214), `commitWallSegmentModel`
|
||||
(2194), `_junctionLimitsIntroduced` (2229) — прочитал целиком
|
||||
(`src/houseplan-editor-runtime.ts:2048-2248`). Порядок операций и место
|
||||
единственного оптимизируемого call site (2795, внутри
|
||||
`_persistActiveDraftSegment`) подтверждены буквально; остальные вызовы
|
||||
`_commitPhysicalGeometry` (2998, 3087…8012) не относятся к
|
||||
`_persistActiveDraftSegment` — исключение их из скоупа (§3.2)
|
||||
соответствует коду;
|
||||
- resize-прецедент (`resizeLiveJunctionRoomIds`, `resizeLiveCandidateSpace`,
|
||||
`_rszSpaceCandidateGeometry`, `_junctionLimitViolations` с
|
||||
`sharedGeometry`/`roomIds`) — прочитал целиком
|
||||
(`src/houseplan-editor-runtime.ts:3640-3772`). Подтверждено: resize уже
|
||||
подставляет урезанное `liveSpace` в полный candidate и гоняет ту же
|
||||
`checkSpacePhysicalGeometry`/`_junctionLimitViolations` с `affected`
|
||||
roomIds и `{status:'lightweight'}` — именно тот механизм, на который ТЗ
|
||||
ссылается в §5–§7 как на существующий сейм;
|
||||
- пороговые числа §8 (15°, 6 стен в узле, 20 см длина, 5 см дистанция,
|
||||
25 см² просвет) сверены с `docs/USER-GUIDE.ru.md:521-533` — совпадают
|
||||
дословно, включая пограничные случаи (14/15°, 6/7, 19/20 см, 4/5 см);
|
||||
- `wall-degraded-extra` (§8) — фикстура реально существует и используется
|
||||
(`src/plan-geometry-preflight.ts`, `test/plan-geometry-preflight.test.mjs`,
|
||||
`test/wall-union-isolation.test.mjs` и др.), не выдумана;
|
||||
- методология перфоманс-профиля (семь замеров после одного warm-up,
|
||||
exact-SHA Linux gate, structural assertions отдельно от timing) сверена с
|
||||
`demo/performance/README.md` и `.github/workflows/performance.yml`
|
||||
(`--samples=7 --warmups=1`) — это существующий паттерн `large-house-*-v1`,
|
||||
не новое изобретение;
|
||||
- `room_drafts` как имя поля подтверждено (`src/align-grid.ts`,
|
||||
`src/coincident-partitions.ts`);
|
||||
- `demo/smoke_wall_chain_merge.mjs` существует — заявленная в AC8 регрессия
|
||||
merge двух drafts опирается на реальный smoke, а не на несуществующий.
|
||||
6. Проверил трассируемость: `docs/specs/README.md:184` содержит строку на
|
||||
#461, коммит несёт трейлеры `Issue: #461 · User-Visible: no` (правка кода
|
||||
ещё не начата — верно для `User-Visible: no`).
|
||||
|
||||
Код не менялся — `npx tsc --noEmit`, `npm test`, `npm run build`, golden,
|
||||
инварианты модели и mutation-gate к этому раунду не относятся: гейты этапа
|
||||
spec — это проверка самого ТЗ на прочитанность и однозначность, а не гейты
|
||||
реализации. Явно не прогонял: ни один из перечисленных в PROCESS.md §8/§10.4
|
||||
кодовых гейтов, потому что диапазон `origin/dev..HEAD` не содержит
|
||||
изменений `src/**`/`test/**`.
|
||||
|
||||
## Находки
|
||||
|
||||
Блокирующих (High) находок нет. Находок уровня Medium в скоупе или вне скоупа
|
||||
нет.
|
||||
|
||||
### Low (не блокирует, оставляю с записью — правка на усмотрение автора)
|
||||
|
||||
1. **§9.3, formula несогласованность между двумя scaling-проверками не
|
||||
объяснена одной фразой.** Node/unit-бенчмарк проверяет «80 сегментов дороже
|
||||
40 не более чем вдвое» (растущий локальный компонент), а
|
||||
production-профиль проверяет remote-вариант («не более `base × 1.5 + 20
|
||||
мс»`) для сегментов, которые НЕ входят в локальный компонент. Формулы разные
|
||||
не потому, что противоречат друг другу — они проверяют разные вещи (рост
|
||||
входящего компонента vs накладные расходы от роста всего конфига при
|
||||
неизменном локальном компоненте), но в тексте это различие не
|
||||
проговорено, и человек, читающий только §9.3, может принять это за
|
||||
непоследовательность. **Снимаю без правки**: раздел «Принятые
|
||||
предположения» (§18, п.4-5) уже даёт ревьюеру право оспорить точные числа
|
||||
без влияния на UX/AC, и code review сможет проверить сам факт двух разных,
|
||||
но не противоречащих друг другу порогов по факту прохождения обоих
|
||||
assertions.
|
||||
2. **§6.2 не проговаривает явно, что закрытие локального компонента —
|
||||
итеративное до fixed point, а не однопроходное.** Формулировка «все rays
|
||||
конкретного junction, если в компонент вошёл хотя бы один его ray» и
|
||||
«room wall atoms… чьи envelopes касаются seed либо первого слоя включённых
|
||||
соседей» подразумевают, что включение нового соседа может открыть новый
|
||||
узел, который открывает следующий слой — то есть закрытие должно быть
|
||||
рекурсивным до неподвижной точки, а не одним проходом. Текст этого не
|
||||
называет явно. **Снимаю без правки**: корректность закрытия в любом случае
|
||||
доказывается не описанием алгоритма, а parity-матрицей §8 и тремя
|
||||
отрицательными мутантами AC4 («выбросить соседнюю комнату», «не включать
|
||||
чужой draft/column», «не замыкать junction rays») плюс независимым
|
||||
terminal full check (§4.3) — если реализация ошибётся с однопроходным
|
||||
закрытием, один из этих гейтов покраснеет вне зависимости от того, назвал
|
||||
ли текст ТЗ слово «fixed point».
|
||||
|
||||
Ни одна из этих двух записей не влияет на выполнимость или проверяемость AC1…AC12,
|
||||
поэтому не поднимаю их до Medium.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Обязательные разделы §7.1** все присутствуют и в правильном порядке:
|
||||
сценарий (§2) → что человек увидит до/после (§2 «До/После») → проблема (§1)
|
||||
→ скоуп/не-скоуп (§3) → контракт поведения (§4-§9) → UX (§4) → модель
|
||||
данных/миграция/i18n (§12) → AC1-AC12 с методом доказательства (§10) → план
|
||||
автотестов (§11) → риски (§15) → откат (§16) → release-артефакты (§17).
|
||||
- **Продуктовая рамка**: привязка к J4/J6 `docs/SCOPE.md` обоснована —
|
||||
задача про десктопный редактор, поддержание плана «true as it evolves»;
|
||||
явно не расширяет touch-паритет (§13, ссылка на `TOUCH-SUPPORT.md`).
|
||||
- **Каждый AC** имеет указанный способ доказательства (unit / smoke /
|
||||
performance / mutation witness / code review / documentation review) —
|
||||
ни одного «просто похоже на правду».
|
||||
- **Явное разделение продуктового и технического** соблюдено: единственное
|
||||
продуктовое решение («не показываем сегмент до verdict», «не удаляем
|
||||
брошенные drafts автоматически») уже принято владельцем прямо в теле/
|
||||
комментариях issue и корректно перенесено в §3.2/§18 как принятое решение,
|
||||
а не как оставленный открытым вопрос. Технические детали (имена файлов,
|
||||
форма кэша) явно помечены в §18 п.5 как «технические и могут меняться при
|
||||
ревью» — ровно то разделение, которое требует §7.1. Отсутствие вопросов
|
||||
владельцу в конце ТЗ не является недосмотром: развилка, которая обычно
|
||||
требовала бы продуктового вопроса («что делать с накопленными кликами при
|
||||
большом плане»), была закрыта до старта ТЗ самим владельцем в комментариях
|
||||
(аналитика: «продуктовых вопросов, блокирующих первую редакцию ТЗ, по
|
||||
текущему описанию нет»).
|
||||
- **Ни одной догадки, выданной за факт, не найдено.** Все числовые пороги
|
||||
(15°, 6 стен, 20 см, 5 см, 25 см², 150/250 мс) либо взяты из
|
||||
`docs/USER-GUIDE.ru.md`, либо прямо привязаны к измерениям владельца
|
||||
(1 278–1 309 мс → 150 мс с десятикратным запасом), либо помечены как
|
||||
bootstrap-потолок в §18 п.4.
|
||||
- **Технические утверждения о текущем коде проверены чтением, не
|
||||
исполнением**, и совпадают построчно: место единственного изменяемого call
|
||||
site, состав операций generic barrier, существование и сигнатура
|
||||
resize-прецедента, существование фикстуры `wall-degraded-extra`, имя поля
|
||||
`room_drafts`, существование smoke `wall_chain_merge`, методология
|
||||
семи-замерного perf-раннера. Ни одной ссылки на несуществующую функцию,
|
||||
файл или fixture не найдено.
|
||||
- **Не-скоуп (§3.2) не противоречит коду**: `_commitPhysicalGeometry`
|
||||
вызывается из ~15 разных мест рантайма, ТЗ ускоряет только один
|
||||
(`_persistActiveDraftSegment`, строка 2795) и явно исключает остальные —
|
||||
это соответствует буквальному тексту «не входит: ускорение остальных
|
||||
generic-вызовов».
|
||||
- **Откат (§16)** реалистичен: fast path — чистая оптимизация одного call
|
||||
site без изменения формата данных, ревёрт не требует миграции.
|
||||
- **i18n/данные (§12)**: schema/model_version не меняются, новых ключей нет —
|
||||
соответствует характеру задачи (внутренний write barrier).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не прогонял `npx tsc --noEmit` / `npm test` / `npm run build` / golden /
|
||||
инварианты модели / mutation-gate — код не менялся, гейты этого раунда не
|
||||
касаются (проверено `git diff origin/dev...HEAD` — только два файла в
|
||||
`docs/specs/`).
|
||||
- Не проверял осуществимость итогового замера «≤150 мс median / ≤250 мс max»
|
||||
на реальном браузере — это будет предметом code review (AC9), сейчас важно
|
||||
только, что бюджет привязан к измерению, а не к произвольному числу, что
|
||||
подтверждено.
|
||||
- Не пересчитывал квадратичную асимптотику `checkSpacePhysicalGeometry`
|
||||
самостоятельно — принял измерения владельца (Node и live-браузер) как
|
||||
первичный источник; это его собственный, дважды перепроверенный замер
|
||||
(первая версия была explicitly отозвана в комментарии тем же автором).
|
||||
|
||||
## Итог
|
||||
|
||||
ТЗ полное, однозначное, каждый AC имеет проверяемый метод доказательства,
|
||||
технические утверждения о текущем коде подтверждены построчным чтением,
|
||||
продуктовая граница зафиксирована самим владельцем, скоуп/не-скоуп совпадает
|
||||
с реальной структурой кода. High-находок нет, Medium нет — ни в скоупе, ни
|
||||
вне его. Две Low-записи оставлены с обоснованием, почему не требуют правки.
|
||||
Вердикт — зелёный, задача может идти в `S5-ready`.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/461-wall-draw-click-performance`, коммит `1ef21037a8c3` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `6a30077b8244b2cfac5d69b3c544256fce730add`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 6a30077b8244
|
||||
```
|
||||
- ТЗ `docs/specs/461-wall-draw-click-performance.md`, блоб `ccf374b1c0797ace0156b7cfb366007f1271fd35`
|
||||
```
|
||||
git log --all --find-object=ccf374b1c0797ace0156b7cfb366007f1271fd35 -- docs/specs/461-wall-draw-click-performance.md
|
||||
```
|
||||
Reference in New Issue
Block a user