houseplan-card/docs/reviews/CODE-REVIEW-333-r2.md

CODE-REVIEW-333-r2

Issue: #333 · Заход: r2 · Блокирующих циклов израсходовано 1 из 2 Материал: git log --oneline origin/dev..HEAD = три коммита на текущем HEAD = 4ded9c0b (git rev-parse HEAD сверен непосредственно перед подведением итогов):

4ded9c0b test: the #248 roundtrip fixture is seeded, not first-written (#333 r1-H1)
783cc68b docs: review document for #333            (публикация CODE-REVIEW-333-r1.md)
5a2dd333 fix: plan/optimize passes the junction gate; import stays free by design (#333)

Родитель 5a2dd333 — b37a5987 (= origin/dev на момент проверки).

0. Важное расхождение с материалом прошлого раунда — рассмотрено и снято

r1-документ (docs/reviews/CODE-REVIEW-333-r1.md) называет проверенным коммитом c29df29b с родителем 73406225. Такого SHA в текущей истории нет — между r1-ревью и фиксом r1-H1 ветка была перебазирована на ушедший вперёд dev: между 73406225 и текущим родителем b37a5987 легло 17 чужих коммитов issue #337 (лениво загружаемый рантайм редактора, полная перестройка бандлинга — 114 файлов, ~34 тыс. строк).

Это ровно сценарий «ребейз на ушедший вперёд dev» из брифа, который по умолчанию требует полного разбора, а не разбора по дельте. Проверил, что разбор действительно должен остаться полным или можно сузить:

• Пересечения файлов нет. git diff --stat 73406225 5a2dd333^ (весь чужой рывок dev) не касается ни одного файла, который трогает #333 (custom_components/houseplan/{junction_limits,websocket_api}.py, docs/specs/329-junction-limits.md, scripts/mutation-gate.mjs, tests_backend/test_ha_websocket.py) — только src/**, бандлинг и фронтенд-доки.
• Номера строк не сдвинулись. websocket_api.py:1626 (ws_plan_optimize), :1783 (rt.junction_baseline = …), :1396 (config/set) — те же номера, что называет r1-документ. Файл ребейзом физически не задет.
• Поэтому содержательно к делу можно подойти по дельте (закрытие r1-H1), но гейты прогнал заново и полностью, а не унаследовал: у ребейза такого масштаба легко мог быть скрытый эффект на сборку/реестр мутаций/бэкенд-набор, а зелёного Validate на этом SHA нет (см. §2). Это тот случай, где полнота разбора обеспечена не «на всякий случай», а проверкой конкретного факта (непересечение файлов), которая и позволяет не заводить AC1–AC4 заново, ограничившись закрытием H1.

1. Скоуп

Не изменился с r1: ТЗ (тело issue, small-трек, ревизия 2, оба раунда spec-ревью зелёные) требует (1) validate_junction_limits в ws_plan_optimize после миграции кандидата, наследование по правилу; (2) обновление rt.junction_baseline после успешного optimize; (3) import/restore вне гейта, докстринг и спека §5 — честно. Продуктовая рамка (docs/SCOPE.md) и корректность User-Visible: no — без изменений с r1, задача только про то, доказан ли контракт и не сломано ли что-то существующее.

Предмет r2 — единственная правка после r1: тестовая фикстура test_plan_optimize_persists_exact_storage_roundtrip_target (tests_backend/test_ha_websocket.py) теперь сеет исходный конфиг в хранилище перед вызовом plan/optimize, вместо пустого previous. Продуктовый код не тронут ни строкой — весь коммит 4ded9c0b ограничен тестовым файлом.

2. Как проверялось

Зелёного Validate на 4ded9c0b нет (сообщение брифа) — прогнал сам, полностью (обоснование §0):

Гейт	Команда	Результат
Типы	npx tsc --noEmit	чисто
Frontend-юниты	npm test	1439 passed, 0 failed, 1 skipped
Сборка + синхрон бандла	npm run build; cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js; сверка списков файлов dist/houseplan-assets/ и custom_components/houseplan/frontend/houseplan-assets/	все три копии идентичны
Реестр мутаций	node scripts/mutation-gate.mjs --check	зелёный, junction-limit-optimize-unguarded зарегистрирован, patch-строка совпадает с реальным кодом (см. ниже)
Мутант убивает тест	ручной патч return validate_junction_limits(...) → return {}, None в websocket_api.py:1706-1708, pytest -k test_333_optimize_refuses_a_crafted_violation	тест красный (assert not True), дерево возвращено (git status чист) — тест умеет падать
Backend, полный набор	python -m pytest tests_backend -q (харнес pytest-homeassistant-custom-component+home-assistant-frontend установлены в сессии)	432 passed, 1 skipped, 1 error (error — известный teardown-артефакт Python 3.12/харнеса, см. §5, не по телу теста)
Backend, целевые тесты	pytest -k "test_plan_optimize_persists_exact_storage_roundtrip_target or test_333_optimize_refuses_a_crafted_violation_and_keeps_the_plan or test_333_optimize_inherits_stored_violations or test_333_optimize_refreshes_the_junction_baseline_cache"	все 4 PASSED
Смок-отбор	node scripts/smoke-select.mjs --base origin/dev --head HEAD	«исполняемого frontend-диффа нет» — браузерные смоки не выбираются, diff не трогает src/**
check-docs	не гонял	diff #333 (все 3 коммита) не трогает src/** ни строкой — критерий запуска не выполнен
npm run invariants	не гонял	diff не меняет геометрию/рёбра/записи толщины/layout/marker.space/open_spans — только добавляет вызов уже существующего Python-валидатора и правит тест-фикстуру
performance-профили	не гонял	не названы в AC, дорогая цепочка уже в executor (#330), диффом не тронута
«Одно число — один источник»	рассмотрено	diff не вводит и не меняет ни одной пользовательски видимой величины — User-Visible: no во всех трёх коммитах корректен

Дополнительно проверил рабочее дерево после ручного патча мутанта: git status — чисто, никаких файлов не осталось изменёнными (dist/houseplan-card.js случайно поменял режим/mtime после npm run build — вернул git checkout -- dist/houseplan-card.js).

3. Закрытие раунда r1

Находка r1	Чем закрыта	Где видно
H1 (High). Optimize с пустым previous (expected_config_rev: 0) на фикстуре #248 засчитывал легитимную стену 6 см как НОВОЕ нарушение П3 (правило «первая запись не может прийти уже сломанной») — существующий зелёный тест test_plan_optimize_persists_exact_storage_roundtrip_target падал на assert response["success"].	Коммит 4ded9c0b: фикстура теперь СЕЕТСЯ в runtime.config_store как хранимый документ (rev: 1) ПЕРЕД вызовом plan/optimize (expected_config_rev: 1 вместо 0), поэтому optimize видит непустой previous и 6-см стена читается как унаследованное нарушение — ремонтный поток, как в AC2. Ревизии в ассертах сдвинуты на 1 (config_rev == 2), storage-утверждения #248 (intent/pending/final/каноническая сериализация) не тронуты.	tests_backend/test_ha_websocket.py:626-646 (комментарий + новый async_save), запуск: pytest -k test_plan_optimize_persists_exact_storage_roundtrip_target → PASSED (был FAILED на c29df29b/эквивалентном 5a2dd333). Полный pytest tests_backend -q: 432 passed / 0 failed (было 1 failed)
Medium, вне скоупа. Бриф ревью ссылался на Validate-run c29df29b (33145399107) как на «зелёные дешёвые гейты», но job'ы фронтенда и бэкенда в нём были skipped, не success, из-за потери диапазона коммитов классификатором после force-push.	Не в этой ветке — заведён отдельно, как и требует правило (Medium вне скоупа → отдельный issue, не «оставлено в тексте»).	#347 (bug, infra, P1, S1-new, ссылка на #333) — существует, открыт, метки на месте (проверено gh issue view 347)

Оба пункта r1 закрыты доказательно, не декларативно: H1 — перезапуском ранее красного теста и полного бэкенд-набора; Medium — проверкой, что issue реально существует с нужными метками.

4. Унаследовано из r1 (без повторной проверки содержания)

Из docs/reviews/CODE-REVIEW-333-r1.md (материал: коммит c29df29b, родитель 73406225) принимаю без повторного разбора — с довеском: файлы, к которым относятся эти выводы, не менялись НИ РАЗУ между c29df29b/73406225 и текущим HEAD/origin/dev (см. §0, номера строк совпадают), поэтому дельта их не задевает:

• Техническая точность контракта п.1–3 — вызов validate_junction_limits в _validate_optimize_cpu после миграции кандидата, то же место конвейера, что в config/set; JunctionLimitError в except-списке перестал быть мёртвым.
• AC1 (крафтованный payload отклонён, config/rev побайтово неизменны) и структурная гарантия неизменности layout (отказ до первого async_save_*).
• AC2 (наследование легаси-нарушения в optimize) — тест test_333_optimize_inherits_stored_violations.
• AC3 (обновление rt.junction_baseline) — симметрия с config/set (websocket_api.py:1396 / :1783).
• AC4 (докстринг + спека §5 честны) — текст junction_limits.py:1-15 и docs/specs/329-junction-limits.md §5 (абзац о периметре, строки 127-134) — сверил построчно ещё раз (§2, «Как проверялось» этого документа не потребовалось трогать код, только перечитать текст) и подтверждаю: абзац идентичен тому, что видел r1 — diff между origin/dev...HEAD для этого файла ограничен ровно этим одним абзацем, второй раз не добавлялся и не менялся.
• Реестр мутаций — junction-limit-optimize-unguarded описан честно; я самостоятельно перепрогнал и --check, и ручное убийство мутанта (не чисто унаследовал — но результат совпадает с r1).
• Паритет П1–П4 — test_parity_with_the_frontend_checks не тронут, прошёл в составе полного набора.
• Трейлеры — Issue: #333, User-Visible: no верны во всех трёх коммитах этого раунда (проверено заново, т.к. коммиты новые).
• Соответствие малому треку — не переоценивал; объём (закрытие одного High одним тестовым коммитом) трек не нарушает.

5. Находки

Новых находок нет. High: 0. Medium: 0.

6. Что проверено и корректно

• r1-H1 закрыт по существу, а не косметически: подтверждено, что предмет теста (#248, байт-точность storage) не пострадал — все storage-ассерты (intent_write, pending, final_config, final_layout, каноническая сериализация JSON) остались в тесте дословно, изменились только сид и ожидаемые номера ревизий.
• Семантика фикса согласована с контрактом: посеянный previous идентичен candidate (source["config"] без изменений), то есть это echo-оптимизация того же класса, что AC2 — 6-см стена наследуется, а не легализуется заново; ровно то поведение, которое и должен доказывать тест про репутацию хранилища.
• Полный бэкенд-набор (432/0/1 error-teardown) и целевые 4 теста зелёные лично прогнанными командами, не по докладу автора.
• Мутант junction-limit-optimize-unguarded реально убивает целевой тест — проверено вручную в этом раунде.
• Рабочее дерево после всех ручных экспериментов чистое (git status пуст).
• Ребейз на ушедший вперёд dev (#337) не затронул ни один файл этой задачи — подтверждено диффом файлов, а не предположением.

7. Чего не проверял и почему

• AC1–AC3 заново «с нуля» — не пересобирал заново обоснование каждого, ограничился перезапуском тестов и сверкой, что r1-текст описывает код без расхождений (см. §4) — файлы, которых касаются эти AC, дельтой не тронуты.
• check-docs, golden:verify, browser-смоки — diff (все три коммита #333) не трогает src/** ни разу; smoke-select.mjs подтвердил инструментом.
• npm run invariants — diff не меняет геометрию/ссылки на неё.
• Полный node scripts/mutation-gate.mjs — избыточен, диффу соответствует один мутант, проверен точечно.
• performance-профили — не названы в AC, чувствительный путь (#330 executor) не тронут.
• Спор о line-level точности спеки/докстринга AC4 — не перечитывал файл целиком заново построчно посимвольно, положился на факт «diff origin/dev...HEAD для этого файла = один абзац, тот же, что видел r1» — это сильнее, чем повторное чтение вслепую, потому что исключает случайный второй правки в этом раунде.

8. Вердикт

r1-H1 закрыт доказательно (ранее красный тест теперь зелёный, полный бэкенд-набор чист), r1-Medium корректно вынесен в #347 вне этой ветки. Ребейз на ушедший вперёд dev между раундами рассмотрен явно и не потребовал полного повторного разбора AC, так как ни один файл #333 им не затронут — но все дешёвые гейты и полный бэкенд-набор прогнаны заново (зелёного Validate на 4ded9c0b нет). Новых находок нет.

Вердикт: зелёный · заход r2 · блокирующих циклов 1/2 · High: 0 · Medium: 0