houseplan-card/docs/reviews/CODE-REVIEW-730-r4.md

CODE-REVIEW #730 · заход r4

Материал: 02dce77c9859ba26d855eb09e62090d126beecb8 (рабочая копия на этом SHA; git diff origin/dev...HEAD / git log --oneline origin/dev..HEAD). Трек: show. Дешёвые гейты (tsc --noEmit, npm test, npm run build со сверкой бандла) подтверждены зелёным Validate на этом же SHA: https://github.com/Matysh/houseplan-card/actions/runs/36817313433 (проверено лично через gh run view 36817313433 — headSha совпадает, conclusion: success) — не перегонялись.

Скоуп

r3 (85d7c6fa) получил зелёный вердикт — ребейз r2 на dev, ушедший на 1 коммит, чистый, без конфликтов. Зелёный вердикт бюджет §4 не тратит и цикла не образует (#227): «блокирующих циклов израсходовано 0 из 2» в заходе r4 корректно.

Между r3 и r4 dev продвинулся ещё на 20 коммитов, включая #726 и #727 (scripts/ship-review.mjs, переработка _ship-review.yml). Повторная публикация документа r3 снова уткнулась в отказ push по праву на workflow (тот же случай AC2), и страж ребейза опять вернул задачу автору на ребейз — автор выполнил его сам (комментарий 2026-10-01T04:54:07Z). На этот раз ребейз не был чисто механическим: автор сообщил о двух настоящих конфликтах — test/publish-push-refusal.test.mjs (тест #726 AC5 и блок #730 для _ship-review.yml независимы, оставлены оба) и docs/reviews/INDEX.md (пересобран reviews-index.mjs). Это не локальная делта в смысле §2.10 («ребейз на ушедший вперёд dev» — явно названный в инструкции повод для полного разбора), поэтому ниже разбираю весь дифф заново, а не только факт переноса SHA, и отдельно проверяю оба места конфликта по существу, а не на слово автора.

Дифф origin/dev...HEAD (файлы, без учёта архивных документов r1–r3): .github/workflows/_beta-derived.yml (17), _process.yml (5), _ship-review.yml (36), PROCESS.md (8), scripts/merge-candidate.mjs (12), test/rebase-generated.test.mjs (18) — построчно идентичны тому, что разбирали r1–r3 (см. «Как проверялось», п.2). Два файла дифф-стата изменились числом строк из-за реального слияния с dev: docs/reviews/INDEX.md (5→переcобран с учётом новых документов #726/#727/#732/#728) и test/publish-push-refusal.test.mjs (217 строк — включает унаследованный тест #726 AC5, который теперь физически соседствует с блоком #730 в одном файле).

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

1. Прочитано тело issue #730 и все одиннадцать комментариев, включая оба отказа push по праву на workflow (02:13 и 03:04), оба самостоятельных ребейза автора и финальный комментарий о втором ребейзе с перечнем конфликтов и прогона (02dce77c поверх dev 3b9f25ea).
2. Побайтово сверены неконфликтные файлы с содержимым, которое разбирали r1–r3: git diff origin/dev...HEAD -- scripts/merge-candidate.mjs, .github/workflows/_process.yml, _ship-review.yml, _beta-derived.yml, PROCESS.md, test/rebase-generated.test.mjs — хуки правки (новая ветка nextStep для PUSH_REFUSAL.workflow, ключи ship-review/beta-derived/rebase в PUBLISHED, --summary="$GITHUB_STEP_SUMMARY" в страже ребейза, разбор push_err в обоих телах публикации, убранный heredoc) текстуально совпадают с тем, что цитируют документы r1–r3. Расхождений нет.
3. Прочитан конфликтный test/publish-push-refusal.test.mjs целиком вокруг места стыка (строки 1–40, 340–560): тест #726 AC5 (строка 350) и блок #730 (// ---------- #730 _ship-review.yml ... c строки 368) идут друг за другом без обрезанных строк, дублирования или следов конфликт-маркеров. Также найдено отличие от версии r1–r3: STEP_SCRIPTS теперь включает 'ship-review.mjs' (было: release-review.mjs, reviews-index.mjs, review-doc-guard.mjs, merge-candidate.mjs). Проверено по существу, не предположением: grep -n "ship-review.mjs" .github/workflows/_ship-review.yml показывает, что именно шаг Опубликовать документ (тот же шаг, в который #730 добавляет разбор push-отказа) сам зовёт scripts/ship-review.mjs в нескольких местах (строки 307, 323, 384) — это код, добавленный в _ship-review.yml соседним #727, уже находящимся на dev. Добавление ship-review.mjs в STEP_SCRIPTS — необходимая адаптация замыкания импортов тестового песочницы под новый сосед по шагу, а не посторонняя правка: без неё песочница не нашла бы транзитивные импорты этого скрипта. Не находка.
4. Весь дифф (git diff origin/dev...HEAD) и всё дерево (git grep) проверены на отсутствие следов незакрытого конфликта: ^<<<<<<<, ^=======$, ^>>>>>>> — единственное совпадение внутри диффа оказалось строкой текста документа r3, описывающей саму эту проверку (цитата, не маркер); по дереву вне diff — пусто.
5. node scripts/reviews-index.mjs --dir=docs/reviews --check → «docs/reviews/INDEX.md свеж» — пересборка индекса после переноса документов r1–r3 и соседних #726/#727/#728/#732 корректна, не полагаюсь на заявление автора «INDEX.md пересобран».
6. Выполнена дисциплина «тест должен уметь падать» лично, на этом SHA, а не унаследована: временно откатил nextStep в scripts/merge-candidate.mjs к тексту до фикса r1/r2 (единая фраза «повтор и ребейз не помогут» для всех исходов) и прогнал test/publish-push-refusal.test.mjs → упал ровно и только тест #730 r1: при отказе по праву на workflow сводка зовёт автора сделать ребейз, а не отговаривает (24 pass / 1 fail из 25). Рабочая копия восстановлена из бэкапа; git status --short и git diff --stat после восстановления пусты.
7. На материале (02dce77c) прогнаны все три связанных тест-файла: node --test test/publish-push-refusal.test.mjs test/rebase-generated.test.mjs test/merge-candidate.test.mjs → 77/77 pass, 0 fail (на один тест больше, чем в r2/r3 — это унаследованный из dev #726 AC5, не новый код этой задачи).
8. node scripts/smoke-select.mjs --base origin/dev --head HEAD на этом SHA → «Исполняемого frontend-диффа нет», src/** по-прежнему не тронут (11 файлов всего, все инфраструктурные/документные).
9. Трейлеры пяти коммитов делты r3→r4, которых не было в r3 (ребейз-результат и повторная публикация документа r3 в новой позиции плюс пересборка индекса): 8e42f4e9/56231394/4eb712be несут Issue: #730, User-Visible: no; 0ab685e3/53bb3bf4 (публикация документа ревью r3, docs-only) и 02dce77c (пересборка индекса) — тот же трейлер, формально не обязателен для docs-only коммитов, но не расходится и не вводит в заблуждение.
10. Подтверждён зелёный Validate именно на этом SHA: gh run view 36817313433 → headSha: 02dce77c9859ba26d855eb09e62090d126beecb8, conclusion: success — ссылка из инструкции совпадает с материалом ревью, не привязка «на слово».
11. Признаков правки продуктового кода, PDF/geometry, custom_components/**/*.py, меток ci:golden нет — golden/pytest/invariants/perf вне применимости, как и в r1–r3.

AC · чем доказан · чем краснеет

AC1–AC3 и закрытая в r2 находка не менялись контентно начиная с r2 — перепроверены исполнением на новом SHA, не только унаследованы, так как делта r3→r4 задела один из файлов-носителей доказательства (конфликт в test/publish-push-refusal.test.mjs):

AC	Доказательство	Чем краснеет (проверено исполнением на 02dce77c)
AC1 (разбор push, ship-review/beta-derived)	test/publish-push-refusal.test.mjs	77/77 зелёных на материале; содержимое блока #730 в файле после слияния с #726 AC5 не повреждено (проверено чтением, п.3)
AC2 (сводка стража ребейза, включая исправленный текст для workflow)	test/rebase-generated.test.mjs	входит в 77/77 выше; файл не конфликтовал при ребейзе, содержимое побайтово совпадает с r1–r3 (п.2)
AC3 (нет heredoc, тонкие файлы не тронуты)	git diff origin/dev...HEAD --stat на тонких ship-review.yml/beta-derived.yml — пусто	статическая проверка, как в r1–r3; ребейз этого не касался
Находка r1/закрытие r2 (текст для исхода workflow)	test/publish-push-refusal.test.mjs:582-589 (#730 r1)	лично откатил nextStep в merge-candidate.mjs к досрочной версии → 1/25 fail ровно на этом тесте (п.6); восстановлено

Закрытие раунда r3

r3 — зелёный вердикт, возврата на правки не было (находок нет). Раздел неприменим по существу как «закрытие находки»; ниже — соответствие заявленному материалу r3 после второго ребейза.

Что было заявлено в r3	Чем подтверждено сейчас
Содержимое AC1–AC3 и находки r1/r2 не меняется ребейзом, только база	Подтверждено и для второго, уже неместного ребейза: неконфликтные файлы побайтово идентичны (п.2); конфликтные файлы прочитаны по существу и логика не искажена (п.3)
76/76 тестов в трёх связанных файлах	На новом SHA — 77/77 (+1 унаследованный из dev тест #726 AC5, не относящийся к #730)

Унаследовано из r1–r3

Принято без повторной проверки «с нуля» — содержимое логики не менялось с r1/r2, в этом раунде перепроверена именно та часть, которую затронул неместный ребейз (пп. 3, 6, 7 выше), а не всё целиком:

• Продуктовая корректность refusalSummary/PUSH_REFUSAL.workflow/PUBLISHED и формулировок для трёх стадий (ship-review, beta-derived, rebase) — документы r1 (находка), r2 (исправление), r3 (живое подтверждение в проде) — не переоценивал содержательно, т.к. scripts/merge-candidate.mjs в дифф-стате к origin/dev не изменился ни на строку со времён r2 (сверено, п.2); «тест умеет падать» перепроверил лично здесь же, а не положился на цитату r2/r3 (п.6).
• Разбор push в _ship-review.yml/_beta-derived.yml настоящим bash/git во временных репозиториях (AC1) — документ r1, пп. 2–4; код шагов не менялся с r1 (п.2), перепроверено исполнением всего файла целиком (п.7).
• Скрипт merge-candidate.mjs берётся из dev, не из тонкого вызывающего репозитория — документ r1, п.9; код чекаута не менялся.
• Токен нигде не светится (noisySecretsGone во всех тестах) — документ r1; состав секретов в тестах не менялся, новый сосед (#726 AC5) — отдельный, независимый тест.
• Полный npm test (3347+/3348, 1 skip) на материале, эквивалентном текущему по содержимому логики — документ r1, п.5; Validate зелёный целиком на самом этом SHA (п.10 выше), точечный прогон трёх файлов (77/77) заменяет повторный полный прогон, т.к. делта к r3 остальные ~3270 тестов не касается.

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

• Второй ребейз, несмотря на реальные конфликты (в отличие от r2→r3), разрешён без потери содержания: оба конфликтных места (тест-файл и индекс документов) проверены по существу, а не по diffstat — структура тестов не повреждена, индекс пересобран инструментом и подтверждён как свежий самим генератором.
• Единственное содержательное отличие теста от версии r1–r3 (STEP_SCRIPTS получил 'ship-review.mjs') объяснимо и корректно: вызвано тем, что сосед по той же строке workflow (#727) теперь сам зовёт этот скрипт внутри того же шага, который разбирает #730 — без этого расширения песочница теста не закрыла бы импорт.
• Дисциплина «тест должен уметь падать» для находки r1/фикса r2 подтверждена лично на этом SHA (не унаследована бездоказательно): откат nextStep красит ровно один целевой тест.
• Validate зелёный именно на материале ревью (02dce77c), сверено напрямую через gh run view, а не процитировано со слов.
• Трейлеры (Issue: #730, User-Visible: no) на месте во всех новых коммитах делты; User-Visible: no оправдан — поведение карточки не меняется, изменение инфраструктурное.
• Признаков незакрытого конфликта (маркеры, обрыв строк, дублирование блоков) нет нигде в дереве и в диффе.

Чего не проверял

• tsc --noEmit, npm run build + сверка трёх копий бандла, полный npm test (3348+ тестов целиком) — не перегонял: подтверждены зелёным Validate на этом же SHA (02dce77c, https://github.com/Matysh/houseplan-card/actions/runs/36817313433, сверено напрямую). Три связанных тест-файла (77 тестов) и дисциплину «тест умеет падать» перегнал лично, см. выше.
• actionlint — бинарник недоступен в окружении ревью (which actionlint — пусто), как и в r1; полагаюсь на то же основание, что в r1: автор заявил «чистый» для затронутых тел, Validate проверяет синтаксическую валидность YAML тем же зелёным прогоном.
• Браузерные смоки — node scripts/smoke-select.mjs --base origin/dev --head HEAD на 02dce77c вернул «исполняемого frontend-диффа нет»; src/** не тронут ни в одном из четырёх раундов.
• npm run golden:verify, python -m pytest tests_backend -q, npm run invariants -- --config …, performance-профили — не применимо: нет метки ci:golden, не менялся custom_components/**/*.py, не менялась геометрия, performance не названа в AC.
• Ручной живой прогон _ship-review.yml/_beta-derived.yml/_process.yml в GitHub Actions с настоящим GitHub-отказом по workflow-праву — не ставил отдельно; сценарий workflow произошёл в реальности дважды между r2 и r4 (см. «Скоуп» r3 и этого документа) и оба раза повёл себя так, как требует AC2 — сильнее постановки вручную.
• Мутанты по диффу — не запрашивались (трек show, #696), не прогонял.

Вердикт

Зелёный. Материал r4 — второй ребейз на ушедший на 20 коммитов вперёд dev (включая #726/#727), на этот раз с двумя настоящими конфликтами в test/publish-push-refusal.test.mjs и docs/reviews/INDEX.md. Оба места разрешены корректно: тестовый файл не потерял и не исказил ни блок #730, ни соседний #726 AC5, добавленная строка STEP_SCRIPTS — обоснованная адаптация к новому соседству в том же workflow-шаге, индекс пересобран и подтверждён генератором как свежий. Логика продукта (merge-candidate.mjs, тела _ship-review.yml/_beta-derived.yml/_process.yml, PROCESS.md) не менялась со времён r2/r3 — перепроверена исполнением заново на этом SHA (77/77 тестов трёх связанных файлов, личный повтор дисциплины «тест умеет падать» для находки r1/фикса r2). Блокирующих находок нет.

---

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

• Ветка: issue/730-derived-push-refusal, коммит 02dce77c9859 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: ab974f07f0b71626f99de337faf835539bd34091

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

• Тело issue: 3d77fb56ed2e308d5df6d329563f8143ebf6d19f6a36b320a92fdb76ee31d8fd
• Вердикт конвейера: green · High 0 · маршрут fix