From cff83e43eccd27c7f0ff0c5ca1c104f42fdf2023 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 08:47:57 +0000 Subject: [PATCH] docs: review document for #663 Issue: #663 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-663-r1.md | 285 +++++++++++++++++++++++++++++ 2 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-663-r1.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 4e3baff9..9566296a 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,9 +1,10 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1079, issue: 382. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1080, issue: 383. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| +| #663 | [SPEC-REVIEW-663-r1.md](SPEC-REVIEW-663-r1.md) | spec · r1 | 🟡 жёлтый | 1 | 0 | §6.2 (wall snap) и §8 (canonicalization/Optimize) описывают два; AC7 называет четыре состояния сломанной; AC13 не называет конкретный бюджет | `docs/CANVAS.md` `docs/WALL-THICKNESS.md` | | #662 | [SPEC-REVIEW-662-r1.md](SPEC-REVIEW-662-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | «Входящие» не существует в UI, и текущая классификация каталога кладёт неразмещённую ле…; «Не скоуп» отсутствует внутри формального ## ТЗ | `src/device-inbox.ts` `USER-GUIDE.ru.md` `docs/USER-GUIDE.ru.md` `demo/smoke_led_strip_bind.mjs` `device-inbox.ts` `SPEC-REVIEW-661-r1.md` | | #661 | [SPEC-REVIEW-661-r1.md](SPEC-REVIEW-661-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #660 | [SPEC-REVIEW-660-r1.md](SPEC-REVIEW-660-r1.md) | spec · r1 | 🔴 красный | 2 | 1 | Раздел ## ТЗ в теле issue отсутствует целиком; Изменение прямо противоречит двум местам; AC «расстояние уменьшено ровно вдвое» не | `docs/process/AUTHOR.md` `REVIEWER.md` `test/core-file-budget.test.mjs` `scripts/smoke-select.mjs` `demo/helpers/hp-test.mjs` `docs/UX-MODES.md` `docs/reviews/SPEC-REVIEW-647-r1.md` | diff --git a/docs/reviews/SPEC-REVIEW-663-r1.md b/docs/reviews/SPEC-REVIEW-663-r1.md new file mode 100644 index 00000000..a61a0d53 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-663-r1.md @@ -0,0 +1,285 @@ +# SPEC-REVIEW-663-r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/663 +- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4) +- **Трек:** полный. Автор сам назвал критерий §5, который задача не проходит: + «новая UX-сущность, новая persisted-модель, несколько поверхностей, + межэтажные ссылки, touch и визуально-производительные риски» — это разбор по + существу, полный трек выбран корректно. +- **Материал:** тело issue #663 (раздел «## ТЗ r1») + все 4 комментария: + (1) аналитика/оценка и SCOPE-сверка, (2) вопросы владельцу Q1–Q8 пачкой с + предложенными default, (3) решения владельца от 26.09.2026 по Q1–Q8, (4) + «ТЗ r1 готово к независимому ревью», `blocked` снят, переход в + `S4-spec-review`. +- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 +- **Роль:** ревьюер ТЗ (не автор) + +## Скоуп ревью + +Новая сущность плана «Лестница»: прямоугольный марш с автогенерируемой +разметкой ступеней (шаг 30 см), стрелкой физического подъёма, transform/magnet +по контракту мебели/декора, односторонней ссылкой `target_space_id` на другое +House Plan space, кликом-переходом в View, вычитанием площади пересечения из +чистой площади помещений и плоской проекцией в 2.5D. На целевом этаже ничего +автоматически не создаётся (решение владельца Q1). Объёмный 2.5D, составные +лестницы, строительные расчёты и коллизии с проёмами/мебелью/устройствами явно +вне скоупа. + +**SCOPE-проверка (`docs/SCOPE.md`):** задача закрывает J1 (физическая связь +этажей видна на плане), J4 (GUI-only, без Inkscape/YAML) и J6 (лестница +участвует в общем плане: undo/redo, optimistic conflict, import/export). Из +списка «никогда не строить» ничего не задевается: раздел 3 и «Не в скоупе» +явно и многократно отказываются от строительного расчёта (уклон, число +подступенков, нормы), отверстия/шахты в перекрытии и объёмной модели — то есть +задача не сползает в «General CAD» (`docs/SCOPE.md`, exception #53 остаётся не +затронутым: экспорт PDF здесь не расширяется). Новой scope-дыры не вижу. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md` целиком; + по ссылкам конспекта открыт PROCESS.md §2.4, §2.5, §4, §7.1, §7.2 (§2.10 не + применялся — это r1). +2. Прочитано тело issue #663 целиком (`gh issue view 663 --json body`) и все 4 + комментария (`gh issue view 663 --json comments`) — история вопрос/ответ + владельца воспроизведена в «Материал» выше. +3. Сверены обязательные разделы §7.1: Сценарий (1), Что человек увидит до/после + (2), Проблема и продуктовая граница (3), Скоуп/Не-скоуп (4–5), Контракт + поведения 6.1–6.5, UX свойств (7), Модель данных и совместимость (8), i18n + (9), AC1–AC13 с указанным способом доказательства (10), План автотестов + (11), Риски (12), Откат (13), Release-артефакты (14), «Принятые технические + предположения» (15) — все на месте и в правильном порядке; первые два + раздела продуктовые и без терминов реализации, как требует §7.1. +4. Все 8 вопросов владельцу (Q1–Q8) сверены построчно с итоговым контрактом + §6.1–6.3, §7: каждый ответ владельца от 26.09.2026 действительно внесён в + текст (одностороннее размещение только на одном этаже — §6.3/решение + Q1 совпадает с «Решение владельца» вводного абзаца; сохранённый + zoom/позиция цели без автоцентрирования — §6.3/Q2; magnet по контракту + декора с учётом толщины стены — §6.2/Q3; шаг 30 см, остаток сверху — + §6.1/Q4; стрелка = физический подъём — §6.1/Q5; вычитание только площади, + без других геометрических/световых эффектов — §6.4/Q6; fixed-floor виден, + переход отключён — §6.3/Q7; 2D можно раньше 2.5D, первая версия 2.5D плоская + — §6.5/Q8). Открытых продуктовых вопросов не осталось — подтверждаю. +5. Контракт transform/magnet (§6.2, §8) и модель площади/навигации сверены с + каноном подсистемы: `docs/CANVAS.md` §9.4 (Shift-семантика), §9.5 + (Optimize/grid-pass), `docs/WALL-THICKNESS.md` (магнит мебели к «сырым» + физическим телам стен), `docs/UX-MODES.md` (`floor`-закреплённая карточка, + свайп/kiosk), `docs/USER-GUIDE.ru.md` (`floor:`/`default_floor`, «сохранённый + вид просмотра» на пространство). Расхождение найдено и разобрано ниже. +6. `git branch -a` / `git log --all --oneline | grep 663` — ветки `issue/663-*` + и коммитов с трейлером `Issue: #663` не существует; `git diff + origin/dev...HEAD` пуст. Продуктового кода для #663 нет — стадия `spec`, + гейты (`tsc`, `test`, `build`, смоки, golden, инварианты) неприменимы, + рассматриваю это как штатное состояние, а не находку. + +## Находки + +### High-1 — §6.2 (wall snap) и §8 (canonicalization/Optimize) описывают два +несовместимых контракта позиционирования, и ТЗ не говорит, какой из них + +**Что не так.** ТЗ последовательно называет transform/magnet лестницы «по +существующему контракту мебели/декора» (разделы 1, 4, 6.2 — «Interaction-модель +повторяет мебель/декор»), как если бы «мебель» и «декор» были одним контрактом. +Канон `docs/CANVAS.md` §9.4 (строки 512–527) описывает их как два **разных** +контракта: + +- у мебели (`furniture`) — свой «wall magnet» (строка 523: «bypassing the + furniture wall magnet»), координаты continuous/не квантуются («Furniture + resize is the explicit exception to positional quantisation… Shift on its + rotation handle snaps to 45°», строки 524–527), и `docs/WALL-THICKNESS.md:529–532` + явно называет это «furniture magnet semantics», привязанной к «сырым» + физическим телам стен, а не к канонической (round-friendly) массе; +- у обычного декора («ordinary decor») — Shift даёт **свободный** поворот, не + привязку к 45° (строка 522: «free ordinary-decor/backdrop rotation» — прямо + противоположно тому, что описывает мебель), никакого «wall magnet» нет + (строка 523–524: «the ordinary decor/room/grid magnet remains active», + отдельно от мебельного), и координаты квантуются на грид (`docs/CANVAS.md:591–592`: + «Other decor kinds and storage-level numeric canonicalization keep their + existing grid contract»). +- `docs/CANVAS.md:588–591` объясняет прямо, зачем это разделение существует: + «The grid pass deliberately excludes the complete transform of `furniture` + and uploaded `image` decor. Their position, size and rotation are + continuously authored values (#383), so changing even one of those fields + would make Optimize create debt from a normal editor operation.» + +ТЗ §6.2 описывает поведение, которое **буквально совпадает** с «мебельным» +wall magnet — «ставит её габарит вплотную к этой грани, а не к оси стены» — +то есть continuous-позиция, зависящая от произвольной толщины стены (толщина +задаётся в сантиметрах непрерывно, 0–100 см, по умолчанию 15 см — +`docs/WALL-THICKNESS.md:383–388`; половина толщины в общем случае не кратна +шагу грида, который определяется физическим масштабом конкретного плана, +`docs/CANVAS.md:97–120`). Одновременно §8 требует: «canonicalization округляет +координаты **тем же lattice-контрактом, что decor**» (issue #663, тело, строка +126) — то есть буквально контракт **обычного** декора, тот самый, который +включён в грид-квантование на каждом сохранении. Тут же в том же перечне §8: +«Optimize не изменяет лестницы» (строка 127) — то есть явный «мебельный» +контракт исключения из Optimize. + +Оба требования из §8 не могут выполняться одновременно с §6.2, если читать +«lattice-контракт decor» буквально: если координаты лестницы округляются на +гриде при каждом сохранении (как у обычного декора), то габарит, магнитно +примкнутый к грани стены на дробном расстоянии (толщина/2), будет сдвинут +на ближайший узел грида **уже на первом же «Сохраняет план»** (сценарий п.5) — +магнит перестаёт держать, ровно та проблема, ради которой канон исключил +мебель из грид-прохода. Если же имелось в виду «контракт мебели» (continuous, +исключён из Optimize) — тогда фраза «тем же lattice-контрактом, что decor» +написана неточно и вводит в заблуждение, поскольку канон использует слово +«decor» именно для контраста с «furniture». + +**Почему это находка ревью ТЗ, а не техническая деталь на усмотрение автора.** +Это не «где хранится состояние» или «какой файл», а прямое противоречие +контракту поведения, наблюдаемому пользователем: либо лестница держит магнит +к стене после сохранения (мебельный контракт), либо не держит и молча +съезжает (декор-контракт) — то есть ровно то расхождение, которое ломает +сценарий п.3 («магнитится вплотную к видимой грани стены») и AC2. Без +исправления реализация с равной вероятностью выберет любой вариант, оба +пройдут AC2 как написан («примыкает… не меняется» ничего не говорит про +persist/Optimize), и один из двух вариантов — задокументированный баг. Раздел +15 «принятые технические предположения» это расхождение не упоминает, хотя оно +влияет на пользовательский контракт («держит ли лестница магнит после +сохранения») — а значит, по правилу §7.1, не может быть закрыто автором +«свободно», не задев наблюдаемое поведение. + +**Что нужно.** Заменить оба места одним однозначным правилом: либо (а) +лестница использует continuous-позиционирование, как мебель — исключена из +`alignAllToGrid`/Optimize-грид-прохода и из посейвовой численной канонизации, +магнит к стене/другой лестнице сохраняется как есть, — либо (б) лестница +квантуется на гриде как обычный декор, и тогда §6.2 должен явно сказать, что +результат wall snap **дополнительно** проецируется/округляется на грид (со +следствием — либо гарантией, что это округление не разрушает «вплотную» +визуально в пределах допуска, либо явным отказом от точного «вплотную»). Любой +из двух вариантов реализуем; невыполненным ТЗ является именно отсутствие +выбора между ними, поскольку оба явно записанных требования (§6.2 и §8) +взаимно исключают друг друга при буквальном прочтении. + +**Серьёзность:** High — блокирует. Затрагивает AC2 (magnet), AC9 (нет +побочных эффектов при Optimize) и AC11 (round-trip после сохранения). + +### Low-1 (снят без возврата) — AC7 называет четыре состояния сломанной +ссылки, контракт определяет три + +§6.3 перечисляет ровно три состояния: «цель отсутствует, равна текущему этажу +или была удалена». AC7 (тело issue, §10) добавляет четвёртое слово: +«пустая, self, удалённая **и недоступная**». Нигде в тексте не определено, чем +«недоступная» отличается от «удалённая» (или это синоним, дублирующий +формулировку §7 «предупреждение о недоступной цели», которая относится ко +всем трём состояниям сразу). Практического расхождения не создаёт — тестируемые +состояния однозначно определены в §6.3, а AC7 в момент реализации будет +проверяться по ним, — но словарь стоит привести к одному термину, чтобы четвёртое +слово не читалось как отдельный незадокументированный кейс (например, +существующий, но временно недоступный space — такого понятия в модели нет). +Снимаю как Low, без возврата автору на цикл; поправить при следующей правке ТЗ +заодно с High-1. + +### Low-2 (снят без возврата) — AC13 не называет конкретный бюджет + +«…большой fixture остаётся в действующих бюджетах» не указывает, какой именно +бюджет/метрику код-ревью будет сверять (frame time? per-space render budget? +`bundle:budget`-подобный числовой порог?). Способ доказательства +(`performance` + ревью кода) назван, что формально закрывает требование DoR +«влияние на производительность названо», но конкретное число решится только на +код-ревью. Это приемлемо для технической детали (сама формулировка «кэшируемая +геометрия, не пересчитывать на каждый live-state update» уже задаёт +проверяемый принцип), но стоит явно сослаться на конкретный существующий +performance-смок/бюджет при реализации, чтобы код-ревью не пришлось изобретать +критерий с нуля. Снимаю как Low. + +## Что проверено и корректно + +- Все обязательные разделы §7.1 присутствуют, в правильном порядке; раздел 1 + называет персону (администратор), поверхность (редактор плана) и момент; + раздел 2 — одной фразой, без терминов реализации. +- Скоуп/Не-скоуп разделены явно и совпадают с §3 (навигационная, не расчётная + сущность) и с ответами владельца Q1–Q8 — расхождений между вопросами, + default-предложениями и итоговым текстом не найдено (см. «Как проверялось» + п.4). +- Контракт стрелки/ступеней (6.1) внутренне согласован и проверяем: полный шаг + 30 см от нижней границы, неполный остаток только у верхней — не оставляет + места для двух прочтений, AC3 численно завершает контракт. +- Контракт навигации (6.3) корректно совпадает с реальным поведением + закреплённой карточки `floor:` (`docs/USER-GUIDE.ru.md:2266–2271`: + «Закреплённая карточка… игнорирует… вкладки других пространств») и с + «сохранённым видом просмотра» на пространство (`docs/USER-GUIDE.ru.md:488`) — + AC5/AC8 проверяемы по существующим механизмам, не по гипотетическим. +- Контракт площади (6.4) однозначно ограничивает эффект только числом + («это расчёт площади, а не отверстие в рендере»); AC4/AC9 закрывают его без + зазора для «а что с полом/светом/vacuum» — отдельно и явно исключено. +- Модель данных (8) — опциональная коллекция, `stairs` отсутствует ⇒ старое + поведение сохраняется — соответствует установленному в проекте паттерну + (аналогично `settings.volumetric_view`/иным опциональным bounded-коллекциям); + ремонт битой ссылки «по стабильным `space.id`, никогда по порядку или + названию» закрывает главный риск раздела «Риски». +- Каждый AC1–AC13 указывает способ доказательства (`unit`/`smoke`/`golden`/ + `backend`/`geometry parity`/`performance`+«ревью кода»), что удовлетворяет + требованию DoR §2.5. +- Продуктовые вопросы Q1–Q8 заданы владельцу одним комментарием, пачкой, + каждый с предложенным default (соответствует форме §7.1); ответы получены, + внесены в тело, `blocked` снят — открытых продуктовых вопросов не осталось. +- i18n-таблица (9) перечисляет ключи по смыслу (инструмент/сущность, длина, + ширина, поворот, направление, «ведёт на этаж», недоступная цель, fixed-floor + no-op) и явно запрещает конкатенацию строк. +- Риски (12) называют главные технические угрозы (SVG-нагрузка на малом zoom, + расхождение площади между потребителями, ложные touch-переходы, битые + ссылки, смешение с decor-транформ-хелперами, объёмный будущий этап) — ровно + те, что также разобраны в «Аналитике» комментария 1; совпадение полное, + ничего не потеряно между аналитикой и ТЗ. +- Откат (13) корректно описывает опциональность коллекции и безопасность + отключения без разрушения пользовательских данных — соответствует правилу + SCOPE.md «никогда не удалять файл/данные пользователя на инференс». +- Release-артефакты (14) называют оба changelog, документацию (User Guide, + UX-MODES, architecture/config-compatibility, 2.5D), screenshots и + пользовательскую заметку о плоской первой версии — полный список, ничего не + забыто относительно затронутых канонических документов (п.6 «Читай в этом + порядке»). + +## Чего не проверял + +- Гейты (`tsc --noEmit`, `npm test`, `npm run build`, `check-docs.mjs`, смоки, + `golden:verify`, инварианты) — не прогонял: этап `spec`, ветки/коммитов для + #663 нет, `git diff origin/dev...HEAD` пуст. Предмет код-ревью после + реализации. +- Не пересчитывал буквально шаг грида на конкретных числовых примерах (взял + общее свойство «половина произвольной толщины стены в общем случае не кратна + шагу грида, зависящему от масштаба плана» из §3/§9.4-9.5 `docs/CANVAS.md` и + диапазона толщины `docs/WALL-THICKNESS.md:383-388`); для High-1 это не + требуется — контракт противоречив независимо от конкретных чисел, при + любом шаге грида, кроме случая, когда автор случайно выберет толщину стены, + кратную шагу (не гарантировано моделью). +- Не проверял осуществимость 2.5D-этапа сверх текста ТЗ (нет макетов + дизайнера, раздел 15 и Q8 явно относят конкретную визуальную реализацию к + «принято предположительно»/будущим материалам) — не предмет ревью ТЗ. +- Не оспаривал сами решения владельца Q1–Q8 — только сверял, что итоговый + контракт им соответствует (см. «Как проверялось» п.4); продуктовая + правомерность выбора («одностороннее размещение», «шаг 30 см» и т.д.) — + решение владельца, не предмет спора ревьюера. +- Не проверял i18n-ключи построчно на предмет конкретных строк RU/EN — раздел 9 + перечисляет смысловые группы ключей, конкретные строки — реализация. + +## Вердикт + +Обязательные разделы ТЗ полны, 8 продуктовых вопросов закрыты владельцем без +остатка, 12 из 13 AC однозначны и не создают внутреннего противоречия. Но +контракт transform/magnet (§6.2) и контракт canonicalization/Optimize (§8) +описывают лестницу одновременно как «мебель» (continuous, wall magnet, +исключена из Optimize) и как «обычный декор» (grid-bound, включена в +posейвовую канонизацию) — два взаимоисключающих поведения канона +`docs/CANVAS.md`, между которыми ТЗ не выбирает. Это не техническая деталь, +свободная для реализации: неверный выбор — реальный, воспроизводимый на первом +же сохранении баг («лестница съезжает со стены»), который AC2 в текущей +формулировке не ловит. Возврат автору для явного выбора одного из двух +контрактов (и правки §6.2 либо §8 под него). + +Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 1 · Medium: 0 → в задаче + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `422221fe9342` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c4e3f48a513e0468285959a11f937aa9b13c9754` + ``` + git log --all --format='%H %T' | grep c4e3f48a513e + ``` +- Тело issue: `a304102f2b7c3f5aae7814eabe642e7aa679d5243df7f25ce579bc490959262c` +- Вердикт конвейера: `yellow` · High 1