Repository navigation
fix: retry CrewAI's first-call empty completion before failing - #624
Conversation
…427) CrewAI's native OpenAI tool-calling path can return an empty completion (finish_reason='stop', no content, no tool_calls) on a turn's very first LLM call, before any tool has run. That's indistinguishable from a real provider outage, so the adapter always failed the delivery outright -- even though nothing ran yet this turn, so a single retry duplicates nothing and often recovers the turn. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fw8AZwnoa3Fen6HBhHEQ7H
band.adapters.crewai is already safely imported at module scope in this file (it defers its own crewai imports to TYPE_CHECKING/function-local), so importlib.import_module inside the test just re-fetched the same cached module object for no reason. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fw8AZwnoa3Fen6HBhHEQ7H
AlexanderZ-Band
left a comment
AlexanderZ-Band
left a comment
There was a problem hiding this comment.
Automated multi-lens review (my-code-review, 6 groups: Style & Hygiene, Design & Reuse, Runtime Safety, Test Quality, Simplification, Logical Bugs; --fix).
3 inline comments below. One additional finding couldn't be placed inline (it's on pre-existing code outside this diff's hunks, per GitHub's line-anchoring rules):
suggestion: src/band/adapters/crewai.py:423 (and the debug log at :428) -- when the retry is exhausted, logger.error("Error processing message: %s", e, exc_info=True) logs the same text CrewAI would raise on a plain first-call failure, with no attempt count. The only other evidence a retry happened is the logger.info at line 500, which fires before the attempt and isn't tied to the eventual outcome -- so a log pipeline filtered to WARNING/ERROR can't tell, from line 423 alone, whether a delivery failed after one attempt or after the retry fired and failed again.
No blocking findings. Logical Bugs (Group F) built 3 targeted repros against the exact head SHA (retry-cap off-by-one, tool-runs-mid-first-call, exception-type confusion) -- none reproduced. Design & Reuse (Group B) confirmed the retry-loop shape matches this repo's existing precedent for small bounded retries (gemini.py, codex/rpc_base.py, crewai_flow.py).
Code review findings on PR #624: - The retry loop's while/attempt-counter machinery was sized for a general N-retry problem the ticket doesn't have (bound is fixed at 1) -- collapsed to a single inline try/except. The now-unused _EMPTY_RESPONSE_FIRST_CALL_RETRIES constant and the comment restating the docstring's rationale go with it. - The retry's own failure had no distinct log line, so an operator filtering at WARNING+ couldn't tell a delivery failed after one attempt from failing after the retry also came back empty -- added a warning log at the point the retry itself fails. - Test assertions on the resulting call count no longer reference a governing constant (removed above), so they stay literal with a short comment instead of drifting toward a fake single source of truth. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fw8AZwnoa3Fen6HBhHEQ7H
/simplify pass on PR #624: - The retry's second attempt logged "came back empty" unconditionally on any exception, not just another empty-response ValueError -- a real failure (timeout, auth, etc.) during the retry would be mislabeled. Now re-checks _is_empty_llm_response on the retry's own exception before choosing the log wording. - The new test's manual `calls` counter duplicated state the mock already tracks (kickoff_async.call_count); reads that instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fw8AZwnoa3Fen6HBhHEQ7H
2 participants
Summary
finish_reason='stop', no content, no tool_calls) on a turn's very first LLM call, before any tool has run — reproduced 8/8 times across local live runs, root-caused and filed upstream at crewAIInc/crewAI#7349.CrewAIAdapteralways failed the whole delivery — even though nothing ran yet this turn, so nothing is duplicated by retrying._EMPTY_RESPONSE_FIRST_CALL_RETRIES = 1) via a new_kickoff_with_empty_response_retryhelper, only for_is_empty_llm_response(e) and not reply_tracker.any_tool_ran. Any other failure, or exhausting the retry, still reports the error and raises exactly as before.any_tool_ranswallow logic (PR fix: treat CrewAI's empty final answer as a finished turn, not a failure #621).Traced from last night's failed nightly E2E run (34307490909) → root-caused → planned and independently fact-verified → implemented, per INT-1427 (plan attached to the ticket).
Test plan
uv run --extra dev-crewai pytest tests/adapters/test_crewai_adapter.py -v— 82 passedtest_empty_answer_with_no_tool_call_still_raisesto assert the retry happens (kickoff_async.call_count == 2) before the final raisetest_empty_first_call_recovers_on_retry: first call empty, second call replies — turn completes, no error eventuv run ruff check ./uv run ruff format .— cleanuv run pyrefly check— 0 errorsuv run pytest tests/ --ignore=tests/integration/ --ignore=tests/e2e/— 5590 passed, 146 skipped🤖 Generated with Claude Code
https://claude.ai/code/session_01Fw8AZwnoa3Fen6HBhHEQ7H