diff --git a/docs/reviews/SPEC-REVIEW-608-r1.md b/docs/reviews/SPEC-REVIEW-608-r1.md new file mode 100644 index 00000000..2e2eb151 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-608-r1.md @@ -0,0 +1,196 @@ +# SPEC-REVIEW-608-r1 + +- **Issue:** #608 «Числовые поля со слайдером (rangeLine) клампят каждый символ» +- **Этап:** spec (PROCESS.md §2.4) +- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (полный трек, лимит 4) +- **Материал:** тело issue #608, раздел `## ТЗ` (17 подразделов), плюс два + комментария-аналитики/вопросов от 2026-09-22. Дополнительных комментариев с + ответами владельца в трассе нет — ответы на Q1–Q3 появляются как раздел 17 + ТЗ («Принятые решения владельца»), дословно совпадая с предложенными + дефолтами; см. «Находки» ниже. + +## Скоуп + +Баг в общем helper `rangeLine` (`src/editors/form-kit.ts`): числовое поле рядом +со слайдером клампит и коммитит значение на каждом символе `input`, а +`.value` привязано без `live()`. Правка переносит коммит на `change`/blur/ +движение слайдера, добавляет привязку к шагу и floating-point-нормализацию. +Правка охватывает 3 диалога (room-settings, space-form, marker-dialog) и все +вызовы `rangeLine`. + +## Как проверялось + +Ревью читает диспозицию по AGENTS.md/PROCESS.md, `docs/SCOPE.md`, тело issue +#608 и код, на который ссылается ТЗ — без исполнения (задача ещё не в +разработке, это ревью ТЗ, а не кода): + +1. `docs/SCOPE.md` — задача чинит надёжность существующих редакторских + диалогов (J6 «Keep the plan true», настройки комнаты/пространства/ + устройства), новой продуктовой территории не открывает; лока/алармов не + касается (лишний контроль актуальности после чтения раздела «lock + invariant» — не относится к задаче). +2. `AGENTS.md`/`PROCESS.md` — трек, трейлеры, лимиты циклов, §7.1 обязательные + разделы, требование «догадка помечена как предположение». +3. Тело issue #608 целиком, включая аналитику (оценка 8/8/4, P1, full track — + обоснование «одна поверхность» не выполняется, поскольку контракт общий + для трёх диалогов, корректно) и блок вопросов Q1–Q3 с дефолтами. +4. Код: прочитаны `src/editors/form-kit.ts` (весь `rangeLine`/`unitInput`), + все 7 фактических вызовов `rangeLine(` (`grep -n "rangeLine(" src/editors/*.ts` + → `form-kit.ts:442` определение + `marker-dialog.ts:476,642,769,780`, + `room-settings-dialog.ts:270`, `space-form.ts:427`), соседние паттерны + «сырая строка до коммита» (`cellCmInput` в `space-form.ts:210/219`, + `glowRadius` в `general-settings-dialog.ts:129-133`/`marker-dialog.ts:557-563`, + `_roomTempMin` в `room-settings-dialog.ts:228-231`). +5. Существующие смок-тесты, которые ТЗ называет как «не видящие» дефект: + `demo/smoke_room_settings_form.mjs:126-133` и + `demo/smoke_dialog_config_parity.mjs:47` — и общий паттерн хелпера + `input(el, value)` (`el.value = value; el.dispatchEvent(new Event('input', + {bubbles:true}))`), используемый в шести+ smoke-файлах. + +## Находки + +Блокирующих (High) находок нет. Ниже — два Low-замечания, оба сняты +(waived) с записью, вердикт не меняют. + +### L1. «Шесть потребителей» на самом деле семь вызовов `rangeLine` + +**Файл:** тело issue #608, разделы «Причина», «Аналитика» (комментарий), +§4 «Scope», §10 «Затронутые файлы», AC4. + +Текст многократно утверждает «все шесть потребителей» и перечисляет: имя +комнаты, подписи комнаты, шрифт карточек пространства, яркость Glow, размер +пульсации, «размер и угол устройства» (последний — одним пунктом). Факт: +`grep -n "rangeLine(" src/editors/*.ts` даёт **семь** реальных вызовов — +`marker-dialog.ts:769` (размер) и `:780` (угол) это два разных вызова с +разным `min/max/step`, сведённые в тексте в один пункт списка. Ни один сайт +не пропущен — угол назван прямо в перечислении, просто не выделен отдельной +строкой в подсчёте. + +**Почему не блокирует:** AC4 доказывается «ревью кода», а не подсчётом по +тексту; код-ревьюер увидит все 7 вызовов через тот же `grep`, независимо от +того, как задача их сгруппировала в прозе. Функционально правка идёт через +общий helper (`rangeLine`/`form-kit.ts`), поэтому она автоматически покрывает +все вызовы вне зависимости от того, что их «шесть» или «семь» — расхождение +чисто текстовое, не архитектурное и не блокирует ни один AC. + +**Решение:** снято (waived). Автору стоит поправить формулировку на «семь +вызовов, шесть смысловых полей» при следующей правке текста, но отдельного +цикла это не требует. + +### L2. Ответ владельца на Q1–Q3 не оставлен отдельным комментарием + +**Файл:** таймлайн issue #608 — `blocked` снят в 04:16:13, `S4-spec-review` +поставлена в 04:17:06, без комментария между этими метками; раздел 17 ТЗ +(«Принятые решения владельца») просто дословно повторяет дефолты из +комментария с вопросами. + +**Почему не блокирует:** оба действия — вопрос и итоговое ТЗ — от одного и +того же аккаунта (`Matysh`, OWNER), а раздел 17 прямо оформлен как решение, +принятое явным блоком в конце ТЗ, как и требует §7.1. Три вопроса — все +продуктовые, дефолты разумны и не противоречат `docs/SCOPE.md`; ни один не +похож на догадку, выданную за факт. Проверяемый след («кто и когда ответил +отдельным комментарием») тоньше обычного, но требования процесса это не +нарушает. + +**Решение:** снято (waived) — фиксирую как наблюдение на будущее: там, где +вопрос-ответ идут без отдельного комментария-ответа, стоит явно пометить +в ТЗ, что ответ получен вне комментария (например, «решено при редактировании +issue», как уже сделано разделом 17), что здесь и произошло. + +## Что проверено и корректно + +- **Причина дефекта грамотна и воспроизводима по коду.** `form-kit.ts:442-453` + — `rangeLine.onInput` действительно делает `Number(raw)` + + `Math.min(max, Math.max(min, n))` на каждом символе; `unitInput` (`:417`) + биндит `.value=${value}` без `live()`. Прогон вручную по описанному + сценарию («1»→50, «2»→«502» clamp 300, backspace→0→min) воспроизводится по + логике кода один в один — не голословное утверждение. +- **Ссылки на «уже сделанные правильно» паттерны точны**: `cellCmInput` + (`space-form.ts:210,219`, `space-form-state.ts:20`), `glowRadius` как + строка-черновик (`general-settings-dialog.ts:129-133`, + `marker-dialog.ts:557-563`), `_roomTempMin` (`room-settings-dialog.ts:228-231`) + — все три действительно хранят сырую строку до коммита без клампа на + каждый `input`, ТЗ ссылается на реальный, не придуманный прецедент. +- **Утверждение «существующие смоки задают значение целиком и дефект не + видят» подтверждено** — `smoke_room_settings_form.mjs:126` и + `smoke_dialog_config_parity.mjs:47` дёргают `input(el, '150')` одним вызовом + через общий хелпер `(el,value)=>{el.value=value;el.dispatchEvent(new + Event('input'))}`, посимвольного набора там нет. +- **Контракт поведения (§6, пп. 1–7) однозначен и без догадок**: коммит по + `change`/blur, кламп → привязка к шагу с округлением половины вверх → + нормализация до точности `step`; пустой/невалидный ввод откатывается без + побочных эффектов; слайдер побеждает во время незавершённого ввода — + все три пункта дословно совпадают с ответами владельца на Q1–Q3, ни один + не оставлен «додуманным». +- **Примеры в п.7 арифметически верны** для реальных диапазонов кода: + 100→120 (шаг 5, on-step, без изменений), 123→125 (расстояние 3 против 2), + 999→300 (клампится к max, max уже on-step для всех 7 вызовов: проверено + вручную для 50–300/5, 0–355/5, 1–100/1, 1–8/0.5, 0.5–3/0.1). +- **AC1–AC6 проверяемы и указывают способ доказательства** (browser smoke ×4, + mutation, typecheck/unit/build); ни один не сформулирован как «работает + корректно» без критерия. +- **Не-скоуп (§5) корректно исключает** только сами диапазоны/шаги/юниты и + «остальные `unitInput`, не входящие в `rangeLine`» — это точное разделение: + прочие `unitInput` (glow radius, room-temp-min, cellCm) уже не имеют + клампа-на-каждый-символ, поэтому исключение не прячет соседний баг. +- **§7.1 обязательные разделы все на месте**: сценарий/персона/поверхность, + что человек увидит до/после, проблема, скоуп и не-скоуп, контракт + поведения, UX/a11y, данные и миграция (явно «не нужна»), i18n (явно «нет + новых строк»), AC1-6 с доказательством, план автотестов, риски, откат, + release-артефакты. +- **i18n, миграция, touch, производительность, golden** обоснованно закрыты + словом «нет»/«не затронуто» с аргументом, а не молча пропущены (§12 п.5 + прямо перечисляет неприменимые классы риска и почему). +- Риск «повторный render может затереть сырую строку» (§14) назван и + привязан к конкретной проверке (smoke в обе стороны) — не спрятан за + общей фразой. + +## Технический риск, замеченный, но не относящийся к спеку + +Существующий хелпер `input(el, value)` в `smoke_dialog_config_parity.mjs` +дёргает только DOM-событие `input`, никогда `change`. После правки коммит +пойдёт по `change`/blur — то есть без доработки самого файла существующий +regression-смок перестанет что-либо коммитить и, вероятно, покраснеет. +Это **не находка к ТЗ**: §10 «Затронутые файлы» уже прямо называет +`demo/smoke_dialog_config_parity.mjs` как файл, который может понадобиться +расширить («если требуется расширение»), то есть автор ТЗ этот риск уже +предусмотрел и явно разрешил редактировать файл в рамках той же задачи +(class B, тот же issue). Технические детали реализации (как именно +досылать `change`) закону ТЗ не подлежат — решает автор кода, проверяет +код-ревью. Оставляю как ориентир для code review на случай, если правка +придёт без учёта этого файла. + +## Чего не проверял и почему + +- **Исполнение кода/тестов** — не применимо к этапу spec: кода ещё нет, + задача в `S4-spec-review`, а не в разработке. Гейты (typecheck/test/build) + не запускались намеренно. +- **golden/screenshots** — не нужны по самой ТЗ (геометрия и визуал не + меняются); не проверял, согласен с обоснованием автора. +- **backend/pytest** — не затронут (`custom_components/**` не в скоупе). +- **Инварианты модели/геометрии** (`npm run invariants`) — не применимо, + диф не касается rooms/edges/thickness/layout. +- Полный список из 261 `demo/smoke_*.mjs` не прогонял и не обязан: этап — + ревью ТЗ, а не кода; выбор смоков для code review будет отдельным шагом на + этапе `S7`. + +## Вывод + +ТЗ полное, однозначное, без догадок, выданных за факт; каждый AC +проверяем и называет способ доказательства; контракт поведения решает +именно тот сценарий, что описан в симптоме. Два Low-замечания сняты +ревьюером с записью, High/Medium — ноль. Готово к разработке. + +--- + + + +## Материал раунда + +- Ветка: `issue/608-range-line-draft-input`, коммит `0b29ccb91d9c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `66badebb05b0276aa596740573dc021bce96224c` + ``` + git log --all --format='%H %T' | grep 66badebb05b0 + ``` +- Тело issue: `518c09d725e03b4ca93b0be8a8b6d61805af4e27b25679ddc71a5237c333055e` +- Вердикт конвейера: `green` · High 0