fix(agent): improve accuracy of semantic descriptions for non-native fc calling #526
Conversation
WalkthroughThis update standardizes the terminology and formatting for tool call records across the codebase, replacing references to "function_call_result" with "function_call_record." It also strengthens and clarifies documentation and constraints regarding the handling, output, and reuse of tool call records within the LLM provider and related presenters, without altering public APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LLMProviderPresenter
participant ToolSystem
User->>LLMProviderPresenter: Sends request
LLMProviderPresenter->>ToolSystem: Initiates tool call
ToolSystem-->>LLMProviderPresenter: Returns tool call response
LLMProviderPresenter->>LLMProviderPresenter: Format as function_call_record JSON
LLMProviderPresenter->>User: Appends tool call record to conversation, prompts to continue
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/presenter/llmProviderPresenter/index.ts (1)
737-742: InconsistentChatMessage.contenttypes for user messages
Here you push the user prompt as an array of{ type, text }parts, whereas most upstream messages use a plain string. Confirm that downstream serialization/UI supports both formats or normalize on one to avoid potential runtime issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/presenter/llmProviderPresenter/baseProvider.ts(2 hunks)src/main/presenter/llmProviderPresenter/index.ts(1 hunks)src/main/presenter/threadPresenter/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (5)
src/main/presenter/llmProviderPresenter/baseProvider.ts (1)
243-245: Down-stream stripping of<function_call>tags is not evidentThe new security & information-hiding rules (6-7) forbid any literal exposure of
<function_call>/</function_call>tags to the end user.
Inside this class we consume those tags, but the codebase also re-injects raw records back into assistant messages for non-native FC (seethreadPresenter/index.tsaround 1967-1976).
Please double-check the rendering layer to ensure those injected tags are removed/escaped before reaching the UI; otherwise the new contract is violated.
If no sanitisation exists a thin post-processing hook that strips or masks the tags before display will be required.src/main/presenter/threadPresenter/index.ts (1)
1967-1976: ```shell
#!/usr/bin/env bashVerify no lingering usages of the old key in code, tests, or docs
rg -n function_call_result || true
rg -n functionCallResult || true
rg -n '"function_call_result"' || true
rg -n '"functionCallResult"' || true</details> <details> <summary>src/main/presenter/llmProviderPresenter/index.ts (3)</summary> `701-706`: **Terminology updated to `function_call_record`** The comment and `formattedToolRecordText` variable correctly reflect the new semantic scope for non-native function calls by embedding a `function_call_record`. --- `713-726`: **Safe append to existing assistant message** The branching logic covers all cases—string, array, or undefined—when adding the formatted tool record, preventing runtime type errors. --- `728-734`: **Fallback creation of standalone assistant message** When no prior assistant message exists, you correctly push a new one containing the record as an array of text parts. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
2 participants
fix(agent): improve accuracy of semantic descriptions for non-native fc calling
Summary by CodeRabbit