diff --git a/docs/reviews/SPEC-REVIEW-459-r2.md b/docs/reviews/SPEC-REVIEW-459-r2.md new file mode 100644 index 00000000..35ce4d73 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-459-r2.md @@ -0,0 +1,82 @@ +# SPEC-REVIEW-459-r2 + +**Issue:** #459 «Настройка «Zigbee links»: подсказка о функции, её ограничениях и легенда цветов и пунктира» +**Этап:** spec (лёгкий трек, `small`), PROCESS.md §2.4 / §5 / §2.10 +**Заход:** r2 · блокирующих циклов израсходовано 1 из 2 (лимит лёгкого трека) — это последний доступный цикл на этом этапе +**Материал:** ТЗ в теле issue #459, текущая редакция (проверено на момент ревью). Предыдущий материал — редакция, зафиксированная в `docs/reviews/SPEC-REVIEW-459-r1.md`, проверенная на `dev` `6af6d7b3`. Репозиторий сейчас на `dev` `c81d1d97` — единственный коммит между ними (`c81d1d97 docs: review document for #459`) не трогает ни один файл, упомянутый в ТЗ (`git log --oneline 6af6d7b3..HEAD -- src/hp-zigbee-topology-settings.ts src/i18n/topology src/logic.ts src/hp-help.ts src/houseplan-editor-runtime.ts test/i18n.test.mjs test/i18n-dead-keys.test.mjs test/core-file-budget.test.mjs demo/smoke_zigbee_topology_hover.mjs` — пусто). Зависимость issue #457 продвинулась `S6-in-progress` → `S7-code-review`, но её спека `docs/specs/457-zigbee-route-arrows.md` (коммит `f3a1dd84`) не менялась ни разу с момента r1 — ссылки #459 на неё остаются валидными без повторной проверки всего документа. + +## Скоуп ревью (по дельте, §2.10) + +Дельта раунда — правка по единственной находке r1 (Medium: AC3 не покрывал содержательную легенду) плюс добавление §4.1 с утверждённым владельцем финальным текстом подсказки. Код этой задачей по-прежнему не тронут (этап ТЗ), рёбейза `dev` не было, контракт поведения не менялся, новая подсистема не задета — дельта локальна, разбор ограничен ею и всем, до чего она дотягивается (сама подсказка как контент, AC3/AC3b, `help_aria`), остальное наследуется из r1. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **Medium** — AC3 требовал только «три состояния линии + обе границы LQI», ни один AC не проверял исходящую/входящие стрелки, линию без стрелки, подпись на конце стрелки, отсутствие стрелки и оговорку о приближённости — черновик текста мог формально пройти все AC1–AC8, не решив задачу | AC3 переписан на семь явных пунктов (см. ниже — в тексте ТЗ они названы «шесть», но перечислены все семь исходных требований контракта §4 п.2 без потерь); добавлен отдельный AC3b на оговорку о приближённости («дерево маршрутов», «не путь пакета»). Владелец утвердил нормативный русский текст (§4.1, 654 знака), и построчная сверка (ниже) показывает, что каждый из семи пунктов и оговорка присутствуют в этом тексте буквально | Тело issue, раздел «Критерии приёмки», строки AC3/AC3b; раздел §4.1 «Утверждённый текст» | +| **Low** — примитив проверки наличия ключа для `topology`-словарей (нужен, поскольку `topologyT()` не умеет отличать «нет перевода» от «есть», в отличие от `_help()`/`hasTranslation`) не назван в «Затронутых файлах» | Не правилось — снято ревьюером r1 без правки как чисто техническая деталь (§7.1: решает реализатор). Автор подтвердил тем же решением, правок не вносил | Комментарий автора «Замечание r1 исправлено…»: «Low принят к сведению без правок, как и снято ревью» | + +## Унаследовано из r1 + +Без повторной проверки в этом раунде, на основании `docs/reviews/SPEC-REVIEW-459-r1.md` (материал: `dev` `6af6d7b3`, дельта после него кода не касалась): + +- лёгкий трек подтверждён (все пять критериев §5 не нарушены); +- точность технических цитат по `hp-zigbee-topology-settings.ts:159-171`, `hp-zigbee-topology-overlay.ts:185-189`, `lqiColor` в `src/logic.ts:8-11`, реальный fail-closed `_help()` в `houseplan-editor-runtime.ts:1228-1233`, структура и паритет четырёх словарей `topology/*.json` (по 25 ключей, плоские имена через `_`), отсутствие покрытия namespace `topology` в `test/i18n.test.mjs` и `test/i18n-dead-keys.test.mjs`, нерелевантность `core-file-budget.test.mjs` к правке (AC7), CSS-вместимость `hp-help` (`max-width`, `max-height`, `overflow: auto`), безопасность `demo/smoke_zigbee_topology_hover.mjs` относительно новой разметки (AC8); +- сверка технических утверждений ТЗ #459 о поведении #457 (ключи `route_device_not_on_plan`, `route_coordinator_not_on_plan`, `route_other_space`, правила исходящей/входящей стрелки и bubble §7.1–7.4, приближённость дерева §6) с принятым текстом `docs/specs/457-zigbee-route-arrows.md` — документ не менялся, вывод r1 остаётся в силе; +- обоснованность AC6 (гейт паритета `topology`, тот же приём, что для `support`) и AC7 (бюджет/core-file не задет косвенно); +- корректность отката (без сохранённого состояния и конфига, обратим одним коммитом); +- Low-находка выше (примитив проверки ключа) остаётся снятой без правки. + +## Как проверялось в этом раунде + +- Построчное сравнение AC3 (r1) с текущим AC3/AC3b, чтобы убедиться, что перечень требований контракта §4 п.2–3 (7 фактов: цвет-по-шкале-LQI, пунктир, исходящая стрелка, входящие стрелки, линия без стрелки, подпись на конце стрелки, отсутствие стрелки; плюс оговорка о приближённости) отражён в критериях приёмки один в один. +- Построчная сверка утверждённого текста §4.1 (нормативная русская строка) против каждого из семи пунктов и оговорки — `python3` подсчёт длины строки (`len(s) == 654`, `'\n' in s == False`) подтвердил фактическую точность заявленных в ТЗ чисел «654 знака, без переносов строк». +- Сверка формулировки владельца о двух сознательно опущенных фактах («у координатора исходящей стрелки нет», «соседи из других пространств не рисуются») с текстом #457 и существующим кодом: `grep` подтвердил, что бейдж «+{n} в других пространствах» и ключ, аналогичный `route_other_space`, уже существуют в `src/i18n/topology/ru.json` и в принятой спеке #457 (`route_other_space` — safe fallback для title пространства, §11). +- Проверка регресса дельты на #457: убедился, что `docs/specs/457-zigbee-route-arrows.md` не менялся с момента r1 (`git log --oneline -- docs/specs/457-zigbee-route-arrows.md` → один коммит, `f3a1dd84`, старше материала r1), значит цитаты ТЗ #459 из #457, проверенные в r1, не устарели. +- Сверка предложенного `help_aria` со **всеми** существующими значениями `*.help.aria` в `src/i18n/ru.json` (18 ключей: `space.cell_cm.help.aria`, `space.zero_wall_style.help.aria`, `gs.glow_radius.help.aria`, `marker.controls.help.aria` и другие) — см. находку Low ниже. +- Гейты не гонялись — код этой задачей по-прежнему не менялся (этап ТЗ), гонять нечего; согласуется с r1. + +## Находки + +### Low — `help_aria` не соответствует заявленному образцу + +ТЗ (§4.1) утверждает: «`help_aria` — «Справка: связи Zigbee», **по образцу существующих подписей**». Это фактическое утверждение неверно: все 18 существующих значений `*.help.aria` в `src/i18n/ru.json` без исключения используют префикс **«Подсказка: …»** (`"space.cell_cm.help.aria": "Подсказка: масштаб пространства"`, `"marker.controls.help.aria": "Подсказка: управление другими источниками света"` и т. д.), а не «Справка: …». Предложенный текст вводит новый, нигде не встречающийся префикс для того же типа элемента (`hp-help`, скринридер-подпись кружка «?»). + +Это ровно тот случай, когда фактическая ссылка на прецедент не подтверждается кодом — но эффект чисто терминологический (sr-only текст кружка справки), не влияет на проверяемость AC1/AC2 и не меняет видимое (глазами) поведение. Снимаю без блокировки цикла, с рекомендацией: либо привести `help_aria` к «Подсказка: связи Zigbee» (тогда формулировка «по образцу» станет верной), либо, если «Справка» — намеренный отход (например, потому что речь не о подсказке-туториале, а о развёрнутой справке/легенде), явно снять слова «по образцу существующих подписей» и объяснить разницу одной фразой. Решение — техническое (именование), автор/реализатор вправе выбрать любой вариант; фиксирую как Low, а не Medium, потому что это единственное словo без последствий для доказуемости AC и без риска для продукта. + +### Low — контракт и AC3 говорят «шесть пунктов легенды», перечисляя семь + +И §4 п.2 («легенда — **шесть** обязательных пунктов: …»), и AC3 («текст называет **все шесть** пунктов легенды: …») сопровождаются перечислением через точку с запятой / запятую, которое фактически содержит семь различимых утверждений: (1) цвет линии по шкале LQI с обеими границами, (2) пунктир = качество не сообщено, (3) исходящая стрелка = путь к координатору, (4) входящие стрелки = маршрутизация через устройство, (5) линия без стрелки = запасной сосед, (6) подпись на конце стрелки = цель не на плане, (7) отсутствие стрелки = путь неизвестен. + +Практического риска для реализации нет: оба места перечисляют каждый пункт по имени, и доказательство AC3 описано как «по одному утверждению на пункт» — тестов будет ровно столько, сколько названо пунктов, число «шесть» в тексте не используется как критерий подсчёта. Тем не менее это внутренняя нестыковка ТЗ (число не совпадает с перечнем), и лучше поправить «шесть» → «семь» в обоих местах для точности документа. Снимаю без блокировки цикла, чисто редакторская правка текста ТЗ. + +## Что проверено и корректно + +- **M1 из r1 закрыт по существу, не по форме**: AC3 (7 явных требований) и AC3b (оговорка о приближённости) вместе покрывают весь набор фактов контракта §4 п.2–3; утверждённый владельцем текст §4.1 содержит все семь пунктов легенды и оговорку буквально — проверено построчным сопоставлением, не поверено на слово автора. +- Длина (654 знака) и отсутствие переносов строк в утверждённом тексте подтверждены счётом, а не переписаны с чужих слов. +- Два сознательно опущенных факта (исходящая стрелка у координатора отсутствует; соседи из других пространств не рисуются) корректно выведены из остального контракта и из уже существующего в коде поведения (бейдж «+{n} в других пространствах», ключ `route_other_space` в принятой спеке #457) — не выдуманы. +- Зависимость от #457 остаётся валидной: спека не менялась, ссылки на неё в #459 (ключи, правила стрелок/bubble) не устарели. +- AC1, AC2, AC4, AC5, AC6, AC7, AC8, мутанты, «Затронутые файлы», откат — не задеты дельтой этого раунда, наследуются из r1 без повторной проверки (см. раздел выше). + +## Чего не проверял и почему + +- Гейты (`typecheck`, `test`, `build`, golden, смоки) — код этой задачей всё ещё не менялся, гонять нечего; согласуется с r1 и с этапом ТЗ. +- Качество перевода EN/DE/FR утверждённого текста — переводы ещё не написаны (нормативна только русская строка), это по-прежнему ответственность код-ревью, как уже зафиксировано в r1. +- Полный повторный обход #457 (все §1–20) — не делал заново: спека не менялась ни на один байт с момента r1 (единственный коммит `f3a1dd84` датирован раньше материала r1), повторная сверка добавила бы нулевую информацию. + +## Вердикт + +Зелёный: единственная Medium-находка r1 закрыта по существу и подтверждена построчной сверкой утверждённого текста, а не заявлением автора. Две новые находки — обе Low, обе без влияния на проверяемость AC или на функциональность, сняты без блокировки цикла с рекомендацией к дешёвой правке при реализации (или в этом же ТЗ, по желанию автора). High-находок нет. + +**Вердикт: зелёный · заход r2 · блокирующих циклов 1/2 · High: 0 · Medium: 0** + +Документ: `docs/reviews/SPEC-REVIEW-459-r2.md` + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Якоря снять не удалось: ветки задачи нет, материал читался по `dev`.