From 927ca2ab45558f8e149d6b227331ee44405f7c2e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 07:01:27 +0000 Subject: [PATCH] docs: review document for #361 Issue: #361 User-Visible: no --- docs/reviews/SPEC-REVIEW-361-r1.md | 186 +++++++++++++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-361-r1.md diff --git a/docs/reviews/SPEC-REVIEW-361-r1.md b/docs/reviews/SPEC-REVIEW-361-r1.md new file mode 100644 index 00000000..bec60003 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-361-r1.md @@ -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) — корневой `` + с `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_.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` для конкретного `` в браузере/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 → +в задаче**