mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
committed by
Sergey Matyunin
parent
e1f0d2310a
commit
474dbd62ca
@@ -0,0 +1,244 @@
|
||||
# SPEC-REVIEW-292-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/292
|
||||
- **Этап:** ревью ТЗ (PROCESS.md §2.4)
|
||||
- **Артефакт ТЗ:** `docs/specs/292-resize-availability-audit.md`, commit `52ad67e7`
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (до этого вердикта)
|
||||
- **Ревьюер:** Claude (роль ревьюер ТЗ, PROCESS.md §6)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Единственный документ на ревью — `docs/specs/292-resize-availability-audit.md`
|
||||
(HEAD detached at `origin/issue/292-resize-availability-audit`, файл введён
|
||||
коммитом `52ad67e7 docs: specify resize availability audit`). Issue не
|
||||
помечен `small`, поэтому наличие отдельного файла в `docs/specs/` обязательно
|
||||
и подтверждено. Первый раунд — деление на дельту (§2.10) не применяется,
|
||||
разбор полный.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитан `docs/SCOPE.md` — задача закрывает job J6 («Keep the plan true as
|
||||
the home evolves» — drag/resize), ограничитель не нарушен.
|
||||
2. Прочитаны `PROCESS.md` (целиком, включая §7.1 обязательные разделы ТЗ,
|
||||
§2.5 чек-лист DoR, §4 лимит циклов) и `AGENTS.md`.
|
||||
3. Прочитано тело issue #292 и все три комментария (уточнение про #293,
|
||||
«Аналитика подтверждена», ссылка на ТЗ).
|
||||
4. Прочитан `docs/RESIZE.md` — канонический контракт Resize, куда #292 обязан
|
||||
вписаться без изменения архитектуры пайплайна.
|
||||
5. Прочитан `docs/USER-GUIDE.ru.md` (термины «Resize», «Оптимизировать
|
||||
планы») — терминология ТЗ совпадает.
|
||||
6. Сверены статусы связанных issue: `gh issue view` для #276 (CLOSED, слит —
|
||||
commit `a6056111 fix: reconcile coincident partitions during Optimize`),
|
||||
#289 (OPEN, `S3-spec`), #290 (OPEN, `S3-spec`), #293 (OPEN,
|
||||
`S4-spec-review`), #277 (CLOSED), #284 (CLOSED).
|
||||
7. Прочитан текущий код: `src/resize.ts` (тип `SafeResizeReason`, 9 значений,
|
||||
без параметров контекста), `src/houseplan-card.ts` (`_rszReasonText`,
|
||||
`_rszDisabledActivate`, `_rszDisabledKey`, разметка `_renderResizeLayer`) —
|
||||
чтобы проверить, что декларируемый в ТЗ контракт технически достижим на
|
||||
существующей архитектуре.
|
||||
8. Сверены i18n-ключи `resize.disabled.*` в `src/i18n/ru.json` с §5 ТЗ.
|
||||
9. Для сравнения формата прочитаны структуры разделов трёх смежных ТЗ той же
|
||||
подсистемы: `docs/specs/276-coincident-partition-reconciliation.md`,
|
||||
`docs/specs/277-safe-resize.md` (прямой предок контракта Resize),
|
||||
`docs/specs/281-resize-zero-range.md`.
|
||||
10. Проверено наличие обеих реальных фикстур,
|
||||
`test/fixtures/real-plan-{first,second}-floor.json` — существуют.
|
||||
11. Проверено происхождение термина «mixed-role»: не изобретён автором ТЗ —
|
||||
это существующий инвариант `checkMixedRoleRecords`
|
||||
(`scripts/model-invariants.mjs`, `test/model-invariants.test.mjs`),
|
||||
закреплённый только что смежным коммитом `523190d8`.
|
||||
|
||||
Код не менялся, продуктовый код не оценивался за пределами чтения контракта
|
||||
для проверки реализуемости ТЗ.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — обещание в §5 не имеет технической опоры и не проверяется ни одним AC
|
||||
|
||||
**Файл:** `docs/specs/292-resize-availability-audit.md`, раздел 5 «Тексты
|
||||
причин», строка «angled / side angle» и абзац после таблицы.
|
||||
|
||||
**Формулировка ТЗ:**
|
||||
|
||||
> angled / side angle — …; предложить Optimize, только если кандидат #290
|
||||
> существует
|
||||
> …
|
||||
> Если Optimize не способен исправить конкретную стену, текст не должен
|
||||
> обещать, что Optimize её исправит.
|
||||
|
||||
Это не пожелание, а нормативное требование к пользовательскому тексту —
|
||||
ровно то, ради чего заведена задача (§29 issue: «пользователь не знает, что
|
||||
виноват невидимый уступ… и что его можно выпрямить через „Оптимизировать“»,
|
||||
но алгоритм не должен врать про стены, которые Optimize не чинит).
|
||||
|
||||
**Почему это дефект, а не мелочь.** Текущая архитектура даёт ровно один
|
||||
текст на одно значение `SafeResizeReason`, без контекста:
|
||||
|
||||
```ts
|
||||
// src/houseplan-card.ts:8567
|
||||
private _rszReasonText(reason: SafeResizeReason): string {
|
||||
return this._t(`resize.disabled.${reason}` as I18nKey);
|
||||
}
|
||||
```
|
||||
|
||||
`SafeResizeReason` (`src/resize.ts:33-42`) — плоский union из 9 значений,
|
||||
без поля «есть ли кандидат на исправление». Два ребра с одинаковым
|
||||
`reason: 'side-angle'` — одно из-за настоящего уступа в один шаг (чинится
|
||||
Optimize), другое из-за принципиально диагональной стены (не чинится) —
|
||||
сегодня физически не различимы на уровне текста. Чтобы выполнить требование
|
||||
§5, нужен один из двух путей:
|
||||
|
||||
- новое поле в `SafeResizeResolution`/новый вариант `SafeResizeReason`,
|
||||
несущий признак «есть кандидат ремонта», плюс отдельные i18n-ключи для двух
|
||||
вариантов сообщения — либо
|
||||
- явный отказ от дифференциации сообщения и смягчение формулировки §5.
|
||||
|
||||
Ни то, ни другое в ТЗ не решено. При этом:
|
||||
|
||||
- **AC6** требует «один resolver и один reason type» между audit и render —
|
||||
то есть удвоения/новой ветки `SafeResizeReason` ТЗ прямо не предполагает,
|
||||
а альтернативы не называет;
|
||||
- ни один AC (1–9) не проверяет само разделение «текст обещает Optimize» vs
|
||||
«текст не обещает Optimize» для двух геометрически разных `side-angle`;
|
||||
- раздел 9 «Ожидаемые файлы» называет `src/i18n/en.json`, `src/i18n/ru.json`
|
||||
без единого перечисленного ключа — DoR (`PROCESS.md` §2.5) требует «i18n:
|
||||
ключи en + ru перечислены», здесь их нет вовсе, что при данном пробеле
|
||||
особенно чувствительно: неизвестно, один ключ на reason останется или
|
||||
появится второй;
|
||||
- источник признака «кандидат #290 существует» зависит от #290, который сам
|
||||
ещё в `S3-spec` и не определил свой интерфейс — то есть #292 обещает
|
||||
использовать сигнал, которого пока не существует даже как контракт.
|
||||
|
||||
**Как воспроизвести последствие, если оставить как есть.** Реализатор,
|
||||
получив ТЗ, либо (а) молча добавит новое поле/вариант reason — незапланированное
|
||||
техническое решение без фиксации в ТЗ, которое ревью кода будет разбирать
|
||||
без опоры на контракт, либо (b) оставит один статический текст на
|
||||
`side-angle`/`diagonal`, и тогда либо утверждение §5 не выполнено (текст
|
||||
всегда одинаковый и либо всегда предлагает Optimize — включая случаи, где
|
||||
это не поможет, — либо никогда не предлагает, обесценивая саму находку
|
||||
issue про «предложить действие»).
|
||||
|
||||
**Что нужно поправить в ТЗ:** добавить явный технический контракт (новое
|
||||
поле контекста reason или отдельный `SafeResizeReason`-вариант с
|
||||
перечисленными i18n-ключами для обоих языков) и минимум один AC, который
|
||||
прямо проверяет «на fixture, где `side-angle` вызван настоящим уступом,
|
||||
текст предлагает Optimize; на fixture, где `side-angle` — не устранимая
|
||||
геометрия, текст этого не предлагает». Либо снять обязательство «текст не
|
||||
должен обещать» из §5 и явно пометить дифференциацию сообщений как
|
||||
предположение вне скоупа/будущей работы после #290.
|
||||
|
||||
### Low — отсутствуют разделы, обязательные по PROCESS.md §7.1 и принятые в соседних ТЗ той же подсистемы
|
||||
|
||||
Обязательный список §7.1: «сценарий · что человек увидит до и после ·
|
||||
проблема · скоуп и не-скоуп · контракт поведения · UX · модель данных и
|
||||
миграция · i18n · AC с доказательством · план автотестов · риски · откат ·
|
||||
release-артефакты».
|
||||
|
||||
В `292-resize-availability-audit.md` отсутствуют как отдельные разделы:
|
||||
|
||||
- **«Сценарий и персона»** — кто, на какой поверхности, в какой момент
|
||||
встречает это поведение. Восстановимо из контекста (админ, десктоп, Plan →
|
||||
Resize), но не написано явно, как в `277-safe-resize.md` §1 и
|
||||
`276-coincident-partition-reconciliation.md` §1.
|
||||
- **«Риски и меры»** — ни одного явного риска не названо отдельным
|
||||
разделом (только «Принятые предположения», которые не то же самое: риск
|
||||
требует меры, предположение — нет). Реальный риск здесь есть и не назван
|
||||
формально: AC1 и AC3 требуют «после rebase поверх #276/#289/#290», а #289 и
|
||||
#290 сейчас оба в `S3-spec` — то есть их собственные ТЗ ещё не прошли
|
||||
ревью. Задача корректно может стоять в очереди `S5-ready` (§2.5: «не
|
||||
работа, а очередь»), и это не блокирует зелёный вердикт ТЗ, но риск
|
||||
«баланс чисел заведомо не финализируется, пока не landed #289/#290»
|
||||
заслуживает отдельной строки, а не одной фразы в предположении №2.
|
||||
- **«Откат»** — DoR (§2.5) требует явно: «как выключить или вернуть назад».
|
||||
У `277-safe-resize.md` и `276-...md` есть выделенный раздел
|
||||
«Release-артефакты и rollback» (например: «Rollback — revert
|
||||
implementation commit»). В ТЗ #292 такого раздела нет вовсе — раздел 10
|
||||
«Release» говорит только про трейлеры и закрытие issue, не про откат.
|
||||
|
||||
Ни одно из этих трёх упущений не меняет технический контракт и не создаёт
|
||||
риска для реализации — это дефект полноты документа относительно
|
||||
зафиксированного формата, а не двусмысленность поведения. Снимается
|
||||
рекомендацией: добавить короткие явные разделы (по 2–4 строки достаточно,
|
||||
как в `281-resize-zero-range.md`, который тоже лаконичен) при следующей
|
||||
правке ТЗ по Medium-находке.
|
||||
|
||||
### Low — AC1–AC8 не помечают явно способ доказательства
|
||||
|
||||
DoR требует «у каждого [AC] указано, чем он доказывается: unit / backend /
|
||||
smoke / golden / «ревью кода»». В тексте способ доказательства всегда
|
||||
угадывается из формулировки (AC1/AC3/AC4/AC6/AC7/AC8 — очевидно `unit` по
|
||||
fixture, AC2 — `unit`, AC5 — явно `production smoke`), противоречий не
|
||||
найдено, но явной пометки нет ни у одного пункта, кроме сводного AC9
|
||||
(гейты). Снимается с записью: смысл всюду однозначен, добавление явных
|
||||
тегов — правка для аккуратности, не для устранения двусмысленности.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Сценарий и продуктовая рамка.** Задача явно закрывает job J6
|
||||
`docs/SCOPE.md` («keep the plan true» — drag/resize остаётся рабочим).
|
||||
Метрика 70% отключённых рукояток верно помечена диагностическим сигналом,
|
||||
а не целью «разрешить любой ценой» (раздел 11, п.1) — соответствует духу
|
||||
RESIZE.md, где часть запретов обязана оставаться.
|
||||
- **Границы с #293 выдержаны.** ТЗ явно исключает починку «активной, но
|
||||
инертной» рукоятки (не входит, раздел 6) — комментарии issue подтверждают,
|
||||
что это разделение согласовано с владельцем 2026-08-24.
|
||||
- **Термины грамотны и не изобретены.** «Оптимизировать планы» соответствует
|
||||
`docs/USER-GUIDE.ru.md:1396`; «mixed-role» соответствует существующему
|
||||
инварианту `checkMixedRoleRecords`; девять причин отказа (`diagonal` …
|
||||
`invalid-geometry`) дословно совпадают с уже описанным приоритетным
|
||||
списком в `docs/RESIZE.md` «Eligibility» — ТЗ не меняет порядок и не
|
||||
добавляет причин молча.
|
||||
- **AC2 не выдумана.** Конкретная пара `room-a` edge 2 / `room-b` edge 2
|
||||
на второй реальной фикстуре подтверждена ранее эмпирически владельцем
|
||||
задачи в комментарии «Про стену между двумя детскими» — `resolveSafeResize`
|
||||
уже возвращал `enabled` на этой паре на момент issue.
|
||||
- **Нет продуктовых вопросов владельцу, и это оправдано.** Комментарий
|
||||
«Аналитика подтверждена» прямо фиксирует «отдельных вопросов владельцу
|
||||
нет: критерии в issue однозначны» — согласуется с §7.1: оставшаяся
|
||||
неопределённость (см. Medium-находку) техническая, а не продуктовая, и её
|
||||
вправе решать автор с ревьюером без эскалации.
|
||||
- **Не входит расширение safe-resize допусков.** Раздел 6 «Не входит»
|
||||
явно запрещает «ослабление safe-resize ограничений для
|
||||
partial/mixed/diagonal topology» — совпадает с предупреждением issue
|
||||
«сама доля запретов не является ошибкой».
|
||||
- **Гейты названы соразмерно этапу.** Раздел 9 «Локальные гейты» верно
|
||||
ограничивается typecheck/test/build/check-docs/targeted-audit-smoke-mutation
|
||||
и явно откладывает полный golden/smoke/performance набор на пре-релиз —
|
||||
соответствует `PROCESS.md` §8.
|
||||
- **AC8 верно привязан к существующему инвариантному инструменту** (wall
|
||||
keys, mixed-role records, references, opening host/fit, geometry preflight)
|
||||
— это тот же набор проверок, что и `scripts/model-invariants.mjs`; ТЗ не
|
||||
придумывает собственный параллельный набор.
|
||||
- **Touch/compatibility не расширяются.** Раздел 8 корректно фиксирует
|
||||
«schema/storage не меняются», touch остаётся best effort с доступом к
|
||||
причине через tap — не создаёт нового UX-контракта сверх
|
||||
`TOUCH-SUPPORT.md`.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Продуктовый код не читался за пределами трёх точек (`resize.ts` типы,
|
||||
`houseplan-card.ts` рендер рукояток и обработчики) — этого разбора
|
||||
достаточно для оценки реализуемости ТЗ, но это не код-ревью.
|
||||
- Не запускались тесты/гейты — на этапе ревью ТЗ они не относятся к делу
|
||||
(код ещё не менялся, есть лишь файл документации класса C).
|
||||
- Не проверялось состояние `docs/specs/README.md` (таблица issue ↔ ТЗ) —
|
||||
вне зоны ответственности ревью ТЗ, это гигиена репозитория.
|
||||
- Не оценивалась связь с #293 на уровне кода — только на уровне
|
||||
формулировок ТЗ/issue, поскольку #293 не входит в скоуп #292.
|
||||
- Точный будущий baseline AC1 не пересчитывался — он и не может быть
|
||||
пересчитан до слияния #289/#290, что сам документ признаёт (раздел 11,
|
||||
предположение 2).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Medium-находка лежит строго в скоупе задачи (это её собственный раздел §5,
|
||||
не соседняя подсистема) → по PROCESS.md §2.4/§4 без High это **жёлтый**
|
||||
вердикт: автор правит ТЗ (либо задаёт технический контракт дифференциации
|
||||
текста плюс AC, либо смягчает формулировку §5), фикс проходит второй заход
|
||||
ревью по дельте (§2.10). Low-находки не блокируют переход, но должны быть
|
||||
закрыты в той же правке, раз документ и так возвращается автору.
|
||||
|
||||
```
|
||||
Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче
|
||||
Документ: docs/reviews/SPEC-REVIEW-292-r1.md
|
||||
```
|
||||
Reference in New Issue
Block a user