fix: fall back to sync call when summarizing with an LLM without acall

[jeon-jihyeon]

Related issue

Fixes #7859

Summary

#7815 made summarization always go through await llm.acall(...). BaseLLM.acall raises NotImplementedError by default, so custom LLMs that only implement call now crash when the context window overflows.

This catches NotImplementedError from acall in _summarize_one and runs llm.call via asyncio.to_thread instead. The fallback sits inside the existing try, so the context-length retry still works. The parallel summarization from #7815 is unchanged, and the Bedrock provider already uses the same to_thread pattern.

Verification

• Tests added or updated for the changed behavior
• Relevant tests and quality checks pass locally

Added TestParallelSummarization::test_falls_back_to_call_when_acall_not_implemented, which summarizes two chunks with a call-only BaseLLM.

before: 1 failed (NotImplementedError from base_llm.py acall)
after:  1 passed
tests/utilities/test_agent_utils.py + test_summarize_integration.py: 116 passed, 3 failed

The 3 failures are the Anthropic, Gemini, and Azure integration tests. They fail the same way without this change, because I don't have those optional extras installed locally.

ruff check, ruff format --check, and mypy on agent_utils.py all pass.

Repro script from #7859:

main @ 8078f91:  FAILED: NotImplementedError NotImplementedError()
this branch:     OK: True

Additional context

This PR should get the llm-generated label per CONTRIBUTING, but I can't add labels as an outside contributor. I used Claude Code to help find the bug and draft the fix. I reviewed the diff and ran the repro and tests myself.

---

Note

Low Risk
Localized change to context-overflow summarization with class-level detection to avoid double-calling LLMs that implement async explicitly.

Overview
Fixes context-window summarization crashing for custom BaseLLM implementations that only override call, after parallel summarization started always using acall.

In _summarize_one, the code now checks whether the LLM class still uses the default BaseLLM.acall. If so, it runs llm.call via asyncio.to_thread (with the same callbacks and summary prompt); otherwise it keeps await llm.acall. Parallel chunk summarization and context-length retry behavior are unchanged.

Tests cover the sync-only path (two chunks, callbacks forwarded) and confirm that an LLM with a real overridden acall that raises does not silently fall back to call.

