mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-06 22:49:16 +00:00
claude[bot]
@@ -0,0 +1,199 @@
|
||||
# SPEC-REVIEW — issue #521 — заход r1
|
||||
|
||||
**Этап:** S4-spec-review (PROCESS.md §2.4) · **Трек:** полный (нарушен критерий §5
|
||||
«нет влияния на производительность и на touch-контракт», названо самим автором) ·
|
||||
**Заход:** r1 (первый) · **Бюджет циклов:** 0/4 израсходовано до этого вердикта.
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Материал — тело issue #521, раздел `## ТЗ` (артефакт по решению владельца
|
||||
2026-09-10, #517; `docs/specs/` архивирован, новый файл не создаётся). Тема:
|
||||
регресс направляющих выравнивания (`.alignline`/`.aligndot`, слой
|
||||
`.alignguides`) во всех трёх живых жестах редактора (`devices`, `decor`,
|
||||
`plan`) после перевода взаимодействия на «живой» путь `src/live-editor.ts`
|
||||
(#451, `c0d61ca3`). Задача — вернуть направляющие в живой шаблон, перевести
|
||||
точку выравнивания устройства на живую позицию, подавить дублирующий осевой
|
||||
слой и переписать свидетеля на настоящие `PointerEvent` вместо фабрикации
|
||||
`_deviceDrag`/`_decorDraft`.
|
||||
|
||||
Продукт: `docs/SCOPE.md` J6 («keep the plan true… drag/resize»), персона —
|
||||
администратор дома, поверхность — три десктопных редактора (референсная
|
||||
среда по `docs/TOUCH-SUPPORT.md`, строки 23–25, процитированы в ТЗ верно).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью только по тексту ТЗ — продуктового кода не менял, ничего не запускал
|
||||
(этап spec, гейты код-ревью здесь не применяются). Проверено:
|
||||
|
||||
- полный текст тела issue #521 и оба комментария (аналитика S2, хендофф
|
||||
«ТЗ готово — на ревью») — оба от `Matysh`, второй такой аккаунт объясняется
|
||||
ролью владельца/публикующей автоматизации в этом репозитории, не находка;
|
||||
- `docs/SCOPE.md`, `PROCESS.md` §§1–10, `AGENTS.md`, `docs/TOUCH-SUPPORT.md`,
|
||||
`docs/USER-GUIDE.ru.md` (полнотекстовый grep на `align`, `выравнивани`,
|
||||
`направляющ`, `магнит`, `пунктир`, `точка-якор` — см. находку ниже),
|
||||
`docs/CANVAS.md`, `docs/STYLING-HOOKS.md`;
|
||||
- сверка каждого технического утверждения ТЗ с текущим кодом на `dev`
|
||||
(`c50bc725`), чтобы отличить корректную диагностику от догадки:
|
||||
- `src/live-editor.ts:273-285` (`editorTemplate`) — подтверждено: режим
|
||||
`devices` не входит ни в одну ветку и возвращает `nothing`; `decor`
|
||||
рисует `_renderDecorLayer`/`_renderBackdropFrame`/`_renderTextFrame` без
|
||||
направляющих; `plan` через `planTemplate` тоже не рисует `.alignguides`;
|
||||
- `src/houseplan-card.ts:12892-12895` (`_alignPoint`, режим `devices`) —
|
||||
подтверждено: путь через `this._pos(d)`, не `_livePos`;
|
||||
- `src/houseplan-card.ts:11698-11701` — подтверждено: осевой слой
|
||||
направляющих и слой разметки используют один и тот же класс
|
||||
`.hp-editor-only-layer`;
|
||||
- `src/live-editor.ts:342-354` (`paintHouseplanEditor`) — подтверждено:
|
||||
подавление `.hp-editor-only-layer:not(.hp-plan-snap-layer)` сегодня
|
||||
выполняется только в ветке `_mode === 'plan'`; для `decor` гасятся лишь
|
||||
`.dtframe, .backdropframe`, для `devices` не гасится ничего — контракт
|
||||
п.3 действительно требует новой работы, а не переиспользования;
|
||||
- `src/houseplan-editor-runtime.ts:11282-11300` (`_renderAlignGuides`) —
|
||||
подтверждено: классы `alignguides`/`alignline`/`aligndot` существуют
|
||||
ровно в заявленном виде;
|
||||
- `demo/smoke_align_guides.mjs:38-43,55-57,71` — подтверждено: сценарии
|
||||
`devices`/`decor` присваивают `c._deviceDrag = {…}` / `c._decorDraft = {…}`
|
||||
напрямую и зовут `c.requestUpdate()`, ровно то, что ТЗ называет
|
||||
фабрикацией; сценарий `#400` (`devGuideComesFromAnotherMarker`) и
|
||||
`noneInView` действительно уже существуют — AC7 не выдумывает новых
|
||||
гарантий;
|
||||
- `package.json` — `benchmark:large-house-interaction` и
|
||||
`benchmark:compare` существуют; `demo/performance/budgets-large-house-interaction.json`
|
||||
существует — профиль в AC9 назван верно и соответствует правилу
|
||||
PROCESS.md §8/#473 (правка `houseplan-card.ts`/`src/live-*` → профиль
|
||||
`large-house-interaction-v1`);
|
||||
- `demo/smoke_isometric_live_touch.mjs`, `demo/smoke_touch_tips.mjs` —
|
||||
существуют, ссылка в разделе «Риски» точна;
|
||||
- установленный паттерн настоящих `PointerEvent` в других смоках
|
||||
(`smoke_decor.mjs`, `smoke_active_chain_ink.mjs` и др.) — подтверждает,
|
||||
что требование AC1–AC4 «настоящий жест» технически осуществимо, не
|
||||
фантазия.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium — заявленный источник поведения не существует в названном документе
|
||||
|
||||
**Файл:** тело issue #521, раздел `## ТЗ` → `### Продуктовая рамка`.
|
||||
|
||||
**Формулировка:** «Ничего нового не появляется: восстанавливается ровно то
|
||||
поведение, которое описано в `docs/USER-GUIDE.ru.md` для выравнивания.» Это
|
||||
утверждение о задокументированном поведении, поданное как факт, не помеченное
|
||||
как предположение.
|
||||
|
||||
**Почему это находка.** Полнотекстовый поиск `docs/USER-GUIDE.ru.md`
|
||||
(2241 строка) по `align`, `выравнивани`, `направляющ`, `точка-якор`,
|
||||
`пунктир` (в контексте выравнивания) даёт **ноль** совпадений на первые три
|
||||
запроса — направляющие выравнивания (дашированная линия + точка-якорь при
|
||||
перетаскивании значка/фигуры/курсора) в этом документе не описаны вовсе, ни
|
||||
под этим именем, ни под каким-либо другим. Раздел «Редактор устройств»
|
||||
(строка 977) описывает только привязку центра маркера к узлу сетки, но не
|
||||
визуальную направляющую к другому значку. Раздел про декор (строка 1502)
|
||||
описывает «лёгкий магнит к углам, серединам, центрам и рёбрам» — это про
|
||||
геометрический магнит подложки, а не про то, что пользователь **видит**
|
||||
(линию/точку), и явно исключает устройства из целей магнита («устройства…
|
||||
не являются целями магнита» — что не то же самое, что и «направляющая от
|
||||
устройства к устройству»).
|
||||
|
||||
`AGENTS.md` («Read this first») требует ровно обратного для видимых
|
||||
изменений: «interface wording comes from there and is not invented, or the
|
||||
UI starts speaking developer» — то есть ссылка на `USER-GUIDE.ru.md` в ТЗ
|
||||
должна быть проверяемой, а не общим местом.
|
||||
|
||||
**Существенно, но не блокирует.** Само восстанавливаемое поведение не
|
||||
выдумано — оно подтверждается независимо: кодом (`_renderAlignGuides`,
|
||||
`.alignline`/`.aligndot`, исключение перетаскиваемого маркера по #400,
|
||||
существующий *до* регрессии смок) и собственным измерением владельца в
|
||||
комментарии S2 на живых `pointerdown`/`pointermove`. AC1–AC9 не опираются на
|
||||
эту фразу — они проверяемы сами по себе, независимо от того, где именно
|
||||
описано прежнее поведение. Поэтому находка не про то, что контракт неверен,
|
||||
а про то, что ТЗ ссылается на несуществующий источник вместо корректного
|
||||
(«восстанавливает поведение, вывезенное #451 и подтверждённое диагностикой
|
||||
в этом же issue» — так и есть на самом деле).
|
||||
|
||||
**Чем закрывается.** Правка одной фразы в «Продуктовая рамка»: убрать ссылку
|
||||
на `docs/USER-GUIDE.ru.md` либо заменить её на точную (код/тесты/коммит
|
||||
`c0d61ca3`/диагностика в issue). Технической правки контракта, AC или кода
|
||||
не требует.
|
||||
|
||||
## Что проверено и признано корректным
|
||||
|
||||
- **Диагноз и причинность.** Оба слома (слой не рисуется в живом шаблоне;
|
||||
`_alignPoint` читает замороженный `_pos` вместо `_livePos`) подтверждены
|
||||
построчно в текущем коде, независимо от текста ТЗ.
|
||||
- **Контракт п.1–п.6** — каждый пункт технически осуществим, ни один не
|
||||
описывает несуществующий API; ссылки на существующие `_renderAlignGuides()`,
|
||||
`_livePos`, `scheduleHouseplanEditor`, `makeTransparent` точны.
|
||||
- **Продуктовая рамка** (без обсуждаемой находки) отвечает на оба
|
||||
обязательных вопроса §7.1: персона/поверхность/момент и что человек увидит
|
||||
до/после, без терминов реализации.
|
||||
- **AC1–AC9** — каждый однозначен и называет способ доказательства (смок-
|
||||
сценарий или performance-профиль) и мутацию/пробу, от которой он краснеет;
|
||||
таблица «AC · чем доказан · чем краснеет» заполнена по всем девяти
|
||||
пунктам без пустых ячеек — требование #435 (в применении к будущему
|
||||
код-ревью) выполнимо уже на этом ТЗ.
|
||||
- AC6 (один расчёт кандидатов на кадр) — механизм, а не наблюдаемое
|
||||
поведение; автор сам вынес это на спор в комментарии «ТЗ готово». Считаю
|
||||
обоснованным: это единственная защита от перф-регресса, ради устранения
|
||||
которого #451 и вводил живой путь, и у него есть доказательство и
|
||||
мутация. Не меняю.
|
||||
- Ссылки на существующие мутанты/сценарии для AC7 (#400, `noneInView`)
|
||||
подтверждены как реально существующие, не выдуманные заново.
|
||||
- **Откат** — один revert, данных/конфига/публичных контрактов не касается;
|
||||
соответствует тому, что диагноз описывает чисто рантайм-регрессию.
|
||||
- **Release-артефакты** — `User-Visible: yes` с текстами обоих changelog
|
||||
названы дословно; фактический прирост пользовательской ценности («вернули
|
||||
то, что было») сформулирован без придуманной новой функциональности.
|
||||
- **Перф и touch** — раздел «Риски» называет ровно то, что требует чек-лист
|
||||
DoR (§2.5): влияние на производительность (AC9 + риск «кадр жеста») и на
|
||||
touch (существующие touch-смоки должны остаться зелёными) — не «нет
|
||||
влияния», а явно описанный риск с проверкой, что и требуется при
|
||||
нарушенном критерии `small`.
|
||||
- **Объём задачи (все три жеста в одной issue).** Это ровно тот вид
|
||||
продуктового вопроса, который решает владелец (§7.1: «какой объём
|
||||
видимых изменений входит в этот issue»), и автор задал его в
|
||||
установленной форме — что неясно, дефолт, приглашение возразить — без
|
||||
блокировки статуса. Это не нарушение процесса: жёсткая блокировка нужна
|
||||
для вопросов, без ответа на которые писать ТЗ нельзя, а здесь дефолт
|
||||
обоснован (общая причина, общий слой) и обратим (владелец может разделить
|
||||
после чтения).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `tsc`/`test`/`build`/смоки — на этапе spec-review код ещё не
|
||||
писан, гейты §8 к этому этапу не относятся; они предмет код-ревью.
|
||||
- Не проверял, действительно ли добавление подавления `.hp-editor-only-layer`
|
||||
для режимов `devices`/`decor` (которого сегодня нет — см. «Как
|
||||
проверялось») реализуемо без дополнительных побочных эффектов на другие
|
||||
слои того же класса (`hp-editor-only-layer` также несёт `_renderMarkupLayer`
|
||||
для режима `plan`) — это станет предметом код-ревью и AC5 там же.
|
||||
- Не оценивал, войдёт ли новая механика тестового счётчика «осевых циклов»
|
||||
(нужна для AC1/AC3/AC4/AC6, готового счётчика в продукте сегодня нет —
|
||||
есть только `_liveEditorPaintCount`, который считает **живые**, а не
|
||||
осевые перерисовки) в бюджет `test/**`/`demo/**` без нового продуктового
|
||||
кода — технический вопрос авторской реализации, не продуктовая
|
||||
неоднозначность.
|
||||
- Не связывался с владельцем по вопросу объёма (все три жеста в одной
|
||||
задаче) — автор уже задал его в тексте в установленной форме; повторный
|
||||
запрос от ревьюера был бы дублированием.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Единственная находка — Medium, в скоупе задачи (текстовая правка одной фразы
|
||||
в теле ТЗ), без High. По PROCESS.md §2.4/§4 это жёлтый вердикт: автор правит
|
||||
ТЗ, фикс проходит следующий заход ревью по дельте (§2.10).
|
||||
|
||||
Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `c50bc725c895` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `75ed6503e9310b9c7f0971f82a69d361a4eaee4a`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 75ed6503e931
|
||||
```
|
||||
- Тело issue: `bf26b17118240771206ed2a7f26f10c82783ea586679def63cbc17e538c38cfe`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user