16 KiB
SPEC-REVIEW-361-r1
Issue: #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 к этому раунду не применяется, второго раунда ещё не было). Проверялись:
- соответствие обязательным разделам ТЗ (PROCESS.md §7.1);
- однозначность и доказуемость каждого AC1…AC9;
- отсутствие догадки, выданной за факт, — каждое техническое утверждение сверено с текущим кодом или каноническим документом;
- соответствие
docs/SCOPE.md(какую строку core user jobs закрывает) и отсутствие расширения скоупа за пределы аналитики; - терминология интерфейса против
docs/USER-GUIDE.ru.md; - согласованность плана автотестов с
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 → в задаче