From eb5e15fb6f37141031e9e4b17b65cbd4f422ce10 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 21:57:57 +0000 Subject: [PATCH] docs: review document for #445 Issue: #445 User-Visible: no --- docs/reviews/SPEC-REVIEW-445-r1.md | 251 +++++++++++++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-445-r1.md diff --git a/docs/reviews/SPEC-REVIEW-445-r1.md b/docs/reviews/SPEC-REVIEW-445-r1.md new file mode 100644 index 00000000..a0f45e84 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-445-r1.md @@ -0,0 +1,251 @@ +# SPEC-REVIEW-445-r1 + +- Issue: https://github.com/Matysh/houseplan-card/issues/445 +- Этап: spec (PROCESS.md §2.4), трек: full (обоснован автором в S2-analysis: сложность/риск выше 3, контракт проходит через несколько модулей — критерий `small` не выполнен) +- Материал: `docs/specs/445-furniture-wall-face-snap.md`, коммит `f6879b37` на ветке `issue/445-furniture-wall-face`, дерево полностью включает `origin/dev` (merge-base = HEAD dev, `560ca214`) — ребейз не требуется +- Заход: r1 · блокирующих циклов израсходовано 0 из 4 (лимит на полном треке — 4) + +## Скоуп разбора + +Первый заход — разбор полный. Материал: тело issue #445, все 4 комментария +(аналитика → вопросы владельцу → решение владельца → «ТЗ готово»), сам файл ТЗ, +код, на который ТЗ ссылается (`_furnWalls`, `snapFurnitureToWall`, +`_rawPhysicalBodiesR`, `_segments`/`roomEdges`, `_innerRoomContour`, +`FURN_WALL_CELLS`, `test/furniture.test.mjs`), `docs/SCOPE.md`, +`docs/WALL-THICKNESS.md`, `docs/FURNITURE.md`, `docs/TOUCH-SUPPORT.md`, +`docs/USER-GUIDE.ru.md` (терминология магнита/мебели). + +## Как проверялось + +1. Прочитан diff коммита `f6879b37` (`git show --stat`): только + `docs/specs/445-furniture-wall-face-snap.md` (новый, 377 строк) и одна + строка в `docs/specs/README.md` (реестр). Класс C, `src/**`/`test/**` не + затронуты. +2. Сверены техническая часть ТЗ («Подтверждённая проблема», карта реализации) + с текущим кодом на этом же SHA: + - `_furnWalls` (`src/houseplan-editor-runtime.ts:4907-4912`) действительно + возвращает `[...this.host._segments, ...faces]`, где `_segments` + (`src/houseplan-card.ts:7385`) — осевые `roomEdges`, а `faces` — грани + только `_rawPhysicalBodiesR()` (drafts/partitions/columns). Подтверждено. + - `snapFurnitureToWall` (`src/furniture.ts:492` и далее) кладёт BACK на + переданный сырой `[x1,y1,x2,y2]` и при `nl < 1e-9` берёт левую нормаль + ребра (`nx = dy/len; ny = -dx/len`) — независимо от того, где комната. + Подтверждено, включая номера строк и комментарий «dead on the wall». + - `test/furniture.test.mjs:171-177` строит `EDGES` через `roomEdges([ROOM])` + без какого-либо каталога стен и ожидает `s.cy = 100 + 45` (осевая + треть + глубины) — тест действительно проверяет центролинейное поведение и не + видит толщины. Подтверждено. + - `docs/WALL-THICKNESS.md` §9 заканчивается фразой «Raw bodies remain + authoritative for hit testing, selection, drag, properties, deletion, + history and furniture magnet semantics» — независимые тела уже физические + грани, повторный offset для partitions/columns действительно был бы + ошибкой; ТЗ учитывает это явно (раздел «Физический кандидат стены», + последний абзац). + Технических неточностей или недоказанных утверждений о текущем поведении + не найдено — «Подтверждённая проблема» и карта реализации описывают код, + который есть на диске, а не воображаемый. +3. Сверены defaults владельца (Q1/Q2, комментарий «Решение владельца») с + разделами «Контракт поведения» и «Принятые предположения» ТЗ — построчное + соответствие есть, ни один owner default не потерян и не расширен без + основания. +4. Сверена терминология с `docs/USER-GUIDE.ru.md` (строки 1306, 1390-1391): + «магнитится к стене», «прижимается к ближайшей стене в радиусе магнита» — + ТЗ не вводит новых пользовательских терминов; текущее руководство не + противоречит новому поведению (обещание «ближайшая стена» остаётся + истинным при переходе с осевой линии на поверхность). +5. Проверено соответствие `docs/SCOPE.md`: задача — уточнение геометрии уже + существующей функции редактора (мебель, wall magnet), в скоупе J4/J6 + («два встроенных редактора», «keep the plan true»); никакой новой сущности + или инструмента не вводится, collision model явно исключена в не-скоуп. + Конфликта со SCOPE нет. +6. Проверена структура разделов на соответствие §7.1 и фактическому шаблону + репозитория — сверено с недавним принятым full-track ТЗ + `docs/specs/442-marker-write-rollback.md` (тот же набор заголовков + один-в-один, включая свёрнутые «UX»/«i18n» в «Touch, клавиатура и + доступность» + «Данные и совместимость»). Отклонения от установленного в + проекте формата нет. +7. Прошёлся по каждому пункту «Контракт поведения» и таблице «Ошибки и + крайние случаи» и сопоставил с AC1…AC9 построчно (см. находку ниже). +8. Гейты (typecheck/test/build) **не прогонялись** — см. «Чего не проверял». + +## Находки + +### Medium (в скоупе задачи) — угловой tie-break не имеет доказательства + +**Файл:** `docs/specs/445-furniture-wall-face-snap.md`, раздел «Контракт +поведения» → «2. Выбор стороны», пункт 6, и таблица «Ошибки и крайние +случаи», строка «угол с двумя равноудалёнными стенами». + +**Формулировка контракта:** «В углу либо при равном расстоянии до нескольких +стен выигрывает физически ближайшая поверхность, затем сторона намерения, +затем стабильный tie-break. Перестановка входных массивов не меняет +результат.» Это новое, видимое пользователю поведение: раньше `_furnWalls` +отдавал единственный плоский список сегментов и ближайший побеждал по +`bestD` без всякого различения «своя стена / другая стена»; теперь список +кандидатов — многослойный (несколько candidate на одну стену + грани +независимых тел), и на прямом углу между двумя РАЗНЫМИ стенами при точном +равенстве расстояний нужен новый, ранее не существовавший tie-break. + +**Проблема:** ни один из AC1…AC9 не называет эту ветку. Сверка построчно: + +| Строка контракта / таблицы | AC, который её доказывает | +|---|---| +| Внешняя стена, pointer внутри/на оси/снаружи | AC1 | +| Общая стена, обе стороны, exact-axis drag/placement, permutation/winding | AC2 | +| Локальная атомарная толщина на переходе 10→20 см | AC3 | +| Радиус от поверхности | AC4 | +| Preview/commit/move один контракт | AC5 | +| Нулевые стены, независимые тела, malformed segment | AC6 | +| Touch safety, схема данных | AC7 | +| Визуальный результат | AC8 | +| Кэш/производительность | AC9 | +| **Угол с двумя равноудалёнными РАЗНЫМИ стенами, порядок массива** | **нет** | + +AC2 доказывает перестановочную инвариантность только для ОДНОЙ общей стены +(смена ролей «комната A / комната B» на одном и том же физическом сегменте), +а не выбор между двумя разными, обычно неколлинеарными стенами, сходящимися +в углу. «План тестирования» упоминает «permutation/winding» одним пунктом без +номера AC, и по контексту (соседний список «внешней/общей стены, intent +point, exact-axis tie») это тоже про AC1/AC2, а не про многостеновой случай. + +**Почему это находка, а не придирка:** сам автор счёл сценарий достаточно +реальным, чтобы вписать его отдельной строкой в таблицу крайних случаев — +то есть это не гипотетический край, а заявленный контракт. Раз для него не +названо ни `unit`, ни `smoke`, ни какого-либо иного способа доказательства, +код-ревью не сможет ответить на вопрос «оно вообще работает» для этой ветки: +по правилу §2.7 «оно либо доказано автотестом, либо разобрано по коду с явной +записью „проверено чтением“» — а без AC ревьюер кода не обязан вообще на неё +смотреть, потому что она не пронумерована. Здесь же для смежных строк того же +уровня риска (§2.6 — общая стена + exact-axis, тоже про несколько кандидатов) +доказательство есть, а для строки про несколько РАЗНЫХ стен в углу — нет. +Отсутствие пронумерованного AC для заявленного поведения — прямое нарушение +DoR §2.5 («AC1…ACn — пронумерованные проверяемые критерии приёмки»), а +не эстетическое пожелание. + +**Чем закрывается (в этой же задаче, без нового issue):** добавить AC10 (или +расширить AC2) — unit-фикстура с двумя неколлинеарными стенами на равном +расстоянии от точки намерения (например, прямой угол комнаты), проверяющая, +что: 1) побеждает физически ближайшая поверхность при неравном расстоянии; +2) при точном равенстве расстояний результат детерминирован и не зависит от +порядка `walls`/`_rawPhysicalBodiesR()` на входе; 3) сторона намерения +учитывается как второй критерий раньше стабильного tie-break. Метод +доказательства — `unit`, по аналогии с остальными AC этого ТЗ. + +Это Medium в скоупе задачи (сам контракт уже описывает поведение, требуется +лишь пронумеровать доказательство) — без High-находок это жёлтый вердикт, +правка ТЗ и повторный цикл в рамках того же issue, отдельный issue не +заводится (#202). + +## Что проверено и корректно + +- Обязательные разделы §7.1 присутствуют и в порядке, совпадающем с недавно + принятым полным ТЗ (#442): сценарий → что человек увидит → проблема → + скоуп/не-скоуп → контракт → данные/совместимость → touch → крайние случаи → + AC → план тестов → карта реализации → риски/rollback → release-артефакты → + принятые предположения. +- «Сценарий» и «Что человек увидит до и после» отвечают на оба продуктовых + вопроса без терминов реализации; персона (Home admin, Редактор подложки) + названа явно. +- Все технические утверждения о текущем поведении («Подтверждённая проблема») + проверены построчно по коду на SHA `f6879b37` и совпадают: `_furnWalls`, + `snapFurnitureToWall`, `nl < 1e-9` ветка, состав `test/furniture.test.mjs`. + Ни одной догадки, выданной за факт: там, где ТЗ вводит новое поведение + (радиус от поверхности, сторона по намерению, tie-break), это либо прямое + происхождение от ответа владельца (Q1/Q2), либо явно помечено в разделе + «Принятые предположения» и открыто для оспаривания. +- Открытые продуктовые вопросы (Q1, Q2) заданы владельцу по правилам §7.1: + пачкой, с предложенным default, `blocked` был выставлен, ответ получен и + корректно перенесён в контракт без искажений — сверено построчно с + комментарием «Решение владельца». +- AC1…AC9 (за вычетом находки выше) однозначны, у каждого назван способ + доказательства (`unit`/`browser smoke`/`golden`/`code review`) и, где AC + заявляет защитное свойство (радиус, отсутствие двойного offset, fallback + на левую нормаль), назван вид mutation, которая обязана его покраснить — + это опережает требование §2.7 и облегчает будущее код-ревью. +- Скоуп/не-скоуп корректно исключает collision model, resize/rotate/flip + (#383), изменение SVG каталога и толщины/рендера самих стен — ни один из + этих пунктов не просачивается в контракт ниже. +- Данные и совместимость: persisted schema не меняется, миграции нет, i18n + ключи не добавляются — согласовано с `CONFIG-COMPATIBILITY.md` духом + (никакой новой версии конфига не описано и не требуется). +- Touch/доступность: safety floor описан терминами, совпадающими с + `docs/TOUCH-SUPPORT.md` («clean tap», «pinch/second pointer/pointercancel + ничего не сохраняют»), а не изобретён заново. +- Ссылка issue ↔ ТЗ двусторонняя: тело ТЗ ссылается на issue, `docs/specs/ + README.md` получил строку на #445 в том же коммите. +- Трейлеры коммита `f6879b37`: `Issue: #445`, `User-Visible: no` — корректно + для документации, класс C. +- SCOPE.md: задача — точечное исправление геометрии существующей функции + (мебель/wall magnet) внутри уже описанных J4/J6, конфликта с гранью скоупа + нет; в out-of-scope списке SCOPE.md ничего из контракта не задето + (не 3D, не collision model интерьера, не automations). + +## Чего не проверял + +- **Гейты `typecheck`/`test`/`build`/`bundle:sync` не прогонялись.** Диапазон + этого коммита (`git show --stat f6879b37`) касается только + `docs/specs/445-furniture-wall-face-snap.md` и одной строки в + `docs/specs/README.md` — класс C, ни одного файла `src/**`/`test/**`/ + `scripts/**`. На этапе ТЗ (spec review, PROCESS.md §2.4) предмет разбора — + выполнимость и проверяемость ТЗ, а не состояние продуктового кода, которого + в этом коммите нет; прогон typecheck/test/build на docs-only дельте не дал + бы никакого сигнала об этой дельте (это баланс тестового бюджета этого же + документа: «полные наборы — предрелизный гейт, а не гейт ревью», и здесь + гейты просто не относятся к изменённым файлам). Они обязательны на + код-ревью, когда появится реализация. +- **`node scripts/check-docs.mjs` не прогонялся** — diff не касается `src/**`, + условие запуска не выполнено. +- **`node scripts/model-invariants.mjs` не прогонялся** — diff не меняет + геометрию, `layout`, `wall_segments` или ссылки на них; это план на будущее + изменение продуктового кода, а не на сам текст ТЗ. +- **Browser smokes / golden / performance / backend pytest не прогонялись** — + нет реализации для запуска; это часть плана тестирования будущего + код-ревью, а не спецификации. +- **Не проверял вручную UI/демо-стенд** — на этапе spec review нет ни кода, + ни поведения для наблюдения; сценарии таблицы «Ошибки и крайние случаи» + проверены логическим разбором формул (offset от `halfDepth`, distance-to- + surface) против описанного в `docs/WALL-THICKNESS.md` контракта атомарной + толщины, не исполнением. +- Не проверял, называет ли будущая реализация конкретный алгоритм + «стабильного tie-break» для нового размещения на оси общей стены (raздел + 2, пункт 4) — ТЗ сознательно оставляет выбор конкретной комнаты автору + («принято предположительно, поменять свободно» не помечено буквально, но + сам владелец в defaults сформулировал это как «детерминированная сторона + ОДНОЙ ИЗ смежных комнат», то есть какая именно — уже не продуктовый вопрос). + Это отличается от находки выше тем, что там отсутствует не алгоритм, а + само доказательство/AC. + +## Материал раунда + +- Ветка: `issue/445-furniture-wall-face` +- SHA материала: `f6879b3788fc6ddc2626482ce0a890edb6b0200b` +- Дерево материала: `docs/specs/445-furniture-wall-face-snap.md` (blob на + этом SHA), `docs/specs/README.md` +- merge-base с `origin/dev`: `560ca214b0d7d9018ff7916b25ebaea96a80e912` + (= HEAD `origin/dev` на момент ревью — дельта пуста, ребейз не требовался) + +## Вердикт + +Одна находка Medium в скоупе задачи, High нет. Согласно §2.4/§2.7 без +High-находок это жёлтый вердикт: автор добавляет AC (или расширяет AC2) для +углового tie-break с явным способом доказательства и перевыставляет +`S4-spec-review` — правка укладывается в один абзац контракта и не требует +пересмотра остальной части ТЗ. + +Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче + +--- + + + +## Материал раунда + +- Ветка: `issue/445-furniture-wall-face`, коммит `f6879b3788fc` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c3ea0c69a5b4fa897e29ec294994f771189b9a1d` + ``` + git log --all --format='%H %T' | grep c3ea0c69a5b4 + ``` +- ТЗ `docs/specs/445-furniture-wall-face-snap.md`, блоб `6c191fb7e9e65afd6e85f5fd0d80576ce4d2e7d3` + ``` + git log --all --find-object=6c191fb7e9e65afd6e85f5fd0d80576ce4d2e7d3 -- docs/specs/445-furniture-wall-face-snap.md + ```