fix(llm): normalize native tool call metadata#337
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughOpenAIService adds internal helper methods to normalize tool-call metadata and arguments, applies them throughout streaming tool-call construction/display/execution, and enables test visibility via InternalsVisibleTo. Tests validate the normalization and safe deserialization behavior. ChangesTool-Call Normalization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs (1)
1098-1100: ⚡ Quick winUse the tolerant argument parser for execution path too.
Line 1099 still uses strict string-dictionary deserialization, while display parsing is now tolerant. Reusing the same conversion strategy here would avoid avoidable per-tool parse failures on non-string argument payloads.
Proposed refactor
- var argsJson = NormalizeToolCallArguments(toolCall.FunctionArguments?.ToString()); - var argsDict = JsonConvert.DeserializeObject<Dictionary<string, string>>(argsJson) - ?? new Dictionary<string, string>(); + var argsDict = DeserializeToolArgumentsForDisplay( + toolCall.FunctionArguments?.ToString());🤖 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 `@TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs` around lines 1098 - 1100, The current execution path deserializes argsJson into Dictionary<string,string> strictly which fails for non-string payloads; change the conversion in OpenAIService.cs (around NormalizeToolCallArguments/toolCall.FunctionArguments usage) to use the same tolerant parser used for display parsing: normalize arguments with NormalizeToolCallArguments, then deserialize into a Dictionary<string,object> (or parse to JObject) and convert values to strings safely (e.g., ToString() or a safe serializer) to build argsDict so non-string argument types won’t cause exceptions during execution.TelegramSearchBot.LLM.Test/Service/AI/LLM/OpenAIToolCallNormalizationTests.cs (1)
6-43: ⚡ Quick winAdd coverage for
NormalizeToolCallNameand null/whitespace argument inputs.You added the helper in production code, but this test file currently doesn’t pin its behavior. Adding those cases would close the normalization test surface.
Suggested additional tests
+ [Fact] + public void NormalizeToolCallName_EmptyOrWhitespace_ReturnsUnknown() { + Assert.Equal("unknown", OpenAIService.NormalizeToolCallName("")); + Assert.Equal("unknown", OpenAIService.NormalizeToolCallName(" ")); + } + + [Fact] + public void NormalizeToolCallArguments_NullOrWhitespace_ReturnsEmptyJsonObject() { + Assert.Equal("{}", OpenAIService.NormalizeToolCallArguments(null)); + Assert.Equal("{}", OpenAIService.NormalizeToolCallArguments(" ")); + }🤖 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 `@TelegramSearchBot.LLM.Test/Service/AI/LLM/OpenAIToolCallNormalizationTests.cs` around lines 6 - 43, Add tests that cover the new NormalizeToolCallName behavior and null/whitespace argument inputs: add a test that calls OpenAIService.NormalizeToolCallName with an empty or whitespace string and asserts it returns a non-empty fallback starting with "call_" and another test that passes a trimmed name like " tool_1 " and expects "tool_1"; also add tests for OpenAIService.NormalizeToolCallArguments (or the existing NormalizeToolCallArguments overload) passing null and whitespace-only inputs and assert they return "{}" (or an empty JSON object string) to mirror the existing empty-string case. Reference the OpenAIService class and the NormalizeToolCallName and NormalizeToolCallArguments method names when adding these tests alongside the existing NormalizeToolCallId/Arguments tests.
🤖 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.
Nitpick comments:
In
`@TelegramSearchBot.LLM.Test/Service/AI/LLM/OpenAIToolCallNormalizationTests.cs`:
- Around line 6-43: Add tests that cover the new NormalizeToolCallName behavior
and null/whitespace argument inputs: add a test that calls
OpenAIService.NormalizeToolCallName with an empty or whitespace string and
asserts it returns a non-empty fallback starting with "call_" and another test
that passes a trimmed name like " tool_1 " and expects "tool_1"; also add tests
for OpenAIService.NormalizeToolCallArguments (or the existing
NormalizeToolCallArguments overload) passing null and whitespace-only inputs and
assert they return "{}" (or an empty JSON object string) to mirror the existing
empty-string case. Reference the OpenAIService class and the
NormalizeToolCallName and NormalizeToolCallArguments method names when adding
these tests alongside the existing NormalizeToolCallId/Arguments tests.
In `@TelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.cs`:
- Around line 1098-1100: The current execution path deserializes argsJson into
Dictionary<string,string> strictly which fails for non-string payloads; change
the conversion in OpenAIService.cs (around
NormalizeToolCallArguments/toolCall.FunctionArguments usage) to use the same
tolerant parser used for display parsing: normalize arguments with
NormalizeToolCallArguments, then deserialize into a Dictionary<string,object>
(or parse to JObject) and convert values to strings safely (e.g., ToString() or
a safe serializer) to build argsDict so non-string argument types won’t cause
exceptions during execution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e4c32d1-b565-44cf-802c-b8a07d365cc3
📒 Files selected for processing (3)
TelegramSearchBot.LLM.Test/Service/AI/LLM/OpenAIToolCallNormalizationTests.csTelegramSearchBot.LLM/Service/AI/LLM/OpenAIService.csTelegramSearchBot.LLM/TelegramSearchBot.LLM.csproj
PR Check ReportSummary
Test Results
Code Quality
Test Artifacts
LinksThis report is auto-generated by GitHub Actions |
ModerRAS
force-pushed
the
codex/fix-agent-tool-error-loop
branch
from
10bb57a to
38603eb
Compare
1 participant
Summary
{}and make tool-call display parsing tolerate non-string or malformed JSON valuesWhy
The previous fix handled
nulltool call IDs, but streaming updates can produce an empty encoded tool call ID. That empty ID is then stored inAssistantChatMessage/ToolChatMessageand can fail during the next OpenAI SDK request with errors likeEmpty encoded value, which bubbles out of the LLMAgent process asAI Agent 执行失败instead of being handled in the tool-call loop.Tests
dotnet test TelegramSearchBot.LLM.Test/TelegramSearchBot.LLM.Test.csproj --filter OpenAIToolCallNormalizationTestsdotnet test TelegramSearchBot.Test/TelegramSearchBot.Test.csproj --filter AgentIntegrationTestsdotnet test TelegramSearchBot.LLM.Test/TelegramSearchBot.LLM.Test.csprojdotnet testSummary by CodeRabbit
Bug Fixes
Tests