diff --git a/docs/reviews/SPEC-REVIEW-683-r1.md b/docs/reviews/SPEC-REVIEW-683-r1.md new file mode 100644 index 00000000..15053a1a --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-683-r1.md @@ -0,0 +1,198 @@ +# SPEC-REVIEW-683-r1 + +Issue: #683 — визуальные свойства и равномерный шаг лестниц (цвет/заливка, +«Вверх/Вниз», равномерные ступени, cursor pointer в View). +Этап: spec (ревью ТЗ, PROCESS.md §2.4). +Заход: r1 · блокирующих циклов израсходовано 0 из 4. +Материал: тело issue #683, раздел «## ТЗ r1» (владелец опубликовал его +2026-09-27T18:46:38Z, после того как в двух предыдущих комментариях снял все +продуктовые вопросы Q1–Q4). Ветка issue/683-stair-visuals объявлена автором +опубликованной от актуального dev, код не менялся — проверяется только текст +ТЗ в теле issue. + +## Скоуп + +Задача обслуживает J1 («что происходит сейчас» — понятный, не завязанный на +тему HA символ) и J4/J6 («GUI без ручной правки конфига» — оба цвета через +стандартный `hp-color-opacity`) из `docs/SCOPE.md`. Видимое поведение меняется: +у лестницы появляются собственные цвет/заливка, «Вперёд/Назад» становится +«Вверх/Вниз», ступени делятся без остатка, экран/2.5D/PDF получают единую +трапецию. `User-Visible: yes` в ТЗ верен. + +## Как проверялось + +Ревью текста ТЗ плюс сверка каждого заявления о «текущем поведении» с +исходниками на `HEAD` (c8fc8838), чтобы отличить проверяемый факт от +недоказанной догадки: + +- `src/stairs.ts` — модель `Stair`, текущая геометрия ступеней/стрелки, + `forward`/`backward`, кеш геометрии по фингерпринту (без стиля). +- `src/stairs-editor-model.ts`, `src/stairs-box.ts`, `src/stairs-editor.ts` — + дефолты создания, `stairs.forward`/`stairs.backward` в диалоге. +- `src/stairs-view.ts` — `stairTargetState` (`active/missing/self/deleted/fixed`). +- `src/space-render.ts`, `src/styles/plan.styles.ts` — текущие классы + `hp-stair-outline/tread/arrow`, `.navigable { cursor: pointer }`, + `--hp-accent`-подсветка только на `.hp-stair-outline`. +- `src/pdf/pdf-scene.ts` — подтверждение, что PDF уже рисует лестницу. +- `custom_components/houseplan/validation.py` — `STAIR_SCHEMA` (`extra=ALLOW_EXTRA`, + §2.7 read сейчас), `_DECOR_COMMON`/`fill_color`/`fill_opacity` у rect/ellipse + как точный прецедент контракта, который ТЗ переиспользует для лестниц. +- `custom_components/houseplan/import_export.py`, `support_package.py` — + позитивные списки полей (`_pick_fields`/`_copy_keys`) для stair-проекций, + подтверждающие, что новые поля туда действительно предстоит добавить. +- `src/plan-optimizer.ts` — подтверждение, что Optimize сегодня не трогает + `stairs` вообще (значит запрет ТЗ «Optimize не меняет цвета» ничего не + ломает и не требует нового исключения). +- `docs/STAIRS.md`, `docs/USER-GUIDE.ru.md`, `docs/PDF-EXPORT.md` — + терминология («Направление подъёма», «Лестница», remainder-поведение) и + граница неполноты документации. +- `demo/fixtures/large-house.mjs` (`STAIR_COUNT = 250`) — заявление AC14 о + «плане с 250 лестницами» и существующих бюджетах. +- `PROCESS.md` §7.1 — список обязательных разделов ТЗ. + +Гейты не запускались: это ревью текста ТЗ, кода нет (автор явно пишет «Код не +изменён: статус пока не разрешает продуктовую реализацию»), гейты этапа spec +не входят в объём этого ревью. + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Что проверено и корректно + +**Обязательные разделы §7.1** — все присутствуют и в правильном порядке: +сценарий (§1) → что человек видит до/после (§2) → проблема/скоуп/не-скоуп +(§3) → контракт цветов (§4) → контракт трапеции и направления (§5) → +равномерные ступени (§6) → View/editor/2.5D/PDF (§7) → UX (§8) → модель +данных и совместимость (§9) → i18n (§10) → файлы (§11) → AC1–AC14 с колонкой +доказательства (§12) → план автотестов (§13) → performance/touch/a11y (§14) → +риски и откат (§15) → release-артефакты (§16) → «принято предположительно» +(§17). + +**Продуктовые развилки закрыты владельцем, а не додуманы.** Q1–Q4 — +единственные вопросы, которые исполнитель вынес до написания ТЗ, все они +продуктовые (что видит пользователь при переключении направления, какая +область красится, наследует ли лестница будущий общий цвет), ответы явно +внесены в текст (§5, §4, §9). Открытых вопросов владельцу в самом ТЗ не +осталось — требование «не бывает сложной задачи без единого открытого +вопроса» выполнено на уровне истории issue, а не текста финального ТЗ (в +финальном тексте вопросы закрыты, как и должно быть после решения). + +**Направление и трапеция внутренне непротиворечивы.** §5 задаёт: стрелка +всегда идёт по фиксированной локальной оси («канонический» вход/выход не +зависит от режима), а таблица режим→legacy→узкое/широкое основание у +входа/наконечника получается автоматически, если трапеция (а не стрелка) +разворачивается при смене «Вверх/Вниз». Я подставил оба режима в таблицу и +получил ровно те строки, что записаны в ТЗ — противоречия нет. Явно +зафиксировано как **намеренное изменение контракта** (старое `backward` +перестаёт разворачивать стрелку) — это совпадает с решением владельца по Q1 +(альтернатива из вопроса, а не предложенный default) и корректно названо +изменением, а не подано как факт без метки. + +**Цветовой контракт — не изобретение, а перенос уже существующего паттерна.** +`color`/`opacity`/`fill_color`/`fill_opacity` с `_COLOR` (`#RRGGBB`) и +`opacity`/`fill_opacity` как `0..1` дословно совпадает с `_DECOR_COMMON` + +rect/ellipse полями в `validation.py:1462-1488`. Это резко снижает риск того, +что защитный AC8 («backend отклоняет плохой color/opacity») окажется +неисполнимым — валидатор для этого паттерна уже написан и обкатан на других +объектах декора. + +**Совместимость со старыми записями проверена по факту, а не по +предположению.** `STAIR_SCHEMA` уже использует `extra=vol.ALLOW_EXTRA` +(validation.py:1580) — «неизвестные поля переживают валидацию» уже так +работает; ТЗ (§9) верно требует лишь **явных** `vol.Optional` полей для +типизированной проверки цвета/opacity, а не выдаёт существующий ALLOW_EXTRA за +готовое решение AC8. Позитивные списки полей в `import_export.py:298-303` и +`support_package.py:382-384` подтверждают, что copy/backup/support сегодня +**не** содержат цветовых полей лестницы — значит правки в этих трёх файлах, +которые ТЗ требует в §11/AC9, реально нужны, а не избыточны. + +**Cursor pointer (AC12) корректно назван регрессионным, а не новым +поведением.** `.hp-stair.navigable { cursor: pointer }` уже существует +(`plan.styles.ts:1588`), `stairTargetState` уже возвращает +`active/missing/self/deleted/fixed` (`stairs-view.ts:101-105`). ТЗ не выдаёт +существующее поведение за новую разработку — совпадает с явным замечанием +автора в комментарии-аналитике («не требует нового поведения, но должен стать +регрессионным AC»). + +**Разделение visual/physical footprint методологически безопасно.** §5, §9, +§10 (риски) прямо называют «смешать трапецию с физическим прямоугольником» +главным риском и явно требуют regression-тестов area/magnet/hit (AC10); +`stairFootprintGeometry`/`stairOutline` в `stairs.ts` сегодня строят footprint +только из `outline` (прямоугольник/круг), трапеция там не участвует — ТЗ не +предлагает трогать этот путь, только геометрию отрисовки. + +**AC14 (производительность) опирается на реальный, а не гипотетический +бюджет.** `demo/fixtures/large-house.mjs` уже генерирует ровно 250 лестниц +(`STAIR_COUNT = 250`) для `budgets-large-house-isometric.json` и соседних +профилей — «план с 250 лестницами остаётся в существующих бюджетах» проверяемо +существующей инфраструктурой, метод доказательства назван корректно +(unit + performance-smoke/review, не полный performance-набор). + +**Единая геометрия across поверхностей — не декларация.** View/Plan editor +уже используют один `cachedStairRenderGeometry`/`stairRenderGeometry` из +`stairs.ts` (`space-render.ts:906`, тот же файл рендерит и Plan, и View), PDF +уже использует его же (`pdf-scene.ts:17-18,467-481`). Запрет «расходящихся +формул в renderers» (§7, §11) закрывает реальный, а не мнимый риск — сегодня +общий путь уже есть, задача — не сломать его, вводя цвет отдельно в каждом +месте. + +**Терминология UI взята из `USER-GUIDE.ru.md`, а не придумана.** Заголовок +поля «Направление подъёма» (docs/USER-GUIDE.ru.md:796) сохраняется по ТЗ §8 — +совпадает; варианты меняются только у straight, spiral остаётся +«По часовой стрелке/Против часовой стрелки» — тоже дословно из руководства. + +**Совместимость версии хранилища.** «Версия общего storage не повышается» — +поля добавляются как optional к уже additive-коллекции `stairs: []` +(docs/STAIRS.md: «Old configs without it remain unchanged») — не противоречит +существующей модели миграций. + +**Один источник числа ступеней.** N нигде не персистируется — он всегда +выводится из хранимых `length`/`radius` по детерминированной формуле (§6); +кеш геометрии (`RENDER_GEOMETRY_CACHE`) сегодня уже не включает производные +величины стиля в фингерпринт, и AC14 отдельно требует не задеть это свойство +(«стиль не создаёт лишний rebuild») — двойного источника величины, видимой +пользователю, я не нашёл. + +## Чего не проверял + +- Не проверял golden-снимки, PDF-рендер и производительность на 250 + лестницах исполнением — код ещё не написан, это гейты этапа code, а не + spec. +- Не проверял состояние других открытых issue/PR, ссылающихся на #663/#676, + кроме факта, что автор в аналитике явно отличает эту задачу от них + («развитие уже выпущенной #663, а не дубликат #663/#676») — цитата принята + без независимой сверки истории тех issue, так как это не влияет на + исполнимость текущего ТЗ. +- Не проверял точный список всех мест backend, где ещё существуют + позитивные списки полей лестницы (например, дополнительные пути + diagnostics/canonicalization за пределами трёх файлов, найденных grep'ом) — + §11 явно оставляет «точный минимальный дифф» реализации, что уместно для + ТЗ этого уровня детализации. +- Не проверял, вызывает ли сегодняшний `.hp-stair.navigable:hover` сброс + `stroke-opacity` при введении пользовательской прозрачности — это чисто + CSS-каскадная деталь реализации (стиль наследует `stroke-opacity`, если + реализация не переопределит её явно в hover-правиле), не продуктовое + решение, и не требует отдельного ответа владельца. + +## Вердикт + +Зелёный. ТЗ содержит все обязательные разделы, однозначные и проверяемые AC1– +AC14 со способом доказательства, устраняет ранее поднятые продуктовые +развилки решениями владельца, а не догадками, и каждое проверяемое утверждение +о существующем поведении подтверждено чтением актуального кода на `HEAD`. +Замечаний, требующих возврата автору, нет. + +--- + + + +## Материал раунда + +- Ветка: `issue/683-stair-visuals`, коммит `c8fc8838d85b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `fc74e5854648d7013fb03e5ed94637d28b29df18` + ``` + git log --all --format='%H %T' | grep fc74e5854648 + ``` +- Тело issue: `5f2e2a957f1cd2869b4e054901cbce315227e529cb9fcc68495857756083a7f6` +- Вердикт конвейера: `green` · High 0