mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 11:49:16 +00:00
@@ -0,0 +1,186 @@
|
||||
# SPEC-REVIEW-361-r1
|
||||
|
||||
Issue: [#361](https://github.com/Matysh/houseplan-card/issues/361) — «Мебель: физическая
|
||||
толщина линий не масштабируется при zoom»
|
||||
ТЗ: `docs/specs/361-furniture-stroke-zoom.md`, commit `ccaf34d6` (`docs: specify furniture
|
||||
stroke zoom`, Issue: #361, User-Visible: no)
|
||||
Этап: spec (§2.4) · трек: полный (аналитика #361 явно назвала критерий `small`,
|
||||
который задача не проходит: сложность/риск 4–5, геометрический render-контракт) ·
|
||||
заход r1 · блокирующих циклов израсходовано 0 из 4 до этого раунда
|
||||
|
||||
## Скоуп проверки
|
||||
|
||||
Ревью первого захода на полном треке — разбор полный (§2.10 к этому раунду не
|
||||
применяется, второго раунда ещё не было). Проверялись:
|
||||
|
||||
1. соответствие обязательным разделам ТЗ (PROCESS.md §7.1);
|
||||
2. однозначность и доказуемость каждого AC1…AC9;
|
||||
3. отсутствие догадки, выданной за факт, — каждое техническое утверждение
|
||||
сверено с текущим кодом или каноническим документом;
|
||||
4. соответствие `docs/SCOPE.md` (какую строку core user jobs закрывает) и
|
||||
отсутствие расширения скоупа за пределы аналитики;
|
||||
5. терминология интерфейса против `docs/USER-GUIDE.ru.md`;
|
||||
6. согласованность плана автотестов с `AGENTS.md`/`PROCESS.md` о порядке
|
||||
запуска гейтов.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Файловое ревью, без исполнения кода (артефакта ещё нет — стадия spec).
|
||||
Прочитаны и сверены с текстом ТЗ:
|
||||
|
||||
- `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` — рамка процесса и продукта;
|
||||
- тело issue #361 и три комментария (аналитика, занятие, хендофф автора ТЗ);
|
||||
- `src/houseplan-card.ts` (строки ~8049–8153, 8074–8076, 8132–8153) — оба
|
||||
render-пути мебели (`_renderFurniturePlacementPreview`, `_renderDecorLayer`
|
||||
ветка `kind === 'furniture'`) и соседние line/rect/ellipse для контраста;
|
||||
- `src/houseplan-card.ts` (строки ~11060–11073) — корневой `<svg class="plan-svg">`
|
||||
с `viewBox` от `_applyView(zoom, …)` и `preserveAspectRatio="xMidYMid meet"` —
|
||||
подтверждает механизм "camera zoom" через `viewBox`, на который и опирается
|
||||
технический контракт ТЗ;
|
||||
- `src/houseplan-editor-runtime.ts` (строки ~10930–10981) — проверено, что
|
||||
другие использования `non-scaling-stroke` (room-draft/partition/column hit-areas)
|
||||
не относятся к мебели и не задеты скоупом;
|
||||
- `src/styles/plan.styles.ts` (строка 704, 713) — `.derasehit` стилизуется
|
||||
отдельным CSS-классом с собственной шириной, подтверждает заявление ТЗ, что
|
||||
erase-hit не станет физической линией;
|
||||
- `docs/FURNITURE.md` — подтверждает нынешнее (ошибочное) канон-утверждение
|
||||
«the user's decor colour, opacity and physical line width remain authoritative»
|
||||
вместе с `non-scaling-stroke», которое AC9 обязуется исправить;
|
||||
- `docs/USER-GUIDE.ru.md` (строки 61, 169, 174, 1247, 1258) — термины
|
||||
«Редактор подложки», «мебель», «толщина… хранится физически» совпадают с ТЗ;
|
||||
- `docs/CANVAS.md` (§ Render frame vs. view, §3) — подтверждает существование
|
||||
величины "screen pixels-per-unit" в существующей модели камеры, на которую
|
||||
ТЗ явно не претендует (оставлено «принято предположительно»);
|
||||
- наличие `demo/smoke_furniture.mjs`, `test/furniture.test.mjs` и прецедентов
|
||||
raster-измерения (`demo/smoke_grid_scale_invariance.mjs`,
|
||||
`demo/smoke_device_icon_pixel_alignment.mjs`) — подтверждает техническую
|
||||
реализуемость заявленного растрового доказательства.
|
||||
|
||||
Код ещё не написан, поэтому гейты (`typecheck`/`test`/`build`/смоки) на этом
|
||||
этапе не запускались — они не относятся к предмету ревью ТЗ.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium — план автотестов противоречит порядку гейтов из `AGENTS.md`
|
||||
|
||||
**Файл:** `docs/specs/361-furniture-stroke-zoom.md`, раздел «План автотестов», пункт 7.
|
||||
|
||||
**Формулировка ТЗ:**
|
||||
> В цикле реализации запускать только `npm run typecheck`, `npm test`,
|
||||
> `npm run build` по принятому процессу. Browser smoke, golden и полный gate
|
||||
> запускаются в предрелизном прогоне; тестовые сценарии и assertions входят в
|
||||
> продуктовый коммит заранее.
|
||||
|
||||
**Почему это находка.** `AGENTS.md` (раздел «Гейты», решение владельца от
|
||||
2026-08-14, issue #151) прямо требует другого: *«before moving an issue to
|
||||
`S7-code-review`, run the smokes named in its AC locally — `node
|
||||
demo/smoke_<name>.mjs`. A red smoke that reaches the review costs a cycle; run
|
||||
locally it costs a minute»*. Для #361 это не абстрактное правило: AC1, AC3,
|
||||
AC4 и AC6 сами называют `demo/smoke_furniture.mjs` способом доказательства. То
|
||||
есть именно этот смок обязателен к локальному прогону **до** перевода issue в
|
||||
`S7-code-review`, а не отложен до предрелизного прогона.
|
||||
|
||||
Пункт 7 в его нынешней редакции — это почти дословный пересказ
|
||||
`PROCESS.md §11.4`, но эта статья описывает другое: набор гейтов, которые
|
||||
физически невозможно прогнать раньше (полный HA-харнесс, `golden`,
|
||||
`performance_smoke`, полный набор `demo/smoke_*.mjs`) — и относится к окну
|
||||
**после** `S8-merged`. Она не отменяет более узкое и более новое правило
|
||||
`AGENTS.md` про смок, названный в AC самой задачи.
|
||||
|
||||
**Сценарий отказа.** Автор реализации следует тексту ТЗ буквально, не
|
||||
запускает `demo/smoke_furniture.mjs` локально, переводит issue в
|
||||
`S7-code-review` с падающим (или просто непроверенным) смоком — ровно тот
|
||||
случай, который `AGENTS.md` называет стоящим целого цикла ревью, при том что
|
||||
локальный прогон стоит минуту. Prewiew/commit parity (AC4) и anisotropic
|
||||
resize (AC2/AC3) — это как раз те инварианты, которые легко сломать при первой
|
||||
реализации и почти невозможно заметить без растрового смока.
|
||||
|
||||
**Как закрыть.** Строка правится на месте — пункт 7 должен предписывать
|
||||
локальный прогон `node demo/smoke_furniture.mjs` перед переводом issue в
|
||||
`S7-code-review` (как того требует `AGENTS.md`), оставляя `golden`, полный
|
||||
набор `demo/smoke_*.mjs` и `performance_smoke` предрелизному прогону. High
|
||||
здесь нет: это правится редактированием одного абзаца ТЗ, не меняет ни один
|
||||
AC, ни контракт поведения, ни скоуп.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Обязательные разделы §7.1** — все на месте: сценарий, «что человек увидит
|
||||
до/после», проблема, скоуп/не-скоуп, контракт поведения, UX, модель данных и
|
||||
миграция, i18n, AC1…AC9 с доказательством, план автотестов, риски, откат,
|
||||
release-артефакты, блок «принято предположительно». Двусторонняя ссылка
|
||||
issue ↔ ТЗ на месте.
|
||||
- **Диагноз дефекта грамотно обоснован кодом, не догадкой.** Проверено
|
||||
построчно: `line/rect/ellipse` (houseplan-card.ts:8093–8130) получают
|
||||
`stroke-width` без `vector-effect` и поэтому масштабируются вместе с
|
||||
`viewBox`; сохранённая мебель (8144–8148) и preview (8071–8076) получают тот
|
||||
же физический `stroke-width`, но дополнительно `vector-effect="non-scaling-stroke"`,
|
||||
который по спецификации SVG игнорирует весь CTM выше элемента — включая и
|
||||
локальный `scale(W2/art.viewW H2/art.viewH)` (нужный), и внешний
|
||||
`viewBox`-масштаб камеры (нежелательный побочный эффект). Корневой
|
||||
`plan-svg` действительно меняет `viewBox` от `_applyView(zoom, …)`
|
||||
(houseplan-card.ts:11069–11073) — то есть камера здесь именно
|
||||
viewBox-driven, а не внешний CSS-transform, и технический контракт ТЗ
|
||||
(«внешний plan CTM/viewBox» как источник camera zoom) сформулирован верно, а
|
||||
не предположительно.
|
||||
- **Anisotropic-инвариант не выдуман.** Причина, по которой простое удаление
|
||||
`vector-effect` создало бы новый дефект, прослеживается по тому же коду:
|
||||
только у мебели есть локальный `scale(x, y)` от `art.viewW/viewH` к боксу
|
||||
`w/h`, у line/rect/ellipse такого transform нет — значит анизотропия
|
||||
штриха возможна только у мебели, и ТЗ верно ограничивает риск именно этим
|
||||
render-путём.
|
||||
- **Erase-hit и миниатюры палитры корректно выведены из скоупа.**
|
||||
`.derasehit` в `plan.styles.ts` стилизуется отдельным CSS-классом с
|
||||
собственной шириной, не зависящей от `width_cm` — заявление «текущая
|
||||
интерактивная ширина не должна сузиться» проверяемо и уже верно сегодня, фикс
|
||||
его не касается по построению.
|
||||
- **Терминология.** «Редактор подложки», «мебель», «толщина… физически» — из
|
||||
`docs/USER-GUIDE.ru.md`, не изобретены.
|
||||
- **Изометрия выведена корректно.** Публичной калибровки iso-режима в скоупе
|
||||
нет (Labs-эксперимент, презентационный слой), при этом требование «не терять
|
||||
мебель и не падать» остаётся — разумная и проверяемая граница, не
|
||||
продуктовый вопрос, требующий владельца.
|
||||
- **Технически реализуемо.** В демо-каталоге уже есть прецеденты растрового
|
||||
измерения (`smoke_grid_scale_invariance.mjs`,
|
||||
`smoke_device_icon_pixel_alignment.mjs`) и оба целевых файла
|
||||
(`demo/smoke_furniture.mjs`, `test/furniture.test.mjs`) существуют и готовы
|
||||
к расширению — план не полагается на несуществующую инфраструктуру.
|
||||
- **Продуктовых вопросов владельцу нет**, и это верно: единственная реальная
|
||||
двусмысленность (нужен ли отдельный golden-сценарий) — техническая, явно
|
||||
оставлена «принято предположительно», разрешается по факту достаточности
|
||||
растрового доказательства на код-ревью, а не эскалацией.
|
||||
- **AC проверяемы и не выданы за уже решённые.** Ни один AC не описывает
|
||||
поведение, которого нет ни в одном документе, без пометки допущения; числовые
|
||||
допуски (`max(1 CSS px, 10%)`) заданы явно там, где они важны для теста, и
|
||||
оставлены свободными там, где это чисто техническая деталь (реализация
|
||||
pixel measurement, точные тестовые symbols).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не оценивалась реализуемость конкретной формулы `min(viewportWidth /
|
||||
viewBoxWidth, viewportHeight / viewBoxHeight)` в деталях (например, как
|
||||
измеряется `viewportWidth` для конкретного `<svg>` в браузере/Playwright) —
|
||||
это заявлено в ТЗ как «принято предположительно, поменять свободно» и не
|
||||
является продуктовым контрактом; будет предметом код-ревью.
|
||||
- Не запускались `npm run typecheck`/`test`/`build` — кода нет, гейты к этому
|
||||
этапу не относятся.
|
||||
- Не проверялась совместимость с `docs/CONFIG-COMPATIBILITY.md` построчно —
|
||||
ТЗ явно заявляет «schema/config version и backend validation не меняются»,
|
||||
и это подтверждено отсутствием изменений в модели данных (persisted schema
|
||||
в разделе ТЗ идентична текущей в `FURNITURE.md`); отдельного анализа
|
||||
миграции не требуется, потому что миграции нет.
|
||||
- Не проверялась текущая numeric-точность существующих raster-тестов
|
||||
decor-line (на который ссылается AC1 «тот же raster tolerance») — она не
|
||||
названа в ТЗ явным числом, и это оставлено на усмотрение автора при
|
||||
реализации теста, а не продуктовое решение.
|
||||
|
||||
## Вердикт
|
||||
|
||||
High: 0. Medium: 1, в скоупе задачи (см. выше) — не заводится отдельным issue,
|
||||
правится в тексте `docs/specs/361-furniture-stroke-zoom.md` в рамках этого же
|
||||
раунда. Технический диагноз проверен по коду и корректен, контракт исполним,
|
||||
AC доказуемы. Единственная находка — процедурная (порядок запуска гейта
|
||||
`demo/smoke_furniture.mjs`), не продуктовая и не архитектурная; правки в
|
||||
поведение, AC или скоуп не требует.
|
||||
|
||||
**Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 →
|
||||
в задаче**
|
||||
Reference in New Issue
Block a user