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

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.

---

Материал раунда

• Ветка: dev, коммит a5a73d1511ae — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 5a1bea4a8cb02a6fc04a3a8742cb8b57e57b7c51

  git log --all --format='%H %T' | grep 5a1bea4a8cb0
  

• Тело issue: 965899906f2230d8473b72e6304be7bb1f58c2045425912b69a9efa788025ff4
• Вердикт конвейера: yellow · High 0