^{Reviewed by Cursor Bugbot for commit 5f932b9. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5649a2c6-a9f1-4b26-99dd-62aee3503546

📥 Commits

Reviewing files that changed from the base of the PR and between bbc3114 and 5f932b9.

📒 Files selected for processing (2)

• lib/crewai/src/crewai/utilities/agent_utils.py
• lib/crewai/tests/utilities/test_agent_utils.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Message summarization uses llm.call in a worker thread when the LLM type inherits BaseLLM.acall unchanged. Otherwise, it awaits llm.acall. Tests cover both paths and callback forwarding.

Changes

Message summarization

Layer / File(s)	Summary
LLM call selection and regression coverage lib/crewai/src/crewai/utilities/agent_utils.py, lib/crewai/tests/utilities/test_agent_utils.py	_summarize_one builds the prompt once. It uses asyncio.to_thread to call llm.call when the LLM type inherits BaseLLM.acall unchanged; otherwise, it awaits llm.acall. Tests verify callback forwarding for synchronous calls and verify that NotImplementedError from an overridden acall propagates without calling llm.call.

Suggested reviewers: joaomdmoura

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 5f932

Sync-only custom LLMs can use message summarization without an identified merge-blocking risk.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5f932

The change restores compatibility while preserving existing asynchronous behavior. However, synchronous calls now run concurrently against the same client and callbacks, which may affect extensions with mutable state. No concrete security exploit was established.

Retained concerns

• Low · reliability · inferred: Synchronous-only extensions acquire an implicit thread-safety requirement: multiple chunks invoke the same client and callback objects concurrently without serialization or isolated ownership. Extensions using mutable per-request state may race and compromise summary integrity or callback-state consistency. This is a bounded contract and failure-containment concern, not a demonstrated security vulnerability.

Security review details

Security Blast Radius

• inferred — Conversation size influences chunk count and therefore the number of synchronous fallback requests. The directly evidenced scope is the supplied client instance, its callbacks and the conversation being summarized. The inspected flow does not establish cross-tenant access or additional tool authority.

Trust Boundaries and Controls

• observed — Fallback selection depends on the configured client's class method, not conversation content or an exception from an overridden asynchronous method. This preserves the asynchronous override's failure behavior instead of silently routing around it through a second request.

Resilience and Maintainability Implications

• observed — The existing context-length recovery advances through three estimation levels and ultimately raises if summarization still cannot fit. Failed collection does not reach history replacement. These controls bound retry depth and local history mutation, but do not provide rollback or deduplication for custom-client and callback side effects.

Hardening Proposals

• proposed — Make concurrency ownership explicit for synchronous-only extensions. Serialize fallback calls unless the client and callbacks support concurrent use, or define an explicit concurrency capability contract.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: falling back to synchronous call when acall is not implemented.
Description check	✅ Passed	The description includes the related issue, explains the problem and fix, reports tests and quality checks, and provides relevant additional context. It also discloses the three integration test failu…
Linked Issues check	✅ Passed	The PR meets the coding requirement in directly linked issue #7859. _summarize_one uses asyncio.to_thread to call llm.call when the LLM class inherits the default BaseLLM.acall; it uses `llm.a…
Out of Scope Changes check	✅ Passed	The implementation and tests only address sync-only custom LLMs during summarization, the behavior requested by #7859. The test for an overridden acall also verifies the boundary of that fallback. N…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[VANDRANKI]

Community review. This does not clear the merge gate.

I checked out the branch and ran tests/utilities/test_agent_utils.py -k summar with --noconftest -o addopts="". On the PR: 30 passed, 2 failed (TestParallelSummarizationVCR::test_parallel_summarize_openai and ..._preserves_files). On base, the same 2 fail (29 passed), so those are environment or cassette issues, not this change. I also put the PR's new test on the base source: test_falls_back_to_call_when_acall_not_implemented fails there and passes on the PR, so it is a real regression test.

I confirmed the cause: BaseLLM.acall in src/crewai/llms/base_llm.py is a bare raise NotImplementedError, so a custom LLM that only implements call fails in _summarize_one. The fallback via asyncio.to_thread(llm.call, ...) is the right shape, and to_thread copies the context vars, so callbacks that read them still work.

Things to consider:

1. The same gap exists in other places that call acall directly. From a grep: agent_utils.py line 633 (aget_llm_response) and utilities/converter.py lines 128 and 133 both call self.llm.acall(...) with no fallback. A custom sync-only LLM will still fail there. If the intent is "sync-only custom LLMs work", a small shared helper (try acall, on NotImplementedError fall back to a thread) would cover all three.

2. except NotImplementedError is broad. If a provider's real acall raises NotImplementedError from somewhere deeper (an unsupported feature in a subclass, for example) after it has already sent the request, the fallback sends a second request. That doubles cost and could repeat side effects. I did not find an instance of that in the repo, so it is a hypothetical, but checking that type(llm).acall is BaseLLM.acall before falling back would make the intent exact.

3. Chunks are summarized concurrently via asyncio.gather, so now N threads call the same llm.call instance at once. Many sync LLM wrappers keep per-instance state (token counters, last-response fields). I did not check which ones do. A short comment, or a bound on parallelism, would help.

4. In the test, SyncOnlyLLM.call ignores callbacks. Passing a callback and asserting it is invoked once per chunk would pin the callbacks=self.callbacks argument on the fallback path.

[jeon-jihyeon]

Thanks for the thorough review, and for checking the new test against base!

• (2) Good call. Switched to getattr(type(llm), "acall", None) is BaseLLM.acall in 5f932b9, so a real acall that raises NotImplementedError now propagates instead of triggering a second request. Added a test for that.
• (4) The fallback test now passes a callback list and asserts each chunk's call receives it.
• (1) Agreed that aget_llm_response and converter.py have the same gap. I'd rather keep this PR scoped to the summarizer and follow up with a shared helper if the maintainers want that direction.
• (3) Fair point. Previously sync-only LLMs couldn't summarize at all, so I kept the existing asyncio.gather concurrency rather than special-casing them. Happy to add a bound if maintainers prefer.