Files
2026-09-28 06:00:47 +00:00

14 KiB
Raw Permalink Blame History

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