docs: review document for #676

Issue: #676
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-27 11:28:14 +00:00
parent f1a87fbad9
commit 9fe7f88cb3
2 changed files with 247 additions and 1 deletions
+2 -1
View File
@@ -1,9 +1,10 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1105, issue: 395. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1106, issue: 396. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|---|---|---|---|---:|---:|---|---|
| #676 | [SPEC-REVIEW-676-r1.md](SPEC-REVIEW-676-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 2 | AC8 не называет проверку двух из трёх ветвей подавления подсказки; риск «рамка перехватывает события под другим инструментом» не привязан ни к одному AC; не задан tie-break для равного протяжки по обеим осям (снимаю сам) | `src/stairs-editor-model.ts` `stairs-view.ts` `demo/smoke_stairs.mjs` `src/furniture.ts` |
| #675 | [CODE-REVIEW-675-r1.md](CODE-REVIEW-675-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #673 | [CODE-REVIEW-673-r1.md](CODE-REVIEW-673-r1.md) | code · r1 | 🟡 жёлтый | 0 | 0 | docs/images/screenshots.json: закоммиченный sourceFingerprint не совпадает с тем, что р…; Комментарий автора в issue #673 называет SHA реализации 557d4e38e26b647437f2c931454737f… | `docs/images/screenshots.json` `demo/golden/matrix.mjs` `scripts/source-fingerprint.mjs` `accept.mjs` `policy.mjs` `AGENTS.md` `baselines-index.json` |
| #673 | [CODE-REVIEW-673-r2.md](CODE-REVIEW-673-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |
+245
View File
@@ -0,0 +1,245 @@
# SPEC-REVIEW-676-r1
- **Issue:** https://github.com/Matysh/houseplan-card/issues/676
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
- **Трек:** полный (метки `bug`, `P1`, `S4-spec-review`; аналитик явно
зафиксировал «лёгкий трек: нет» — несколько поверхностей, новый
UX-контракт размещения протяжкой и подсказки, сложность > 3).
- **Материал:** тело issue #676, раздел `## ТЗ` (редакция 1, 27.09) + 3
комментария: (1) «Взял» аналитика, (2) полная аналитика — причины 9
дефектов по коду и воспроизведению, решение «латать или переписывать»,
оценка, (3) факт публикации ТЗ и перевод в `S4-spec-review`.
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
- **Роль:** ревьюер ТЗ (не автор)
## Скоуп ревью
Первая итерация лестниц (#663) даёт 9 дефектов редактирования, воспроизведённых
владельцем и подтверждённых аналитиком чтением кода и браузерным пробником
(`demo/probe_stairs_bugs.mjs`, не гейт). Задача переписывает слой
редактирования лестниц (`src/stairs-editor.ts`, `stairs-editor-model.ts`,
рамка узлов в `houseplan-card.ts`, подсказка в `stairs-view.ts`, стили) на
box-контракт, общий с декором/мебелью — жесты, курсоры, магнит по краям,
диалог свойств. Модель `stairs.ts`, площадь (#669), навигация по клику в
Просмотре и 2.5D не трогаются.
**SCOPE-проверка (`docs/SCOPE.md`):** сценарий закрывает J6 («keep the plan
true», редактирование объектов плана без регресса в сохранённых данных) и
J1/J2 через корректную навигацию/подсказку лестницы в Просмотре. Из
«никогда не строить» ничего не задевается; из «партиально покрыто» —
touch-эргономика редакторов («best effort», не блокирует).
## Как проверялось
1. Прочитаны `docs/SCOPE.md`, `docs/process/REVIEWER.md` целиком; по ссылкам
конспекта открыт PROCESS.md §2.4, §2.5, §4, §7.1, §7.2 (§2.10 не
применялся — это r1).
2. Прочитано тело issue #676 целиком (`gh issue view 676 --json body`) и все
3 комментария.
3. Прочитан канонический `docs/STAIRS.md` — контракт К1–К8 ему не
противоречит; раздел «Implementation boundary» уже называет
`stairs-editor-model.ts` владельцем чистых Plan-трансформаций — задача
продолжает существующее разделение, а не изобретает новое.
4. Сверены обязательные разделы §7.1: Сценарий, Что человек увидит до/после,
Проблема, Скоуп/Не-скоуп, Контракт поведения (К1–К8), UX, Модель данных и
миграция, i18n, Критерии приёмки AC1–AC11 со способом доказательства, План
автотестов, Риски, Откат, Release-артефакты — все на месте; первые два —
продуктовые, без терминов реализации.
5. Прочитан код, лежащий в основании диагностики «Проблема», чтобы отделить
факт от догадки:
- `src/stairs-editor.ts:2,3,5` — подтверждён импорт и вызов
`resizeFurnitureTransform`, `snapFurnitureToWall`,
`furnitureWallSurfacesFor` для текущего (дефектного) поведения;
- `src/houseplan-editor-runtime.ts:2214-2223,3333-3334` — подтверждён
`_markupClick`/`_suppressClick`, объясняющий дефекты 1/2/8 (копия при
ресайзе, потеря выделения);
- `src/wall-thickness.ts:222-243` — подтверждено, что `fieldToCm`/
`cmToField`/`clampWallCm` зажимают в `[WALL_MIN_CM, WALL_MAX_CM]` = не
[30, 10000], а диапазон толщины стены — дефект 4 («100 см потолок»)
реален и назван верно;
- `src/houseplan-card.ts:9097,11069-11088` — подтверждён порядок отрисовки:
`stairs.renderLayer()` стоит ДО `_renderWallBodies`, `_renderTextFrame`
(декор) — ПОСЛЕ; перенос рамки лестницы к `_renderTextFrame` (Скоуп)
действительно решает дефект 3 и AC7;
- `src/stairs-editor.ts:427-435`, `src/styles/plan.styles.ts:1595-1605` —
подтверждён единый класс `hp-stair-resize` (`nwse-resize` для всех 8
узлов) и радиус `_gridPitch * 0.65` (единицы плана, не экрана) — дефекты
5 и 9 реальны, К2 их закрывает;
- `src/furniture-placement.ts:7` (`FURN_WALL_CELLS = 6`) и
`src/furniture.ts:140-141` (`FURN_MIN_CM = 1`, `FURN_MAX_CM = 10000`) —
константы контракта (`STAIR_MAX_CM = 10000`, довод магнита 6 клеток)
совпадают с источником, на который ссылается ТЗ;
- `src/stairs-editor.ts:317-323` (`undoActiveDrag`) — Ctrl+Z во время
жеста уже реализован и работает (откат к `drag.original`, сброс жеста);
К6 в этой части не меняет контракт, а фиксирует уже верное поведение;
- i18n: `stairs.fixed_floor`, `history.stair_move` уже существуют во всех
четырёх словарях (`src/i18n/{en,ru,de,fr}.json`) — новые ключи
(`stairs.tooltip_navigate`, `history.stair_resize`,
`history.stair_rotate`) не конфликтуют и следуют тому же
нейминг-паттерну; `test/i18n.test.mjs` действительно проверяет
плейсхолдер-паритет всех словарей — AC10 доказуем как заявлено;
- golden: 4 существующих сцены лестниц подтверждены (`stairs-flat-hover-
dark`, `stairs-flat-normal-light`, `stairs-flat-selected-light`,
`stairs-isometric-dark`, `demo/golden/baselines/`) — AC9 корректно
называет ровно одну меняющуюся и три неизменные;
- `test/stairs-editor-model.test.mjs`, `demo/smoke_stairs.mjs` — второй
файл существует и расширяется, первый отсутствует и создаётся с нуля,
как и заявлено («новый»).
Диагностика «Проблема» и вся фактическая база контракта проверена по коду
и подтверждена — ни одного случая догадки, выданной за факт, не найдено.
6. Проверена math-корректность сектора курсора К2 (`round(θ/45) mod 8`, 0°=
+x экрана по часовой): раскладка по секторам (0/4→ew, 2/6→ns, 1/5→nwse,
3/7→nesw) арифметически верна для экранной системы координат (y вниз) и
соответствует стандартным CSS-именам диагоналей (SE/NW действительно
лежат на диагонали `nwse-resize`, NE/SW — на `nesw-resize`).
7. Проверено покрытие исходных 9 дефектов отчёта критериями приёмки — все
девять сведены к AC1–AC11 (таблица построена вручную, см. «Что проверено
и корректно»).
8. Прогон гейтов **не выполнялся** — см. «Чего не проверял»: на этапе
`S4-spec-review` кода задачи ещё нет, ветка `issue/676-*` не создана,
рабочее дерево на `f1a87fba` содержит только исходную (дефектную)
реализацию #663.
## Находки
### Medium (в скоупе) — AC8 не называет проверку двух из трёх ветвей подавления подсказки
**Что не так.** К8 описывает три причины, по которым подсказка Просмотра НЕ
показывается: «Лестница без цели, с битой целью **или** в карточке с
закреплённым этажом». Это ровно три состояния `stairTargetState()`
(`missing`, `deleted`, `fixed` — функция уже существует,
`src/stairs-editor-model.ts:97-102`). AC8 же называет доказательством только
одну ветвь: «наведение на лестницу с целью `garden` даёт … ; уход убирает;
**лестница без цели** подсказки не даёт» — про «битую цель» (deleted) и
«закреплённый этаж» (fixed) в столбце «Доказательство» нет ни слова.
**Почему это находка, а не придирка.** Тултип — новый код (существующий
`_showTip` только переиспользуется, сам вызов на новом обработчике наведения
в `stairs-view.ts` — новый), а не старая, уже покрытая гвардом навигация;
то, что `stairTargetState` для missing/deleted/fixed уже протестирован для
навигации (не-скоуп), не гарантирует, что тот же guard правильно подключён
именно к вызову тултипа — это ровно тот код, который эта задача добавляет.
Реализация, читающая AC8 буквально, закроет её одним смоук-шагом («нет
цели → нет подсказки») и формально будет «выполнена», оставив нерабочим (или
непроверенным) случай удалённой цели и случай `floor`-карточки — то есть
именно тот регресс, который К8 прямо запрещает, останется недоказанным.
**Чем закрывается в скоупе задачи.** Расширить столбец «Доказательство» AC8
(или явно расщепить его на AC8a/AC8b) двумя смоук-утверждениями: наведение на
лестницу с удалённым/несуществующим `target_space_id` не показывает тултип;
наведение на лестницу с валидной целью в карточке, сконфигурированной с
`floor`, не показывает тултип. Технический характер уточнения (какой именно
смоук-шаг доказывает уже сформулированное продуктовое поведение) — решение
автора ТЗ, не продуктовый вопрос владельцу.
### Medium (в скоупе) — риск «рамка перехватывает события под другим инструментом» не привязан ни к одному AC
**Что не так.** К2 формулирует обязательное условие: «Рамка не перехватывает
события под другими инструментами плана». Раздел «Риски» повторяет его же
как угрозу регресса: «Рамка в оверлее должна оставаться инертной под
инструментами кроме «Выбор»/«Лестница» (К2), иначе перехватит стены». Ни в
таблице AC1–AC11, ни в «Плане автотестов» нет ни одной строки, которая бы
это проверяла — ни unit, ни smoke.
**Почему это находка, а не придирка.** Рамка теперь стоит В ВЕРХНЕМ
ОВЕРЛЕЕ, выше тел стен (это и есть исправление дефекта 3) — именно
геометрическое перекрытие с кликабельной зоной инструмента «Стены» создаёт
риск, названный самим автором. DoR требует не просто «риски перечислены», а
чтобы задача имела для риска обозначенный путь проверки; риск, названный, но
не проверяемый ни одним AC, с равной вероятностью останется бумажным
предупреждением и уйдёт в релиз непроверенным — ровно то, ради чего вообще
существует колонка «Доказательство».
**Чем закрывается в скоупе задачи.** Добавить в «План автотестов» один
смоук-шаг (или новый AC12): под инструментом «Стены» (или любым другим
не-«Выбор»/не-«Лестница» инструментом) клик в точке, где визуально
находится узел выделенной лестницы, должен воздействовать на стену/плоскость
плана, а не быть перехвачен рамкой лестницы (`pointer-events: none` на
оверлее вне активных инструментов — assert через `elementFromPoint` или
эквивалент, как уже делает `demo/smoke_stairs.mjs` для гвардов навигации).
### Low — не задан tie-break для равного протяжки по обеим осям (снимаю сам)
К1: «длинная ось протяжки — ось подъёма» для случая `|B−A|x == |B−A|y`
(ровно диагональная протяжка под 45°) не называет, какая ось считается
доминирующей — угол результата не определён детерминированно текстом ТЗ.
Не блокирует: это событие нулевой меры для реального указателя, а в
кодовой базе уже есть прецедент детерминированного тай-брейка при похожем
равенстве (`resizeFurnitureTransform`, `src/furniture.ts`: «Equal deltas
deliberately prefer width for deterministic ties»). Снимаю сам с
рекомендацией придерживаться того же соглашения (например, при точном
равенстip предпочитать +x/-x перед +y/-y) — чисто техническое решение,
владельца не касается.
## Что проверено и корректно
- Обязательные разделы §7.1 присутствуют и в правильном порядке; «Сценарий»
и «Что человек увидит» — продуктовые, без терминов реализации, называют
персону 1/2 по `docs/SCOPE.md`.
- Все 9 дефектов из «Что происходит» сведены к контрактам К1–К8 и покрыты
критериями: 1/2 → К6/AC6; 3 → К2/AC7; 4 → К7/AC5; 5 → К2/AC4; 6 → К4/AC3;
7 → К8/AC8; 8 → К3+К4/AC3+AC11; 9 → К2/AC7. Ни один исходный дефект не
остался без критерия приёмки.
- Диагностика «Проблема» и числовые/структурные утверждения контракта
(константы, файлы, существующие функции) сверены с кодом и подтверждены
фактом, а не приняты на веру — см. «Как проверялось», п. 5.
- AC1, AC2, AC3, AC9, AC10, AC11 однозначны, называют способ доказательства
и воспроизводимы буквально по тексту.
- Модель данных не меняется, откат («revert коммитов, данные читаются
обеими версиями») корректен и согласован с `docs/CONFIG-COMPATIBILITY.md`
(задача явно заявляет, что не задевает этот документ, и это верно —
персистентная схема стула не меняется).
- Touch учтён минимально необходимо: К1 включает touch/pen в размещение
(курсорный жест общий для указателя любого типа, ничего специфичного не
требуется), AC8 явно фиксирует, что касание не показывает тултип
(наведения нет) и не меняет навигацию касанием — единственное
туч-чувствительное новое поведение проговорено. Экраны редактора вне View
— best effort по `docs/TOUCH-SUPPORT.md`, отдельного пункта не требуется.
- «Принято предположительно» корректно фиксирует семь технических развилок
как решённые автором (направление подъёма, поведение клика, шаг вращения,
предел доворота магнита 5°, минимальный размер 30 см, база отсчёта
магнита, техническая реализация рамки/подавления клика) — ни одна не
является продуктовым вопросом, эскалации владельцу не требовалось и не
произошло.
- Release-артефакты (changelog RU+EN, `docs/STAIRS.md`, руководство,
golden, «performance — нет, security — нет») названы явно, а не оставлены
подразумеваемыми; бюджет бандла назван риском с конкретным запасом (~2 КБ)
и существующим гейтом (`bundle:budget`).
## Чего не проверял
- Не запускал `npm run typecheck`/`npm test`/`npm run build` и
`node scripts/check-docs.mjs` — на этапе `S4-spec-review` кода задачи ещё
нет: ветка `issue/676-*` не создана, рабочее дерево на `f1a87fba` содержит
только исходную реализацию #663 без единой правки этой задачи. Дифф для
этих гейтов просто не существует; они относятся к этапу код-ревью.
- Не запускал `demo/smoke_stairs.mjs`, `node scripts/smoke-select.mjs`,
`golden:verify` и инварианты модели — по той же причине: браузерные смоки
и golden проверяют код, а на этапе ТЗ проверяемый код не существует. Их
прогон целиком ложится на код-ревью после реализации.
- Не проверял `demo/probe_stairs_bugs.mjs` (пробник аналитика) по существу —
он явно не гейт и не идёт в коммит; использовал только код продукта
(`src/*.ts`), который он якобы демонстрирует, и подтвердил утверждения
чтением этого кода напрямую (см. «Как проверялось», п. 5), а не доверием к
выводу пробника.
- Не проверял #663 и #669 по существу (смежные задачи) — использовал только
факт их состояния («работают, не трогаются» / «работает, #669 закрыт») как
вводную для не-скоупа этой задачи.
## Вердикт
Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 2 → в задаче
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `f1a87fbad9d4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `4c3048e855e543bdf5ecf9edbdfed8146f31c648`
```
git log --all --format='%H %T' | grep 4c3048e855e5
```
- Тело issue: `fe591d135b54a53f4596f5e657f3139e5fb064331dbd48ff529b63472395175d`
- Вердикт конвейера: `yellow` · High 0