mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,150 @@
|
||||
# SPEC-REVIEW-384-r1
|
||||
|
||||
Issue: [#384](https://github.com/Matysh/houseplan-card/issues/384) ·
|
||||
этап: `S4-spec-review` · трек: `small` (ТЗ в теле issue) · заход r1 ·
|
||||
SHA рабочего дерева на момент ревью: `9c162c09`, ветка `dev`.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Родитель — #373 (`fit: house`, docs/specs/373-space-card-house-fit.md, в
|
||||
`dev` с v1.69.0). Баг: невидимые тела стен (`canonicalWallGeometry.components`),
|
||||
extras (независимые стены/драфты/колонны) и нулевые стены безусловно участвуют
|
||||
в structure-цикле кадра `fit: house` (`src/space-render.ts:428-448`), даже
|
||||
когда `show_borders: false` и они физически не рисуются. Видимые проёмы уже
|
||||
корректно гейтятся `!disp.hideOpenings` (:449).
|
||||
|
||||
ТЗ (лёгкий трек, в теле issue) предлагает:
|
||||
1. гейтить вклад тел стен (~:432), extras (~:438) и нулевых стен (~:442) тем же
|
||||
`disp.showBorders`, которым уже гейтится их рендер (:657, :882, :914);
|
||||
2. вернуть `needsCanonicalWallGeometry` (:388) к виду до #373:
|
||||
`walls.length || (extras.length && disp.showBorders)`.
|
||||
|
||||
Единственная затронутая поверхность — `src/space-render.ts`; трек `small`
|
||||
заявлен корректно (см. «Проверка трека» ниже).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Прочитаны `docs/SCOPE.md`, `PROCESS.md` (§1–§9), `AGENTS.md` полностью.
|
||||
- Прочитано тело issue #384 и комментарий аналитики (единственный комментарий).
|
||||
- Сверены построчные ссылки ТЗ с текущим кодом `src/space-render.ts` (чтением,
|
||||
без исполнения): :388, :424-467, :654-657, :882, :914 — все совпадают
|
||||
дословно с описанием в issue.
|
||||
- `git log -p -S"needsCanonicalWallGeometry"` и `-S"fit === 'house'"` по
|
||||
`src/space-render.ts` — подтверждено, что условие `walls.length ||
|
||||
(extras.length && disp.showBorders)` было исходным (коммит `955de3e6`,
|
||||
до #373), а `|| fit === 'house'` добавлен именно коммитом #373
|
||||
(`0d33691f`). Заявление ТЗ «возвращается к виду до #373» фактически точное,
|
||||
а не догадка.
|
||||
- Прочитан `docs/specs/373-space-card-house-fit.md` (контракт-родитель) —
|
||||
контракт п.5 говорит о «visible architectural extents», что и является
|
||||
опорой для этого фикса.
|
||||
- Прочитан `demo/smoke_space_card.mjs` целиком — существующая tight-фикстура
|
||||
жёстко фиксирует `show_borders: true` (:16), поэтому её текущие ассерты
|
||||
(`tightFrame`, `tightPaintedEnvelope.contained` ±0.51px, `tightNoTitleFrame`)
|
||||
физически не заденет предложенный гейт: AC2 (регресс) проверяем.
|
||||
- Прочитаны `docs/USER-GUIDE.md`, `docs/USER-GUIDE.ru.md` (разделы про
|
||||
`fit: house`) и `docs/CANVAS.md` §4 (структурный кадр) — см. находку ниже.
|
||||
- Код продукта не менялся, реализации нет — это ревью ТЗ, не код-ревью.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — «Доки не задеты» неверно: три документа обещают безусловное участие стен/колонн в кадре
|
||||
|
||||
**Файл:** тело issue #384, раздел «7. i18n / Release-артефакты».
|
||||
|
||||
ТЗ утверждает: *«Доки: не задеты (спека #373 уже описывает желаемое поведение;
|
||||
USER-GUIDE про fit не детализирует слои)»*. Это фактически неверно — оба
|
||||
пользовательских гайда и канонический документ подсистемы (CANVAS.md, из
|
||||
обязательного списка чтения AGENTS.md) уже детализируют состав кадра, и
|
||||
детализируют его **безусловно**, без оговорки на `show_borders`:
|
||||
|
||||
- `docs/USER-GUIDE.md:798-800`: *«`house` removes that intentional padding and
|
||||
fits every room, wall, partition, column and opening symbol»* — без единого
|
||||
слова о видимости стен;
|
||||
- `docs/USER-GUIDE.ru.md:1543-1544`: *«Карточка сохранит в кадре все комнаты,
|
||||
стены, перегородки, колонны и полный размах символов проёмов»* — то же самое
|
||||
по-русски, снова безусловно;
|
||||
- `docs/CANVAS.md:232-233`: *«every sane room, positive/zero wall, independent
|
||||
wall or saved draft, column and complete door/window/gate symbol envelope
|
||||
participates»* — канонический контракт подсистемы, тоже без оговорки.
|
||||
|
||||
После фикса это перестаёт быть правдой для `show_borders: false`: стены,
|
||||
extras и нулевые стены **больше не участвуют** в кадре, и все три документа
|
||||
станут вводить в заблуждение читателя ровно про тот кейс, который эта задача
|
||||
чинит. Симметричная оговорка для проёмов (`hide_openings`) в этих документах
|
||||
уже отсутствует тоже, но её отсутствие не создаёт эта задача — а вот
|
||||
безусловность по стенам она напрямую разрушает.
|
||||
|
||||
**Сценарий воспроизведения:** пользователь читает `docs/USER-GUIDE.ru.md`,
|
||||
включает `show_borders: false` и `fit: house`, ожидает (по документации) что
|
||||
«все стены» останутся в кадре — и после фикса получает кадр без них,
|
||||
документация при этом не предупреждает.
|
||||
|
||||
Находка в скоупе: правки нужны в тех же трёх файлах, которые описывают именно
|
||||
эту функцию (`fit: house`), и правило PROCESS.md §11 («документация — в том
|
||||
же коммите, что и поведение») требует их обновить вместе с кодом. Без High
|
||||
это не блокирует полностью, но раздел 7 ТЗ должен назвать все три файла
|
||||
(`docs/USER-GUIDE.md`, `docs/USER-GUIDE.ru.md`, `docs/CANVAS.md`) как
|
||||
затронутые release-артефакты, с оговоркой вида «скрытые стены (`show_borders:
|
||||
false`) не голосуют» рядом с уже существующей оговоркой про backdrop/decor.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Фактическая база ТЗ.** Все построчные ссылки (:388, :428-449, :657, :882,
|
||||
:914) и утверждение о происхождении условия `needsCanonicalWallGeometry`
|
||||
подтверждены чтением кода и git-историей — ни одной догадки, выданной за
|
||||
факт, не найдено в техническом описании.
|
||||
- **Согласованность предложенного гейта.** Прослежено, что реверт
|
||||
`needsCanonicalWallGeometry` не меняет поведение при `disp.showBorders ===
|
||||
true` (условие остаётся истинным через тот же дизъюнкт) и не задевает
|
||||
независимый путь «чистого пола» (`floorMinusBodies` в :791 использует сырой
|
||||
`extras`, а не `canonicalWallGeometry`, и не зависит от этого флага) —
|
||||
побочных эффектов на рендер пола вокруг колонн не возникает.
|
||||
- **AC1–AC3** однозначны, каждый называет способ доказательства (юнит/смок) и
|
||||
проверяемы: AC1/AC3 — новая twin-фикстура с `show_borders: false`, сравнение
|
||||
с кадром «только комнаты+проёмы»; AC2 — регресс на существующих
|
||||
tight-ассертах, которые используют фиксированный `show_borders: true` и
|
||||
физически не пересекаются с новым гейтом.
|
||||
- **AC4** («полный гейт зелёный; бюджет ≈ 0») корректен: изменение — два
|
||||
булевых гейта плюс возврат одного условия, дополнительных вычислений не
|
||||
вводит (скорее исключает лишний union там, где он был не нужен).
|
||||
- **План тестов** называет конкретный механизм (twin-фикстура) и даже
|
||||
mutation-gate («убрать новый гейт → красный смок») — это ровно та
|
||||
дисциплина «тест умеет падать», которую требует код-ревью впоследствии;
|
||||
для ТЗ-этапа это не обязательно, но повышает доверие к плану.
|
||||
- **Откат** — обычный `git revert`, без флагов/миграций/персистентных данных;
|
||||
корректно, изменение чисто в рендер-функции.
|
||||
- **Раздел 8 («принятые предположения»)** — единственное явное предположение
|
||||
(проёмы продолжают голосовать при `show_borders: false`) явно помечено как
|
||||
предположение, а не выдано за факт; технически корректно и не требует
|
||||
продуктового вопроса владельцу.
|
||||
- **Проверка трека `small`.** Один модуль (`src/space-render.ts`), один
|
||||
структурный цикл, ни миграции конфига, ни нового UX-контракта (контракт уже
|
||||
зафиксирован #373, фикс восстанавливает его), ни влияния на perf/touch —
|
||||
все пять критериев §5 выполняются одновременно; аналитика (единственный
|
||||
комментарий issue) содержит явные оценки и не эскалирует продуктовых
|
||||
вопросов владельцу, что соответствует §2.2.
|
||||
- **Открытых продуктовых вопросов нет**, и это верно: поведение до/после
|
||||
однозначно описано контрактом #373 («visible architectural extents»),
|
||||
решать нечего — только привести код к уже принятому контракту.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал никакие гейты (`tsc`, `test`, `build`, смоки) — это ревью ТЗ,
|
||||
реализации ещё не существует; кода для проверки нет.
|
||||
- Не проверял en/de/fr переводы USER-GUIDE (нет отдельного файла с этим
|
||||
текстом кроме `docs/USER-GUIDE.md`, который и есть канонический английский
|
||||
источник) — задета только формулировка кадра, i18n строк интерфейса
|
||||
(label'ы) фикс не касается, ТЗ верно говорит «i18n: не задето» в смысле
|
||||
`src/i18n/*.json`.
|
||||
- Не проверял поведение `fit: content` — по коду и ТЗ оно физически не
|
||||
пересекается с правкой (гейт живёт только внутри блока `if (fit ===
|
||||
'house')`, :424-467), поэтому не разбирал отдельно.
|
||||
- Не оценивал производительность практически (профили не прогонял) — по
|
||||
характеру изменения (два добавленных булевых условия) риск отсутствует, а
|
||||
AC4 явно требует лишь «бюджет ≈ 0», не отдельного профиля.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый. High: 0. Medium: 1, в скоупе задачи — правится в этом же ТЗ (не
|
||||
блокирует переход после исправления, но требует ещё один цикл ревью ТЗ).
|
||||
Reference in New Issue
Block a user