mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 12:49:56 +00:00
claude[bot]
@@ -0,0 +1,161 @@
|
||||
# SPEC-REVIEW-250-r1
|
||||
|
||||
- Issue: [#250](https://github.com/Matysh/houseplan-card/issues/250) — «Проёмы с сохранённым flip_v остались на грани стены: символ обязан стоять на осевой всегда»
|
||||
- Этап: ТЗ на ревью (PROCESS.md §2.4)
|
||||
- Заход: r1 (первый), блокирующих циклов израсходовано **0 из 4**
|
||||
- Ревьюер: Claude (роль «ревьюер ТЗ», отдельная от автора — Codex)
|
||||
- Артефакт ТЗ: `docs/specs/250-opening-centerline.md`
|
||||
- SHA ТЗ: `2c776b0c6d00b1276c04d0b51c696f405beddcbb` (ветка `issue/250-opening-centerline`, коммит «docs: specify opening centerline invariant»)
|
||||
- Трек: обычный (метки `bug`, `P2`, `polish`, `S4-spec-review`; `small` не выставлена — файл в `docs/specs/` обязателен, что и сделано)
|
||||
|
||||
## Скоуп проверки
|
||||
|
||||
Первый заход — разбор полный, раунд по дельте (§2.10) неприменим.
|
||||
|
||||
Прочитано перед вердиктом:
|
||||
- `docs/SCOPE.md` (Core user jobs, лок-инвариант, правило про удаление файлов — не задействовано здесь);
|
||||
- `AGENTS.md`, `docs/PROCESS.md` целиком (§1–§14, включая §2.4, §4, §7.1, §7.2);
|
||||
- тело issue #250 и оба комментария (аналитика владельца-эксперта и хендофф автора ТЗ на ревью);
|
||||
- `docs/USER-GUIDE.ru.md` (раздел «Настройки проёма», «Толстые стены»);
|
||||
- `docs/WALL-THICKNESS.md`, `docs/ARCHITECTURE.md` (модель `OpeningCfg`), `docs/ISOMETRIC.md` (структурный Iso-контракт);
|
||||
- сам ТЗ `docs/specs/250-opening-centerline.md` целиком.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
ТЗ утверждает конкретные факты о текущем коде и текущих документах — эти факты
|
||||
проверялись построчно, а не принимались на слово (это и есть работа
|
||||
ревьюера ТЗ: не согласиться, а найти невыполнимое или непроверяемое):
|
||||
|
||||
1. **Диагноз §3** (`openingSymbolOffset()` даёт ненулевой offset для door/window
|
||||
при `flip_v: true`, gate/passage уже нулевые) — сверен с
|
||||
`src/opening-symbol-placement.ts:20-36`. Подтверждено буквально: строка 26
|
||||
`if (!flipV || type === 'gate' || type === 'passage') return { ox: 0, oy: 0 };`
|
||||
и вычисление `half = Math.hypot(face.ox, face.oy)` дальше. Тест
|
||||
`test/opening-symbol-placement.test.mjs` («saved door and window flips use
|
||||
one canonical local edge») прямо ожидает `{ ox: 0, oy: 20 }` для
|
||||
`positiveFace = { cm: 20, … }` — соответствует утверждению issue.
|
||||
2. **Использование helper на трёх уровнях** — сверено: `src/render/opening-symbol.ts`
|
||||
вызывает `openingSymbolOffset` и транслирует только swing-группу (arc/leaf/glass)
|
||||
через `swingTx/swingTy`, при этом косяки (`<line>` на `x=±half`) рисуются вне
|
||||
этой группы и не двигаются — значит смещение сегодня действует только на
|
||||
вращающуюся часть символа, а не на полную глубину косяка; это согласуется с
|
||||
AC6 ТЗ («jamb depth остаётся 70 см независимо от flip»). `src/iso-openings.ts:87-89`
|
||||
строит `origin = [x + offset.ox, y + offset.oy]` и передаёт его в `hinge` —
|
||||
при offset≡0 origin всегда `(x, y)`, а направление у door/window всё равно
|
||||
мутируется через `sy` внутри `transformVector` на **quarterVector** (не на
|
||||
hinge), то есть анимация открывания меняет сторону без переноса точки
|
||||
крепления — ровно то, что заявляет §7.2 ТЗ.
|
||||
3. **Gate не трогается** — `face.side * 10` в `render/opening-symbol.ts` и
|
||||
`iso-openings.ts` действительно не зависит от `openingSymbolOffset`; мутант
|
||||
`opening-gate-flip-cancels-turn` в `scripts/mutation-gate.mjs:1697` целится в
|
||||
отдельную строку (`gateAngle = spec.face.side * 10 * amount`), не в offset —
|
||||
ТЗ корректно относит его к «сохраняется», а не «выводится из реестра».
|
||||
4. **Устаревающие мутанты (§14)** — проверены `opening-symbol-default-uses-room-face`,
|
||||
`opening-symbol-partition-follows-endpoints`, `opening-gate-flip-translates-leaves`
|
||||
в `scripts/mutation-gate.mjs:1660-1692`: все три патчат строку с условием
|
||||
`if (!flipV || type === 'gate' || …)`. Если реализация делает
|
||||
`openingSymbolOffset` тождественным нулём (вариант, прямо разрешённый
|
||||
владельцем в теле issue и продублированный в §19.1 ТЗ), эта строка исчезает
|
||||
и `find`-паттерн патча перестаёт совпадать — три анкера действительно
|
||||
становятся непредметными, а не произвольно списанными.
|
||||
5. **Golden-сцены** — все четыре ID (`opening-symbol-room-wall-light`,
|
||||
`opening-symbol-diagonal-partition-dark`, `opening-symbol-flip-pairs-light`,
|
||||
`isometric-opening-symbol-parity-dark`) существуют в
|
||||
`demo/golden/matrix.mjs:288-301` и в `demo/golden/baselines/baselines-index.json`.
|
||||
`openingFlipContract` (строки 39-55) уже содержит пары `flipV:true` с
|
||||
`offset: 'edge'` — это ровно то значение, которое реализация обязана сменить
|
||||
на `'center'`; `harness.mjs:161-179` подтверждает, что `offset` — размеченное,
|
||||
проверяемое поле контракта (`'center'|'edge'`), а не décor. Технически
|
||||
заявленный план (§13.3) выполним без структурных изменений сцены.
|
||||
6. **Документы, которые «больше не обещают edge alignment» (AC8)** — найдены
|
||||
ровно те формулировки, которые ТЗ обязуется поменять: `docs/WALL-THICKNESS.md:107-110`,
|
||||
`docs/USER-GUIDE.ru.md:616-620`, `docs/ARCHITECTURE.md:582`,
|
||||
`docs/ISOMETRIC.md:161-162`. Все действительно говорят «flip_v: true
|
||||
edge-aligns door/window» на текущем `dev` — значит AC8 указывает на реальные
|
||||
строки, а не на воображаемую проблему.
|
||||
7. **Обязательные разделы ТЗ (§7.1 PROCESS.md)** — сверены по списку: сценарий (§1),
|
||||
что человек увидит до/после (§2), проблема (§3), скоуп и не-скоуп (§6),
|
||||
контракт поведения (§7), UX (§8), модель данных и migration (§9), i18n/a11y/touch
|
||||
(§10), AC1…AC9 с доказательством (§12), план автотестов (§13), риски (§18),
|
||||
откат (§17), release-артефакты (§16) — присутствуют все.
|
||||
8. **Продуктовая неоднозначность** — решение владельца зафиксировано прямо в теле
|
||||
issue («Все проёмы … располагаются на осевой линии стены, независимо от
|
||||
настроек») и повторено в аналитике владельца-эксперта («Решение владельца
|
||||
однозначно; продуктовых вопросов для ТЗ нет»). ТЗ ничего не додумывает сверх
|
||||
этого текста; раздел §19 явно и корректно отделяет только техническые
|
||||
допущения («принято предположительно, поменять свободно») от решённых
|
||||
владельцем пунктов.
|
||||
|
||||
## Находки
|
||||
|
||||
Блокирующих (High) находок нет. Находок Medium в скоупе или вне скоупа нет.
|
||||
|
||||
**Low (снята с записью, не возвращается автору):** ТЗ #250 не добавлено строкой
|
||||
в таблицу `docs/specs/README.md` (там пока только #242 и #248 из соседних
|
||||
задач). Раздел не входит в обязательные §7.1/DoR-пункты, сама таблица отмечена
|
||||
в PROCESS.md §7.3 как технический долг с другой стороной проблемы (дублирующая
|
||||
колонка статуса, а не отсутствие строки), и ничего не блокирует ни автора, ни
|
||||
ревьюера кода. Снимаю без возврата; при желании строка добавляется тем же
|
||||
коммитом, что и реализация.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Сценарий и персона (§1) соответствуют J1/J6 из `docs/SCOPE.md`: «план должен
|
||||
одинаково и физически понятно показывать проёмы», «сохранённая настройка не
|
||||
должна скрыто менять их положение» — обе фразы буквально из аналитики
|
||||
владельца, не изобретены автором ТЗ.
|
||||
- «Что человек увидит» (§2) сформулировано без терминов реализации, одной
|
||||
фразой до/после — соответствует требованию PROCESS.md §7.1.
|
||||
- Контракт (§7) точен и проверяем численно: `{ox:0, oy:0}` — не «около нуля», а
|
||||
точное значение; 70-см стена → офсет 0, jamb 35 см от оси в каждую сторону,
|
||||
что для стены 70 см даёт полный охват — арифметика непротиворечива.
|
||||
- Матрица AC1 (4 типа × 2 flip × horizontal/vertical/diagonal × positive/
|
||||
negative/zero/malformed face) полностью покрывает пространство входов текущей
|
||||
сигнатуры `openingSymbolOffset(type, flipV, angle, face)` — заведомо не
|
||||
half-baked.
|
||||
- Скоуп/не-скоуп (§6) корректно исключает `flip_h`, `compareOpeningSides`,
|
||||
`face.side`, `partitionOpeningFace`, wall cut/tunnel/jambs/Glow/sun, lock
|
||||
badge, миграцию/backend, новый UI-контрол и публикацию скрытого Iso —
|
||||
совпадает с диагнозом (§3), который показывает, что все эти механизмы читают
|
||||
`face`/`side`/`cm` напрямую, а не через удаляемый offset.
|
||||
- Модель данных (§9) корректно называет это intentional семантическим
|
||||
изменением уже сохранённого поля, а не миграцией: формат `OpeningCfg.flip_v`
|
||||
(boolean) не меняется, что действительно не требует version bump по
|
||||
`docs/CONFIG-COMPATIBILITY.md`.
|
||||
- Риски (§18) названы предметно и с митигацией через конкретные AC/мутанты, а
|
||||
не общими словами; §14 mutation guards корректно разводит «анкер, который
|
||||
перестанет существовать» и «анкер, который обязан продолжать падать».
|
||||
- Откат (§17) реалистичен: одна code revision восстанавливает старую ветку,
|
||||
данные не трогаются.
|
||||
- Технические допущения (§19) явно помечены как решаемые автором/ревьюером, а
|
||||
не выданы за продуктовое решение — соответствует «догадка, записанная как
|
||||
факт, — худший вид дефекта».
|
||||
- Владельцу за этот раунд не задано ни одного вопроса — и это оправдано: все
|
||||
продуктовые развилки (что видно, что нельзя трогать) уже закрыты текстом
|
||||
самого issue.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `npm run typecheck`/`npm test`/`npm run build` — на этапе ТЗ нет
|
||||
кода для сборки (диапазон изменений `git diff origin/dev..HEAD` содержит
|
||||
только новый файл `docs/specs/250-opening-centerline.md`, 348 строк, класс C).
|
||||
- Не проверял golden/browser smoke — они относятся к код-ревью, здесь нет
|
||||
реализации, которую можно было бы прогнать.
|
||||
- Не проверял `check-docs`/`process-gate --issues`, упомянутые автором в
|
||||
хендоффе, как отдельные команды — это инфраструктурные гейты уровня коммита
|
||||
документации, а не предмет содержательного ревью ТЗ; факт «класс C коммит,
|
||||
трейлеры на месте» проверен `git show --stat` (см. выше), этого достаточно
|
||||
для этого этапа.
|
||||
- Не оценивал `test/opening-symbol.test.mjs` и `test/iso-openings.test.mjs` на
|
||||
предмет их будущей полной переписки — план автотестов (§13.1) описывает
|
||||
направление правки, конкретные assertions появятся в реализации и будут
|
||||
предметом код-ревью, а не ревью ТЗ.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. ТЗ выполнимо, каждый AC проверяем и привязан к способу доказательства,
|
||||
продуктовая неоднозначность отсутствует (решена владельцем в теле issue),
|
||||
диагноз и заявленный релиз-impact подтверждены построчным чтением текущего кода
|
||||
и документов, а не приняты на слово автора.
|
||||
|
||||
`Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче`
|
||||
Reference in New Issue
Block a user