houseplan-card/docs/reviews/CODE-REVIEW-587-r1.md

CODE-REVIEW-587-r1

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

Материал: 978035c49193a7d367d3dba6a22bebef149e1eef (совпадает с текущим HEAD, рабочая копия детачнута на нём). Трек: инфраструктурный (метки bug, P2, infra, S7-code-review; без продуктового S*).

Скоуп

Один коммит, диапазон origin/dev..HEAD = сам коммит 978035c4. Класс B целиком: .github/workflows/performance.yml, scripts/mutation-registry.mjs, demo/performance/README.md, плюс новые scripts/performance-baseline.mjs и test/performance-baseline.test.mjs. Продуктовый код (src/**, custom_components/**) не тронут — подтверждено git diff origin/dev...HEAD --stat.

Проблема из issue: гейт «Полных бенчмарков» для кандидата стабильного релиза сравнивал линейку саму с собой, потому что performance.yml запускается только на push в main, а базой относительного сравнения брался «предыдущий push в main» — то есть первый коммит той же релизной линейки. Наблюдалось вживую на v1.76.0: честный красный прогон 35097102695 на c3d64789 и зелёный 35109734519 на 9683a590 независимо от правки бюджетов.

Решение: выбор базы вынесен из shell в scripts/performance-baseline.mjs; стабильный кандидат (head несёт трейлер Release: vX.Y.Z без пре-релизного суффикса) сравнивается с предыдущим стабильным тегом, остальные случаи (бета, обычный push, ручной comparison_ref) — без изменений.

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

Дешёвые гейты для этого SHA уже подтверждены зелёным прогоном Validate (системная метка ревью ссылается на 35135781823; сам исполнитель в хендоффе называет соседний прогон 35135276439 — оба независимо проверены мной, headSha обоих совпадает с материалом ревью, conclusion: success): npx tsc --noEmit, npm test, npm run build не перегонялись повторно.

Что прогнал сам, потому что это точечные, дешёвые и по существу задачи проверки:

Гейт	Команда	Результат
Целевой тест AC2	node --test --test-name-pattern="AC2" test/performance-baseline.test.mjs	зелёный, 1/1
Мутационный гейт по задаче	node scripts/mutation-gate.mjs --id=stable-candidate-compares-against-itself	«заявленный тест покраснел на мутанте» — оба шага (чистый прогон, прогон с мутацией) зелёные в ожидаемом смысле
Реестр мутантов (целостность якорей)	node scripts/mutation-gate.mjs --check	прошёл, новый якорь stable-candidate-compares-against-itself в списке
Выбор смоков по дифу	node scripts/smoke-select.mjs --base origin/dev --head HEAD	«Исполняемого frontend-диффа нет… браузерные смоки этим диффом не выбираются» — src/** не тронут, смоки не нужны

Не прогонял и почему:

• npm test целиком, npx tsc --noEmit, npm run build со сверкой бандла — уже зелёные на этом SHA (см. выше), диф не в src/**/custom_components/**, перегонять нет смысла;
• node scripts/check-docs.mjs — диф не трогает src/**, отпечаток скриншотов документации им не задет; исполнитель называет тот же известный устаревший отпечаток (#586), не относящийся к этой задаче;
• инварианты модели (npm run invariants) — диф не касается геометрии, стен, layout, marker.space, open_spans;
• npm run golden:verify — диф не меняет рендер/геометрию/стили;
• python -m pytest tests_backend -q — custom_components/**/*.py не тронут;
• performance-профили (реальный прогон бенчмарков) — задача меняет только то, с чем сравнивается прогон, а не измеряемый код; AC не требуют реального запуска performance.yml, только доказательства выбора базы, что и покрывают юнит-тесты с инжектированным git.

Дополнительно прочитал код инвариантов, на которые опирается AC4 (см. ниже), и сверил регекспы/веточную логику scripts/performance-baseline.mjs построчно против старой shell-реализации, которую он заменяет.

Разбор по AC (issue #587)

AC1 — «Push в main коммита с трейлером Release: vX.Y.Z (стабильный) берёт базой предыдущий стабильный тег; это видно в сводке прогона.» Доказано тестом AC1: кандидат стабильного релиза судится о предыдущий стабильный тег (test/performance-baseline.test.mjs:43) — прогнан лично, зелёный. Видимость в сводке: resolveComparisonBase возвращает source, запись в $GITHUB_STEP_SUMMARY не убрана (scripts/performance-baseline.mjs:206-208), проверено чтением — идентична строке из прежней shell-версии. Чем краснеет: тот же мутант, что и AC2 (see below) убирает ветку previous-stable целиком — тест AC1 тоже упал бы на этом мутанте (не зарегистрирован отдельно, но код-путь общий: см. «Защитный AC» ниже).

AC2 — «Два коммита подряд одной линейки не могут дать зелёный гейт "сам с собой" — второй сравнивается с тем же стабильным тегом, что и первый.» Доказано тестом AC2 — прогнан лично, зелёный. Защитный AC доказан таблицей:

AC	чем доказан	чем краснеет
AC2	node --test --test-name-pattern="AC2" test/performance-baseline.test.mjs	мутант stable-candidate-compares-against-itself (scripts/mutation-registry.mjs) убирает условие входа в ветку «предыдущий стабильный тег»; прогнал лично node scripts/mutation-gate.mjs --id=stable-candidate-compares-against-itself → «заявленный тест покраснел на мутанте»

Для чистого юнита (не дорогой гейт — смок/бэкенд/golden) прогона со снятой защитой достаточно по §2.7; сделано.

AC3 — «Бета-кандидат и обычный push сохраняют текущую базу.» Доказано тестом AC3 (тот же файл) — три под-случая (бета с трейлером, push без трейлера, workflow_dispatch без comparison_ref), прогнан лично, зелёный.

AC4 — «Заодно проверить, не опирается ли на "родителя" ещё какой-нибудь релизный гейт.» Разобрано чтением, не исполнением:

• validate.yml использует github.event.before только как FALLBACK для диапазона классифицируемых файлов (base/range_base, строки 240–295, 606, 700); действующая база диапазона — «самый новый предок с успешно завершённым Validate» (#387/#388), не сырой родитель — комментарий на строке 249 прямо это фиксирует как урок предыдущего инцидента (#86 r5). Циклическая проблема из #587 к этой логике неприменима: она не измеряет релизную линейку саму собой, а определяет объём проверки файлов.
• scripts/release-gate.mjs, release-contract.mjs, release-membership.mjs, release-bookkeeping.mjs, e2e-gate.mjs — grep на parent|HEAD^|event.before ничего не находит; docs/DEVELOPMENT.md:485-504 подтверждает, что release.yml резолвит кандидата по точному тегу/SHA и требует полное доказательство Validate для этого SHA, родителя не касаясь. Вывод соответствует утверждению исполнителя; независимая проверка грепом и чтением документации его подтверждает.

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

• Регекспы releaseTrailerTag/isStableTag соответствуют существующей конвенции трейлера Release: в scripts/classify-changes.mjs и scripts/process-gate.mjs — это не новый придуманный контракт, а уже используемый в кодовой базе (docs/DEVELOPMENT.md:489: стабильный релиз обязан нести Release: <tag> на кандидате).
• previousStableTag корректно исключает и тег кандидата (по номеру версии, independent от текущего SHA — для случая «ремонт уже выпущенного релиза»), и тег, стоящий на самой голове; оба случая покрыты тестом «свой тег и тег на голове предыдущим не считаются».
• Порядок отказов (недоступная база → родитель → последний достижимый релизный тег → ошибка) сохранён из старой shell-реализации построчно; тесты «непригодная база уводит...», «без предыдущего тега...», «база старше HP-PERF-01...» подтверждают эквивалентность поведения не-стабильных путей.
• Тест workflow берёт базу из скрипта... (test/performance-baseline.test.mjs:161) проверяет сам YAML-файл: вызов scripts/performance-baseline.mjs на месте, HEAD_MESSAGE прокинут, старая shell-развилка (source="candidate parent") удалена целиком — защищает именно от риска «два места принятия одного решения расходятся», названного исполнителем.
• Коммит несёт корректные трейлеры Issue: #587, User-Visible: no — продуктовое поведение не меняется, changelog не требуется.
• demo/performance/README.md обновлён в том же коммите — документация подсистемы синхронна с кодом.
• Одно число — один источник: изменение не вводит новых пользовательских величин, дублирования нет (CI-внутренний артефакт, не UI).

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

• Реальный прогон performance.yml на push в main с трейлером Release: — недоступно из код-ревью (нет push-события), поведение доказано юнит-тестами с инжектированным git, а не сквозным прогоном workflow. Риск невысокий: единственная переменная, которую тесты не видят, — фактический формат github.event.head_commit.message, но он прокидывается без обработки как строка, и releaseTrailerTag уже проверен на многострочных сообщениях с трейлерами после текста коммита (тест «трейлер релиза читается...»).
• npm run mutation-gate полный прогон всех ~760 мутантов (гонял только --check для целостности реестра и точечный --id для новой записи) — полный прогон не относится к дельте этой задачи и дорог.

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

• SHA: 978035c49193a7d367d3dba6a22bebef149e1eef
• Дерево: git rev-parse HEAD^{tree} на момент ревью соответствует рабочей копии (детач на материале, без локальных изменений — git status чист).

---

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

• Ветка: issue/587-stable-perf-baseline, коммит 978035c49193 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 8f80b742f032f854e5b54f5184adb4c8daa52cc1

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

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