docs: review document for #707

Issue: #707
User-Visible: no

docs/reviews/INDEX.md

@@ -1,6 +1,6 @@
 # Индекс ревью
 
-Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 208, issue: 101. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
+Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 209, issue: 102. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
 
 | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
 |---|---|---|---|---:|---:|---|---|
@@ -15,6 +15,7 @@
 | #711 | [CODE-REVIEW-711-r1.md](CODE-REVIEW-711-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
 | #709 | [CODE-REVIEW-709-r1.md](CODE-REVIEW-709-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | canon TESTING.md противоречит себе | `docs/TESTING.md` `scripts/smoke-select.mjs` `scripts/pre-push-gate.mjs` `TESTING.md` `process-digests.test.mjs` |
 | #709 | [CODE-REVIEW-709-r2.md](CODE-REVIEW-709-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |
+| #707 | [SPEC-REVIEW-707-r1.md](SPEC-REVIEW-707-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | К1/AC1: strings.json заявлен источником риска ux, но класс A его не видит | `strings.json` `change-classes.mjs` `custom_components/houseplan/strings.json` `scripts/change-classes.mjs` `manifest.json` |
 | #706 | [CODE-REVIEW-706-r1.md](CODE-REVIEW-706-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
 | #705 | [CODE-REVIEW-705-r1.md](CODE-REVIEW-705-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
 | #704 | [CODE-REVIEW-704-r1.md](CODE-REVIEW-704-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |

docs/reviews/SPEC-REVIEW-707-r1.md

