diff --git a/docs/reviews/CODE-REVIEW-684-r1.md b/docs/reviews/CODE-REVIEW-684-r1.md new file mode 100644 index 00000000..6f40fd7b --- /dev/null +++ b/docs/reviews/CODE-REVIEW-684-r1.md @@ -0,0 +1,171 @@ +# CODE-REVIEW-684-r1 + +Issue: #684 · этап: code · заход: r1 · блокирующих циклов израсходовано 0 из 4 +Материал: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD`, +ровно `e79f3cbfbbe0fd72a923fad6bb54ec391555a94b` (рабочая копия на нём). +Validate на этом SHA: success, run 36383798628. + +## Скоуп + +Issue #684 — находка вне скоупа из код-ревью #680 (r1, `docs/reviews/CODE-REVIEW-680-r1.md`): +`docs/UX-MODES.md` и `docs/TOUCH-SUPPORT.md` утверждали, что Esc и Reset трея +**не** завершают открытую цепочку Walls, тогда как код и `docs/WALL-THICKNESS.md` +§11 «Finishing a chain» (добавлен #680) говорят обратное. Задача — привести оба +документа к формулировке §11. + +Маршрут: инфраструктурный (PROCESS.md §1) — правка только класса C +(`docs/UX-MODES.md`, `docs/TOUCH-SUPPORT.md`), ни одного файла класса A, поэтому +вход сразу в `S7-code-review` корректен. ТЗ не требуется и не заводилось — +верно для этого маршрута. + +Единственный коммит `e79f3cbf` поверх `dev@03a3dd99`: + +``` + docs/TOUCH-SUPPORT.md | 9 ++++++--- + docs/UX-MODES.md | 17 ++++++++++------- + 2 files changed, 16 insertions(+), 10 deletions(-) +``` + +Продуктовый код (`src/**`) не тронут — изменение не влияет на поведение, +только на документацию. + +## Как проверялось + +Раз это чистая документная правка без AC-таблицы (задача не защитная, поведение +не менялось), я вместо таблицы «AC · чем доказан · чем краснеет» сверил каждое +предложение диффа напрямую с кодом, а не поверил формулировке автора. + +1. Прочитал `docs/WALL-THICKNESS.md` §11 «Finishing a chain» (строки 662–673) — + источник истины, на который оба документа теперь ссылаются. +2. Построчно сверил новые формулировки `UX-MODES.md` (≈178, ≈268) и + `TOUCH-SUPPORT.md` (≈154) с §11 — читал, не исполнял (docs — не код). +3. Проверил чтением каждую цитируемую в issue и коммите строку кода, что она + существует и делает заявленное: + - `src/houseplan-card.ts:2941-2958` — обработчик `Escape`: при + `_tool === 'draw' && _path.length` вызывает `this._finishWallChain()` — + подтверждено (строки 2953–2958 в HEAD). + - `src/houseplan-editor-runtime.ts:4928-4932` — кнопка `btn.reset` при + активном `draw`-пути вызывает `_finishWallChain()` — подтверждено. + - `src/houseplan-editor-runtime.ts:1203-1213` — `_finishWallChain()` вызывает + `_finalizeWallChainPartitions()`, которая (строка 1169) вызывает + `finalizeWallChainSpace()` из `src/writer-fixed-point.ts:64` — тот же + финализатор, что описан в §11 — подтверждено. + - `src/houseplan-editor-runtime.ts:1217` — `_activateMarkupTool`: ранний + `return` при `tool === host._tool` — подтверждает, что повторный выбор + Walls **не** finish-действие, как и утверждает новый текст. + - `src/houseplan-card.ts:2363` (`_onHashChange`) и + `src/houseplan-editor-runtime.ts:7371` (`_keepClosedAsPartitions`) — вызовы + `_finishWallChain()` / `_finalizeWallChainPartitions()` на смене хэша и на + отказе от всех граней — подтверждено, оба упомянуты в новом тексте + `TOUCH-SUPPORT.md`. +4. Прогнал `grep` по обоим файлам и по `docs/*.md` на предмет оставшихся + противоречащих формулировок («not finish actions», «does not finish») — + единственное совпадение осталось корректным (новый список non-finish + действий в `UX-MODES.md:183`), других противоречий не нашёл. +5. Проверил, что третье место в `UX-MODES.md` (~268, «independent physical + objects»), которое issue не называл явно, тоже обновлено (добавлены Esc и + Reset) — не было ошибкой (не содержало отрицания), но правка делает три + места консистентными; не регрессия. +6. Сверился с `docs/reviews/INDEX.md:14` — строка задачи #680 подтверждает, что + находка действительно была заведена как отдельный issue, а не «оставлена в + тексте ревью» (§12). +7. Гейты (см. ниже) и трейлеры коммита. + +## Гейты + +| Гейт | Прогнан | Результат | +|---|---|---| +| Validate CI на материале | нет (уже зелёный на этом SHA) | success, run 36383798628 — покрывает `tsc --noEmit`, `npm test`, `npm run build` + сверку бандла | +| `node scripts/check-docs.mjs --screenshots=warn` | да, сам | `Documentation checks passed (7 files, 12 external links)`, exit 0; голый `check-docs.mjs` без флага падает на несвязанном с задачей протухшем отпечатке скриншотов (#479) — ожидаемо, не признак этой правки | +| `node scripts/process-gate.mjs --issues` | да, сам | `диапазон origin/dev..HEAD, коммитов 1`, `гейт пройден, предупреждений 0` | +| `npm test` / `npx tsc --noEmit` / `npm run build` | нет отдельно | не требуются: диф не трогает `src/**`, зелёный Validate уже подтвердил их на этом SHA | +| смоки (`smoke-select.mjs`) | нет | диф не содержит исполняемого frontend-кода — автор зафиксировал это в хендоффе, я согласен: `git diff --stat` показывает только два `.md` | +| `golden:verify` | нет | нет изменения рендера | +| `pytest tests_backend` | нет | нет изменений в `custom_components/**/*.py` | +| инварианты модели | нет | нет изменений геометрии/ссылок на неё | +| mutation / «чем краснеет» | не применялось | задача не защитная: правка не меняет поведение, а приводит документацию в соответствие уже существующему и ранее подтверждённому (#680) коду; тестировать «падение» нечего — это не AC вида валидации/лимита/отказа | + +Трейлеры коммита `e79f3cbf`: `Issue: #684` есть; `User-Visible: no` — верно, +правка не меняет ничего наблюдаемого пользователем House Plan (только +внутренняя документация для читающих код/докс агентов и контрибьюторов); +CHANGELOG-правка поэтому не требуется и отсутствует, что корректно. Класс +файлов — только C, `Release:`/`Baseline-Reviewed` не нужны (baselines не +тронуты). + +Одно число, видимое дважды: изменение не вводит и не дублирует числовых +величин, видимых пользователю — не применимо. + +## Что проверено и корректно + +- Обе новые формулировки (`UX-MODES.md` дважды, `TOUCH-SUPPORT.md` один раз) + дословно согласуются с `WALL-THICKNESS.md` §11 и с реальным поведением кода: + Esc, Reset трея, смена tool/editor/floor, уход по маршруту/хэшу и отказ от + всех граней — все ведут к `_finishWallChain()` → `finalizeWallChainSpace()`. +- Список **не**-finish действий (re-selecting Walls без смены инструмента, + pan, pinch, второй указатель, `pointercancel`, подавленный синтетический + клик) сохранён и подтверждён кодом (`_activateMarkupTool` ранний return). + Заявление автора об отсутствии тестов, проверяющих эти формулировки, тоже + подтверждено: `grep` по `test/`, `scripts/`, `demo/` на характерные фразы + документов пуст. +- Находка #680(r1) закрыта содержательно, а не косметически: старое + противоречие (документ говорит «не завершает», код и §11 — «завершает») + устранено в обе стороны диффа, без новых противоречий. +- Инфраструктурный маршрут применён корректно (только класс C), трейлеры на + месте, коммит один, гейты соразмерны изменению. +- Строка `docs/reviews/INDEX.md` для #680 подтверждает происхождение задачи и + что находка не была «закрыта» просто оставлением в тексте прошлого ревью. + +## Находки + +Нет находок уровня High или Medium. Расхождение, ради которого заведена +задача, устранено полностью и без побочных регрессий. + +Низкоуровневое наблюдение (Low, снимаю без правки — не искажает модель, не +блокирует): второе место в `UX-MODES.md` (~268, «independent physical +objects») после правки перечисляет «changing tool/editor/floor, `Esc` or the +tray's Reset», но не упоминает route/hash-уход и отказ от всех граней — в +отличие от первого места (~178) и `TOUCH-SUPPORT.md`, которые ссылаются на +полный список §11. Формулировка не заявляет исчерпывающести («only») и потому +не противоречит коду, но небольшая неполнота остаётся. Не блокирую: это не +регрессия относительно состояния до коммита (там этого места вообще не +касались формулировки Esc/Reset), а улучшение сверх минимально требуемого +issue объёма. + +## Чего не проверял + +- Не перегонял `tsc --noEmit`, `npm test`, `npm run build` отдельно — доверился + зелёному Validate на этом SHA (правило §8 разрешает это для дешёвых гейтов, + когда прогон уже есть). +- Не гонял браузерные смоки и `golden:verify` — диф не содержит исполняемого + кода, `smoke-select.mjs` не даёт значимых кандидатов при пустом + frontend-диффе (сам инструмент не запускал, так как выборка тривиально пуста + — единственные изменённые файлы это `.md`, вне зоны любого смоука). +- Не проверял английскую версию `docs/USER-GUIDE.md`/`.ru.md` на предмет + аналогичного расхождения про Esc/Reset — issue не называл эти файлы, и это + за пределами заявленного скоупа задачи; если расхождение там есть, это + повод для отдельного issue, а не блокер этого ревью. +- Не проверял всю остальную часть `WALL-THICKNESS.md`, `UX-MODES.md`, + `TOUCH-SUPPORT.md` построчно на прочие несвязанные несоответствия — только + участки, которые задевает дифф, и цитируемые в issue строки кода, по правилу + «объём разбора по дельте» (это первый раунд, но задача сама узкая и + точечная). + +## Вердикт + +Зелёный. AC (формулировка issue: формулировки обоих документов приведены к +§11 WALL-THICKNESS.md) выполнены и подтверждены чтением кода и §11; High/Medium +нет; маршрут, трейлеры и гейты соразмерны докс-only инфраструктурной задаче. + +--- + + + +## Материал раунда + +- Ветка: `issue/684-wall-chain-finish-docs`, коммит `e79f3cbfbbe0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `35954ac38b7d847d005425fb2ea97409a1279a94` + ``` + git log --all --format='%H %T' | grep 35954ac38b7d + ``` +- Тело issue: `a7b76e3a4e928c4c8d8bbe055a1425e98afcd72bcf4709ebf6c34372aac95594` +- Вердикт конвейера: `green` · High 0