mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
committed by
Sergey Matyunin
parent
dd3a50426d
commit
cf01489fa9
@@ -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`.
|
||||
Замечаний, требующих возврата автору, нет.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/683-stair-visuals`, коммит `c8fc8838d85b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `fc74e5854648d7013fb03e5ed94637d28b29df18`
|
||||
```
|
||||
git log --all --format='%H %T' | grep fc74e5854648
|
||||
```
|
||||
- Тело issue: `5f2e2a957f1cd2869b4e054901cbce315227e529cb9fcc68495857756083a7f6`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user