From 9592092b8eb95976ac91303cfa17310edf25df20 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 10 Sep 2026 18:49:42 +0000 Subject: [PATCH] docs: review document for #521 Issue: #521 User-Visible: no --- docs/reviews/SPEC-REVIEW-521-r1.md | 199 +++++++++++++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-521-r1.md diff --git a/docs/reviews/SPEC-REVIEW-521-r1.md b/docs/reviews/SPEC-REVIEW-521-r1.md new file mode 100644 index 00000000..5dc86c68 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-521-r1.md @@ -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 → в задаче + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `c50bc725c895` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `75ed6503e9310b9c7f0971f82a69d361a4eaee4a` + ``` + git log --all --format='%H %T' | grep 75ed6503e931 + ``` +- Тело issue: `bf26b17118240771206ed2a7f26f10c82783ea586679def63cbc17e538c38cfe` +- Вердикт конвейера: `yellow` · High 0