feat: add language-aware system prompt routing for multi-model agents#6491
feat: add language-aware system prompt routing for multi-model agents#6491ashusnapx wants to merge 1 commit into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds language-aware system prompt translation for agents and async kickoff support for awaitable callbacks. It introduces model profiles, prompt translation utilities, agent prompt-translation settings, async kickoff preparation, and tests for the new prompt and kickoff flows. ChangesPrompt translation feature
Async kickoff callback handling
Sequence Diagram(s)sequenceDiagram
participant Agent
participant _build_execution_prompt
participant optimize_system_prompt
participant get_model_profile
participant translate_text
Agent->>_build_execution_prompt: build execution prompt
_build_execution_prompt->>_build_execution_prompt: check auto_translate_prompt and llm.model
_build_execution_prompt->>optimize_system_prompt: prompt text, model name, glossary
optimize_system_prompt->>optimize_system_prompt: detect_language(prompt)
optimize_system_prompt->>get_model_profile: model_name
get_model_profile-->>optimize_system_prompt: ModelProfile or None
optimize_system_prompt->>optimize_system_prompt: estimate token savings
optimize_system_prompt->>translate_text: protected text, source_lang, target_lang, glossary
translate_text-->>optimize_system_prompt: translated text
optimize_system_prompt-->>_build_execution_prompt: optimized prompt
_build_execution_prompt-->>Agent: final prompt result
sequenceDiagram
participant Crew.akickoff
participant aprepare_kickoff
participant before_kickoff_callbacks
participant after_kickoff_callbacks
Crew.akickoff->>aprepare_kickoff: kickoff inputs
aprepare_kickoff->>before_kickoff_callbacks: invoke callbacks
before_kickoff_callbacks-->>aprepare_kickoff: awaitable or value
aprepare_kickoff->>aprepare_kickoff: await awaitable results
aprepare_kickoff-->>Crew.akickoff: prepared inputs
Crew.akickoff->>after_kickoff_callbacks: run callbacks
after_kickoff_callbacks-->>Crew.akickoff: awaitable or value
Crew.akickoff->>Crew.akickoff: await awaitable results
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Requesting ReviewHey @caike @zeroasterisk @renatonitta @diegocn — would love your eyes on this PR for #6428 (Language-Aware System Prompt Routing). Quick Test# Install from this branch
pip install git+https://github.com/ashusnapx/crewAI@feat/language-aware-prompt-routing
# Test it out
from crewai import Agent
agent = Agent(
role="Researcher",
goal="你是一个专业的研究人员。请帮我找到关于AI代理的信息。",
backstory="你有十年的研究经验。",
llm="llama-3.1-70b-versatile", # LLaMA has poor Chinese support
)
# The Chinese prompt will be auto-translated to English before reaching LLaMA
# because LLaMA's tokenizer is ~50-65% less efficient for ChineseWhat to look for
Key files to review
Let me know if you have any questions or want changes! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
lib/crewai/tests/utilities/test_model_profiles.py (1)
44-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing parametrized coverage for
"o1","o3","o4","command", and"jamba"registry entries.Five of the sixteen registry entries have no test coverage in
test_known_models. Given the PR objective emphasizes model profile correctness, these should be included to prevent regressions if profiles are modified.💡 Suggested additions
("amazon.nova-pro-v1:0", "amazon", "en"), + ("o1-mini", "openai", "en"), + ("o3-mini", "openai", "en"), + ("o4-mini", "openai", "en"), + ("command-r-plus", "cohere", "en"), + ("jamba-1.5-large", "ai21", "en"), ], ids=[ "gpt-4o", @@ -78,6 +84,11 @@ "mixtral", "nova", + "o1-mini", + "o3-mini", + "o4-mini", + "command", + "jamba", ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/utilities/test_model_profiles.py` around lines 44 - 80, The parametrized coverage in test_known_models is missing several registry entries, so extend the existing model_name/expected_family/expected_lang cases to include the o1, o3, o4, command, and jamba profiles. Add them to the same pytest.mark.parametrize block and keep the ids list aligned so the model profile mapping is exercised through the existing test function.lib/crewai/src/crewai/utilities/prompt_translator.py (1)
44-44: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider bounding the translation cache size.
_TRANSLATION_CACHEis an unbounded module-level dict. For agents with fixed system prompts this is fine, but dynamic or templated prompts could cause unbounded memory growth in long-running processes. Consider using a size-limited cache (e.g.,collections.OrderedDictwithmove_to_end/popitem, orfunctools.lru_cachewrapping) to cap memory usage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/utilities/prompt_translator.py` at line 44, The module-level _TRANSLATION_CACHE in prompt_translator is unbounded and can grow indefinitely for dynamic prompts; replace it with a size-limited caching approach so memory use stays capped in long-running processes. Update the translation lookup/write path in the prompt translation logic to enforce eviction, using the existing _TRANSLATION_CACHE symbol as the entry point and keeping behavior the same for repeated prompt translations.lib/crewai/tests/utilities/test_prompt_translator.py (1)
157-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen
test_json_objectandtest_env_varassertions.Both tests only assert
isinstance(cleaned, str), which passes even if no extraction occurs. The JSON regex should match{"key": "value", "count": 42}and the env var regex should matchAPI_KEY=secret123. Assert the segments are actually extracted and the placeholders appear in the cleaned text.As per coding guidelines, tests should focus on behavior rather than implementation details — these assertions don't verify the extraction behavior.
♻️ Proposed stronger assertions
def test_json_object(self): text = 'Config: {"key": "value", "count": 42}' cleaned, segments = extract_untranslatable_segments(text) - # May match JSON or not depending on regex complexity - # At minimum, the text should be processable - assert isinstance(cleaned, str) + assert len(segments) == 1 + assert '{"key": "value", "count": 42}' in segments[0][1] + assert "__SEGMENT_0__" in cleaned + assert "Config:" in cleaned def test_env_var(self): text = "Set API_KEY=secret123 before running." cleaned, segments = extract_untranslatable_segments(text) - # ENV var pattern may or may not match - assert isinstance(cleaned, str) + assert len(segments) == 1 + assert "API_KEY=secret123" in segments[0][1] + assert "__SEGMENT_0__" in cleaned + assert "Set" in cleaned🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/utilities/test_prompt_translator.py` around lines 157 - 168, `test_json_object` and `test_env_var` in `test_prompt_translator.py` are too weak because they only check `cleaned` is a string and don’t verify extraction behavior. Update `extract_untranslatable_segments` expectations so `segments` contains the JSON object `{"key": "value", "count": 42}` and the env assignment `API_KEY=secret123`, and assert the returned `cleaned` text includes the corresponding placeholder markers. Use the existing test methods `test_json_object` and `test_env_var` to confirm both segment extraction and placeholder replacement, not just basic type output.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/agent/core.py`:
- Around line 1018-1036: The auto-translation block in _build_execution_prompt
should fail safe instead of letting prompt optimization break agent execution.
Wrap the optimize_system_prompt calls inside the auto_translate_prompt and
self.llm path in a try/except, using the existing prompt handling around
SystemPromptResult and prompt.prompt, and on any exception log a warning/error
and keep the original prompt values unchanged so execution can continue.
- Around line 1018-1036: The auto-translation path in the prompt handling logic
is currently a no-op because optimize_system_prompt is called without the
resolved LLM context. Update the prompt translation flow in the agent core
method that handles SystemPromptResult and regular prompts so each
optimize_system_prompt invocation receives self.llm (the same resolved LLM
already checked by auto_translate_prompt) along with the prompt text,
model_name, and prompt_glossary. Ensure both prompt.system/prompt.user and
prompt.prompt branches thread the LLM through consistently.
In `@lib/crewai/src/crewai/utilities/model_profiles.py`:
- Around line 64-84: The model profile registry entries for ModelProfile in
model_profiles.py use short keys like "o1", "o3", and "o4", which can cause
false-positive matches in get_model_profile because it relies on substring
checks. Update the matching logic in get_model_profile to be boundary-aware (for
example, only match at the start of the model name or after a separator such as
"/" or "-"), or make the registry keys more specific so they do not match
unrelated model names. Keep the OpenAI profiles intact while ensuring only
intended model identifiers resolve to those entries.
In `@lib/crewai/src/crewai/utilities/prompt_translator.py`:
- Around line 200-201: The compiled patterns `_XML_TAG_RE` and `_MD_HEADING_RE`
are currently dead code in `extract_untranslatable_segments`. Either wire them
into the segment-extraction flow so XML tags and markdown headings are preserved
during translation, or remove both definitions and any related references if
they are not needed. If you keep `_XML_TAG_RE`, tighten its matching behavior
before using it so it does not accidentally capture comparison operators in
normal text.
- Around line 314-317: The translation cache in `prompt_translator.py` is keyed
only by `text` and the language pair, so it can return stale results when
different glossaries are used. Update the cache lookup in the translation flow
to include the glossary (or a stable representation of it) in `_cache_key`, and
make sure the same glossary input produces the same key while different
glossaries produce different keys. Keep the change localized around the cache
access and any helper used by `translate`/`_cache_key` so translations remain
correct across agents with different glossary terms.
In `@lib/crewai/tests/utilities/test_prompt_translator_integration.py`:
- Around line 148-165: The test in test_system_prompt_result_optimized is too
weak because it only checks that the returned prompt exists, not that
optimize_system_prompt actually changed the content. Update the assertions on
the Agent._build_execution_prompt result so they verify the optimized
SystemPromptResult fields carry the "optimized:" prefix (for both system and
user where applicable), using the mocked optimize_system_prompt behavior to
validate the translation flow rather than just non-null output.
---
Nitpick comments:
In `@lib/crewai/src/crewai/utilities/prompt_translator.py`:
- Line 44: The module-level _TRANSLATION_CACHE in prompt_translator is unbounded
and can grow indefinitely for dynamic prompts; replace it with a size-limited
caching approach so memory use stays capped in long-running processes. Update
the translation lookup/write path in the prompt translation logic to enforce
eviction, using the existing _TRANSLATION_CACHE symbol as the entry point and
keeping behavior the same for repeated prompt translations.
In `@lib/crewai/tests/utilities/test_model_profiles.py`:
- Around line 44-80: The parametrized coverage in test_known_models is missing
several registry entries, so extend the existing
model_name/expected_family/expected_lang cases to include the o1, o3, o4,
command, and jamba profiles. Add them to the same pytest.mark.parametrize block
and keep the ids list aligned so the model profile mapping is exercised through
the existing test function.
In `@lib/crewai/tests/utilities/test_prompt_translator.py`:
- Around line 157-168: `test_json_object` and `test_env_var` in
`test_prompt_translator.py` are too weak because they only check `cleaned` is a
string and don’t verify extraction behavior. Update
`extract_untranslatable_segments` expectations so `segments` contains the JSON
object `{"key": "value", "count": 42}` and the env assignment
`API_KEY=secret123`, and assert the returned `cleaned` text includes the
corresponding placeholder markers. Use the existing test methods
`test_json_object` and `test_env_var` to confirm both segment extraction and
placeholder replacement, not just basic type output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1998c5b2-67f3-4ba5-9d58-c51ba0ec4f41
📒 Files selected for processing (7)
lib/crewai/src/crewai/agent/core.pylib/crewai/src/crewai/agents/agent_builder/base_agent.pylib/crewai/src/crewai/utilities/model_profiles.pylib/crewai/src/crewai/utilities/prompt_translator.pylib/crewai/tests/utilities/test_model_profiles.pylib/crewai/tests/utilities/test_prompt_translator.pylib/crewai/tests/utilities/test_prompt_translator_integration.py
| @patch("crewai.utilities.prompt_translator.optimize_system_prompt") | ||
| def test_system_prompt_result_optimized(self, mock_optimize): | ||
| """When SystemPromptResult is returned, both system and user should be optimized.""" | ||
| mock_optimize.side_effect = lambda prompt, model, glossary=None: f"optimized:{prompt}" | ||
|
|
||
| agent = Agent( | ||
| role="test", | ||
| goal="test", | ||
| backstory="test", | ||
| llm="gpt-4o", | ||
| auto_translate_prompt=True, | ||
| use_system_prompt=True, | ||
| ) | ||
|
|
||
| prompt, _, _ = agent._build_execution_prompt([]) | ||
|
|
||
| # The prompt should have been optimized | ||
| assert prompt is not None |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Weak assertion doesn't verify the behavior described by the test.
test_system_prompt_result_optimized mocks optimize_system_prompt to prefix output with "optimized:", but only asserts prompt is not None — it never checks that prompt.system/prompt.user actually carry the "optimized:" prefix. As written, this test would pass even if the translation call were removed entirely.
As per path instructions, **/tests/**/*.py: "Write unit tests for new functionality that focus on behavior rather than implementation details."
✅ Proposed fix
prompt, _, _ = agent._build_execution_prompt([])
- # The prompt should have been optimized
- assert prompt is not None
+ # Both system and user segments should have been optimized
+ assert prompt.system is not None and prompt.system.startswith("optimized:")
+ assert prompt.user is None or prompt.user.startswith("optimized:")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @patch("crewai.utilities.prompt_translator.optimize_system_prompt") | |
| def test_system_prompt_result_optimized(self, mock_optimize): | |
| """When SystemPromptResult is returned, both system and user should be optimized.""" | |
| mock_optimize.side_effect = lambda prompt, model, glossary=None: f"optimized:{prompt}" | |
| agent = Agent( | |
| role="test", | |
| goal="test", | |
| backstory="test", | |
| llm="gpt-4o", | |
| auto_translate_prompt=True, | |
| use_system_prompt=True, | |
| ) | |
| prompt, _, _ = agent._build_execution_prompt([]) | |
| # The prompt should have been optimized | |
| assert prompt is not None | |
| `@patch`("crewai.utilities.prompt_translator.optimize_system_prompt") | |
| def test_system_prompt_result_optimized(self, mock_optimize): | |
| """When SystemPromptResult is returned, both system and user should be optimized.""" | |
| mock_optimize.side_effect = lambda prompt, model, glossary=None: f"optimized:{prompt}" | |
| agent = Agent( | |
| role="test", | |
| goal="test", | |
| backstory="test", | |
| llm="gpt-4o", | |
| auto_translate_prompt=True, | |
| use_system_prompt=True, | |
| ) | |
| prompt, _, _ = agent._build_execution_prompt([]) | |
| # Both system and user segments should have been optimized | |
| assert prompt.system is not None and prompt.system.startswith("optimized:") | |
| assert prompt.user is None or prompt.user.startswith("optimized:") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai/tests/utilities/test_prompt_translator_integration.py` around
lines 148 - 165, The test in test_system_prompt_result_optimized is too weak
because it only checks that the returned prompt exists, not that
optimize_system_prompt actually changed the content. Update the assertions on
the Agent._build_execution_prompt result so they verify the optimized
SystemPromptResult fields carry the "optimized:" prefix (for both system and
user where applicable), using the mocked optimize_system_prompt behavior to
validate the translation flow rather than just non-null output.
Source: Path instructions
|
@ashusnapx impressive work — 80 tests with clean module separation in under 48 hours is solid. A few observations from reading through the implementation: Design boundary: The original proposal (#6428) explicitly scoped translation to system prompts only. In core.py, optimize_system_prompt is being applied to both prompt.user and prompt.prompt as well. User messages carry intent — machine-translating them risks altering meaning in ways that are hard to detect. The PR description says "system prompt only" but the code doesn't enforce this boundary. This is the one thing I'd flag before merge. Token estimation: The target_profile construction in optimize_system_prompt (swapping english/non-english ratios) works directionally but the assumption breaks for en→non-en translations. Not a blocker — the heuristic ordering (which language costs more tokens) is still correct, which is all the decision gate needs. Unicode approach: Using script ranges instead of pulling in a full NLP library was the right call for a zero-dependency MVP. The CJK ambiguity handling (checking for kana to distinguish zh vs ja) is exactly the right pattern. Happy to help review further if the maintainers want a second pass. I wrote the original proposal so I can speak to design intent. |
- core.py: wrap optimize_system_prompt in try/except for fail-safe behavior (log and keep original prompt on error) - core.py: pass self.llm as llm_caller so translation actually fires - core.py: only translate system prompt (not user messages) per original design scope - model_profiles.py: replace substring matching with boundary-aware regex to prevent false positives on short keys (o1, o3, o4) - model_profiles.py: normalize separators (./-/_ all equivalent) for robust matching across provider naming conventions - prompt_translator.py: remove dead code (_XML_TAG_RE, _MD_HEADING_RE) - prompt_translator.py: include glossary in cache key to prevent stale translations across different glossaries - tests: strengthen assertion in test_system_prompt_result_optimized to actually verify the prefix is applied - tests: add test_no_false_positive_on_unrelated_names for boundary match - tests: add test_o1_matches_with_boundary for o1 pattern validation Addresses feedback from CodeRabbit and @YuhaoLin2005 on PR crewAIInc#6491
Feedback AddressedAll review feedback has been fixed and pushed. Here's what changed: @coderabbitai feedback
@YuhaoLin2005 feedback
Fixed. The code now only translates Test ResultsReady for re-review. @caike @zeroasterisk @renatonitta @diegocn |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
cd /tmp
gh pr view 6491 --repo crewAIInc/crewAI --json commits,files -q '.files[].path' 2>/dev/null || trueLength of output: 640 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr diff 6491 --repo crewAIInc/crewAI -- lib/crewai/src/crewai/agent/core.py
echo "-----"
gh pr diff 6491 --repo crewAIInc/crewAI -- lib/crewai/src/crewai/utilities/prompt_translator.py | head -300Length of output: 574 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr diff 6491 --repo crewAIInc/crewAI > /tmp/pr6491.diff
wc -l /tmp/pr6491.diff
echo "---- core.py section ----"
awk '/^diff --git a\/lib\/crewai\/src\/crewai\/agent\/core.py/,/^diff --git a\/lib\/crewai\/src\/crewai\/agents/' /tmp/pr6491.diff
echo "---- prompt_translator.py section ----"
awk '/^diff --git a\/lib\/crewai\/src\/crewai\/utilities\/prompt_translator.py/,0' /tmp/pr6491.diff | head -400Length of output: 16743 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '400,481p' /tmp/pr6491.diff
echo "==== continue after 300 lines ===="
awk '/^diff --git a\/lib\/crewai\/src\/crewai\/utilities\/prompt_translator.py/,0' /tmp/pr6491.diff | sed -n '300,500p'
echo "==== model_profiles.py diff ===="
awk '/^diff --git a\/lib\/crewai\/src\/crewai\/utilities\/model_profiles.py/,/^diff --git a\/lib\/crewai\/tests/' /tmp/pr6491.diffLength of output: 34845
Nice work addressing all of these cleanly. One minor thing worth a follow-up look (not blocking): in Looks solid overall — will let the human maintainers ( |
Bonus fix: token savings estimation@coderabbitai flagged that the "estimated savings" log could be misleading in edge cases. Fixed by rewriting the heuristic. Before: Swapped After: Maps source-language token counts to their target-language equivalents. After translation, the text composition inverts: Chinese chars become English words and vice versa. 82 tests passing, lint clean, mypy clean. Ready for final review. @caike @zeroasterisk @renatonitta @diegocn |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/crewai/src/crewai/utilities/prompt_translator.py (1)
347-366: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFailed translations are cached, preventing future retries.
_TRANSLATION_CACHE[key] = translatedexecutes unconditionally — even when the LLM call raises an exception, returns a non-string/empty response, or nollm_calleris provided. In all these casestranslatedremains the originaltext, so the untranslated fallback is cached. A transient LLM failure (network hiccup, rate limit) thus permanently suppresses translation for that prompt until the cache is cleared or the process restarts. In multi-agent setups, the first failure blocks all subsequent agents sharing the same prompt.🔒 Proposed fix: only cache successful translations
translated = text # fallback if llm_caller is not None: try: messages = [{"role": "user", "content": prompt}] response = llm_caller.call(messages) if isinstance(response, str) and response.strip(): translated = response.strip() + _TRANSLATION_CACHE[key] = translated except Exception: logger.debug( "Translation LLM call failed; returning original text.", exc_info=True, ) else: # No LLM available — return original (safe fallback) logger.debug( "No LLM caller provided for translation; returning original text." ) - _TRANSLATION_CACHE[key] = translated return translated🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/utilities/prompt_translator.py` around lines 347 - 366, The prompt translation logic in prompt_translator should not cache fallback text when translation fails or no LLM is available. Update the translation flow around llm_caller.call and the _TRANSLATION_CACHE write so that only a valid, non-empty translated result is stored; if the call raises, returns an invalid response, or llm_caller is None, return the original text without populating the cache. Use the existing translated variable and _TRANSLATION_CACHE key handling to keep the fix localized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/crewai/src/crewai/utilities/prompt_translator.py`:
- Around line 347-366: The prompt translation logic in prompt_translator should
not cache fallback text when translation fails or no LLM is available. Update
the translation flow around llm_caller.call and the _TRANSLATION_CACHE write so
that only a valid, non-empty translated result is stored; if the call raises,
returns an invalid response, or llm_caller is None, return the original text
without populating the cache. Use the existing translated variable and
_TRANSLATION_CACHE key handling to keep the fix localized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2cac7789-8d0d-40d1-8118-17b16073c738
📒 Files selected for processing (5)
lib/crewai/src/crewai/agent/core.pylib/crewai/src/crewai/utilities/model_profiles.pylib/crewai/src/crewai/utilities/prompt_translator.pylib/crewai/tests/utilities/test_model_profiles.pylib/crewai/tests/utilities/test_prompt_translator_integration.py
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/crewai/src/crewai/agent/core.py
- lib/crewai/tests/utilities/test_model_profiles.py
- lib/crewai/tests/utilities/test_prompt_translator_integration.py
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr diff 6491 --repo crewAIInc/crewAI > /tmp/pr6491_v2.diff
wc -l /tmp/pr6491_v2.diff
echo "---- prompt_translator.py latest diff (search for target_profile / estimate savings) ----"
grep -n "target_profile\|estimated savings\|savings" /tmp/pr6491_v2.diff | head -50Length of output: 883 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '740,850p' /tmp/pr6491_v2.diffLength of output: 3996
This is a meaningfully more accurate heuristic than the previous swap-based approach, and the observability log ( |
Fix: only cache successful translations@coderabbitai caught that fallback text was being cached when translation failed or no LLM was available — permanently blocking retries until cache cleared. Before: After: Cache write moved inside the success branch — only valid LLM responses are cached. Transient failures allow subsequent requests to retry. 82 tests passing. No other known issues remain. @caike @zeroasterisk @renatonitta @diegocn |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr diff 6491 --repo crewAIInc/crewAI > /tmp/pr6491_v3.diff
grep -n "_TRANSLATION_CACHE\[" /tmp/pr6491_v3.diff
echo "----context----"
grep -n "_TRANSLATION_CACHE\|def translate_text\|llm_caller\|try:\|except" /tmp/pr6491_v3.diff | sed -n '1,80p'Length of output: 2534 🏁 Script executed: #!/bin/bash
sed -n '695,720p' /tmp/pr6491_v3.diffLength of output: 1291
Clean fix, no side effects on the caching-on-success path ( |
Also fixes #6481 — async callbacks in akickoffIn addition to #6428, this PR now also fixes #6481 (before/after_kickoff_callbacks do not support async callables in akickoff). What was brokenasync def my_callback(result):
await some_async_operation() # never awaited
return result
crew = Crew(..., after_kickoff_callbacks=[my_callback])
await crew.akickoff() # callback silently discardedWhat was fixed
Pattern followedSame pattern used by cb_result = callback(result)
if inspect.isawaitable(cb_result):
await cb_resultTests added
All 14 async crew tests pass. No regressions. |
ashusnapx
force-pushed
the
feat/language-aware-prompt-routing
branch
from
b6ae1f4 to
86cf68c
Compare
ashusnapx
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/crewai/src/crewai/crews/utils.py (1)
402-407: 🩺 Stability & Availability | 🔵 TrivialAsymmetry: sync
kickoff()/prepare_kickoff()won't await async callbacks.Now that async
before_kickoff_callbacks/after_kickoff_callbacksare officially supported viaakickoff, note that the synckickoff()path (prepare_kickoff) has no equivalentinspect.isawaitablehandling for before-callbacks. If a crew configured with an async before-callback is invoked viakickoff()instead ofakickoff(),normalizedwould end up holding an unawaited coroutine object, likely breaking downstream input interpolation/file extraction with a confusing error (or an "coroutine was never awaited" warning) rather than a clear message.Worth considering either a clear guard/error when a coroutine-returning callback is used with
kickoff(), or documenting that async callbacks requireakickoff().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/crews/utils.py` around lines 402 - 407, The sync kickoff path in prepare_kickoff is missing the same async-callback handling that kickoff_kickoff_async uses for before_kickoff_callbacks, so a coroutine-returning callback can leak into normalized and break later steps. Update the prepare_kickoff flow around the before_kickoff_callbacks loop to either explicitly reject awaitable results with a clear error or route them through the async path, and make sure the behavior matches the existing inspect.isawaitable handling used in the async kickoff logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/crews/utils.py`:
- Around line 362-475: `aprepare_kickoff` is duplicating almost all of
`prepare_kickoff`, which risks the two paths drifting apart. Extract the shared
post-callback workflow (event emission, input/file normalization, task callback
setup, agent setup, and planning) into a common helper, and have both
`prepare_kickoff` and `aprepare_kickoff` call it after their respective
before-kickoff callback handling. Keep the async-specific await logic only in
`aprepare_kickoff`, and preserve the existing behavior of symbols like
`CrewKickoffStartedEvent`, `store_files`, `setup_agents`, and
`crew._handle_crew_planning()`.
- Around line 423-431: The async kickoff path in `aprepare_kickoff()` is
blocking on the event bus future returned by `crewai_event_bus.emit`, which can
stall `Crew.akickoff()`. Update the `CrewKickoffStartedEvent` handling branch to
avoid calling `future.result()` synchronously; either remove the wait in this
async path or await the returned future with an async-compatible mechanism such
as `asyncio.wrap_future`. Refer to `aprepare_kickoff`, `crewai_event_bus.emit`,
and the `CrewKickoffStartedEvent` branch when making the change.
---
Nitpick comments:
In `@lib/crewai/src/crewai/crews/utils.py`:
- Around line 402-407: The sync kickoff path in prepare_kickoff is missing the
same async-callback handling that kickoff_kickoff_async uses for
before_kickoff_callbacks, so a coroutine-returning callback can leak into
normalized and break later steps. Update the prepare_kickoff flow around the
before_kickoff_callbacks loop to either explicitly reject awaitable results with
a clear error or route them through the async path, and make sure the behavior
matches the existing inspect.isawaitable handling used in the async kickoff
logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f7c08b5e-d38b-49eb-be49-bd12c7827d34
📒 Files selected for processing (3)
lib/crewai/src/crewai/crew.pylib/crewai/src/crewai/crews/utils.pylib/crewai/tests/crew/test_async_crew.py
| async def aprepare_kickoff( | ||
| crew: Crew, | ||
| inputs: dict[str, Any] | None, | ||
| input_files: dict[str, FileInput] | None = None, | ||
| ) -> dict[str, Any] | None: | ||
| """Async version of prepare_kickoff for use with akickoff(). | ||
|
|
||
| Mirrors :func:`prepare_kickoff` but awaits async before-kickoff | ||
| callbacks so that coroutines are not silently discarded. | ||
|
|
||
| Args: | ||
| crew: The crew instance to prepare. | ||
| inputs: Optional input dictionary to pass to the crew. | ||
| input_files: Optional dict of named file inputs for the crew. | ||
|
|
||
| Returns: | ||
| The potentially modified inputs dictionary after before callbacks. | ||
| """ | ||
| from crewai.events.base_events import reset_emission_counter | ||
| from crewai.events.event_bus import crewai_event_bus | ||
| from crewai.events.event_context import ( | ||
| get_current_parent_id, | ||
| reset_last_event_id, | ||
| ) | ||
| from crewai.events.types.crew_events import CrewKickoffStartedEvent | ||
|
|
||
| resuming = crew.checkpoint_kickoff_event_id is not None | ||
|
|
||
| if not resuming and get_current_parent_id() is None: | ||
| reset_emission_counter() | ||
| reset_last_event_id() | ||
|
|
||
| normalized: dict[str, Any] | None = None | ||
| if inputs is not None: | ||
| if not isinstance(inputs, Mapping): | ||
| raise TypeError( | ||
| f"inputs must be a dict or Mapping, got {type(inputs).__name__}" | ||
| ) | ||
| normalized = dict(inputs) | ||
|
|
||
| for before_callback in crew.before_kickoff_callbacks: | ||
| if normalized is None: | ||
| normalized = {} | ||
| normalized = before_callback(normalized) | ||
| if inspect.isawaitable(normalized): | ||
| normalized = await normalized | ||
|
|
||
| if resuming and crew._kickoff_event_id: | ||
| if crew.verbose: | ||
| from crewai.events.utils.console_formatter import ConsoleFormatter | ||
|
|
||
| fmt = ConsoleFormatter(verbose=True) | ||
| content = fmt.create_status_content( | ||
| "Resuming from Checkpoint", | ||
| crew.name or "Crew", | ||
| "bright_magenta", | ||
| ID=str(crew.id), | ||
| ) | ||
| fmt.print_panel( | ||
| content, "\U0001f504 Resuming from Checkpoint", "bright_magenta" | ||
| ) | ||
| else: | ||
| started_event = CrewKickoffStartedEvent(crew_name=crew.name, inputs=normalized) | ||
| crew._kickoff_event_id = started_event.event_id | ||
| future = crewai_event_bus.emit(crew, started_event) | ||
| if future is not None: | ||
| try: | ||
| future.result() | ||
| except Exception: # noqa: S110 | ||
| pass | ||
|
|
||
| crew._task_output_handler.reset() | ||
| crew._logging_color = "bold_purple" | ||
|
|
||
| _flow_files = baggage.get_baggage("flow_input_files") | ||
| flow_files: dict[str, Any] = _flow_files if isinstance(_flow_files, dict) else {} | ||
|
|
||
| if normalized is not None: | ||
| unpacked_files = _extract_files_from_inputs(normalized) | ||
|
|
||
| all_files = {**flow_files, **(input_files or {}), **unpacked_files} | ||
| if all_files: | ||
| store_files(crew.id, all_files) | ||
|
|
||
| crew._inputs = normalized | ||
| crew._interpolate_inputs(normalized) | ||
| else: | ||
| all_files = {**flow_files, **(input_files or {})} | ||
| if all_files: | ||
| store_files(crew.id, all_files) | ||
| crew._set_tasks_callbacks() | ||
| crew._set_allow_crewai_trigger_context_for_first_task() | ||
|
|
||
| agents_to_setup: list[BaseAgent] = list(crew.agents) | ||
| seen_agent_ids: set[int] = {id(agent) for agent in agents_to_setup} | ||
| for task in crew.tasks: | ||
| if task.agent is not None and id(task.agent) not in seen_agent_ids: | ||
| agents_to_setup.append(task.agent) | ||
| seen_agent_ids.add(id(task.agent)) | ||
|
|
||
| setup_agents( | ||
| crew, | ||
| agents_to_setup, | ||
| crew.embedder, | ||
| crew.function_calling_llm, | ||
| crew.step_callback, | ||
| ) | ||
|
|
||
| if crew.planning: | ||
| crew._handle_crew_planning() | ||
|
|
||
| return normalized | ||
|
|
||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract shared logic instead of duplicating prepare_kickoff.
aprepare_kickoff re-implements nearly the entire body of prepare_kickoff (event emission, input normalization, file extraction, task/agent setup, planning) verbatim, differing only in the await-aware before-callback loop. Any future bug fix or behavior change to this shared logic now has to be applied in two places, and it's easy to let them drift apart.
Consider factoring the post-callback logic (everything from event emission through crew._handle_crew_planning()) into a shared helper that both prepare_kickoff and aprepare_kickoff call after running their respective (sync/async-aware) before-callback loops.
♻️ Sketch of a shared-helper extraction
+def _finish_kickoff_preparation(
+ crew: Crew,
+ normalized: dict[str, Any] | None,
+ input_files: dict[str, FileInput] | None,
+ resuming: bool,
+) -> None:
+ """Shared post-before-callback preparation logic."""
+ if resuming and crew._kickoff_event_id:
+ ...
+ else:
+ ...
+ crew._task_output_handler.reset()
+ crew._logging_color = "bold_purple"
+ ...
+ if crew.planning:
+ crew._handle_crew_planning()
def prepare_kickoff(...):
...
for before_callback in crew.before_kickoff_callbacks:
if normalized is None:
normalized = {}
normalized = before_callback(normalized)
- ...(duplicated body)...
+ _finish_kickoff_preparation(crew, normalized, input_files, resuming)
+ return normalized
async def aprepare_kickoff(...):
...
for before_callback in crew.before_kickoff_callbacks:
if normalized is None:
normalized = {}
normalized = before_callback(normalized)
if inspect.isawaitable(normalized):
normalized = await normalized
- ...(duplicated body)...
+ _finish_kickoff_preparation(crew, normalized, input_files, resuming)
+ return normalized🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai/src/crewai/crews/utils.py` around lines 362 - 475,
`aprepare_kickoff` is duplicating almost all of `prepare_kickoff`, which risks
the two paths drifting apart. Extract the shared post-callback workflow (event
emission, input/file normalization, task callback setup, agent setup, and
planning) into a common helper, and have both `prepare_kickoff` and
`aprepare_kickoff` call it after their respective before-kickoff callback
handling. Keep the async-specific await logic only in `aprepare_kickoff`, and
preserve the existing behavior of symbols like `CrewKickoffStartedEvent`,
`store_files`, `setup_agents`, and `crew._handle_crew_planning()`.
| else: | ||
| started_event = CrewKickoffStartedEvent(crew_name=crew.name, inputs=normalized) | ||
| crew._kickoff_event_id = started_event.event_id | ||
| future = crewai_event_bus.emit(crew, started_event) | ||
| if future is not None: | ||
| try: | ||
| future.result() | ||
| except Exception: # noqa: S110 | ||
| pass |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect crewai_event_bus.emit to determine its return type and whether
# waiting on the returned future can block for a meaningful duration.
fd event_bus.py lib/crewai/src/crewai/events --exec cat -n {}Repository: crewAIInc/crewAI
Length of output: 42854
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the async kickoff path and the exact code around aprepare_kickoff.
rg -n "async def aprepare_kickoff|def prepare_kickoff|akickoff|future\.result\(" lib/crewai/src/crewai -S
echo
echo "---- utils.py around the reviewed lines ----"
sed -n '390,450p' lib/crewai/src/crewai/crews/utils.py
echo
echo "---- kick off call sites / await handling ----"
rg -n "wrap_future\(|await .*future|future\.result\(" lib/crewai/src/crewai -SRepository: crewAIInc/crewAI
Length of output: 8573
Avoid blocking aprepare_kickoff() on the event bus future. crewai_event_bus.emit(...) can return a future for async or threaded handlers, and calling future.result() here blocks Crew.akickoff()'s event loop until those listeners finish. If this path needs to wait, use an async wait such as await asyncio.wrap_future(future); otherwise, skip the synchronous wait in the async path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai/src/crewai/crews/utils.py` around lines 423 - 431, The async
kickoff path in `aprepare_kickoff()` is blocking on the event bus future
returned by `crewai_event_bus.emit`, which can stall `Crew.akickoff()`. Update
the `CrewKickoffStartedEvent` handling branch to avoid calling `future.result()`
synchronously; either remove the wait in this async path or await the returned
future with an async-compatible mechanism such as `asyncio.wrap_future`. Refer
to `aprepare_kickoff`, `crewai_event_bus.emit`, and the
`CrewKickoffStartedEvent` branch when making the change.
Closes crewAIInc#6428 Adds a lightweight pre-inference layer that detects the language of an agent system prompt and automatically translates it to the language the target model handles best, before the prompt reaches the LLM. New modules: - utilities/model_profiles.py: Boundary-aware regex registry for 15 model families (gpt-4o, claude, llama, qwen, deepseek, gemini, etc.) - utilities/prompt_translator.py: Core pipeline — language detection via Unicode script analysis, token estimation, code block/URL/JSON extraction, translation via LLM, content-hash caching Agent changes: - Add auto_translate_prompt (default True) field to BaseAgent - Add prompt_glossary field to BaseAgent for domain-term preservation - Integration in Agent._build_execution_prompt() with try/except fail-safe, system-prompt-only scope Design: - System prompt only — user messages never translated - Opt-out via Agent(auto_translate_prompt=False) - Fail-safe: any exception logs and keeps original prompt - Only cache successful translations (transient failures can retry) - Zero new hard dependencies Tests: 82 passing (model profiles + translator + integration). ruff: all checks passed. mypy: no issues found.
ashusnapx
force-pushed
the
feat/language-aware-prompt-routing
branch
from
86cf68c to
47374ee
Compare
ashusnapx
changed the title
2 participants
Summary
Adds a lightweight pre-inference layer that detects the language of an agent's system prompt and automatically translates it to the language the target model handles best, before the prompt reaches the LLM.
Closes #6428
Problem
Instruction-following quality varies by language (Multi-IF benchmark, arXiv:2410.15553). Tokenizer efficiency varies dramatically by model family — a 2000-character Chinese system prompt costs ~3500 tokens on LLaMA 3 but only ~1500 tokens translated to English. Today, developers must manually research and maintain per-model, per-language prompt variants.
Solution
A pure-function pipeline inserted between prompt construction and LLM invocation:
Usage
Model Profiles
Design Decisions
Files Changed
utilities/model_profiles.pyutilities/prompt_translator.pyagents/agent_builder/base_agent.pyauto_translate_prompt+prompt_glossaryfieldsagent/core.py_build_execution_prompt()tests/utilities/test_model_profiles.pytests/utilities/test_prompt_translator.pytests/utilities/test_prompt_translator_integration.pyTotal: +1,517 lines, 7 files, 82 tests
Test Results
What to Test Locally