@@ -0,0 +1,222 @@
+# SPEC-REVIEW-707-r1
+
+Issue: #707 · этап: spec · трек: ask · заход: r1 · блокирующих циклов израсходовано (после этого раунда): 1/4
+
+## Скоуп
+
+#707 — часть разбиения семипунктовой аналитики 29.09 (#707/#726/#727/#728/#729). В этот
+issue входят п.1, п.2, п.4 исходного объёма: единое правило риска по изменённым
+участкам диффа (К1), единая функция трека/основания/лимита (К2), решение конвейера
+на `S7` и заметка ревьюеру show/ask (К3), новые разделы `task-packet.mjs` (К4),
+правка канона и конспектов (К5). Продуктового кода задача не трогает (класс A не
+затрагивается), `User-Visible: no`. ТЗ живёт в теле issue под `## ТЗ`, комментарии
+проверены (аналитика 29.09 и передача на ревью 30.09).
+
+## Как проверялось
+
+Ревью текстовое (этап spec, ветки продуктового кода нет — п.12 ТЗ «этап spec,
+ветки нет, инфраструктурный дифф → риск пуст» сам это фиксирует). Проверка шла в
+три слоя:
+
+1. Обязательные разделы §7.1 и однозначность каждого AC — чтением тела issue.
+2. **Проверка каждого технического утверждения по текущему коду `dev`**, а не на
+   слово: все имена файлов, функций, регэкспов, строк конфигурации, упомянутых в
+   ТЗ, сверены с `grep`/`node -e` на рабочей копии. Список того, что именно
+   проверено — ниже, в «Что проверено и корректно». Это сделано, потому что ТЗ
+   содержит десятки конкретных технических утверждений о существующем поведении
+   («в файле X на строке Y», «список Z даёт…», «сейчас такой проверки нет») —
+   каждое из них либо доказуемо чтением кода, либо является непроверенной
+   догадкой, которая по §7.1 — находка.
+3. Прогон гейтов не требовался: задача не создаёт и не меняет ни одного файла —
+   только текст ТЗ в issue. `npx tsc --noEmit` / `npm test` / `npm run build`
+   нечего было бы проверять на этом этапе (см. «Чего не проверял»).
+
+## Находки
+
+### Medium — К1/AC1: `strings.json` заявлен источником риска `ux`, но класс A его не видит
+
+**Файл:** тело issue #707, раздел «6. Контракт поведения» → К1, строка таблицы
+`ux` (столбец «Токен в изменённой строке»).
+
+**Воспроизведение.** К1 открывается условием: «Судятся только файлы класса A
+(`classify` из `change-classes.mjs`); `test/**`, `scripts/**`, `demo/**`, `docs/**`
+риска не дают.» Строка `ux` того же К1 называет три источника нового ключа:
+`src/i18n/*.json`, `custom_components/**/translations/*.json` **и
+`strings.json`**. Проверка самой функции `classify` на рабочей копии:
+
+```
+$ node -e "import('./scripts/change-classes.mjs').then(({classify}) =>
+  console.log(classify('custom_components/houseplan/strings.json')))"
+?
+```
+
+`custom_components/houseplan/strings.json` не подпадает ни под один паттерн
+`CLASS_A` в `scripts/change-classes.mjs` (там только `.py`, `manifest.json` и
+`translations/`-каталог) — `classify()` возвращает `'?'`, не `'A'`. Значит файл,
+который К1 сам называет входом правила `ux`, будет отфильтрован ещё на первом
+шаге («судятся только файлы класса A») и никогда не дойдёт до токен-правила.
+Часть контракта AC1 описывает поведение, которое реализовать как написано
+невозможно: либо `strings.json` тихо выпадает из правила `ux` (реализация
+разойдётся с текстом ТЗ, и это не будет видно в тестах — ни один сценарий
+AC1(а)–(и) не проверяет именно этот путь), либо реализатору придётся по своей
+инициативе расширить `CLASS_A` в `change-classes.mjs` — файл, которого нет в
+разделе «13. Затронутые файлы», и который используется не только этой задачей:
+он же определяет `shipLimitViolations` (рамки `ship`) и признак «инфраструктура»
+в `resolveTrack`/guard. Расширение класса A задним числом — это побочный эффект
+за пределами скоупа, никак не описанный в контракте и не покрытый AC.
+
+`strings.json` — не гипотетический путь: это реальный, вручную редактируемый файл
+конфигурации HA-интеграции (`custom_components/houseplan/strings.json`, история
+правок есть), т.е. случай практический, а не краевой теоретический.
+
+**Почему это находка, а не мелочь.** AC1 — защитный/классифицирующий критерий: он
+обязан однозначно определять классы для указанных путей (DoR, §7.1 — «однозначность
+каждого AC»). Здесь для одного из трёх явно названных путей поведение не определено
+— оно противоречит собственному первому условию правила.
+
+**Что нужно от автора.** Явно решить одно из двух и записать в ТЗ: (а) убрать
+`strings.json` из строки `ux` К1 (ux-риск для HA config-flow строк не отслеживается
+этим правилом), либо (б) явно включить путь `custom_components/*/strings.json` в
+проверяемый список К1 отдельной оговоркой, не трогая `change-classes.mjs` (например,
+нишевым исключением в самой риск-функции, а не в общем `classify`), и добавить
+сценарий в план AC1. Любой из двух вариантов дёшев; выбор — продуктовое решение
+о том, что RU/EN-строки конфиг-флоу интеграции достойны такого же сигнала риска,
+что и `src/i18n/*.json`, — не то, что ревьюер решает за автора.
+
+Без High в задаче это жёлтый вердикт, правка ТЗ и повторный цикл (§2.4, §4).
+
+## Что проверено и корректно
+
+ТЗ необычно тщательно сверено с текущим кодом: я перепроверил около 30 отдельных
+технических утверждений, и, кроме находки выше, все подтвердились дословно:
+
+- Существование и сигнатуры `resolveTrack`, `trackFromLabels`, `hasTrackLabel`,
+  `shipLimitViolations`, `parseNumstat`, `parseNameStatus`, `SHIP_SRC_LINE_LIMIT`
+  в `scripts/process-track.mjs`.
+- **Реальное расхождение трека при двух метках**, которое К2/AC2 называет
+  дефектом и чинит: `trackFromLabels` при `track:ship`+`track:ask` возвращает
+  `'ship'` (проверяет `track:ship` первым), тогда как bash guard в
+  `.github/workflows/_process.yml` (строки 92–97) при `track:ask` всегда сбрасывает
+  `SMALL=false` — т.е. сегодня guard даёт `ask`/4, а `process-track.mjs` — `ship`.
+  Утверждение ТЗ подтвердилось построчно.
+- `task-packet.mjs:223` — ровно та строка про «перед S7 ребейз» безусловно для
+  любого `behind > 0`, которую К4/AC8 переписывает; устарелость с #696
+  подтверждается (строка не различает track).
+- `classify`/`CLASS_A`/`CLASS_B`/`CLASS_C`/`CLASS_D` в `scripts/change-classes.mjs`
+  и их точное содержимое (кроме отсутствия `strings.json` — см. находку).
+- Порядок шагов в `.github/workflows/_process.yml`: шаг «Трек задачи и рамки ship
+  (#696)» (строка 424) действительно идёт до шага «Привести ветку к dev» (строка
+  481), как того требует К3.
+- Существование и сигнатуры `SHIP_MERGE_MARKER_RE`, `renderShipBrief` в
+  `scripts/ship-review.mjs`; маркер `hp:ship-merge material=...` в конвейере
+  (строка 1706).
+- Все четыре якоря реестра мутаций, которые ТЗ требует перенести, а не удалить:
+  `guard-infra-keeps-ask-limit`, `packet-infra-track-ignores-show-default`,
+  `pipeline-ship-ignores-limits`, `ship-review-ignores-merge-marker` — все
+  существуют в `scripts/mutation-registry.mjs`.
+- Отсутствие сегодня `bash -n`-проверки шагов `_process.yml` (AC4) — подтверждено
+  пустым результатом `grep -rn "bash -n" test/ scripts/`.
+- Порог «300 файлов» в guard (`.github/workflows/_process.yml:239`,
+  `files.length < 300`) — дословное совпадение с описанием К2 «300 файлов и
+  больше → инфраструктура не доказана».
+- Полный список файлов геометрии/touch/devices/perf/visual из таблицы К1 (src/*.ts
+  и custom_components/houseplan/*.py) — каждое имя, которое я сверил построчно
+  (physical-geometry, wall-*, junction-limits, coincident-partitions,
+  coordinate-canonicalization, opening-*, partition-openings, open-spans,
+  near-axis, align-grid, grid-scale, room-fit, resize*, stairs*, radar-geometry,
+  zigbee-topology-geometry, device-marker-geometry, plan-geometry-preflight,
+  plan-optimizer, zero-walls, iso-projection; pointer-modality,
+  pointer-move-queue, touch-gesture-click-guard, live-interaction-runtime,
+  live-viewport, viewport-transition, room-gear-drag; device-toggle,
+  marker-toggle-entity, integration-provider, virtual-light-state, vacuum*,
+  device-hit-owner; auth.py, http_api.py, websocket_api.py, virtual_lights.py,
+  vacuum_routes.py, store.py, geometry_migration.py, import_export.py,
+  validation.py, coordinate_canonicalization.py, junction_limits.py,
+  wall_segment_model.py, radar_geometry.py, projection.py;
+  houseplan-render-lifecycle, iso-scene-render, glow-*, day-cycle-render,
+  initial-load, boot-soft-layout; paper-scene.ts; space-render, stairs-view,
+  device-visual, device-face; styles.ts/styles/) — существует ровно как названо.
+- Наличие `test/process-track.test.mjs` с существующим паттерном извлечения текста
+  шага `_process.yml` регэкспом (строка 120, 167) — подтверждает реалистичность
+  плана автотестов АС4/АС6 «как существующие тесты #696».
+- Обязательные разделы §7.1 (сценарий, что увидит человек, проблема, скоуп и
+  не-скоуп, контракт поведения, UX/данные/i18n/perf/touch, AC1–AC14 со способом
+  доказательства, план автотестов, риски, откат, release-артефакты) — все
+  присутствуют и в правильном порядке.
+- «Принято предположительно» (§10) — 8 пунктов, каждый — реальная техническая
+  развилка (где живёт код, эвристика К1, длина заметки и т.д.), не маскирует
+  продуктовый вопрос под техническое решение.
+- **§10 п.8** (сужение «метка владельца окончательна» до «метка, подтверждённая
+  строкой владельца») — автор прямо просит проверить его на ревью. Обоснование в
+  тексте ТЗ внутренне непротиворечиво: текущий §5 PROCESS.md не различает метку,
+  поставленную человеком-владельцем, от метки, поставленной аналитиком/агентом
+  от того же аккаунта (о чём и сам §5 говорит: «Аналитик предлагает трек… и ставит
+  метку»), и per §10 п.1 задачи по actor события их сегодня действительно не
+  отличить. Уточнение закрывает реальный пробел, не меняет ничьих прав, которых
+  не было прежде, и не требует отдельного продуктового вопроса владельцу — трек
+  решается вердиктом по §7.1 («технический спор автора и ревьюера решается
+  вердиктом»). Принимаю формулировку как есть.
+- Лимиты циклов, таблица `ask`=4/`show`,`ship`=2, «зелёный вердикт цикла не
+  образует» — согласуются с §4 буквально.
+- Соответствие «Не входит» реальным границам: `smoke-select.mjs`, `process-reconcile`,
+  `process-resume`, `merge-candidate`, тонкий `process.yml` в `main` — задача их не
+  трогает; список «затронутые файлы» (раздел 13) их действительно не включает.
+- Метки на самом issue: `track:ask`, `S4-spec-review`, `P2`, `infra`, `process`,
+  `tech-debt` — соответствуют заявленному в аналитике треку и статусу.
+- П.7 (черновая реализация до вердикта, меняющая правило №1) явно вынесен в #729 и
+  не входит в контракт #707; раздел «11. Вопрос владельцу» корректно помечен
+  «не блокирует #707, `blocked` не ставится» — в #707 остаётся только справочная
+  ссылка, лишнего продуктового вопроса в этой задаче нет.
+
+## Чего не проверял
+
+- **Гейты (`npx tsc --noEmit`, `npm test`, `npm run build`) не прогонял** — задача
+  не меняет ни одного файла репозитория на этом этапе, проверять нечего; на
+  `S7-code-review` они станут обязательны и будут применяться к реальному диффу.
+- Не проверял golden/смоки/бэкенд-pytest/invariants — задача класса A не
+  содержит, `User-Visible: no`, визуальных и геометрических изменений продукта
+  нет (сама ТЗ фиксирует это в разделе 7 и 16).
+- Не оценивал реальную эффективность эвристики К1 (долю ложных срабатываний/пропусков
+  на исторических диффах) — это явно измеряемый, но не гейтируемый на этом этапе
+  вопрос; ТЗ сама называет это «принято предположительно» (§10 п.2) и переносит
+  измерение эффекта в #728.
+- Не проверял, действительно ли `git diff --unified=0` с настройками git по
+  умолчанию в GitHub Actions runner надёжно распознаёт чистые переименования
+  (сценарий AC1(и)) — это деталь реализации команды диффа, не зафиксированная в
+  контракте явным флагом (`-M`/`--find-renames`); при стандартном `diff.renames=true`
+  (git ≥2.9) поведение ожидаемо корректно, а при сбое эффект безопасен (ложный
+  риск, а не пропуск) — не поднимаю отдельной находкой, но это стоит держать в
+  уме при написании AC1(и) как юнит-теста на встроенном диффе, а не на реальном
+  git.
+
+## Вердикт
+
+Один Medium **в скоупе задачи** (AC1/К1: `strings.json` заявлен источником риска
+`ux`, но исключён условием «только класс A» того же правила) — без High это
+жёлтый вердикт с возвратом автору по §2.4/§2.7. Всё остальное, вплоть до
+мелких деталей вроде номеров строк и имён функций, подтвердилось построкой
+сверкой с кодом `dev`; спор на ревью — только вокруг одной несостыковки К1.
+
+**Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче**
+
+## Материал раунда
+
+- Этап: spec. Материал — тело issue #707 на момент комментария Matysh
+  2026-09-30T20:59:00Z («ТЗ написано… Передаю на ревью ТЗ»).
+- Кода/ветки продукта не существует (инфраструктурная задача до реализации).
+- Сверка кода `dev`: `git rev-parse HEAD` рабочей копии ревью =
+  `a5a73d1511ae4c09277e16a41c076bfd5eb1fd8e`.
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `a5a73d1511ae` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `5a1bea4a8cb02a6fc04a3a8742cb8b57e57b7c51`
+  ```
+  git log --all --format='%H %T' | grep 5a1bea4a8cb0
+  ```
+- Тело issue: `965899906f2230d8473b72e6304be7bb1f58c2045425912b69a9efa788025ff4`
+- Вердикт конвейера: `yellow` · High 0