From 474dbd62ca4330e8d5fb9659fb4da43dfebcc36d Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 11:00:54 +0000 Subject: [PATCH] docs: review document for #292 Issue: #292 User-Visible: no --- docs/reviews/SPEC-REVIEW-292-r1.md | 244 +++++++++++++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-292-r1.md diff --git a/docs/reviews/SPEC-REVIEW-292-r1.md b/docs/reviews/SPEC-REVIEW-292-r1.md new file mode 100644 index 00000000..bd652d8a --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-292-r1.md @@ -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 +```