diff --git a/docs/reviews/CODE-REVIEW-565-r2.md b/docs/reviews/CODE-REVIEW-565-r2.md new file mode 100644 index 00000000..63b4db09 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-565-r2.md @@ -0,0 +1,273 @@ +# CODE-REVIEW — issue #565, заход r2 + +**Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0** + +## Скоуп + +Материал ревью: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD` +на SHA `dded4fbd5c6d51e4127b4ddfd5d9a1c547f21de2` (рабочая копия на нём же). + +Ветка приведена к `dev` конвейером до ревью: `origin/dev` продвинулся с `7d08a28d` +(база r1) до `84a7773e` (два новых коммита — `54265023` мутанты #569 и `84a7773e` +публикация их ревью, оба трогают только `scripts/mutation-registry.mjs` и не +задевают ни один файл #565). Задача автора `82ce5aae` перебазирована конвейером +в `dded4fbd`. Это другой код по определению §7.2, поэтому разбор — полный, не +по дельте: ниже проверены не только новые строки, но и то, что рёбейз не +исказил уже принятый материал r1 (сверка `git diff origin/dev..HEAD -- +scripts/mutation-registry.mjs` — все шесть записей #565 присутствуют одним +чистым блоком, ни одного фрагмента #569 не задето и не продублировано). + +Диапазон — четыре коммита: `6103e7de`/`c23c7927` (публикация SPEC/CODE-REVIEW-565-r1 +конвейером, класс C), `6aca447f` (продуктовый коммит r1, уже разобран +`docs/reviews/CODE-REVIEW-565-r1.md`), `dded4fbd` — **предмет этого раунда**, +единственный продуктовый коммит после r1 (`fix: сохранить клавиатурную +подсказку (#565)`, трейлеры `Issue: #565` / `User-Visible: yes` на месте). + +Единственная находка r1 — **M1 (Medium, в скоупе)**: клавиатурная focus-tooltip +устройства тихо стиралась наведением мыши на любую комнату, а симметричная +защита у другого устройства была избыточно широкой (блокировала ЛЮБОЙ hover, а +не только сфокусированного устройства). Коммит `dded4fbd` — целевой фикс этой +находки. Файлы: `src/houseplan-card.ts`, `src/live-hover.ts`, +`src/live-interaction-runtime.ts`, `demo/smoke_household_journeys.mjs`, +`scripts/mutation-registry.mjs`, `docs/CHANGELOG{.ru,}.md`, `docs/TESTING.md`, +`docs/UX-MODES.md` — соответствует хендоффу. Ни `device-presentation.ts`, ни +`space-render.ts`, ни i18n, ни тесты `test/*.test.mjs` не тронуты — AC1–AC4, +AC8, AC9 дельта не задевает. + +## Разбор дельты (M1 → fix) + +`src/live-hover.ts` вводит `focusTips: WeakMap` — состояние +клавиатурной подсказки хранится отдельно от `host._tip` (то, что реально +отрисовано): + +- `showDevicePointerTip` — убрана защита `if (host._tip?.source === 'focus') + return;`. Это и была избыточно широкая половина M1: она блокировала + legitimate hover ЛЮБОГО устройства мышью, пока где-то держался клавиатурный + фокус. Теперь наведение мышью на другое устройство работает как обычно. +- `showDeviceFocusTip` — пишет tip и в `focusTips`, и в `host._tip`. +- `hideDeviceFocusTip` (blur) — чистит запись в `focusTips` по `deviceId`, + сбрасывает `host._tip` только если текущий показ и есть focus-tooltip. +- `clearPointerHover` (`pointerleave` с устройства/комнаты) — `host._tip = + focusTips.get(value) || null`: возвращает сфокусированную подсказку, если + она ещё жива, иначе `null`. +- `reconcileDeviceFocusTip` (на каждый `commit()`) — переведена на проверку + `focusTips`, а не `host._tip`, поэтому больше не путает временный + pointer-tip с владением фокуса. +- Новые экспорты `deviceFocusTipActive`/`clearDeviceFocusTip`. + +`src/houseplan-card.ts`: + +- `_showTip(..., room)` — новая охрана `if (room && this._liveRt?. + deviceFocusActive()) return;` (houseplan-card.ts:7434) — это вторая, + недостающая половина M1: комнатный `pointermove` (`tip()`, + houseplan-card.ts:11677) больше не перезаписывает активную клавиатурную + подсказку. Прочитан весь путь: `enterRoom` (`pointerenter`) по-прежнему + ставит `_hoverRoom` для визуальной подсветки комнаты независимо от фокуса — + это не тот контракт, который защищает M1 (подсветка, не подпись), и в ТЗ/ + находке не упоминается. +- `_clearTransientHover` — добавлен `this._liveRt?.clearDeviceFocus();` перед + остальной очисткой. Вызывается на границах жизненного цикла: смена + режима/пространства (`_setMode`, строка 7601, `this._clearTransientHover + (true)`), смена модальности на touch/pen (`_notePointer`), множественный + touch-контакт, `pointerup`/`pointercancel` на touch. Без этой правки + `focusTips` пережил бы границу и `clearPointerHover` мог бы «воскресить» + устаревшую подсказку после смены пространства — прочитано и подтверждено, + что путь перекрыт для всех вызывающих `_clearTransientHover`. +- `_clearPointerHover` — тернарник вместо `?? `; функционально то же самое, но + устраняет реальную ловушку: `pointerLeave(): void` возвращает `undefined`, + и старая форма `this._liveRt?.pointerLeave() ?? this._clearTransientHover()` + **всегда** дополнительно вызывала `_clearTransientHover()` (правый операнд + `??` срабатывает на `undefined`), т.е. любой `pointerleave` с живым + `_liveRt` стирал вообще весь transient-hover, включая только что + восстановленную focus-подсказку. Мутант `device-pointer-leave-clears-focus- + fallback` воспроизводит именно эту старую форму — назван ниже. + +`src/live-interaction-runtime.ts` — тонкие делегаты `deviceFocusActive()` / +`clearDeviceFocus()` на `LiveRuntime`, используются в двух местах выше плюс +`dispose()`. + +**Симметрия достигнута**: единственный инвариант «пока `focusTips` содержит +запись для хоста, посторонний `pointermove` не должен ни блокировать чужой +hover (было — избыточно), ни затирать текущую подсказку без предусмотренного +события (было — дыра M1)» реализован в одном источнике (`focusTips`), а не +двумя независимыми точечными проверками, как раньше. + +## Что проверено исполнением (не только чтением) + +Локально пересобран бандл (`npm run build && npm run bundle:sync`) — три копии +байт-в-байт совпали с материалом ревью (`git status --short` пуст после +сборки), typecheck прошёл без ошибок. + +| Гейт | Команда | Результат | +|---|---|---| +| typecheck + build | `npm run build` | зелёный, `tsc --noEmit` без ошибок, `dist` пересобран идентично коммиту | +| bundle sync | `npm run bundle:sync` | зелёный, `git status` чист — все 3 копии синхронны | +| check-docs | `node scripts/check-docs.mjs --screenshots=warn` | зелёный: `Documentation checks passed (7 files, 12 external links)`; единственное `WARN` — уже известный устаревший screenshot-фингерпринт (#479, ожидаемо на плейн-пуше, не блокирует; тот же вывод был у автора и в r1) | +| **`node demo/smoke_household_journeys.mjs`** | прямое совпадение по diff (`_clearTransientHover, _showTip, _tip`), несёт сами новые проверки `j4.room_hover_does_not_replace_keyboard_tooltip`, `j4.other_device_keeps_ordinary_pointer_hover`, `j4.pointer_leave_restores_keyboard_tooltip` | **зелёный, `OK`** (0 записей в `_failures`) | +| **`node demo/smoke_touch_tips.mjs`** | прямое совпадение (`_hoverRoom, _notePointer, _showTip, _tip`); `_clearTransientHover` — тронутая функция, защищает AC7 | **зелёный, `OK`** (`touchClearsAllTransientHover`, `noTipOnTouch` и др. — все `true`) | +| **`node demo/smoke_room_tooltip_toggle.mjs`** | прямое совпадение (`_hoverRoom, _pointerModality, _tip`); ровно тот путь комнатного `tip()`, который получил новую охрану | **зелёный, `OK`** (`roomTipRestoredOnMove`, `disabledRoomTip`, `deviceTipSurvives` и др. — все `true`) | + +`node scripts/smoke-select.mjs --base origin/dev --head HEAD` (полный вывод +приложен решением по каждой строке ниже): 33 прямых совпадения (в основном по +широким символам `_mode`/`_tip`/`_drag`, которые встречаются в большинстве +смоков файла) + 1 «зарегистрированная связь» (`smoke_value_face_source.mjs` +← `resolveDevicePresentation`, не относится к этой дельте — `device- +presentation.ts` не менялся в `dded4fbd`) + одна широкая помеченная явно +(`_space`, не учитывалась инструментом). Прогнаны три смока, которые прямо +покрывают изменённые функции и являются целью новых мутантов/носителем новых +проверок (`household_journeys`, `touch_tips`, `room_tooltip_toggle`); остальные +30 совпадений — по обиходным символам, использующимся в файле почти +повсеместно (`_mode`/`_drag`/`_config` встречаются в самом файле независимо от +tooltip-логики), не запускались: ни один из них не входит в файлы, изменённые +`dded4fbd`, а прочитанный код изменения (см. «Разбор дельты» выше) не касается +drag/resize/config/mode-переключателей как таковых — только маршрутизацию +tooltip между pointer и focus источниками. Полный набор — предрелизный гейт, а +не гейт ревью (PROCESS.md §8). + +## Дешёвые гейты на этом SHA (подтверждены дважды) + +Validate на точном материале `dded4fbd` зелёный: +https://github.com/Matysh/houseplan-card/actions/runs/34816298250 — покрывает +`npm test` (я не перезапускал: воспроизвёл typecheck/build локально, они +совпали, дополнительных причин сомневаться в юнитах, которые delta не трогает, +нет), `no-new-any`, `process-gate`, mutation-gate (шардируется в review-candidate +прогоне, покрывает все 6 witness'ов #565, включая 2 новых, — см. ниже), +perf-smoke триггер (`live-hover.ts`/`live-interaction-runtime.ts`/ +`houseplan-card.ts` входят в `large-house-interaction-v1`, прогнан тем же +Validate). Не требуются и не прогонялись: `golden:verify` (стили/подписи не +тронуты, диффом подтверждено — как и в r1), `model-invariants` (геометрия не +тронута), `pytest tests_backend` (бэкенд не тронут). + +## Таблица «AC · чем доказан · чем краснеет» — обновление к AC6 + +| AC | Чем доказан | Чем краснеет | +|---|---|---| +| AC6 (комнатный триггер, закрывает M1) | `j4.room_hover_does_not_replace_keyboard_tooltip` в `demo/smoke_household_journeys.mjs`, прогнан лично, `OK` | мутант `device-focus-tooltip-room-hover-overwrites` (снимает охрану `if (room && this._liveRt?.deviceFocusActive()) return;`), гард — тот же smoke; часть Validate review-candidate прогона на этом SHA | +| AC6 (восстановление после hover другого устройства, закрывает M1) | `j4.other_device_keeps_ordinary_pointer_hover` + `j4.pointer_leave_restores_keyboard_tooltip`, прогнаны лично, `OK` | мутант `device-pointer-leave-clears-focus-fallback` (возвращает старую форму `?? `, которая при живом `_liveRt` **всегда** доп. вызывает `_clearTransientHover()`, стирая восстановленную подсказку), гард — тот же smoke; часть Validate review-candidate прогона | + +Обе новые записи `scripts/mutation-registry.mjs` матчатся `find` ровно на +текущий текст (`houseplan-card.ts:7434`, `houseplan-card.ts:5748`) — сверено +построчно, не только по имени функции. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| M1 (Medium, в скоупе): клавиатурная focus-tooltip тихо стирается наведением мыши на комнату; симметричная защита у другого устройства избыточно широка | 1) Охрана `if (room && this._liveRt?.deviceFocusActive()) return;` внутри `_showTip` не даёт комнатному `pointermove` затереть активную focus-подсказку. 2) Снята избыточная защита `if (host._tip?.source === 'focus') return;` в `showDevicePointerTip` — hover другого устройства мышью снова разрешён (устраняет вторую половину асимметрии). 3) `focusTips` WeakMap хранит владение независимо от текущего показа и восстанавливает его в `clearPointerHover` при уходе указателя. 4) Границы жизненного цикла (`_clearTransientHover` → `clearDeviceFocus()`) чистят состояние, чтобы восстановление не «воскрешало» устаревшую подсказку | `src/houseplan-card.ts:7434` (новая охрана), `src/live-hover.ts:64-118` (весь механизм `focusTips`), `src/houseplan-card.ts:7304` (`clearDeviceFocus()` в `_clearTransientHover`); поведенчески — три новых проверки `j4.room_hover_does_not_replace_keyboard_tooltip` / `j4.other_device_keeps_ordinary_pointer_hover` / `j4.pointer_leave_restores_keyboard_tooltip` в `demo/smoke_household_journeys.mjs`, лично прогнаны и зелёные; защищены мутантами `device-focus-tooltip-room-hover-overwrites` и `device-pointer-leave-clears-focus-fallback` | + +## Унаследовано из r1 + +Принято без повторной проверки кода (сам код этих участков не менялся в +`dded4fbd`, что подтверждено `git show dded4fbd --stat` — ни один из +перечисленных файлов/областей не входит в дифф этого коммита), опираясь на +`docs/reviews/CODE-REVIEW-565-r1.md` на материале `127cd07ba78fc75cd776ebd6518248a6bef91e82`: + +- **AC1/AC2** (именованная навигация пространств, единственный `aria-current`, + Tab/Enter/Space не тронуты) — `src/houseplan-card.ts`, `