fix: serialize non-standard FunctionResponse dicts in AnthropicLlm#4807
fix: serialize non-standard FunctionResponse dicts in AnthropicLlm#4807giulio-leone wants to merge 2 commits intogoogle:mainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Hi @giulio-leone , Thank you for your contribution! It appears you haven't yet signed the Contributor License Agreement (CLA). Please visit https://cla.developers.google.com/ to complete the signing process. Once the CLA is signed, we'll be able to proceed with the review of your PR. Thank you! |
db815f8 to
936a699
Compare
|
Rebased onto latest The upstream branch introduced improved Both the upstream PDF document tests and this PR's non-standard response test are included. |
936a699 to
31301a3
Compare
|
@rohityan Thanks for the heads-up! I'll get the Google CLA signed. Will follow up once it's done. |
|
I have signed the Google CLA. Could you please re-run the CLA check? Thank you! |
8adbe49 to
a931305
Compare
|
Hi @rohityan — the CLA is now signed and passing ✅ (it was a |
Rebased onto latest main, resolved conflicts with upstream json.dumps improvement. Keeps both upstream's json.dumps serialization for standard results AND the fallback path for non-standard response dicts (e.g. SkillToolset).
Move the non-standard FunctionResponse assertions back into the regression test that exercises the fallback serialization path, and keep the PDF document test focused on document serialization only. Validated with pyink --check, the full anthropic_llm test file, and a runtime proof script covering both the fallback tool_result path and the PDF document path.
ddb12da to
bc87808
Compare
|
Refreshed this branch onto current Validation (two consecutive clean passes, no code changes between passes):
Direct behavioral proof for the non-standard
So after refreshing the branch, the runtime fix itself is now already present upstream as well; the remaining contributor-side value of this PR is the explicit regression coverage that locks the non-standard response path in place and the fact that the branch is no longer stale/conflicting. |
✅ Autonomous Validation — Pass 2/2 CleanBranch: Test Results (double-pass, no code changes between passes)
Runner: Validated autonomously — double-pass strict protocol |
Summary
Fixes #4779
part_to_message_block()only handledfunction_responsedicts containing"content"or"result"keys. Any other dict structure (e.g. SkillToolset returning{"skill_name": ..., "instructions": ...}) fell through to an empty string, causing Claude to never see the tool output.Changes
anthropic_llm.py: Addedjson.dumps()fallback after the existing"content"/"result"checks, so non-standard response dicts are serialized to JSON text instead of being silently dropped.test_anthropic_llm.py: Added regression test verifying that a SkillToolset-style response dict is round-tripped correctly throughpart_to_message_block().Testing
test_anthropic_llm.pytests pass (including the new one)