Repository navigation
fix: apply reviewed maintenance repairs for #2930 - #3696
kovtcharov wants to merge 1 commit into
Conversation
Changed: .github/workflows/email_scorecard_refresh.yml; hub/agents/email/python/packaging/assert_llm_coverage.py; hub/agents/email/python/tests/test_assert_llm_coverage.py; tests/unit/eval/test_email_refresh_timeouts.py.
Verdict: Approve with suggestionsThis makes the email scorecard-refresh workflow size its own timeouts from the dispatch inputs, so a repeat run ( Three things worth fixing before merge, none blocking:
Real-world evidenceN/A — CI configuration and tests only; no 🔍 Technical details🟡 Important1. Re-derives the The workflow defines
2. No upper bound on the derived budget ( The guard rejects Pick whatever ceiling matches intent — the point is that there be one. 🟢 Minor3. New error isn't actionable ( CLAUDE.md's "Actionable errors name three things" (what failed / what to do / where to look) — and the two sibling branches at lines 88 and 112 all do. This one names neither the offending value nor the file. The summary line also drops the ❌ its two siblings use, so the job summary renders inconsistently. The existing assertion ( 4. Non-numeric input still throws a raw traceback (
5. Stale comment ( "Keep BOTH values BELOW the step's own timeout-minutes (210 full / 45 subset)" — those are now per-experiment multiplicands, not the step's timeout. Still true as a floor, but the parenthetical no longer describes what the step is set to. Strengths
|
Follow-up fixes for #2930. Changed: .github/workflows/email_scorecard_refresh.yml; hub/agents/email/python/packaging/assert_llm_coverage.py; hub/agents/email/python/tests/test_assert_llm_coverage.py; tests/unit/eval/test_email_refresh_timeouts.py.
This PR targets the original feature branch because the current account cannot push to
amd/gaia. It carries the prepared maintenance changes for that PR.Test plan