From 19edda83e35e95e7fdc66beeab8cb56d6a2756b6 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:42:17 +0000 Subject: [PATCH] docs: review document for #676 Issue: #676 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-676-r2.md | 211 +++++++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-676-r2.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 1b5b623a..6c33c068 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,10 +1,11 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1106, issue: 396. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1107, issue: 396. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| | #676 | [SPEC-REVIEW-676-r1.md](SPEC-REVIEW-676-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 2 | AC8 не называет проверку двух из трёх ветвей подавления подсказки; риск «рамка перехватывает события под другим инструментом» не привязан ни к одному AC; не задан tie-break для равного протяжки по обеим осям (снимаю сам) | `src/stairs-editor-model.ts` `stairs-view.ts` `demo/smoke_stairs.mjs` `src/furniture.ts` | +| #676 | [SPEC-REVIEW-676-r2.md](SPEC-REVIEW-676-r2.md) | spec · r2 | 🟡 жёлтый | 0 | 1 | AC8/К8 перечисляют четыре ветви stairTargetState, а не пять; состояние self выпадает из… | `src/stairs-editor-model.ts` `docs/STAIRS.md` `src/stairs.ts` `stairs-view.ts` | | #675 | [CODE-REVIEW-675-r1.md](CODE-REVIEW-675-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #673 | [CODE-REVIEW-673-r1.md](CODE-REVIEW-673-r1.md) | code · r1 | 🟡 жёлтый | 0 | 0 | docs/images/screenshots.json: закоммиченный sourceFingerprint не совпадает с тем, что р…; Комментарий автора в issue #673 называет SHA реализации 557d4e38e26b647437f2c931454737f… | `docs/images/screenshots.json` `demo/golden/matrix.mjs` `scripts/source-fingerprint.mjs` `accept.mjs` `policy.mjs` `AGENTS.md` `baselines-index.json` | | #673 | [CODE-REVIEW-673-r2.md](CODE-REVIEW-673-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-676-r2.md b/docs/reviews/SPEC-REVIEW-676-r2.md new file mode 100644 index 00000000..642c767f --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-676-r2.md @@ -0,0 +1,211 @@ +# SPEC-REVIEW-676-r2 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/676 +- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4) +- **Трек:** полный (метки `bug`, `P1`, `S4-spec-review`; лёгкий трек — нет, + подтверждено аналитиком в r1: несколько поверхностей, новый UX-контракт + размещения протяжкой и подсказки, сложность > 3). +- **Материал:** тело issue #676, раздел `## ТЗ` (редакция 2, 27.09) + 6 + комментариев (без изменений добавился только комментарий «редакция 2» и + вердикт r1). +- **Заход:** r2 · блокирующих циклов израсходовано 1 из 4 (эта находка сделает + 2 из 4) +- **Роль:** ревьюер ТЗ (не автор) + +## Скоуп ревью (по дельте, PROCESS.md §2.10) + +Предмет r2 — ровно три точечных правки редакции 2, объявленные автором в +комментарии `IC_kwDOTOcLQM8AAAABXQNDXw` и в шапке ТЗ («по ревью r1: AC8 +покрывает все три ветви подавления, AC12 — инертность рамки, tie-break +диагонали в К1»): + +1. AC8 расширен до перечисления всех ветвей подавления подсказки Просмотра + (К8). +2. Новый AC12 — инертность рамки узлов лестницы под инструментами, отличными + от «Выбор»/«Лестница». +3. К1 получил детерминированный tie-break для протяжки ровно по диагонали + 45°. + +Код задачи не существует (ветка `issue/676-*` не создана; см. «Как +проверялось», п. 1) — ТЗ по-прежнему живёт только в теле issue, `docs/specs/` +не создаётся (issue открыт после 2026-09-10). Остальные разделы ТЗ (Сценарий, +Контракт К2–К7, i18n, модель данных, риски, откат, release-артефакты) +дельтой r1→r2 не задеты — они наследуются из r1 без повторной проверки (см. +раздел «Унаследовано из r1» ниже), кроме одного пересечения: расширенная +верификация К8/AC8 в этом раунде вскрыла состояние, не покрытое ни в r1, ни +в правке r2 (см. «Находки»). + +**SCOPE-проверка (`docs/SCOPE.md`):** не меняется относительно r1 — сценарий +закрывает J6 (редактирование объектов плана без регресса) и J1/J2 через +корректную навигацию/подсказку лестницы в Просмотре. + +## Как проверялось + +1. Подтверждено, что кода задачи всё ещё нет: `git branch -a | grep -i 676` + не находит рабочую ветку (только `d03a68b8 feat: добавить лестницы между + этажами (#663)` — это #663, не #676); `git diff f1a87fba..HEAD --stat` + между материалом r1 и текущим `HEAD` (`9fe7f88c`) показывает только два + файла — `docs/reviews/INDEX.md` и публикацию `SPEC-REVIEW-676-r1.md`. + Дифф кода для этой задачи не существует; гейты `typecheck`/`npm test`/ + `npm run build`/`check-docs.mjs` неприменимы на этом этапе — см. «Чего не + проверял». +2. Прочитано тело issue #676 целиком (`gh issue view 676 --json body`, + sha256 `0b58e92a…0bdd`) и все 6 комментариев (`gh issue view 676 --json + comments`), включая новый (редакция 2) и вердикт r1. +3. Сверены ровно три правки редакции 2 с текстом находок r1 построчно: + - **Medium-1 (AC8, r1):** требовал отдельного смоук-шага на каждую из + веток `missing`/`deleted`/`fixed`. Новый AC8 перечисляет **четыре** + шага с отдельными флагами: `garden` (active) → тултип и снятие; + `target_space_id: null` (missing) → `null`; `'no-such-space'` + (deleted) → `null`; карточка с `floor: 'f1'` (fixed) → `null`. Текстуально + закрывает ровно то, что просил r1. + - **Medium-2 (инертность рамки, r1):** рекомендовал новый AC12 со + смоук-шагом «под инструментом не-«Выбор»/не-«Лестница» клик по точке + узла воздействует на инструмент, а не перехватывается рамкой». + Новый AC12 в точности содержит эту проверку (`.hp-stair-frame` + отсутствует в DOM, `_path.length` растёт под «Стенами»). К2 также + получил явное «рамка не рендерится вовсе» — согласовано с AC12. + - **Low (tie-break, r1):** К1 получил «при равных |dx| и |dy| доминирует + x», с прямой ссылкой на прецедент `resizeFurnitureTransform` — снято. +4. Проверено, что AC12 ссылается на реально существующий, а не + выдуманный контракт: `src/houseplan-editor-runtime.ts:2213-2229` + (`_markupClick`) подтверждает, что клик под инструментом `draw` («Стены») + продвигает `this.host._path` (`_path.length` растёт при добавлении точки, + см. `_canAppendWallPoint`, `:2200-2210`) — заявление AC12 о «клик ставит + точку стены» не расходится с существующим кодом инструмента. +5. Так как AC8 — именно та зона, которую r1 уже признал недостаточно + покрытой веткой, для r2 она перепроверена не только текстуально, а по + исходной функции, которую К8/AC8 описывают: + `stairTargetState()` (`src/stairs-editor-model.ts:97-104`) возвращает + **пять** состояний — `active | missing | self | deleted | fixed`, — не + четыре. `self` (`target_space_id === currentSpaceId`) — не выдуманное + состояние: оно документировано в каноне + `docs/STAIRS.md:21-22` («A missing, self or deleted target leaves the + stair visible and editable but shows a repair warning...») в одной группе + с `missing`/`deleted`, и данные, порождающие его, не отвергаются моделью + (`src/stairs.ts:84` проверяет только тип строки `target_space_id`, не + запрещает равенство текущему пространству). Диалог свойств лишь + исключает текущее пространство из выпадающего списка на **вкладке, где + лестница редактируется сейчас** (`src/stairs-editor.ts:505`, + `.filter((item) => item.id !== this.owner._space)`) — это не мешает + `self` возникнуть после переноса/слияния помещений или ручной правки + конфигурации, поэтому уже существующий код `stairs-view.ts:64-73` + отдельно проверяет ровно пять веток через ту же функцию и намеренно не + считает `self` навигируемым (`active = interactive && targetState === + 'active'`). К8/AC8 этой задачи вводят новый вызов той же функции для + тултипа, но описывают только **четыре** из пяти её исходов; вывод — в + «Находки». +6. Разделы, не тронутые дельтой (К2–К7 кроме упомянутого в п. 3 уточнения, + i18n, модель данных, откат, release-артефакты, план автотестов кроме + строки AC12), не перечитывались заново построчно — они приняты из r1 без + повторной проверки, см. «Унаследовано из r1». + +## Находки + +### Medium (в скоупе) — AC8/К8 перечисляют четыре ветви `stairTargetState`, а не пять; состояние `self` выпадает из контракта тултипа + +**Что не так.** К8: «Лестница без цели, с битой целью или в карточке с +закреплённым этажом подсказки не показывает» — это `missing`, `deleted`, +`fixed`. AC8 после правки r2 прямо называет себя «все четыре ветви» и +перечисляет `active`/`missing`/`deleted`/`fixed`. Но функция, которую К8 +описывает, `stairTargetState()` (`src/stairs-editor-model.ts:97-104`), +возвращает **пять** значений: `active | missing | self | deleted | fixed`. +Ветвь `self` — лестница с валидным, непустым `target_space_id`, который +совпадает с ID текущего пространства, — не упомянута ни в К8, ни в AC8, ни +в плане смоук-шагов. + +**Почему это находка, а не придирка.** `self` — не гипотетическое, а +документированное состояние: `docs/STAIRS.md:21-22` группирует его вместе +с `missing`/`deleted` («shows a repair warning... makes View activation a +no-op»), и модель (`src/stairs.ts:84`) не отвергает данные, где +`target_space_id` равен текущему пространству — такое может возникнуть при +переносе/слиянии помещений (J6, не защищено на этом уровне валидацией) или +ручной правке YAML. Существующий, не-скоуповый код Просмотра +(`stairs-view.ts:64-73`) уже трактует `self` как ненавигируемое наравне с +`missing`/`deleted`/`fixed` через ту же функцию. Реализация тултипа, +написанная буквально по К8 («не показывать при missing, deleted или +fixed»), — то есть перечислением трёх суппрессирующих веток, а не +инверсией `targetState !== 'active'`, — пропустит `self` через «иначе» и +покажет пользователю подсказку «Переход на этаж {название текущего же +этажа}» на лестнице, которая никуда не ведёт (клик по ней не +сработает, потому что навигация уже верно проверяет `=== 'active'`). Это +ровно тот класс дефекта, который r1 уже указал для этой же строки ТЗ: +неполная ветка гварда, которая пройдёт AC1–AC12 буквально и оставит +конкретное поведение и непроверенным, и вероятно неверным. Формулировка +«все четыре ветви» (в отличие от r1, где пропуск был очевиден по счёту) +создаёт ложное чувство полноты — именно поэтому дельта r1→r2 не отловила +её сама. + +**Чем закрывается в скоупе задачи.** Один из двух путей, оба технические +(не продуктовый вопрос владельцу): (а) переформулировать К8/AC8 как +инверсию — «подсказка показывается только при `targetState === 'active'`, +во всех остальных случаях, включая уже перечисленные и `self`, не +показывается» и добавить пятый смоук-шаг (лестница с +`target_space_id`, равным ID текущего пространства, — `_tip` остаётся +`null`); либо (б) явно решить, что `self` в контексте тултипа — не +предусмотренная владельцем ситуация и обосновать, почему её можно +не тестировать (например, если правка К8 формулируется как «показывать +только при active», для чего пятая ветка тривиально следует из первых +четырёх и не требует отдельного теста, а лишь явного упоминания в тексте +контракта). Любой вариант не меняет скоуп и не требует эскалации +владельцу — решение техническое, не продуктовое. + +## Что проверено и корректно + +- Обе Medium-находки r1 закрыты именно так, как рекомендовал r1: AC8 — + четырьмя отдельными флагами вместо одного; AC12 — новым смоук-шагом, + подтверждённым существующим кодом инструмента «Стены» + (`_markupClick`/`_canAppendWallPoint`). +- Low-находка r1 (tie-break диагонали) закрыта явным правилом «доминирует + x» со ссылкой на прецедент. +- AC12 корректно вставлен в «План автотестов» (`demo/smoke_stairs.mjs`: + «расширить … шагами … AC12»), а не оставлен только в таблице АС без + соответствующей строки плана. +- К2 согласован с AC12 текстуально: оба говорят «рамка не рендерится вовсе» + под другими инструментами, а не «рендерится, но игнорирует события» — + единственное число, один источник истины для этого поведения. +- Правки редакции 2 не меняют ничего вне заявленных трёх пунктов: секции + Сценарий, К1 (кроме tie-break), К2 (кроме одной фразы), К3–К7, модель + данных, i18n, риски, откат и release-артефакты текстуально идентичны + тому, что уже проверено в r1 (сверено построчным чтением текущего тела + issue против содержания, процитированного в SPEC-REVIEW-676-r1.md). + +## Чего не проверял + +- Не запускал `npm run typecheck`/`npm test`/`npm run build`, + `node scripts/check-docs.mjs`, смоки, `golden:verify`, инварианты модели — + как и в r1, кода задачи не существует (`git branch -a` не находит ветку + `issue/676-*`; `git diff f1a87fba..HEAD` содержит только публикацию + документа r1). Дифф для этих гейтов не существует, они относятся к этапу + код-ревью. +- Не проверял заново по коду весь фактический фундамент контракта + (константы, файлы, существующие функции, 4 golden-сцены, i18n-паритет, + math сектора курсора К2) — это сделано в r1 и не задето дельтой r2, кроме + точки пересечения с `stairTargetState`, которую я перепроверил заново + специально из-за характера правки AC8 (см. «Как проверялось», п. 5). +- Не проверял, действительно ли `self` практически достижим через + существующий UI (перенос/слияние помещений, J6) в разбивке по шагам — + для находки достаточно, что модель (`stairs.ts`) не отвергает такие + данные и канон (`STAIRS.md`) явно документирует состояние как + поддерживаемое; глубже, чем чтение этих двух источников, не уходил. +- Не проверял #663/#669 по существу — не в скоупе этой задачи, как и в r1. + +## Вердикт + +Вердикт: жёлтый · заход r2 · блокирующих циклов 2/4 · High: 0 · Medium: 1 → в задаче + +--- + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `9fe7f88cb3ec` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `85bef1cf4119abfcaf8c9ff7275010213306fb9a` + ``` + git log --all --format='%H %T' | grep 85bef1cf4119 + ``` +- Тело issue: `e28517c301f2de5676ccf6e30f0997db7f3a5a36d10f7a1d66070909bb1b1d9d` +- Вердикт конвейера: `yellow` · High 0