Fix Snowflake Claude incomplete tool result histories#6006
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds Claude-specific message sanitization to SnowflakeCompletion. When formatting messages for Claude models, the implementation removes incomplete assistant tool-use turns—assistant messages with tool_calls that lack matching tool-result messages—before ensuring the conversation ends with a user message. Non-Claude models remain unchanged. ChangesClaude Incomplete Tool-Use Removal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/llms/providers/snowflake/completion.py`:
- Around line 191-193: The sanitizer currently appends the entire contiguous
block messages[index + 1 : lookahead] once
expected_ids.issubset(tool_result_ids) is true, which can reintroduce malformed
or unrelated tool messages; change this so after confirming expected_ids ⊆
tool_result_ids you only extend sanitized with those messages from
messages[index + 1 : lookahead] that are role "tool" and contain a valid
tool_call_id present in tool_result_ids (i.e., filter the slice by
msg.get("role") == "tool" and msg.get("tool_call_id") in tool_result_ids) so
only valid tool-result pairs are preserved (refer to variables expected_ids,
tool_result_ids, sanitized, messages, index, lookahead).
🪄 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: f9a90b77-5109-4ee5-835a-c7a306caca19
📒 Files selected for processing (2)
lib/crewai/src/crewai/llms/providers/snowflake/completion.pylib/crewai/tests/llms/snowflake/test_snowflake.py
|
Addressed the CodeRabbit comment in latest commit. Change made:
Validation:
................... [100%]
@coderabbitai review |
|
Clean summary of the CodeRabbit fix:
Validation:
@coderabbitai review |
|
(◕‿◕✿)🐇 ✅ Actions performedReview triggered.
|
|
(◕‿◕✿)🐇 ✅ Actions performedReview triggered.
|
|
You were right: the previous fix only validated OpenAI-style tool_calls. The failing runtime path can produce Claude/Bedrock-style content blocks with toolUse/toolResult, which Snowflake Claude validates directly. Updated fix:
Validation:
Real sanitized messages: @coderabbitai review |
|
A couple of things worth confirming when the review runs:
Triggering the fresh review now. (◕‿◕✿)🐇 ✅ Actions performedReview triggered.
|
|
Updated after testing against the full failing event shape. Root cause from the attached log:
Fix in latest commit:
Validation:
Real formatted messages had no historical tool_calls and no role=tool messages, only a user message summarizing the previous tool results. @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch the current state of the snowflake completion.py and test file
fd "completion.py" --full-path --search-path lib/crewai/src/crewai/llms/providers/snowflake
fd "test_snowflake.py" --full-path --search-path lib/crewai/tests/llms/snowflakeLength of output: 411 🏁 Script executed: #!/bin/bash
cat lib/crewai/src/crewai/llms/providers/snowflake/completion.py
echo "---TEST FILE---"
cat lib/crewai/tests/llms/snowflake/test_snowflake.pyLength of output: 20285
Looking at the code currently on the branch, it reflects an earlier iteration (the version that preserves complete pairs as-is). The latest commit with the collapsing strategy and
Triggering a fresh review now to pick up the latest commit. ✅ Actions performedReview triggered.
|
Summary
Why
Snowflake Claude returns this 400 when a prior tool-use is incomplete:
Each 'toolUse' block must be accompanied with a matching 'toolResult' block.CrewAI can hit this when a tool cycle is interrupted/retried and the next LLM call includes a dangling assistant
tool_callsmessage.Tests
uv run pytest lib/crewai/tests/llms/snowflake/test_snowflake.py— 18 passeduv run ruff check lib/crewai/src/crewai/llms/providers/snowflake/completion.py lib/crewai/tests/llms/snowflake/test_snowflake.py— passeduv run ruff format --check lib/crewai/src/crewai/llms/providers/snowflake/completion.py lib/crewai/tests/llms/snowflake/test_snowflake.py— passedSummary by CodeRabbit