ADFA-5410 | Implement native tool calling for Gemini and OpenAI agents - #86
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Declare tools natively on both backends so calls arrive as structured data instead of unparseable prose, and read the dialects models still emit — Gemini's print(default_api.x()) and OpenAI's tool_calls deltas that carried no content — with a fallback to undeclared requests where a server refuses them. Report unreadable calls and abandoned runs honestly rather than COMPLETED, and name the source, layout and manifest paths up front.
e10b9e2 to
6fa029a
Compare
Daniel-ADFA
left a comment
There was a problem hiding this comment.
Requesting changes for two findings; the rest are nits. Plugin-api usage is clean (only public symbols present in the current plugin-api.api), and the unit tests pass locally (ai-core 454, gemini 36, openai 173; CI does not compile plugins).
1. Regression: long prose replies are discarded on MAX_TOKENS. GeminiBackend.kt ~L385 reports an error on finishReason == MAX_TOKENS whenever toolCallCount == 0, even when finalText is non-blank. streamContents is also the plain streaming path, and ChatViewModel.onError deletes the bubble and fails the turn, so a long answer that hits maxOutputTokens now vanishes where it used to be shown truncated. OpenAI keeps the partial text in the same case (OpenAiBackend.kt ~L389 errors only when calls.isEmpty() && finalText.isBlank()). Gate on blank text and complete with the partial text otherwise.
2. Native calls are stored in history as text envelopes. ChatViewModel.withNativeCalls (~L1284) renders native calls back into <tool_call> text, that merged string is what onComplete hands the loop (deferred.complete(text) L1208) and what AgentLoop stores as the ASSISTANT turn (~L160). From turn 2 on, the model sees its own prior turns written in the format NATIVE_CALL_FORMAT (GeminiSystemPrompt.kt L22-26) says "is NOT read ... and will not run", which invites imitation and the native+text double emission the ...ExactlyOneWay test guards against. Keep the envelope rendering for extraction, but store the history turn without envelopes (or in native form).
Nits:
ChatViewModel.ktL1207: the comment "Return the RAW text to the loop" is stale; the text now carries appended envelopes.ChatViewModel.kt~L999 andAgentReplyRenderer.ktL66-69 both show the truncated/malformed advice, so UNPARSABLE renders the same string twice; pick one place.GeminiToolProtocol.ktL149: rundeclarable()onitemstoo; a free-form object array item currently yields a propertyless OBJECT and an API 400 that therunCatchingatGeminiBackend.kt~L745 does not catch.OpenAiBackend.kt~L525: when the server rejectstools, surface that to the user once instead of only logging; the run otherwise continues with a native-mode prompt and no tools.- One 2.9k-line commit mixes native calling, the
tool_codefallback,ProjectLayout, loop stop semantics, tracing and prompt wording; please split before merge. Also overlaps #85 inGeminiBackend.streamContents, so whichever lands second needs a rebase.
…tory Gemini only errors on MAX_TOKENS when the reply is also blank, and AgentLoop takes a ModelReply so envelopes reach extraction but never the transcript. Also: declarable() on items, a toast when a server refuses tools, unparsed advice left to the bubble.
Daniel-ADFA
left a comment
There was a problem hiding this comment.
Both blocking findings are fixed and pinned by tests: Gemini now errors on MAX_TOKENS only when the text is also blank (GeminiBackend.kt L378), and AgentLoop.ModelReply keeps the <tool_call> envelopes out of the stored ASSISTANT turn (AgentLoop.kt L52/L177, two AgentLoopTest cases). The four code nits are addressed: items through declarable(), tools-refused Toast, single unparsed-advice site, comment. Unit tests pass locally: ai-core 456, gemini 37, openai 173.
Still open, none blocking:
- The branch carries the Java 21
libs/gradle-plugin.jarfromd09f05aand will not configure on the repo's JDK 17 until main (055e6dc) is merged again. - It still conflicts with #85 in
GeminiBackend.streamContents; whichever lands second needs a rebase. - The commit split was not done; fine if this squash-merges.
- Please confirm on device that a native call with no prose beside it is accepted by the Gemini API: since the empty ASSISTANT turn is now skipped, the model receives two consecutive
usercontents.
A native call with no prose leaves no ASSISTANT turn, so the user message and its tool results were sent as two adjacent "user" contents.
|
@Daniel-ADFA Nits addressed and verified in device Screen_Recording_20260908_093542_Code.on.the.Go.mp4 |
Conflict in GeminiBackend.streamContents: ADFA-5254 wrapped the SSE loop in withTrafficTag(NetworkTags.INFERENCE) while ADFA-5410 rewrote its body for native function calls. Kept both — the tool-call parsing now runs inside the tagged block, using the hoisted requestJob since withTrafficTag is non-inline and its lambda cannot suspend.
A run now keeps one transcript row, rewritten in place as each tool starts and closed as a one-line summary; failures keep their own message and Retry. Removals also reach the session's message list, or syncMessageToSession republishes a dropped bubble and it animates its dots for the rest of the chat.
Description
This PR migrates the Gemini and OpenAI agent backends to use their native function-calling APIs instead of relying on plain text tool call envelopes. This addresses the issue where LLMs produced malformed text payloads (such as unescaped quotes or newlines) which caused valid tool calls to be skipped or silently fail. By correctly declaring tool schemas and updating the system prompts to stop teaching the
<tool_call>syntax when native calling is available, the agents can now execute actions reliably.Details
ToolCallingBackendcapabilities to bothGeminiBackendandOpenAiBackend.GeminiToolProtocolandOpenAiToolProtocolto manage provider-specific JSON schemas, declarations, and chunk parsing.ToolCallExtractorto diagnose and report malformed or truncated tool replies, preventing the agent loop from incorrectly reporting success.ToolCodeParserto intercept and extract tool calls from Gemini's undocumenteddefault_api.<tool>(...)fallback syntax.ToolSchemato standardise and provide structured parameter schemas for all tool handlers (e.g.,AddDependencyHandler,CreateFileHandler).ProjectLayoutcontext to the system prompt to explicitly inform the agent of the module structure, significantly reducing wastedlist_filessteps.OpenAI
Screen_Recording_20260904_104112_Code.on.the.Go.mp4
Gemini API
Screen_Recording_20260904_105334_Code.on.the.Go.mp4
Ticket
ADFA-5410
Observation
The
AgentLoophas also been updated to halt accurately when tool calls are unparsable or abandoned, providing specific diagnostic messages to the user rather than failing silently.