diff --git a/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiBackend.kt b/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiBackend.kt index 2d06e877..586f847c 100644 --- a/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiBackend.kt +++ b/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiBackend.kt @@ -2,10 +2,12 @@ package com.itsaky.androidide.plugins.aiagentgemini.backend import android.content.SharedPreferences import android.os.Looper +import android.util.Log import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aiagentgemini.R import com.itsaky.androidide.plugins.aiagentgemini.errors.GeminiErrorFormatter import com.itsaky.androidide.plugins.aiagentgemini.errors.GeminiFailure +import com.itsaky.androidide.plugins.aiagentgemini.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aiagentgemini.preferences.GeminiPreferences import com.itsaky.androidide.plugins.aiagentgemini.prompt.GeminiSystemPrompt import com.itsaky.androidide.plugins.aiagentgemini.security.secureApiKeyStore @@ -27,6 +29,16 @@ import kotlinx.coroutines.launch import org.json.JSONArray import org.json.JSONObject +/** + * Tool-protocol tracing, under the tag suffix `ai-core` uses for the other half of the same run: + * `adb logcat -s AiCore.AgentTrace:V AiAgentGemini.AgentTrace:V` reads a run end to end. + * + * These lines go through [Log] rather than `context.logger`, which the host funnels into its own + * class's tag with only a `[pluginId]` prefix — unfilterable, and why a captured log of an agent + * run showed nothing from this plugin at all. + */ +private const val TAG = "$LOG_PREFIX.AgentTrace" + /** * Gemini API backend for cloud-based LLM inference. * @@ -38,7 +50,7 @@ import org.json.JSONObject */ class GeminiBackend( private val context: PluginContext -) : HistoryCapableBackend, CancellableBackend, ConfigurableBackend { +) : HistoryCapableBackend, CancellableBackend, ConfigurableBackend, ToolCallingBackend { private val scope = CoroutineScope(Dispatchers.IO) @@ -66,6 +78,9 @@ class GeminiBackend( /** Server-sent-events streaming variant of [METHOD_GENERATE_CONTENT]. */ private const val METHOD_STREAM_GENERATE_CONTENT = "streamGenerateContent" + + /** `finishReason` for a reply the model's output cap cut short. */ + private const val FINISH_REASON_MAX_TOKENS = "MAX_TOKENS" } /** This plugin's own settings, written by its settings pane and read here at request time. */ @@ -232,7 +247,8 @@ class GeminiBackend( config: LlmConfig, callback: StreamCallback ) { - streamContents(JSONArray().put(contentJson("user", buildPrompt(prompt, config))), config, callback) + val contents = JSONArray().put(contentJson("user", buildPrompt(prompt, config))) + streamContents(contents, config, emptyList(), callback.asToolCallback()) } /** @@ -262,7 +278,8 @@ class GeminiBackend( ChatMessage.Role.ASSISTANT -> "model" // Gemini has no system role; a mid-conversation system note goes as a user turn. ChatMessage.Role.SYSTEM -> "user" - // No native function calling here, so a tool result rides in as a user turn. + // A functionResponse part pairs with a functionCall one, and the assistant turn + // in history is text, so a tool result rides in as a user turn instead. ChatMessage.Role.TOOL -> "user" } contents.put(contentJson(role, msg.content)) @@ -276,12 +293,14 @@ class GeminiBackend( * * @param contents the request's `contents[]` turns * @param config sampling settings for this request - * @param callback receives tokens, completion, and errors + * @param tools the tools to declare, or empty to stream plain text + * @param callback receives tokens, tool calls, completion, and errors */ private fun streamContents( contents: JSONArray, config: LlmConfig, - callback: StreamCallback + tools: List, + callback: ToolStreamCallback ) { currentJob = scope.launch { try { @@ -292,12 +311,21 @@ class GeminiBackend( } val startTime = System.currentTimeMillis() - context.logger.info("GeminiBackend: Streaming response over ${contents.length()} turns") - - val body = buildRequestJson(contents, config) + val body = buildRequestJson(contents, config, tools) + // `declared` counts what reached the body, not what was asked for: a schema + // [buildRequestJson] had to drop falls back to text calls without saying so here. + Log.i( + TAG, + "REQUEST | model=${getModelName()} turns=${contents.length()} " + + "tools=${tools.size} declared=${declaredToolCount(body)} " + + tools.joinToString(",") { it.name } + ) val fullText = StringBuilder() var chunkCount = 0 + var toolCallCount = 0 + // Last chunk's reason wins: only the final one carries why generation stopped. + var finishReason: String? = null val conn = openConnection(getModelName(), METHOD_STREAM_GENERATE_CONTENT, sse = true, apiKey = apiKey) val cancelHandle = coroutineContext[Job]?.invokeOnCompletion { cause -> if (cause != null) conn.disconnect() @@ -313,14 +341,24 @@ class GeminiBackend( val payload = line.substringAfter("data:").trim() if (payload.isEmpty() || payload == "[DONE]") continue // A malformed/non-JSON chunk must not abort the whole stream; skip it. - val chunk = runCatching { extractText(JSONObject(payload)) }.getOrElse { - context.logger.warn("GeminiBackend: skipping malformed SSE chunk: ${it.message}") - "" + val chunk = runCatching { GeminiToolProtocol.parseChunk(JSONObject(payload)) }.getOrElse { + Log.w(TAG, "CHUNK | skipped malformed SSE chunk: ${it.message}") + GeminiToolProtocol.StreamChunk.EMPTY } - if (chunk.isNotEmpty()) { + chunk.finishReason?.let { finishReason = it } + if (chunk.text.isNotEmpty()) { chunkCount++ - fullText.append(chunk) - callback.onToken(chunk) + fullText.append(chunk.text) + callback.onToken(chunk.text) + } + for (call in chunk.calls) { + toolCallCount++ + Log.i( + TAG, + "FUNCTION_CALL | tool=${call.name} " + + "args=${call.args.orEmpty().keys.joinToString(",")}" + ) + callback.onToolCall(call) } } } @@ -330,18 +368,32 @@ class GeminiBackend( } val finalText = fullText.toString() - if (finalText.isBlank()) { - callback.onError("Empty response from Gemini API") - } else { - val tokenCount = finalText.split("\\s+".toRegex()).size - context.logger.info("GeminiBackend: Streamed ${finalText.length} chars in $chunkCount chunks, ~$tokenCount tokens") - callback.onComplete(LlmResponse.success(finalText, tokenCount, System.currentTimeMillis() - startTime)) + Log.i( + TAG, + "STREAM | chars=${finalText.length} chunks=$chunkCount calls=$toolCallCount " + + "finish=$finishReason" + ) + when { + // Truncation is why a request to write a whole file came back unusable, and it + // reads as an empty or half-finished reply unless the reason is reported. + toolCallCount == 0 && finishReason == FINISH_REASON_MAX_TOKENS -> + callback.onError(userMessage(GeminiFailure.ReplyTruncated)) + + toolCallCount == 0 && finalText.isBlank() -> + callback.onError("Empty response from Gemini API") + + else -> { + val tokenCount = finalText.split("\\s+".toRegex()).size + callback.onComplete( + LlmResponse.success(finalText, tokenCount, System.currentTimeMillis() - startTime) + ) + } } } catch (e: CancellationException) { throw e } catch (e: Exception) { ensureActive() - context.logger.error("GeminiBackend: Error in streaming", e) + Log.e(TAG, "STREAM | failed: ${e.message}", e) callback.onError(formatErrorMessage(e)) } } @@ -553,7 +605,31 @@ User: $userPrompt""" config: LlmConfig, callback: StreamCallback ) { - streamContents(buildContents(history, prompt, config), config, callback) + streamContents(buildContents(history, prompt, config), config, emptyList(), callback.asToolCallback()) + } + + /** + * Streams a turn with [tools] declared to the API, reporting each `functionCall` part through + * [ToolStreamCallback.onToolCall]. + * + * This is the path the agent takes. Declaring the tools is what stops Gemini writing a call as + * prose the caller has to parse back: the arguments arrive already structured, so a file whose + * contents contain quotes or newlines can no longer break the call carrying it (ADFA-5410). + * + * @param prompt the current user turn + * @param history the conversation so far, oldest first + * @param config sampling settings; its system prompt becomes the leading turn pair + * @param tools the tools to declare; an empty list streams plain text + * @param callback receives tokens, tool calls, completion, and errors + */ + override fun generateStreamingWithTools( + prompt: String, + history: List, + config: LlmConfig, + tools: List, + callback: ToolStreamCallback + ) { + streamContents(buildContents(history, prompt, config), config, tools, callback) } /** Cancel any in-flight generation (user pressed Stop). */ @@ -645,10 +721,15 @@ User: $userPrompt""" * * @param contents the `contents` array of role/parts turns * @param config supplies temperature and max output tokens + * @param tools the tools to declare; omitted from the body when empty * @return the request JSON */ - private fun buildRequestJson(contents: JSONArray, config: LlmConfig): JSONObject = - JSONObject() + private fun buildRequestJson( + contents: JSONArray, + config: LlmConfig, + tools: List = emptyList(), + ): JSONObject { + val body = JSONObject() .put("contents", contents) .put( "generationConfig", @@ -656,6 +737,31 @@ User: $userPrompt""" .put("temperature", config.temperature.toDouble()) .put("maxOutputTokens", config.maxTokens) ) + if (tools.isEmpty()) return body + // A schema this side cannot express must not cost the user the whole request: dropping the + // declarations degrades to the text envelope the prompt still describes. + val declarations = runCatching { GeminiToolProtocol.functionDeclarations(tools) }.getOrElse { + Log.w(TAG, "REQUEST | could not declare tools, falling back to text calls", it) + return body + } + return body.put("tools", JSONArray().put(JSONObject().put("functionDeclarations", declarations))) + } + + /** + * How many function declarations [body] ended up carrying. + * + * Read back off the body rather than counted from the tool list, so a run whose declarations + * were dropped is distinguishable in the trace from one that was never offered any tools. + * + * @param body a request built by [buildRequestJson] + * @return the declaration count, or 0 when the body declares no tools + */ + private fun declaredToolCount(body: JSONObject): Int = + body.optJSONArray("tools") + ?.optJSONObject(0) + ?.optJSONArray("functionDeclarations") + ?.length() + ?: 0 /** * Build a single `contents` turn. @@ -669,6 +775,18 @@ User: $userPrompt""" .put("role", role) .put("parts", JSONArray().put(JSONObject().put("text", text))) + /** + * Adapts a plain stream callback to the tool-aware one [streamContents] takes. + * + * @return a [ToolStreamCallback] that forwards every event and reports no tool calls + */ + private fun StreamCallback.asToolCallback(): ToolStreamCallback = object : ToolStreamCallback { + override fun onToken(token: String) = this@asToolCallback.onToken(token) + override fun onToolCall(request: ToolCallRequest) = Unit + override fun onComplete(response: LlmResponse) = this@asToolCallback.onComplete(response) + override fun onError(error: String) = this@asToolCallback.onError(error) + } + /** * Extract and concatenate the text parts of the first candidate. * @@ -730,6 +848,9 @@ User: $userPrompt""" GeminiFailure.Unreachable -> resources.getString(R.string.gemini_error_unreachable) + GeminiFailure.ReplyTruncated -> + resources.getString(R.string.gemini_error_truncated) + is GeminiFailure.Failed -> failure.reason?.let { resources.getString(R.string.gemini_error_failed_reason, it) } ?: resources.getString(R.string.gemini_error_failed) diff --git a/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiToolProtocol.kt b/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiToolProtocol.kt new file mode 100644 index 00000000..0b932fb3 --- /dev/null +++ b/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiToolProtocol.kt @@ -0,0 +1,164 @@ +package com.itsaky.androidide.plugins.aiagentgemini.backend + +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolCallRequest +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolDefinition +import org.json.JSONArray +import org.json.JSONObject + +/** + * Gemini's half of the native function-calling protocol: tool schemas out, `functionCall` parts in. + * + * Pure and free of Android types, so the shapes that decide whether a tool call runs at all are + * unit-testable without a device or a network — see [GeminiSystemPrompt] for the same reasoning. + */ +internal object GeminiToolProtocol { + + /** Gemini's `Type` for an object, which its enum spells in upper case. */ + private const val TYPE_OBJECT = "OBJECT" + + /** Gemini's `Type` for a string, the only shape a free-form object can be declared as. */ + private const val TYPE_STRING = "STRING" + + /** Appended when an object argument has to be declared as JSON text; see [declarable]. */ + private const val AS_JSON_TEXT = " Written as a JSON object." + + /** + * One parsed stream chunk. + * + * @property text the chunk's text parts, concatenated. + * @property calls the chunk's `functionCall` parts. + * @property finishReason why generation stopped, on the chunk that carries it. + */ + data class StreamChunk( + val text: String, + val calls: List, + val finishReason: String?, + ) { + companion object { + /** A chunk carrying nothing, for a payload that would not parse. */ + val EMPTY = StreamChunk("", emptyList(), null) + } + } + + /** + * Splits the first candidate of [response] into text, tool calls, and a finish reason. + * + * @param response a generateContent response (or a single stream chunk) + * @return what the chunk carried; [StreamChunk.EMPTY] when it has no candidate + */ + fun parseChunk(response: JSONObject): StreamChunk { + val candidates = response.optJSONArray("candidates") ?: return StreamChunk.EMPTY + if (candidates.length() == 0) return StreamChunk.EMPTY + val candidate = candidates.getJSONObject(0) + val finishReason = candidate.optString("finishReason").takeIf { it.isNotEmpty() } + val parts = candidate.optJSONObject("content")?.optJSONArray("parts") + ?: return StreamChunk("", emptyList(), finishReason) + + val text = StringBuilder() + val calls = mutableListOf() + for (i in 0 until parts.length()) { + val part = parts.getJSONObject(i) + val functionCall = part.optJSONObject("functionCall") + if (functionCall != null) calls += toolCallOf(functionCall) else text.append(part.optString("text")) + } + return StreamChunk(text.toString(), calls, finishReason) + } + + /** + * Reads one `functionCall` part. + * + * Gemini pairs a `functionResponse` by name rather than by id, so a call with no `id` of its + * own is identified by its name — never by a synthetic id the API would not recognise. + * + * @param functionCall the part's `functionCall` object + * @return the call, with its arguments already structured + */ + fun toolCallOf(functionCall: JSONObject): ToolCallRequest { + val name = functionCall.optString("name") + val args = mutableMapOf() + functionCall.optJSONObject("args")?.let { declared -> + for (key in declared.keys()) args[key] = declared.get(key) + } + return ToolCallRequest(functionCall.optString("id").ifEmpty { name }, name, args) + } + + /** + * The `functionDeclarations` array for [tools]. + * + * @param tools the tools to declare + * @return one declaration per tool, parameters omitted unless the tool names arguments + */ + fun functionDeclarations(tools: List): JSONArray { + val declarations = JSONArray() + for (tool in tools) { + val declaration = JSONObject() + .put("name", tool.name) + .put("description", tool.description.orEmpty()) + val parameters = tool.parametersSchema?.takeIf { it.isNotEmpty() }?.let { schemaJson(it) } + // Only when it names arguments: Gemini rejects an OBJECT with no properties outright + // ("should be non-empty for OBJECT type"), which fails the whole request, every tool + // in it included. A tool that names none is declared the way a no-arg tool is. + if (parameters != null && namesProperties(parameters)) { + declaration.put("parameters", parameters) + } + declarations.put(declaration) + } + return declarations + } + + /** Whether [schema] declares at least one property, which an OBJECT must for Gemini. */ + private fun namesProperties(schema: JSONObject): Boolean = + (schema.optJSONObject("properties")?.length() ?: 0) > 0 + + /** + * [schema] in a form Gemini will accept as one argument. + * + * An object whose keys are not known ahead of time cannot be declared as an OBJECT here at + * all, so it is declared as the JSON text the model should write instead — which every caller + * of this protocol already accepts for such an argument. + * + * @param schema one property's schema, already converted. + * @return the schema to declare, unchanged unless it is a propertyless object. + */ + private fun declarable(schema: JSONObject): JSONObject { + if (schema.optString("type") != TYPE_OBJECT || namesProperties(schema)) return schema + val description = schema.optString("description").trim() + return JSONObject() + .put("type", TYPE_STRING) + .put("description", (description + AS_JSON_TEXT).trim()) + } + + /** + * Converts a JSON Schema to the OpenAPI subset Gemini accepts. + * + * Only the keywords Gemini documents survive: anything else (`additionalProperties`, `$ref`, + * `oneOf`) is rejected outright by the API, and a contributed tool is free to carry them. + * + * @param schema the tool's JSON Schema, as [ToolDefinition] carries it + * @return the equivalent Gemini schema + */ + fun schemaJson(schema: Map<*, *>): JSONObject { + val json = JSONObject() + // Gemini's Type is an enum, so its values are upper case; JSON Schema writes them lower. + (schema["type"] as? String)?.let { json.put("type", it.uppercase()) } + (schema["description"] as? String)?.let { json.put("description", it) } + (schema["format"] as? String)?.let { json.put("format", it) } + (schema["enum"] as? Collection<*>)?.let { values -> + json.put("enum", JSONArray().apply { values.forEach { put(it.toString()) } }) + } + (schema["items"] as? Map<*, *>)?.let { json.put("items", schemaJson(it)) } + (schema["properties"] as? Map<*, *>)?.let { properties -> + val rendered = JSONObject() + for ((name, value) in properties) { + if (value is Map<*, *>) rendered.put(name.toString(), declarable(schemaJson(value))) + } + if (rendered.length() > 0) json.put("properties", rendered) + } + (schema["required"] as? Collection<*>)?.let { required -> + if (required.isNotEmpty()) { + json.put("required", JSONArray().apply { required.forEach { put(it.toString()) } }) + } + } + return json + } +} diff --git a/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/errors/GeminiErrorFormatter.kt b/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/errors/GeminiErrorFormatter.kt index 677a1100..3e3d89a6 100644 --- a/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/errors/GeminiErrorFormatter.kt +++ b/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/errors/GeminiErrorFormatter.kt @@ -50,6 +50,12 @@ sealed interface GeminiFailure { /** No response at all — no network, DNS failure, timeout. */ data object Unreachable : GeminiFailure + /** + * Generation stopped at the output cap (`finishReason: MAX_TOKENS`) with nothing runnable. + * Not an API error — the request succeeded — so it is never produced by [GeminiErrorFormatter]. + */ + data object ReplyTruncated : GeminiFailure + /** Everything else, including failures that never reached the network. */ data class Failed(val reason: String?) : GeminiFailure } diff --git a/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/prompt/GeminiSystemPrompt.kt b/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/prompt/GeminiSystemPrompt.kt index cf69c324..7db34ae3 100644 --- a/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/prompt/GeminiSystemPrompt.kt +++ b/ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/prompt/GeminiSystemPrompt.kt @@ -18,6 +18,13 @@ internal object GeminiSystemPrompt { */ private const val FALLBACK_EXAMPLE_PATH = "app/src/main/java/com/example/MainActivity.kt" + /** How to call a tool when the caller takes calls through the provider's own API. */ + private val NATIVE_CALL_FORMAT = """ + TOOL CALL FORMAT — the tools above are declared to you: call one through the function-calling + API. A call written into your reply text is NOT read by this system and will not run. + Do NOT describe the action in prose (e.g. "Okay, I'll open the file…") — narrating does nothing. + """.trimIndent() + /** * Builds the prompt for [request]. * @@ -61,7 +68,7 @@ internal object GeminiSystemPrompt { val workflow = """ WORKFLOW: 1. Understand the user's request - 2. List files to understand the project structure + 2. Locate what you need with ONE search_project call — the IDE CONTEXT block above already names the source, layout and manifest paths 3. Create/modify files with complete implementations 4. Add dependencies if needed 5. Sync gradle and verify compilation @@ -69,7 +76,10 @@ internal object GeminiSystemPrompt { 7. Report success and what was built """.trimIndent() - val syntax = request.toolCallSyntax ?: return head + "\n\n" + workflow + // Null syntax means the caller reads calls off the function-calling API instead. Saying so + // is what stops the model writing one as text, where nothing would run it (ADFA-5410). + val syntax = request.toolCallSyntax ?: return listOf(head, NATIVE_CALL_FORMAT, workflow) + .joinToString("\n\n") val callFormat = """ TOOL CALL FORMAT — to run a tool, emit a single line in EXACTLY this format and nothing after it: diff --git a/ai-agent-gemini/src/main/res/values/strings.xml b/ai-agent-gemini/src/main/res/values/strings.xml index d5345f3e..ce795a40 100644 --- a/ai-agent-gemini/src/main/res/values/strings.xml +++ b/ai-agent-gemini/src/main/res/values/strings.xml @@ -10,6 +10,7 @@ Gemini is temporarily unavailable (HTTP %1$d). Try again in a moment. Gemini returned an error (HTTP %1$d). Gemini returned an error (HTTP %1$d). %2$s + The reply hit the model\'s output limit before the action was complete, so nothing was changed. Ask for a smaller step, or raise the output limit in AI Settings. Could not reach Gemini. Check your internet connection and try again. The Gemini request failed. The Gemini request failed. %1$s diff --git a/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiBackendTest.kt b/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiBackendTest.kt index 24f5570e..469007ad 100644 --- a/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiBackendTest.kt +++ b/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiBackendTest.kt @@ -22,11 +22,12 @@ class GeminiBackendTest { } @Test - fun givenTheBackend_whenAskedForItsCapabilities_thenItDeclaresHistoryButNotToolCalling() { - // Dropping HistoryCapableBackend compiles and silently turns chat into one-shot prompting. + fun givenTheBackend_whenAskedForItsCapabilities_thenItDeclaresBothHistoryAndToolCalling() { + // Dropping either compiles and degrades silently: history turns chat into one-shot + // prompting, and tool calling drops the agent back to parsing calls out of the reply text. val declared: LlmBackend = backend assertTrue(declared is HistoryCapableBackend) - assertFalse(declared is ToolCallingBackend) + assertTrue(declared is ToolCallingBackend) } } diff --git a/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiToolProtocolTest.kt b/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiToolProtocolTest.kt new file mode 100644 index 00000000..90d667a0 --- /dev/null +++ b/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiToolProtocolTest.kt @@ -0,0 +1,203 @@ +package com.itsaky.androidide.plugins.aiagentgemini.backend + +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolDefinition +import org.json.JSONArray +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Unit tests for [GeminiToolProtocol]. Focus: ADFA-5410, where Gemini wrote its tool calls as text + * and a payload containing quotes could not be read back. Declaring the tools is what stops that, + * so the declaration and the `functionCall` parsing are both pinned down here. + */ +class GeminiToolProtocolTest { + + private fun schema(vararg properties: Pair>, required: List) = + mapOf( + "type" to "object", + "properties" to properties.toMap(), + "required" to required, + ) + + @Test + fun givenAToolWithArguments_whenDeclared_thenItsSchemaTravelsWithIt() { + val declarations = GeminiToolProtocol.functionDeclarations( + listOf( + ToolDefinition( + "read_file", + "Read a file", + schema( + "file_path" to mapOf("type" to "string", "description" to "Path to read."), + required = listOf("file_path"), + ), + ) + ) + ) + + assertEquals(1, declarations.length()) + val declaration = declarations.getJSONObject(0) + assertEquals("read_file", declaration.getString("name")) + assertEquals("Read a file", declaration.getString("description")) + val parameters = declaration.getJSONObject("parameters") + assertEquals("OBJECT", parameters.getString("type")) + assertEquals("STRING", parameters.getJSONObject("properties").getJSONObject("file_path").getString("type")) + assertEquals("file_path", parameters.getJSONArray("required").getString(0)) + } + + @Test + fun givenAFreeFormObjectArgument_whenDeclared_thenItIsDeclaredAsJsonText() { + // Gemini rejects an OBJECT with no properties ("should be non-empty for OBJECT type"), + // and that 400 fails the whole request — every other tool in it included. + val declarations = GeminiToolProtocol.functionDeclarations( + listOf( + ToolDefinition( + "generate_from_template", + "Generate from a template", + schema( + "template_name" to mapOf("type" to "string"), + "variables" to mapOf("type" to "object", "description" to "Variables."), + required = listOf("template_name"), + ), + ) + ) + ) + + val variables = declarations.getJSONObject(0) + .getJSONObject("parameters") + .getJSONObject("properties") + .getJSONObject("variables") + assertEquals("STRING", variables.getString("type")) + assertTrue(variables.getString("description").contains("JSON object")) + } + + @Test + fun givenASchemaThatNamesNoProperties_whenDeclared_thenNoParametersAreSent() { + // Same rejection at the top level, where there is nothing to degrade it to. + val declarations = GeminiToolProtocol.functionDeclarations( + listOf(ToolDefinition("gradle_sync", "Sync", mapOf("type" to "object"))) + ) + + assertFalse(declarations.getJSONObject(0).has("parameters")) + } + + @Test + fun givenAToolThatTakesNoArguments_whenDeclared_thenNoParametersAreSent() { + val declarations = GeminiToolProtocol.functionDeclarations( + listOf(ToolDefinition("run_app", "Build and run", emptyMap())) + ) + + assertFalse(declarations.getJSONObject(0).has("parameters")) + } + + @Test + fun givenASchemaCarryingKeywordsGeminiRejects_whenConverted_thenTheyAreDropped() { + // A contributed (MCP) tool is free to send these; the API 400s on them. + val json = GeminiToolProtocol.schemaJson( + mapOf( + "type" to "object", + "additionalProperties" to false, + "\$schema" to "https://json-schema.org/draft/2020-12/schema", + "properties" to mapOf("q" to mapOf("type" to "string")), + ) + ) + + assertFalse(json.has("additionalProperties")) + assertFalse(json.has("\$schema")) + assertEquals("STRING", json.getJSONObject("properties").getJSONObject("q").getString("type")) + } + + @Test + fun givenASchemaWithNoRequiredArguments_whenConverted_thenRequiredIsOmitted() { + val json = GeminiToolProtocol.schemaJson( + mapOf("type" to "object", "properties" to mapOf("directory" to mapOf("type" to "string"))) + ) + + assertFalse(json.has("required")) + } + + @Test + fun givenAFunctionCallPart_whenParsed_thenItsArgumentsArriveAlreadyStructured() { + // The edit ADFA-5410 lost: the value carries the quotes and newlines that broke text mode. + val layout = "\n" + val chunk = GeminiToolProtocol.parseChunk( + JSONObject() + .put( + "candidates", + JSONArray().put( + JSONObject().put( + "content", + JSONObject().put( + "parts", + JSONArray().put( + JSONObject().put( + "functionCall", + JSONObject() + .put("name", "edit_file") + .put("args", JSONObject().put("new_string", layout)), + ) + ) + ) + ) + ) + ) + ) + + assertEquals(1, chunk.calls.size) + assertEquals("edit_file", chunk.calls[0].name) + assertEquals(layout, chunk.calls[0].args!!["new_string"]) + assertTrue(chunk.text.isEmpty()) + } + + @Test + fun givenAFunctionCallWithNoIdOfItsOwn_whenParsed_thenItIsIdentifiedByName() { + // Gemini pairs a functionResponse by name, so the name is the only id there is. + val call = GeminiToolProtocol.toolCallOf(JSONObject().put("name", "gradle_sync")) + + assertEquals("gradle_sync", call.callId) + assertEquals("gradle_sync", call.name) + assertTrue(call.args!!.isEmpty()) + } + + @Test + fun givenATextPart_whenParsed_thenItIsReportedAsTextWithNoCalls() { + val chunk = GeminiToolProtocol.parseChunk( + JSONObject().put( + "candidates", + JSONArray().put( + JSONObject().put( + "content", + JSONObject().put("parts", JSONArray().put(JSONObject().put("text", "Working on it."))), + ) + ) + ) + ) + + assertEquals("Working on it.", chunk.text) + assertTrue(chunk.calls.isEmpty()) + } + + @Test + fun givenAReplyStoppedAtTheOutputCap_whenParsed_thenTheFinishReasonIsReported() { + val chunk = GeminiToolProtocol.parseChunk( + JSONObject().put( + "candidates", + JSONArray().put(JSONObject().put("finishReason", "MAX_TOKENS")), + ) + ) + + assertEquals("MAX_TOKENS", chunk.finishReason) + } + + @Test + fun givenAChunkWithNoCandidates_whenParsed_thenNothingIsReported() { + val chunk = GeminiToolProtocol.parseChunk(JSONObject()) + + assertTrue(chunk.text.isEmpty()) + assertTrue(chunk.calls.isEmpty()) + assertNull(chunk.finishReason) + } +} diff --git a/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/prompt/GeminiSystemPromptTest.kt b/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/prompt/GeminiSystemPromptTest.kt new file mode 100644 index 00000000..b8bc91fa --- /dev/null +++ b/ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/prompt/GeminiSystemPromptTest.kt @@ -0,0 +1,61 @@ +package com.itsaky.androidide.plugins.aiagentgemini.prompt + +import com.itsaky.androidide.plugins.services.LlmInferenceService.SystemPromptRequest +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolDefinition +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Unit tests for [GeminiSystemPrompt]. Focus: the prompt teaches exactly one way to call a tool. + * Teaching both (ADFA-5410) is how a call ends up written as text that nothing runs. + */ +class GeminiSystemPromptTest { + + private companion object { + const val SYNTAX = """{"tool":"TOOL_NAME","args":{"arg":"value"}}""" + } + + private fun prompt(toolCallSyntax: String?) = GeminiSystemPrompt.build( + SystemPromptRequest( + listOf(ToolDefinition("read_file", "Read a file", emptyMap())), + toolCallSyntax, + "app/src/main/java/com/example/MainActivity.kt", + ) + ) + + @Test + fun givenAnEnvelopeSyntax_whenBuilding_thenItIsReproducedVerbatim() { + assertTrue(prompt(SYNTAX).contains(SYNTAX)) + } + + @Test + fun givenNoEnvelopeSyntax_whenBuilding_thenTheEnvelopeIsNeverTaught() { + // The caller parses no envelope here, so an example of one is a call that would not run. + assertFalse(prompt(null).contains("")) + } + + @Test + fun givenNoEnvelopeSyntax_whenBuilding_thenTheFunctionCallingApiIsNamedInstead() { + assertTrue(prompt(null).contains("function-calling")) + } + + @Test + fun givenEitherMode_whenBuilding_thenTheToolsAreAlwaysListed() { + listOf(SYNTAX, null).forEach { syntax -> + assertTrue("tools must be listed either way", prompt(syntax).contains("read_file")) + } + } + + @Test + fun givenEitherMode_whenBuilding_thenTheWorkflowDoesNotContradictTheRuleAgainstWalkingTheTree() { + // WORKFLOW step 2 used to say "List files to understand the project structure", against a + // RULE forbidding exactly that. A run followed the workflow and spent 7 of 16 turns on it. + listOf(SYNTAX, null).forEach { syntax -> + val prompt = prompt(syntax) + + assertFalse(prompt.contains("2. List files")) + assertTrue(prompt.contains("Never walk the tree with repeated list_files calls")) + } + } +} diff --git a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiBackend.kt b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiBackend.kt index 7b2ef771..5b86828a 100644 --- a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiBackend.kt +++ b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiBackend.kt @@ -1,12 +1,14 @@ package com.itsaky.androidide.plugins.aiagentopenai.backend import android.content.SharedPreferences +import android.util.Log import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aiagentopenai.R import com.itsaky.androidide.plugins.aiagentopenai.errors.OpenAiErrorFormatter import com.itsaky.androidide.plugins.aiagentopenai.errors.OpenAiFailure import com.itsaky.androidide.plugins.aiagentopenai.errors.OpenAiFailureMessages import com.itsaky.androidide.plugins.aiagentopenai.errors.OpenAiHttpException +import com.itsaky.androidide.plugins.aiagentopenai.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aiagentopenai.preferences.OpenAiPreferences import com.itsaky.androidide.plugins.aiagentopenai.prompt.OpenAiSystemPrompt import com.itsaky.androidide.plugins.aiagentopenai.security.ApiKeyCache @@ -28,6 +30,15 @@ import kotlinx.coroutines.launch import org.json.JSONArray import org.json.JSONObject +/** + * Tool-protocol tracing, under the tag suffix `ai-core` uses for the other half of the same run: + * `adb logcat -s AiCore.AgentTrace:V AiAgentOpenAi.AgentTrace:V` reads a run end to end. + * + * Through [Log] rather than `context.logger`, which the host funnels into its own class's tag with + * only a `[pluginId]` prefix — unfilterable, and so absent from a captured log of an agent run. + */ +private const val TAG = "$LOG_PREFIX.AgentTrace" + /** * OpenAI-compatible backend: one transport for every server that speaks `chat/completions`. * @@ -41,7 +52,7 @@ import org.json.JSONObject */ class OpenAiBackend( private val context: PluginContext -) : HistoryCapableBackend, CancellableBackend, ConfigurableBackend { +) : HistoryCapableBackend, CancellableBackend, ConfigurableBackend, ToolCallingBackend { private val scope = CoroutineScope(Dispatchers.IO) @@ -55,6 +66,13 @@ class OpenAiBackend( @Volatile private var currentJob: Job? = null + /** + * Base URL that answered a tool declaration with a refusal, so the next turn does not pay the + * same round trip. Keyed by the URL itself, so pointing the setting elsewhere re-probes. + */ + @Volatile + private var toolsRejectedBy: String? = null + companion object { /** Backend id, as persisted by AI Core when the user selects this backend. */ const val BACKEND_ID = "openai" @@ -197,7 +215,8 @@ class OpenAiBackend( streamMessages( OpenAiRequestBuilder.messages(emptyList(), prompt, config.systemPrompt), config, - callback + emptyList(), + callback.asToolCallback() ) } @@ -253,53 +272,132 @@ class OpenAiBackend( streamMessages( OpenAiRequestBuilder.messages(history, prompt, config.systemPrompt), config, + emptyList(), + callback.asToolCallback() + ) + } + + /** + * Streams a turn with [tools] declared to the server, reporting each `tool_calls` entry through + * [ToolStreamCallback.onToolCall]. + * + * This is the path the agent takes. Declaring the tools is what stops the model writing a call + * as prose the caller has to parse back: the arguments arrive already structured, so a file + * whose contents contain quotes or newlines can no longer break the call carrying it + * (ADFA-5410). + * + * @param prompt the current user turn + * @param history the conversation so far, oldest first + * @param config sampling settings; its system prompt becomes the leading `system` turn + * @param tools the tools to declare; an empty list streams plain text + * @param callback receives tokens, tool calls, completion, and errors + */ + override fun generateStreamingWithTools( + prompt: String, + history: List, + config: LlmConfig, + tools: List, + callback: ToolStreamCallback + ) { + streamMessages( + OpenAiRequestBuilder.messages(history, prompt, config.systemPrompt), + config, + tools, callback ) } + /** + * Adapts a plain stream callback to the tool-aware one [streamMessages] takes. + * + * @return a [ToolStreamCallback] that forwards every event and reports no tool calls + */ + private fun StreamCallback.asToolCallback(): ToolStreamCallback = object : ToolStreamCallback { + override fun onToken(token: String) = this@asToolCallback.onToken(token) + override fun onToolCall(request: ToolCallRequest) = Unit + override fun onComplete(response: LlmResponse) = this@asToolCallback.onComplete(response) + override fun onError(error: String) = this@asToolCallback.onError(error) + } + /** * Streams one `chat/completions` request over the already-built [messages]. * * @param messages the request's `messages[]` turns * @param config sampling settings for this request - * @param callback receives tokens, completion, and errors + * @param tools the tools to declare, or empty to stream plain text + * @param callback receives tokens, tool calls, completion, and errors */ private fun streamMessages( messages: JSONArray, config: LlmConfig, - callback: StreamCallback + tools: List, + callback: ToolStreamCallback ) { currentJob = scope.launch { try { val startTime = System.currentTimeMillis() - context.logger.info("OpenAiBackend: Streaming over ${messages.length()} turns") val fullText = StringBuilder() var chunkCount = 0 var outcome = StreamOutcome() - // The retry exists because reasoning models and third-party servers disagree about - // max_tokens/temperature; see RequestTuning. - withParameterRetry(config) { tuning -> - fullText.clear() - chunkCount = 0 - val body = OpenAiRequestBuilder.body( - messages, getModelName(), stream = true, config = config, tuning = tuning - ) - outcome = streamOnce(body) { chunk -> - chunkCount++ - fullText.append(chunk) - callback.onToken(chunk) + var calls = emptyList() + withToolRetry(tools) { declared -> + // The retry exists because reasoning models and third-party servers disagree + // about max_tokens/temperature; see RequestTuning. + withParameterRetry(config) { tuning -> + fullText.clear() + chunkCount = 0 + val accumulator = OpenAiToolProtocol.CallAccumulator() + val body = OpenAiRequestBuilder.body( + messages, + getModelName(), + stream = true, + config = config, + tuning = tuning, + tools = declared, + ) + Log.i( + TAG, + "REQUEST | model=${getModelName()} turns=${messages.length()} " + + "tools=${declared.size} " + + declared.joinToString(",") { it.name } + ) + outcome = streamOnce(body, accumulator) { chunk -> + chunkCount++ + fullText.append(chunk) + callback.onToken(chunk) + } + calls = accumulator.requests() + outcome.droppedCalls = accumulator.droppedCalls } } val finalText = fullText.toString() - if (finalText.isBlank()) { + Log.i( + TAG, + "STREAM | chars=${finalText.length} chunks=$chunkCount calls=${calls.size} " + + "dropped=${outcome.droppedCalls} finish=${outcome.finishReason}" + ) + // Reported after the stream, so a call the retry replaced never reaches the caller. + for (call in calls) { + Log.i( + TAG, + "FUNCTION_CALL | tool=${call.name} " + + "args=${call.args.orEmpty().keys.joinToString(",")}" + ) + callback.onToolCall(call) + } + + // A turn that called a tool and said nothing is the normal agent turn, so only a + // reply with neither text nor a call is empty. + if (calls.isEmpty() && finalText.isBlank()) { // The request succeeded and the stream ended, so this is not a failed request; // say which of the empty-reply cases it was instead of a generic error. context.logger.warn( "OpenAiBackend: stream produced no reply text " + "(skipped=${outcome.skippedChunks}, " + "reasoningChars=${outcome.reasoningChars}, " + + "droppedCalls=${outcome.droppedCalls}, " + "finishReason=${outcome.finishReason})" ) callback.onError(failureMessages.of(emptyReplyFailure(outcome))) @@ -312,6 +410,7 @@ class OpenAiBackend( throw e } catch (e: Exception) { ensureActive() + Log.e(TAG, "STREAM | failed: ${e.message}", e) context.logger.error("OpenAiBackend: Error in streaming", e) callback.onError(formatErrorMessage(e)) } @@ -328,11 +427,13 @@ class OpenAiBackend( * @param skippedChunks payloads the parser could not use * @param reasoningChars thinking text seen, which is never part of the reply * @param finishReason the last `finish_reason` the server sent, if any + * @param droppedCalls tool calls whose arguments never parsed, i.e. arrived half-written */ private data class StreamOutcome( var skippedChunks: Int = 0, var reasoningChars: Int = 0, var finishReason: String? = null, + var droppedCalls: Int = 0, ) /** @@ -341,10 +442,12 @@ class OpenAiBackend( * Tokens already delivered before a mid-stream failure stay delivered; the caller resets its * buffer before a retry, which only ever happens on a 400 raised before any token arrived. * + * @param accumulator collects the native tool-call fragments the stream carries * @return what else the stream carried, for diagnosing an empty reply */ private suspend fun streamOnce( body: JSONObject, + accumulator: OpenAiToolProtocol.CallAccumulator, onChunk: (String) -> Unit ): StreamOutcome { val outcome = StreamOutcome() @@ -368,6 +471,12 @@ class OpenAiBackend( when (val event = SseChunk.parse(line)) { is SseChunk.Event.Token -> onChunk(event.text) + is SseChunk.Event.ToolCalls -> { + accumulator.accept(event.deltas) + // Prose the same chunk carried; usually empty, never dropped. + if (event.text.isNotEmpty()) onChunk(event.text) + } + // Not shown, but proof the model was working; see StreamOutcome. is SseChunk.Event.Reasoning -> outcome.reasoningChars += event.text.length @@ -407,9 +516,49 @@ class OpenAiBackend( private fun emptyReplyFailure(outcome: StreamOutcome): OpenAiFailure = when { outcome.reasoningChars > 0 -> OpenAiFailure.ReasoningOnly outcome.finishReason == "length" -> OpenAiFailure.TruncatedBeforeReply + // Arguments that stop mid-JSON are a cut-off reply, whatever the server said stopped it. + outcome.droppedCalls > 0 -> OpenAiFailure.TruncatedBeforeReply else -> OpenAiFailure.EmptyReply(outcome.skippedChunks) } + /** + * Run [attempt] with [tools] declared and, if this server refuses a tool declaration, run it + * once more with none. + * + * The refusal is remembered per server so only the first turn pays for it. What it costs is + * real: the system prompt for this run was built for native calling, so it teaches no envelope + * and the model has no other way to reach a tool — the turn answers in prose. Servers that + * take `tools` are the overwhelming majority, and this keeps the rest chatting rather than + * failing outright. + * + * @param tools the tools this turn wants declared; an empty list skips straight through + * @param attempt the request to make, given the tools to actually declare + */ + private suspend fun withToolRetry( + tools: List, + attempt: suspend (List) -> Unit + ) { + val baseUrl = getBaseUrl() + if (tools.isEmpty() || toolsRejectedBy == baseUrl) { + attempt(emptyList()) + return + } + try { + attempt(tools) + } catch (e: CancellationException) { + throw e + } catch (e: OpenAiHttpException) { + if (!UnsupportedTools.rejectedIn(e.statusCode, e.body)) throw e + toolsRejectedBy = baseUrl + Log.w(TAG, "REQUEST | server refused a tool declaration; retrying with none") + context.logger.warn( + "OpenAiBackend: $baseUrl does not accept tool declarations; " + + "the agent cannot call tools on this server" + ) + attempt(emptyList()) + } + } + /** * Run [attempt] and, if the server rejected one optional parameter, run it once more without it. * diff --git a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiRequestBuilder.kt b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiRequestBuilder.kt index b95376f2..57d018d7 100644 --- a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiRequestBuilder.kt +++ b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiRequestBuilder.kt @@ -2,6 +2,7 @@ package com.itsaky.androidide.plugins.aiagentopenai.backend import com.itsaky.androidide.plugins.services.LlmInferenceService.ChatMessage import com.itsaky.androidide.plugins.services.LlmInferenceService.LlmConfig +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolDefinition import org.json.JSONArray import org.json.JSONObject @@ -43,7 +44,8 @@ internal object OpenAiRequestBuilder { ChatMessage.Role.USER -> ROLE_USER ChatMessage.Role.ASSISTANT -> ROLE_ASSISTANT ChatMessage.Role.SYSTEM -> ROLE_SYSTEM - // No native function calling here, so a tool result rides in as a user turn. + // A `tool` role is only legal after an assistant turn carrying the matching + // `tool_calls`, which ChatMessage gives the assistant turn no way to hold. ChatMessage.Role.TOOL -> ROLE_USER } messages.put(message(role, entry.content)) @@ -59,6 +61,7 @@ internal object OpenAiRequestBuilder { * @param stream true to ask for the SSE token stream * @param config supplies the token cap and temperature * @param tuning decides which optional parameters are sent at all + * @param tools the tools to declare; omitted from the body when empty * @return the request JSON */ fun body( @@ -67,12 +70,19 @@ internal object OpenAiRequestBuilder { stream: Boolean, config: LlmConfig, tuning: RequestTuning, + tools: List = emptyList(), ): JSONObject { val body = JSONObject() .put("model", model) .put("messages", messages) .put("stream", stream) + // Declared, not described in the prompt: the arguments then arrive already structured, so + // a file whose contents carry quotes or newlines can no longer break the call (ADFA-5410). + if (tools.isNotEmpty()) { + body.put("tools", OpenAiToolProtocol.toolsArray(tools)) + } + if (config.maxTokens > 0) { body.put(tuning.tokenParam, config.maxTokens) } diff --git a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiToolProtocol.kt b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiToolProtocol.kt new file mode 100644 index 00000000..30d569df --- /dev/null +++ b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiToolProtocol.kt @@ -0,0 +1,212 @@ +package com.itsaky.androidide.plugins.aiagentopenai.backend + +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolCallRequest +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolDefinition +import org.json.JSONArray +import org.json.JSONObject + +/** + * This backend's half of the native function-calling protocol: `tools[]` out, `tool_calls` in. + * + * Pure and free of Android types, so the shapes that decide whether a tool call runs at all are + * unit-testable without a device or a network — see [OpenAiSystemPrompt] for the same reasoning. + */ +internal object OpenAiToolProtocol { + + /** + * Nesting a declared schema may carry. A contributed (MCP) schema is provider-supplied text, + * and a pathologically deep one would otherwise recurse until the host process dies. + */ + private const val MAX_SCHEMA_DEPTH = 12 + + /** + * One `tool_calls` fragment as it arrives on the stream. + * + * A call is spread across as many chunks as its arguments need, so no single fragment is a + * call; [CallAccumulator] joins them. + * + * @property index the call's position in the turn, which is what fragments are joined on. + * @property id the provider's call id, present on the first fragment only. + * @property name the tool's name, likewise present once. + * @property arguments this fragment's slice of the arguments JSON, possibly a partial token. + */ + data class ToolCallDelta( + val index: Int, + val id: String?, + val name: String?, + val arguments: String, + ) + + /** + * The `tools[]` array declaring [tools] to the server. + * + * @param tools the tools to declare. + * @return one `{"type":"function","function":{…}}` entry per tool. + */ + fun toolsArray(tools: List): JSONArray { + val declarations = JSONArray() + for (tool in tools) { + val function = JSONObject() + .put("name", tool.name) + .put("description", tool.description.orEmpty()) + .put("parameters", parametersJson(tool.parametersSchema)) + declarations.put(JSONObject().put("type", "function").put("function", function)) + } + return declarations + } + + /** + * The `parameters` value for a tool. + * + * An empty schema becomes a bare object rather than being omitted: omitting `parameters` + * declares a tool that takes none, and the model would then call it with nothing. + * + * @param schema the tool's JSON Schema, empty when it publishes none. + * @return the schema to declare. + */ + fun parametersJson(schema: Map?): JSONObject { + if (schema.isNullOrEmpty()) return JSONObject().put("type", "object") + return schemaJson(schema, MAX_SCHEMA_DEPTH) + } + + /** + * Converts a JSON Schema to JSON. + * + * Passed through keyword for keyword, unlike the Gemini transport's whitelist: this protocol + * takes plain JSON Schema, which is the dialect a contributed tool already arrives in. + * + * @param schema the tool's JSON Schema. + * @param depth how much further nesting to render; a deeper subtree is dropped. + * @return the equivalent JSON. + */ + private fun schemaJson(schema: Map<*, *>, depth: Int): JSONObject { + val json = JSONObject() + if (depth <= 0) return json + for ((key, value) in schema) { + val name = key as? String ?: continue + json.put(name, jsonValue(value, depth)) + } + return json + } + + /** One schema value: a nested schema, a list of them, or a scalar as it stands. */ + private fun jsonValue(value: Any?, depth: Int): Any = when (value) { + null -> JSONObject.NULL + is Map<*, *> -> schemaJson(value, depth - 1) + is Collection<*> -> JSONArray().apply { value.forEach { put(jsonValue(it, depth)) } } + else -> value + } + + /** + * The `tool_calls` fragments carried by one streamed `delta` (or one-shot `message`). + * + * @param delta the chunk's `delta` or `message` object, or null when it has neither. + * @return the fragments, empty when the chunk carries no call. + */ + fun toolCallDeltas(delta: JSONObject?): List { + val calls = delta?.optJSONArray("tool_calls") ?: return emptyList() + val deltas = mutableListOf() + for (i in 0 until calls.length()) { + val call = calls.optJSONObject(i) ?: continue + val function = call.optJSONObject("function") + deltas += ToolCallDelta( + // Absent on servers that send a whole call per chunk; position in the array then. + index = if (call.has("index")) call.optInt("index") else i, + id = call.optString("id").takeIf { it.isNotBlank() }, + name = function?.optString("name")?.takeIf { it.isNotBlank() }, + arguments = function?.optString("arguments").orEmpty(), + ) + } + return deltas + } + + /** + * Joins streamed [ToolCallDelta] fragments back into whole calls. + * + * Not thread-safe: it belongs to the one reader loop consuming a single response body. + */ + class CallAccumulator { + + /** One call under construction, fed by every fragment carrying its index. */ + private class Entry(val id: String?, var name: String?) { + val arguments = StringBuilder() + } + + /** Every call this turn has begun, in arrival order. */ + private val entries = mutableListOf() + + /** The call each index is still receiving fragments for. */ + private val open = HashMap() + + /** + * Calls whose arguments never parsed, as of the last [requests] call. + * + * The diagnostic for a turn that asked for a tool and ran none: a cut-off stream leaves + * arguments half-written, which is a truncated reply rather than an empty one. + */ + var droppedCalls: Int = 0 + private set + + /** + * Folds one chunk's fragments in. + * @param deltas the fragments, in the order the chunk carried them. + */ + fun accept(deltas: List) { + for (delta in deltas) { + val existing = open[delta.index] + // A new id at a live index means a second call, not more of the first one. + val entry = if (existing == null || (delta.id != null && delta.id != existing.id)) { + Entry(delta.id, delta.name).also { entries += it; open[delta.index] = it } + } else { + existing.apply { if (name == null) name = delta.name } + } + entry.arguments.append(delta.arguments) + } + } + + /** + * The calls accumulated so far, in the order the stream began them. + * + * A call whose arguments will not parse is left out and counted in [droppedCalls] rather + * than reported with empty arguments, which would run the tool on nothing. + * + * @return the whole calls; empty when the turn carried none. + */ + fun requests(): List { + val requests = mutableListOf() + var dropped = 0 + for (entry in entries) { + val name = entry.name + if (name.isNullOrBlank()) { + dropped++ + continue + } + val args = argsOf(entry.arguments.toString()) + if (args == null) { + dropped++ + continue + } + // Paired by name when the server sent no id, never by an id it would not recognise. + requests += ToolCallRequest(entry.id ?: name, name, args) + } + droppedCalls = dropped + return requests + } + } + + /** + * Reads one call's `arguments` string. + * + * @param arguments the accumulated JSON; blank for a tool called with none. + * @return the arguments, or null when the JSON is incomplete or malformed. + */ + fun argsOf(arguments: String): Map? { + val text = arguments.trim() + if (text.isEmpty()) return emptyMap() + val json = runCatching { JSONObject(text) }.getOrNull() ?: return null + val args = mutableMapOf() + // Values stay as org.json types, as the Gemini transport also hands them over. + for (key in json.keys()) args[key] = json.get(key) + return args + } +} diff --git a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/RequestTuning.kt b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/RequestTuning.kt index 10bc6cf0..a4463844 100644 --- a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/RequestTuning.kt +++ b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/RequestTuning.kt @@ -105,7 +105,7 @@ internal object UnsupportedParameter { fun nameIn(body: String?): String? { if (body.isNullOrBlank()) return null val text = body.lowercase() - if (!soundsUnsupported(text)) return null + if (!saysUnsupported(text)) return null // The structured field is authoritative when the server supplies one. paramField(body)?.let { param -> if (param in ADJUSTABLE) return param } @@ -123,9 +123,47 @@ internal object UnsupportedParameter { OpenAiErrorFormatter.errorObjectIn(body)?.optString("param") ?.takeIf { it.isNotBlank() }?.lowercase() - /** True when the body says the parameter is not accepted, rather than that its value is bad. */ - private fun soundsUnsupported(text: String): Boolean = listOf( - "unsupported", "not supported", "unrecognized", "unknown", "unexpected", - "is not permitted", "instead", "deprecated", "extra inputs", - ).any { text.contains(it) } } + +/** + * Whether a server refused the request over the `tools` declaration itself. + * + * Its own detector rather than another [UnsupportedParameter] entry: dropping `tools` changes which + * protocol the run uses, so it is recovered from once per server instead of once per request. + */ +internal object UnsupportedTools { + + /** Statuses a compatible server uses to refuse a parameter outright. */ + private val REFUSAL_STATUSES = setOf(400, 404, 422) + + /** Request fields that only exist to carry tools, in either the modern or legacy spelling. */ + private val TOOL_FIELDS = listOf("tools", "tool_choice", "functions", "function_call") + + /** + * True when [body] says this server will not take a tool declaration. + * + * A rejected *schema* lands here too and degrades the same way, which is a turn that still + * answers rather than a lost one — see `OpenAiBackend.withToolRetry` for what that costs. + * + * @param statusCode the HTTP status the server answered with + * @param body the response body of that failure + * @return true to retry the request with no tools declared + */ + fun rejectedIn(statusCode: Int, body: String?): Boolean { + if (statusCode !in REFUSAL_STATUSES) return false + if (body.isNullOrBlank()) return false + val text = body.lowercase() + if (!saysUnsupported(text)) return false + return TOOL_FIELDS.any { text.contains(it) } + } +} + +/** + * True when an error body says a request field is not accepted, rather than that its value is bad. + * + * @param text the response body, lowercased + */ +private fun saysUnsupported(text: String): Boolean = listOf( + "unsupported", "not supported", "unrecognized", "unknown", "unexpected", + "is not permitted", "instead", "deprecated", "extra inputs", +).any { text.contains(it) } diff --git a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/SseChunk.kt b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/SseChunk.kt index a330eaef..c3719fab 100644 --- a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/SseChunk.kt +++ b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/SseChunk.kt @@ -24,6 +24,17 @@ internal object SseChunk { /** Visible reply text to append and hand to the caller. */ data class Token(val text: String) : Event + /** + * Fragments of one or more native tool calls, for [OpenAiToolProtocol.CallAccumulator]. + * + * @property deltas the chunk's `tool_calls` fragments. + * @property text prose the same chunk carried, which a call riding along must not drop. + */ + data class ToolCalls( + val deltas: List, + val text: String, + ) : Event + /** * Thinking text, which is **not** part of the reply. * @@ -99,6 +110,7 @@ internal object SseChunk { val content = StringBuilder() val reasoning = StringBuilder() + val toolCalls = mutableListOf() var finishReason: String? = null for (i in 0 until choices.length()) { val choice = choices.optJSONObject(i) ?: continue @@ -106,11 +118,15 @@ internal object SseChunk { val delta = choice.optJSONObject("delta") ?: choice.optJSONObject("message") content.append(delta?.optString("content").orEmpty()) reasoning.append(reasoningOf(delta)) + toolCalls += OpenAiToolProtocol.toolCallDeltas(delta) choice.optString("finish_reason").takeIf { it.isNotBlank() && it != "null" } ?.let { finishReason = it } } return when { + // Ahead of the text, which rides along: a call delta carries no `content`, so a chunk + // of them was `Ignored` before native calling and the turn came back empty. + toolCalls.isNotEmpty() -> Event.ToolCalls(toolCalls, content.toString()) content.isNotEmpty() -> Event.Token(content.toString()) reasoning.isNotEmpty() -> Event.Reasoning(reasoning.toString()) finishReason != null -> Event.Finish(finishReason) diff --git a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/prompt/OpenAiSystemPrompt.kt b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/prompt/OpenAiSystemPrompt.kt index 5b287fa4..24b8102e 100644 --- a/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/prompt/OpenAiSystemPrompt.kt +++ b/ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/prompt/OpenAiSystemPrompt.kt @@ -21,12 +21,19 @@ internal object OpenAiSystemPrompt { */ private const val FALLBACK_EXAMPLE_PATH = "app/src/main/java/com/example/MainActivity.kt" + /** How to call a tool when the caller takes calls through the provider's own API. */ + private val NATIVE_CALL_FORMAT = """ + TOOL CALL FORMAT — the tools above are declared to you: call one through the function-calling + API. A call written into your reply text is NOT read by this system and will not run. + Do NOT describe the action in prose (e.g. "Okay, I'll open the file…") — narrating does nothing. + """.trimIndent() + /** * Builds the prompt for [request]. * * [SystemPromptRequest.toolCallSyntax] is reproduced verbatim — a paraphrase would produce - * replies nothing reads — and a null one means the caller parses no envelope, so the format - * section and its examples are left out rather than taught in a syntax nothing reads back. + * replies nothing reads — and a null one means the caller reads calls off the provider's own + * function-calling API, so [NATIVE_CALL_FORMAT] replaces the envelope rather than joining it. * * @return the system prompt, without the caller's IDE-context block */ @@ -57,7 +64,6 @@ internal object OpenAiSystemPrompt { - old_string must be the text currently in the file and new_string what it should become. If they are identical the edit is rejected. - Never fabricate tool output. Emit a tool call, then wait for the real result before continuing. - Never write "User:", "Assistant:", a block, or a ```tool_response fence — the system supplies real results. Any tool output you write yourself is a hallucination and will be ignored. - - Do NOT use your provider's native function-calling channel. Tool calls travel in your reply text, in exactly the format below; a structured tool call is not read by this system. - Paths are relative to the project root and must be complete. If you don't know a file's exact path, find it with search_project or list_files first, then act on the real path — don't guess. - For plain chat (e.g. "Hi"), just reply briefly with no tool call. When the task is done, either give a short summary with no tool call, or end with a single respond call carrying that summary in its "message" — never an empty respond. """.trimIndent() @@ -65,7 +71,7 @@ internal object OpenAiSystemPrompt { val workflow = """ WORKFLOW: 1. Understand the user's request - 2. List files to understand the project structure + 2. Locate what you need with ONE search_project call — the IDE CONTEXT block above already names the source, layout and manifest paths 3. Create/modify files with complete implementations 4. Add dependencies if needed 5. Sync gradle and verify compilation @@ -73,12 +79,16 @@ internal object OpenAiSystemPrompt { 7. Report success and what was built """.trimIndent() - val syntax = request.toolCallSyntax ?: return head + "\n\n" + workflow + // Null syntax means the caller reads calls off the function-calling API instead. Saying so + // is what stops the model writing one as text, where nothing would run it (ADFA-5410). + val syntax = request.toolCallSyntax ?: return listOf(head, NATIVE_CALL_FORMAT, workflow) + .joinToString("\n\n") val callFormat = """ TOOL CALL FORMAT — to run a tool, emit a single line in EXACTLY this format and nothing after it: $syntax Do NOT describe the action in prose (e.g. "Okay, I'll open the file…") — narrating does nothing. + Do NOT use your provider's native function-calling channel either; a structured tool call is not read by this system. The tool only runs when you emit the tool call line itself. FORMAT EXAMPLES (the tool call is the entire reply; the paths are this project's — reuse a path diff --git a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiBackendTest.kt b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiBackendTest.kt new file mode 100644 index 00000000..27fa42aa --- /dev/null +++ b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiBackendTest.kt @@ -0,0 +1,35 @@ +package com.itsaky.androidide.plugins.aiagentopenai.backend + +import com.itsaky.androidide.plugins.services.LlmInferenceService.CancellableBackend +import com.itsaky.androidide.plugins.services.LlmInferenceService.HistoryCapableBackend +import com.itsaky.androidide.plugins.services.LlmInferenceService.LlmBackend +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolCallingBackend +import io.mockk.mockk +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * What this backend declares to the caller. Each interface is optional, so dropping one compiles + * and degrades silently — which is the only way these regress. + */ +class OpenAiBackendTest { + + private val backend = OpenAiBackend(mockk(relaxed = true)) + + @Test + fun givenTheBackend_whenAskedForItsIdentity_thenItRegistersAsOpenAi() { + assertEquals("openai", backend.getId()) + } + + @Test + fun givenTheBackend_whenAskedForItsCapabilities_thenItDeclaresToolCallingToo() { + // Dropping ToolCallingBackend drops the agent back to parsing calls out of the reply text, + // and takes the prompt's envelope instructions with it (ADFA-5410). + val declared: LlmBackend = backend + + assertTrue(declared is HistoryCapableBackend) + assertTrue(declared is CancellableBackend) + assertTrue(declared is ToolCallingBackend) + } +} diff --git a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiRequestBuilderTest.kt b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiRequestBuilderTest.kt index 96965a04..38cae968 100644 --- a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiRequestBuilderTest.kt +++ b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiRequestBuilderTest.kt @@ -2,6 +2,7 @@ package com.itsaky.androidide.plugins.aiagentopenai.backend import com.itsaky.androidide.plugins.services.LlmInferenceService.ChatMessage import com.itsaky.androidide.plugins.services.LlmInferenceService.LlmConfig +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolDefinition import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue @@ -69,7 +70,8 @@ class OpenAiRequestBuilderTest { @Test fun givenAToolResult_whenBuildingMessages_thenItRidesInAsAUserTurn() { - // This backend declares no ToolCallingBackend, so there is no tool role to send it as. + // A `tool` role is only legal after an assistant turn carrying the matching `tool_calls`, + // which ChatMessage gives the assistant turn no way to hold. val history = listOf(ChatMessage.toolResult("call_1", "read_file", "file contents")) val messages = OpenAiRequestBuilder.messages(history, "next", null) @@ -157,4 +159,37 @@ class OpenAiRequestBuilderTest { ) assertFalse(body.has("stop")) } + + @Test + fun givenTools_whenBuildingTheBody_thenTheyAreDeclaredToTheServer() { + val body = OpenAiRequestBuilder.body( + OpenAiRequestBuilder.messages(emptyList(), "Hi", null), + "gpt-4o", + stream = true, + config = config(), + tuning = defaultTuning, + tools = listOf(ToolDefinition("read_file", "Read a file", emptyMap())), + ) + + val declared = body.getJSONArray("tools") + assertEquals(1, declared.length()) + assertEquals( + "read_file", + declared.getJSONObject(0).getJSONObject("function").getString("name") + ) + } + + @Test + fun givenNoTools_whenBuildingTheBody_thenNoToolsFieldIsSent() { + // Plain chat, and a server that rejects an empty `tools` array would 400 on the request. + val body = OpenAiRequestBuilder.body( + OpenAiRequestBuilder.messages(emptyList(), "Hi", null), + "gpt-4o", + stream = true, + config = config(), + tuning = defaultTuning, + ) + + assertFalse(body.has("tools")) + } } diff --git a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiToolProtocolTest.kt b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiToolProtocolTest.kt new file mode 100644 index 00000000..c3b65a57 --- /dev/null +++ b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiToolProtocolTest.kt @@ -0,0 +1,201 @@ +package com.itsaky.androidide.plugins.aiagentopenai.backend + +import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolDefinition +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Unit tests for [OpenAiToolProtocol]. Focus: ADFA-5410, where a tool call written as reply text + * could not be read back once its arguments carried quotes. Declaring the tools is what stops + * that, so the declaration and the `tool_calls` accumulation are both pinned down here. + */ +class OpenAiToolProtocolTest { + + private fun schema(vararg properties: Pair>, required: List) = + mapOf( + "type" to "object", + "properties" to properties.toMap(), + "required" to required, + ) + + private fun deltas(json: String) = + OpenAiToolProtocol.toolCallDeltas(JSONObject(json)) + + @Test + fun givenAToolWithArguments_whenDeclared_thenItsSchemaTravelsWithIt() { + val declarations = OpenAiToolProtocol.toolsArray( + listOf( + ToolDefinition( + "read_file", + "Read a file", + schema( + "file_path" to mapOf("type" to "string", "description" to "Path to read."), + required = listOf("file_path"), + ), + ) + ) + ) + + assertEquals(1, declarations.length()) + val entry = declarations.getJSONObject(0) + assertEquals("function", entry.getString("type")) + val function = entry.getJSONObject("function") + assertEquals("read_file", function.getString("name")) + assertEquals("Read a file", function.getString("description")) + val parameters = function.getJSONObject("parameters") + // Lower case, unlike the Gemini transport: this protocol takes plain JSON Schema. + assertEquals("object", parameters.getString("type")) + assertEquals( + "string", + parameters.getJSONObject("properties").getJSONObject("file_path").getString("type") + ) + assertEquals("file_path", parameters.getJSONArray("required").getString(0)) + } + + @Test + fun givenAToolWithNoSchema_whenDeclared_thenParametersIsStillAnObject() { + // Omitting `parameters` declares a tool that takes none, and the model would then call it + // with nothing at all. + val declarations = OpenAiToolProtocol.toolsArray( + listOf(ToolDefinition("run_app", "Build and run", emptyMap())) + ) + + val parameters = declarations.getJSONObject(0).getJSONObject("function") + .getJSONObject("parameters") + assertEquals("object", parameters.getString("type")) + } + + @Test + fun givenASchemaCarryingKeywordsGeminiWouldReject_whenConverted_thenTheySurvive() { + val parameters = OpenAiToolProtocol.parametersJson( + mapOf( + "type" to "object", + "additionalProperties" to false, + "properties" to mapOf("q" to mapOf("type" to "string")), + "required" to listOf("q"), + ) + ) + + assertFalse(parameters.getBoolean("additionalProperties")) + assertEquals("q", parameters.getJSONArray("required").getString(0)) + } + + @Test + fun givenAnAbsurdlyNestedSchema_whenConverted_thenItStopsRatherThanRecursingForever() { + // A contributed (MCP) schema is provider-supplied; unbounded recursion would take the host + // process down with it. + var schema = mapOf("type" to "string") + repeat(200) { schema = mapOf("type" to "object", "properties" to mapOf("next" to schema)) } + + val parameters = OpenAiToolProtocol.parametersJson(schema) + + assertEquals("object", parameters.getString("type")) + } + + @Test + fun givenArgumentsSplitAcrossChunks_whenAccumulated_thenTheCallIsWholeAgain() { + val accumulator = OpenAiToolProtocol.CallAccumulator() + + accumulator.accept( + deltas("""{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"create_file","arguments":"{\"file_path\":\"a.kt\","}}]}""") + ) + accumulator.accept( + deltas("""{"tool_calls":[{"index":0,"function":{"arguments":"\"content\":\"val s = \\\"hi\\\"\"}"}}]}""") + ) + val calls = accumulator.requests() + + assertEquals(1, calls.size) + assertEquals("call_1", calls[0].callId) + assertEquals("create_file", calls[0].name) + assertEquals("a.kt", calls[0].args.orEmpty()["file_path"]) + // The payload ADFA-5410 lost: a quoted string inside an argument value. + assertEquals("""val s = "hi"""", calls[0].args.orEmpty()["content"]) + assertEquals(0, accumulator.droppedCalls) + } + + @Test + fun givenTwoCallsInOneTurn_whenAccumulated_thenEachKeepsItsOwnArguments() { + val accumulator = OpenAiToolProtocol.CallAccumulator() + + accumulator.accept( + deltas("""{"tool_calls":[ + {"index":0,"id":"a","function":{"name":"read_file","arguments":"{\"file_path\":\"x\"}"}}, + {"index":1,"id":"b","function":{"name":"open_file","arguments":"{\"file_path\":\"y\"}"}} + ]}""") + ) + val calls = accumulator.requests() + + assertEquals(listOf("read_file", "open_file"), calls.map { it.name }) + assertEquals("x", calls[0].args.orEmpty()["file_path"]) + assertEquals("y", calls[1].args.orEmpty()["file_path"]) + } + + @Test + fun givenWholeCallsWithNoIndex_whenAccumulated_thenANewIdStartsANewCall() { + // Some compatible servers send a complete call per chunk and number none of them. + val accumulator = OpenAiToolProtocol.CallAccumulator() + + accumulator.accept( + deltas("""{"tool_calls":[{"id":"a","function":{"name":"read_file","arguments":"{}"}}]}""") + ) + accumulator.accept( + deltas("""{"tool_calls":[{"id":"b","function":{"name":"list_files","arguments":"{}"}}]}""") + ) + + assertEquals(listOf("read_file", "list_files"), accumulator.requests().map { it.name }) + } + + @Test + fun givenACallCutOffMidArguments_whenAccumulated_thenItIsDroppedRatherThanRunOnNothing() { + val accumulator = OpenAiToolProtocol.CallAccumulator() + + accumulator.accept( + deltas("""{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"edit_file","arguments":"{\"file_path\":\"a"}}]}""") + ) + val calls = accumulator.requests() + + assertTrue(calls.isEmpty()) + assertEquals(1, accumulator.droppedCalls) + } + + @Test + fun givenACallWithNoArguments_whenAccumulated_thenItRunsWithNone() { + val accumulator = OpenAiToolProtocol.CallAccumulator() + + accumulator.accept( + deltas("""{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"gradle_sync","arguments":""}}]}""") + ) + val calls = accumulator.requests() + + assertEquals(1, calls.size) + assertTrue(calls[0].args.orEmpty().isEmpty()) + assertEquals(0, accumulator.droppedCalls) + } + + @Test + fun givenACallWithNoId_whenAccumulated_thenItIsIdentifiedByName() { + val accumulator = OpenAiToolProtocol.CallAccumulator() + + accumulator.accept( + deltas("""{"tool_calls":[{"index":0,"function":{"name":"respond","arguments":"{\"message\":\"done\"}"}}]}""") + ) + + assertEquals("respond", accumulator.requests()[0].callId) + } + + @Test + fun givenAChunkWithNoToolCalls_whenRead_thenNoFragmentsComeBack() { + assertTrue(deltas("""{"content":"Hello"}""").isEmpty()) + assertTrue(OpenAiToolProtocol.toolCallDeltas(null).isEmpty()) + } + + @Test + fun givenMalformedArguments_whenRead_thenTheyAreRefusedRatherThanGuessedAt() { + assertNull(OpenAiToolProtocol.argsOf("""{"file_path":}""")) + assertEquals(emptyMap(), OpenAiToolProtocol.argsOf(" ")) + } +} diff --git a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/RequestTuningTest.kt b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/RequestTuningTest.kt index 5b5b09bf..bafd9351 100644 --- a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/RequestTuningTest.kt +++ b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/RequestTuningTest.kt @@ -162,4 +162,27 @@ class UnsupportedParameterTest { assertNull(UnsupportedParameter.nameIn(null)) assertNull(UnsupportedParameter.nameIn("")) } + + @Test + fun givenAServerThatRefusesToolDeclarations_whenClassified_thenTheRetryDropsThem() { + val body = """{"error":{"message":"Unsupported parameter: 'tools'","param":"tools"}}""" + assertTrue(UnsupportedTools.rejectedIn(400, body)) + // Some compatible servers answer an unknown field with 422 instead. + assertTrue(UnsupportedTools.rejectedIn(422, """{"error":"unknown field: functions"}""")) + } + + @Test + fun givenAFailureAboutSomethingElse_whenClassified_thenToolsAreKept() { + // Dropping tools changes which protocol the run uses, so only a refusal of the + // declaration itself may trigger it. + assertFalse(UnsupportedTools.rejectedIn(400, """{"error":{"message":"model not found"}}""")) + assertFalse( + UnsupportedTools.rejectedIn( + 400, + """{"error":{"message":"Unsupported parameter: 'temperature'"}}""" + ) + ) + assertFalse(UnsupportedTools.rejectedIn(401, """{"error":"unsupported tools"}""")) + assertFalse(UnsupportedTools.rejectedIn(400, null)) + } } diff --git a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/SseChunkTest.kt b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/SseChunkTest.kt index 3dbf7bdb..ef76e596 100644 --- a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/SseChunkTest.kt +++ b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/SseChunkTest.kt @@ -148,4 +148,43 @@ class SseChunkTest { """data: {"choices":[{"delta":{"content":"a"}},{"delta":{"content":"b"}}]}""" assertEquals(SseChunk.Event.Token("ab"), SseChunk.parse(line)) } + + @Test + fun givenAToolCallDelta_whenParsed_thenItsFragmentsComeBackInsteadOfBeingIgnored() { + // Such a delta carries no `content`, so it used to fall through to Ignored and the turn + // came back empty — which is how a declared tool call ran nothing (ADFA-5410). + val line = """data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_1",""" + + """"type":"function","function":{"name":"read_file","arguments":"{\"a\":1}"}}]}}]}""" + + val event = SseChunk.parse(line) + + assertTrue("expected ToolCalls, got $event", event is SseChunk.Event.ToolCalls) + val deltas = (event as SseChunk.Event.ToolCalls).deltas + assertEquals(1, deltas.size) + assertEquals("read_file", deltas[0].name) + assertEquals("call_1", deltas[0].id) + assertEquals("""{"a":1}""", deltas[0].arguments) + } + + @Test + fun givenAChunkCarryingBothProseAndACall_whenParsed_thenNeitherIsLost() { + val line = """data: {"choices":[{"delta":{"content":"On it. ","tool_calls":""" + + """[{"index":0,"function":{"name":"run_app","arguments":"{}"}}]}}]}""" + + val event = SseChunk.parse(line) as SseChunk.Event.ToolCalls + + assertEquals("On it. ", event.text) + assertEquals("run_app", event.deltas[0].name) + } + + @Test + fun givenAWholesaleAnswerCarryingACall_whenParsed_thenTheCallIsStillRead() { + // The same servers that ignore stream:true answer a tool call in `message` too. + val line = """data: {"choices":[{"message":{"role":"assistant","tool_calls":""" + + """[{"index":0,"id":"c1","function":{"name":"gradle_sync","arguments":"{}"}}]}}]}""" + + val event = SseChunk.parse(line) as SseChunk.Event.ToolCalls + + assertEquals("gradle_sync", event.deltas[0].name) + } } diff --git a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/prompt/OpenAiSystemPromptTest.kt b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/prompt/OpenAiSystemPromptTest.kt index 4ebb9135..b35f5533 100644 --- a/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/prompt/OpenAiSystemPromptTest.kt +++ b/ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/prompt/OpenAiSystemPromptTest.kt @@ -2,6 +2,7 @@ package com.itsaky.androidide.plugins.aiagentopenai.prompt import com.itsaky.androidide.plugins.services.LlmInferenceService.SystemPromptRequest import com.itsaky.androidide.plugins.services.LlmInferenceService.ToolDefinition +import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test @@ -49,16 +50,25 @@ class OpenAiSystemPromptTest { } @Test - fun givenNoToolCallSyntax_whenBuilt_thenTheEnvelopeSectionIsLeftOut() { - // A null syntax means the caller parses no envelope; teaching one produces unread replies. + fun givenNoToolCallSyntax_whenBuilt_thenTheEnvelopeIsNeverTaught() { + // A null syntax means the caller parses no envelope; an example of one is a call that + // would not run. val prompt = OpenAiSystemPrompt.build(request(syntax = null)) - assertFalse(prompt.contains("TOOL CALL FORMAT")) assertFalse(prompt.contains("")) assertTrue(prompt.contains("AVAILABLE TOOLS:")) assertTrue(prompt.contains("WORKFLOW:")) } + @Test + fun givenEitherMode_whenBuilt_thenExactlyOneWayToCallAToolIsTaught() { + // Teaching both (ADFA-5410) is how one call runs twice: the provider carries it and the + // text copy is extracted as a second call. + listOf(request(), request(syntax = null)).forEach { request -> + assertEquals(1, OpenAiSystemPrompt.build(request).split("TOOL CALL FORMAT").size - 1) + } + } + @Test fun givenNoExamplePath_whenBuilt_thenTheExamplesStillCarryAConcretePath() { val prompt = OpenAiSystemPrompt.build(request(examplePath = null)) @@ -67,12 +77,22 @@ class OpenAiSystemPromptTest { } @Test - fun givenAnyRequest_whenBuilt_thenNativeFunctionCallingIsForbidden() { - // This backend declares no ToolCallingBackend, so a model using its own channel would hang. + fun givenAToolCallSyntax_whenBuilt_thenNativeFunctionCallingIsForbidden() { + // In envelope mode nothing reads the provider's channel, so a model using it would hang. val prompt = OpenAiSystemPrompt.build(request()) assertTrue(prompt.contains("native function-calling channel")) } + @Test + fun givenNoToolCallSyntax_whenBuilt_thenTheFunctionCallingApiIsNamedInstead() { + // The reverse of the rule above: the tools are declared, so the channel is the only way in + // and forbidding it would leave the model no way to call anything. + val prompt = OpenAiSystemPrompt.build(request(syntax = null)) + + assertTrue(prompt.contains("function-calling")) + assertFalse(prompt.contains("Do NOT use your provider's native function-calling channel")) + } + @Test fun givenAnyRequest_whenBuilt_thenOneToolCallPerReplyIsRequired() { assertTrue(OpenAiSystemPrompt.build(request()).contains("Emit ONE tool call per reply")) diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/logging/AgentTrace.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/logging/AgentTrace.kt index f645aa03..3d0187f4 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/logging/AgentTrace.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/logging/AgentTrace.kt @@ -8,6 +8,10 @@ import java.util.concurrent.atomic.AtomicInteger * One log stream for a whole agent run: prompt, model turns, parsing, guards, approval, result. * Every line shares one tag, a per-run id, elapsed ms and a sequence number (`adb logcat -s * AiCore.AgentTrace:*`). Runs never overlap, so the current run is object state, not a parameter. + * + * A backend plugin cannot reach this class — it ships in `ai-core` — so one that traces its own + * half of a run logs under `.AgentTrace` instead, and + * `adb logcat -s AiCore.AgentTrace:V AiAgentGemini.AgentTrace:V` follows both halves in order. */ object AgentTrace { @@ -17,11 +21,15 @@ object AgentTrace { const val PREVIEW_CHARS = 120 /** - * Whether previewed content (prompt, code snippets, model replies) reaches logcat: debug only. - * The structured head of each line is what a trace is read for and always logs; a release build - * has no reason to write the user's source into a log it does not own. + * Whether previewed content (prompt, code snippets, model replies) reaches logcat. + * + * On in a debug build. A release `.cgp` — what the Plugin Manager installs — writes only the + * structured head of each line, which is what a trace is read for, until someone asks for the + * content with `adb shell setprop log.tag.AiCore.AgentTrace VERBOSE`. Read per line rather than + * cached, so that takes effect on the next message instead of the next IDE restart. */ - private val CONTENT_LOGGING = BuildConfig.DEBUG + private val contentLogging: Boolean + get() = BuildConfig.DEBUG || Log.isLoggable(TAG, Log.VERBOSE) @Volatile private var runId: String = "-" @@ -121,6 +129,6 @@ object AgentTrace { private fun line(stage: String, detail: String, preview: String?): String { val elapsed = if (runStartMs == 0L) 0 else System.currentTimeMillis() - runStartMs val head = "[$runId +${elapsed}ms #${sequence.incrementAndGet()}] $stage | $detail" - return if (preview == null || !CONTENT_LOGGING) head else "$head | $preview" + return if (preview == null || !contentLogging) head else "$head | $preview" } } diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/AgentLoop.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/AgentLoop.kt index ffa3268d..31624aab 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/AgentLoop.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/AgentLoop.kt @@ -15,6 +15,8 @@ class AgentLoop( private val toolOutputCharLimit: Int = DEFAULT_TOOL_OUTPUT_CHAR_LIMIT, private val maxConsecutiveRepeats: Int = DEFAULT_MAX_CONSECUTIVE_REPEATS, private val extractToolCalls: (String) -> List = ToolCallExtractor::extractToolCalls, + private val diagnoseUnparsedReply: (String) -> ToolCallExtractor.UnparsedReply? = + ToolCallExtractor::diagnoseUnparsedReply, private val terminalTool: String? = null, ) { @@ -65,19 +67,63 @@ class AgentLoop( */ suspend fun onRepeatedToolCalls(turns: Int) {} + /** + * The model re-issued the batch it had just run successfully, which the loop reads as the + * work being finished rather than as a repeat to abort on. + * + * The only completion that reaches the user with nothing said: it fires instead of + * [onFinalAnswer], because the model never called the terminal tool. + * + * @param turn 1-based turn index. + */ + suspend fun onRepeatAfterSuccess(turn: Int) {} + /** * The model called the terminal tool to finish. * @param turn 1-based turn index. * @param message the model's final answer. */ suspend fun onFinalAnswer(turn: Int, message: String) {} + + /** + * The reply read like a tool call but none could be parsed out of it, so nothing ran. + * @param turn 1-based turn index. + * @param reason why the call could not be read. + */ + suspend fun onUnparsedReply(turn: Int, reason: ToolCallExtractor.UnparsedReply) {} + + /** + * The model stopped calling tools with the last batch's failure unaddressed. + * + * Distinct from [onFinalAnswer]: the task did not finish, so a run that ends here must not + * be reported as completed. + * + * @param turn 1-based turn index. + */ + suspend fun onAbandonedAfterFailure(turn: Int) {} } /** Why the loop stopped. */ - enum class StopReason { COMPLETED, MAX_ITERATIONS, REPEATED } + enum class StopReason { + /** The model ended the run itself, with nothing outstanding. */ + COMPLETED, + + /** The step budget ran out first. */ + MAX_ITERATIONS, + + /** The model kept re-issuing a batch that was not working. */ + REPEATED, + + /** A reply meant to call a tool and no call could be read out of it. */ + UNPARSABLE, + + /** The model gave up: it stopped calling tools with a failed one unaddressed. */ + ABANDONED, + } /** - * Outcome of a run; [completed] is true when the model ended on its own. + * Outcome of a run; [completed] is true when the model ended on its own with nothing left + * outstanding, which is not the same as the loop simply having stopped. * @property turns model turns executed. * @property reason why the loop stopped. */ @@ -104,7 +150,8 @@ class AgentLoop( var turn = 0 var previousSignature: String? = null var consecutiveRepeats = 0 - var previousBatchSucceeded = false + // Null until a batch has run: "no tools yet" and "the tools failed" end a run differently. + var previousBatchSucceeded: Boolean? = null while (turn < maxIterations) { turn++ @@ -114,6 +161,19 @@ class AgentLoop( val calls = extractToolCalls(text) if (calls.isEmpty()) { + // A reply with no call is an ordinary answer; one that meant to call and failed to + // is a silent dead end, and reporting it COMPLETED is what hid it from the user. + val unparsed = diagnoseUnparsedReply(text) + if (unparsed != null) { + events.onUnparsedReply(turn, unparsed) + return Result(turn, StopReason.UNPARSABLE) + } + // Prose after a failed batch is the model giving up, not finishing: the run ends + // with the user's request unmet, so reporting it COMPLETED overstates the outcome. + if (previousBatchSucceeded == false) { + events.onAbandonedAfterFailure(turn) + return Result(turn, StopReason.ABANDONED) + } return Result(turn, StopReason.COMPLETED) } @@ -131,7 +191,8 @@ class AgentLoop( val signature = signatureOf(realCalls) if (signature == previousSignature) { - if (previousBatchSucceeded) { + if (previousBatchSucceeded == true) { + events.onRepeatAfterSuccess(turn) return Result(turn, StopReason.COMPLETED) } consecutiveRepeats++ diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/Executor.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/Executor.kt index fdbda2fb..7b6a0991 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/Executor.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/Executor.kt @@ -236,11 +236,23 @@ class Executor( toolExecutionTracker?.logToolCall(toolName, toolDuration) Log.i(TAG, "($executionMode): Result: ${result.toResultMap()}") - AgentTrace.stage( - "EXEC", - "$toolName done success=${result.success} tookMs=$toolDuration", - AgentTrace.preview(result.message), - ) + // A failure's reason goes in the head, not the preview: it is the handler's own short + // message rather than the user's content, and a release build drops previews — which is + // what left a run_app that failed in 0ms indistinguishable from one that ran and failed. + if (result.success) { + AgentTrace.stage( + "EXEC", + "$toolName done success=true tookMs=$toolDuration", + AgentTrace.preview(result.message), + ) + } else { + AgentTrace.refusal( + "EXEC", + "$toolName done success=false tookMs=$toolDuration " + + "reason=${AgentTrace.preview(result.message, 80)}", + result.error_details.orEmpty(), + ) + } return result } diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCallExtractor.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCallExtractor.kt index 69a9890d..e4581820 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCallExtractor.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCallExtractor.kt @@ -10,6 +10,19 @@ import org.json.JSONObject * `{"tool":...}` JSON second. Works for both the cloud and local backends. */ class ToolCallExtractor { + + /** + * Why a reply that reads like a tool call produced none. Each state needs different advice, + * so the caller can tell the user what to do rather than dump the raw reply. + */ + enum class UnparsedReply { + /** The envelope was never closed; the reply hit the model's output cap mid-call. */ + TRUNCATED, + + /** The envelope closed but its JSON would not parse, usually an unescaped quote. */ + MALFORMED, + } + companion object { private const val TAG = "$LOG_PREFIX.ToolCallExtractor" @@ -17,6 +30,76 @@ class ToolCallExtractor { private val TOOL_CALL_REGEX = Regex("""\s*(.+?)\s*""", RegexOption.DOT_MATCHES_ALL) + /** Opening envelope tag, as [TOOL_CALL_REGEX] matches it and [renderEnvelope] writes it. */ + private const val OPEN_TAG = "" + + /** Closing envelope tag; see [OPEN_TAG]. */ + private const val CLOSE_TAG = "" + + /** The key that marks an object as a call. Scanned per character, so kept a plain string. */ + private const val TOOL_KEY = "\"tool\"" + + /** + * The `"tool"` key of a bare (unenveloped) call, as a JSON key rather than as the word. + * + * The colon is what makes it a key: a reply that merely says the word `"tool"` in quotes is + * prose, and reading it as a broken call reports a failure that never happened. + */ + private val BARE_TOOL_KEY_REGEX = Regex(""""tool"\s*:""") + + /** + * Classifies a reply that [extractToolCalls] found nothing in. + * + * Only meaningful for such a reply: a parsed envelope matches these shapes too, so calling + * it on a reply that yielded calls reports a failure that did not happen. + * + * @param text the model's raw reply. + * @return the failure, or null when the reply is ordinary prose and nothing went wrong. + */ + fun diagnoseUnparsedReply(text: String): UnparsedReply? { + val opened = text.indexOf(OPEN_TAG) + if (opened >= 0) { + val closed = text.indexOf(CLOSE_TAG, opened + OPEN_TAG.length) + return if (closed < 0) UnparsedReply.TRUNCATED else UnparsedReply.MALFORMED + } + // A `tool_code` block Strategy 3 could not read, e.g. one passing arguments positionally. + if (ToolCodeParser.looksLikeToolCode(text)) return UnparsedReply.MALFORMED + // No envelope at all, but a bare call the JSON strategy could not read. + return if (containsBareToolCall(text)) UnparsedReply.MALFORMED else null + } + + /** + * Whether [text] holds something shaped like a bare `{"tool":…}` call. + * + * Requires the key to sit inside an object, so neither the diagnosis nor the prose filter + * fires on a sentence that quotes the word. + * + * @param text the text to inspect. + * @return true when a bare call is present. + */ + private fun containsBareToolCall(text: String): Boolean { + val key = BARE_TOOL_KEY_REGEX.find(text) ?: return false + return text.lastIndexOf('{', key.range.first) >= 0 + } + + /** + * Renders one call as the canonical envelope, escaping through [JSONObject]. + * + * This is how a backend's native function call re-enters the text pipeline: encoding it + * here rather than trusting the model to is what puts such a call out of + * [UnparsedReply.MALFORMED]'s reach. + * + * @param name the tool's name. + * @param args its arguments. + * @return the envelope, ready to append to the reply text. + */ + fun renderEnvelope(name: String, args: Map): String { + val argsJson = JSONObject() + for ((key, value) in args) argsJson.put(key, value ?: JSONObject.NULL) + val call = JSONObject().put("tool", name).put("args", argsJson) + return "$OPEN_TAG$call$CLOSE_TAG" + } + /** * A tool result written by the model itself, as a ```tool_response fence or a * `` tag. Both system prompts forbid these and promise such output is @@ -58,8 +141,9 @@ class ToolCallExtractor { fun proseOutsideToolCalls(text: String): String? { val remainder = TOOL_CALL_REGEX.replace(beforeFabricatedResult(text), "\n").trim() if (remainder.isEmpty()) return null - // A leftover `"tool"` key is an unenveloped call; raw JSON is worse than nothing. - if (remainder.contains("\"tool\"")) return null + // A leftover bare call is not prose; raw JSON is worse than nothing. + if (containsBareToolCall(remainder)) return null + if (ToolCodeParser.looksLikeToolCode(remainder)) return null return remainder } @@ -77,14 +161,31 @@ class ToolCallExtractor { // Anything past a tool result the model wrote itself is an invented continuation. val body = beforeFabricatedResult(text) + // Which strategy read the reply, for the trace: a call that arrives as `tool_code` + // rather than `envelope` is the model ignoring the protocol, not this side failing. + var strategy = "none" + // Strategy 1: Explicit XML tags toolCalls.addAll(extractFromXmlTags(body)) + if (toolCalls.isNotEmpty()) strategy = "envelope" // Strategy 2: Bare JSON objects if no XML found if (toolCalls.isEmpty()) { toolCalls.addAll(extractFromJsonObjects(body)) + if (toolCalls.isNotEmpty()) strategy = "bare_json" + } + + // Strategy 3: Gemini's own `default_api` dialect, which neither prompt asks for. + if (toolCalls.isEmpty()) { + val fromToolCode = ToolCodeParser.parse(body) + if (fromToolCode.isNotEmpty()) { + Log.d(TAG, "Strategy 3 (tool_code): Found ${fromToolCode.size} matches") + toolCalls.addAll(fromToolCode) + strategy = "tool_code" + } } + AgentTrace.detail("EXTRACT", "strategy=$strategy calls=${toolCalls.size} chars=${body.length}") Log.d(TAG, "Extracted ${toolCalls.size} tool calls from response (${body.length} chars)") // Warn if we found incomplete tool calls @@ -153,7 +254,7 @@ class ToolCallExtractor { } // Check if this substring contains "tool" - if (!hasToolField && text.substring(i, minOf(j + 1, text.length)).contains("\"tool\"")) { + if (!hasToolField && text.substring(i, minOf(j + 1, text.length)).contains(TOOL_KEY)) { hasToolField = true } diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCodeParser.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCodeParser.kt new file mode 100644 index 00000000..1508d605 --- /dev/null +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCodeParser.kt @@ -0,0 +1,258 @@ +package com.itsaky.androidide.plugins.aicore.tool + +/** + * Parses Gemini's own call dialect — `print(default_api.run_app())`, usually inside a `` + * block — into [ToolCall]s. + * + * Gemini falls back to this surface when the tools are described in prose rather than declared + * through its function-calling API. It is not the envelope either system prompt teaches, so a reply + * written in it ran nothing and was pasted into the chat instead (ADFA-5410). + * + * Pure and free of Android types, so the quoting rules that decide whether a call runs are + * unit-testable without a device. + */ +object ToolCodeParser { + + /** The call itself. Anchored on `default_api.`, which no ordinary prose contains. */ + private val DEFAULT_API_CALL = Regex("""default_api\.([A-Za-z_]\w*)\s*\(""") + + /** Anything that reads as this dialect, whether or not a call could be parsed out of it. */ + private val TOOL_CODE_MARKER = + Regex("""|```+\s*tool_code|default_api\.""", RegexOption.IGNORE_CASE) + + /** A keyword argument's name. */ + private val IDENTIFIER = Regex("""[A-Za-z_]\w*""") + + /** + * Whether [text] is written in this dialect. + * + * True even for a block [parse] rejects, so a call this side cannot read is still reported to + * the user rather than mistaken for an ordinary prose reply. + * + * @param text the model's raw reply. + * @return true when the reply reads as `tool_code`. + */ + fun looksLikeToolCode(text: String): Boolean = TOOL_CODE_MARKER.containsMatchIn(text) + + /** + * Every `default_api.(…)` call in [text], in the order they appear. + * + * Only keyword arguments are read. Mapping a positional argument to a parameter needs the + * tool's schema, which is not here, and a tool run with its arguments in the wrong slots is + * worse than one that did not run — so such a call is skipped for [looksLikeToolCode] to report. + * + * @param text the model's raw reply. + * @return the calls found; empty when there are none this side can read. + */ + fun parse(text: String): List { + val calls = mutableListOf() + for (match in DEFAULT_API_CALL.findAll(text)) { + val open = match.range.last + val close = matchingBracket(text, open) + if (close < 0) continue + val args = parseKeywordArgs(text.substring(open + 1, close)) ?: continue + calls += ToolCall(match.groupValues[1], args) + } + return calls + } + + /** + * The index of the bracket closing the one at [open], skipping over string literals. + * + * @return the closing index, or -1 when the call is unterminated. + */ + private fun matchingBracket(text: String, open: Int): Int { + var depth = 0 + var i = open + while (i < text.length) { + val c = text[i] + if (c == '"' || c == '\'') { + val end = endOfString(text, i) + if (end < 0) return -1 + i = end + continue + } + if (c == '(' || c == '[' || c == '{') depth++ + if (c == ')' || c == ']' || c == '}') { + depth-- + if (depth == 0) return i + } + i++ + } + return -1 + } + + /** + * The index just past the string literal starting at [start], triple-quoted or not. + * + * Triple quotes matter: a model writing a file's contents reaches for them, and reading one as + * an empty `''` would end the argument list in the middle of the payload. + * + * @return the index just past the literal, or -1 when it is unterminated. + */ + private fun endOfString(text: String, start: Int): Int { + val quote = text[start] + val delimiter = if (text.startsWith("$quote$quote$quote", start)) "$quote$quote$quote" else "$quote" + var i = start + delimiter.length + while (i < text.length) { + if (text[i] == '\\') { + i += 2 + continue + } + if (text.startsWith(delimiter, i)) return i + delimiter.length + i++ + } + return -1 + } + + /** + * Reads `name=value` pairs out of an argument list. + * + * @param args the text between the call's brackets. + * @return the arguments, or null when one of them is positional or malformed. + */ + private fun parseKeywordArgs(args: String): Map? { + if (args.isBlank()) return emptyMap() + val parsed = mutableMapOf() + for (part in splitTopLevel(args) ?: return null) { + val assignment = topLevelAssignment(part) ?: return null + val name = part.substring(0, assignment).trim() + if (!IDENTIFIER.matches(name)) return null + parsed[name] = parseValue(part.substring(assignment + 1).trim()) + } + return parsed + } + + /** + * Splits an argument list on the commas that separate arguments, not the ones inside them. + * + * @return the arguments, or null when a string literal is unterminated. + */ + private fun splitTopLevel(args: String): List? { + val parts = mutableListOf() + var depth = 0 + var start = 0 + var i = 0 + while (i < args.length) { + val c = args[i] + if (c == '"' || c == '\'') { + val end = endOfString(args, i) + if (end < 0) return null + i = end + continue + } + if (c == '(' || c == '[' || c == '{') depth++ + if (c == ')' || c == ']' || c == '}') depth-- + if (c == ',' && depth == 0) { + parts += args.substring(start, i) + start = i + 1 + } + i++ + } + parts += args.substring(start) + return parts.filter { it.isNotBlank() } + } + + /** + * The index of the `=` binding an argument's name to its value. + * + * @return the index, or null when the part carries no top-level assignment (so, positional). + */ + private fun topLevelAssignment(part: String): Int? { + var depth = 0 + var i = 0 + while (i < part.length) { + val c = part[i] + if (c == '"' || c == '\'') { + val end = endOfString(part, i) + if (end < 0) return null + i = end + continue + } + if (c == '(' || c == '[' || c == '{') depth++ + if (c == ')' || c == ']' || c == '}') depth-- + // Not `==`, `!=`, `<=` or `>=`: those are an expression, never a keyword argument. + if (c == '=' && depth == 0 && part.getOrNull(i + 1) != '=' && + part.getOrNull(i - 1) !in listOf('=', '!', '<', '>') + ) { + return i + } + i++ + } + return null + } + + /** + * Python string prefixes, which reach a path argument as part of its value if not stripped. + * + * The lookahead is what keeps this off an ordinary bare word: only a prefix immediately + * followed by a quote is one. + */ + private val STRING_PREFIX_REGEX = Regex("""^[rbuf]{1,2}(?=["'])""", RegexOption.IGNORE_CASE) + + /** + * Reads one argument value: a string literal, a Python literal, or the raw text. + * + * @param raw the value as written, already trimmed. + * @return the value, with a string literal unquoted and unescaped. + */ + private fun parseValue(raw: String): Any? { + if (raw.isEmpty()) return "" + // `r"app/src/…"` is still a path; leaving the prefix on fails the file operation instead. + val literal = raw.substring(STRING_PREFIX_REGEX.find(raw)?.value?.length ?: 0) + val quote = literal[0] + if ((quote == '"' || quote == '\'') && endOfString(literal, 0) == literal.length) { + val delimiter = if (literal.startsWith("$quote$quote$quote")) 3 else 1 + return unescape(literal.substring(delimiter, literal.length - delimiter)) + } + return when (raw) { + "True" -> true + "False" -> false + "None" -> null + // A list or dict stays raw text, as it does coming out of the JSON envelope. + else -> raw.toLongOrNull() ?: raw.toDoubleOrNull() ?: raw + } + } + + /** + * Resolves the escape sequences in a string literal's body. + * + * @param value the literal's contents, without its quotes. + * @return the text the model meant; an unknown escape is left as written. + */ + private fun unescape(value: String): String { + if (!value.contains('\\')) return value + val out = StringBuilder(value.length) + var i = 0 + while (i < value.length) { + if (value[i] != '\\' || i == value.length - 1) { + out.append(value[i]) + i++ + continue + } + when (val escape = value[i + 1]) { + 'n' -> out.append('\n') + 't' -> out.append('\t') + 'r' -> out.append('\r') + '\\' -> out.append('\\') + '"' -> out.append('"') + '\'' -> out.append('\'') + // A backslash before a newline is a line continuation: both characters go. + '\n' -> Unit + 'u' -> { + val hex = value.substring(i + 2, minOf(i + 6, value.length)) + val code = hex.takeIf { it.length == 4 }?.toIntOrNull(16) + if (code != null) { + out.append(code.toChar()) + i += 6 + continue + } + out.append('\\').append(escape) + } + else -> out.append('\\').append(escape) + } + i += 2 + } + return out.toString() + } +} diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolSchema.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolSchema.kt new file mode 100644 index 00000000..b90336db --- /dev/null +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolSchema.kt @@ -0,0 +1,57 @@ +package com.itsaky.androidide.plugins.aicore.tool + +/** + * Builders for the JSON Schema a tool publishes as [ToolHandler.parametersSchema]. + * + * Plain JSON Schema, the dialect contributed (MCP) tools already arrive in, so a backend needs one + * conversion rather than two. Adapting it to a provider's dialect belongs to that backend. + */ +object ToolSchema { + + /** + * An object schema over [properties]. + * + * `required` is omitted when empty rather than sent as an empty array: a provider that + * validates the schema is entitled to reject the empty form. + * + * @param properties the arguments, each built by [string], [boolean] or [freeform]. + * @param required the arguments a call must carry. + * @return the schema. + */ + fun objectOf( + vararg properties: Pair>, + required: List = emptyList(), + ): Map = buildMap { + put("type", "object") + put("properties", properties.toMap()) + if (required.isNotEmpty()) put("required", required) + } + + /** + * A string argument. + * @param description what the argument means, as the model will read it. + * @return the property schema. + */ + fun string(description: String): Map = + mapOf("type" to "string", "description" to description) + + /** + * A boolean argument. + * @param description what the argument means, as the model will read it. + * @return the property schema. + */ + fun boolean(description: String): Map = + mapOf("type" to "boolean", "description" to description) + + /** + * An argument holding an object whose keys are not known ahead of time. + * + * A backend whose provider cannot declare one (Gemini rejects an object with no properties) + * degrades it to JSON text, so a handler reading such an argument must accept either shape. + * + * @param description what the argument means, as the model will read it. + * @return the property schema. + */ + fun freeform(description: String): Map = + mapOf("type" to "object", "description" to description) +} diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/AddDependencyHandler.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/AddDependencyHandler.kt index 4b069d3f..d117cbc5 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/AddDependencyHandler.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/AddDependencyHandler.kt @@ -5,6 +5,7 @@ import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aicore.models.ToolResult import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema import com.itsaky.androidide.plugins.services.IdeProjectManipulationService private const val TAG = "$LOG_PREFIX.AddDependencyHandler" @@ -16,6 +17,15 @@ class AddDependencyHandler( private val pluginContext: PluginContext ) : ToolHandler { override val toolName = "add_dependency" + override val parametersSchema = ToolSchema.objectOf( + "dependency" to ToolSchema.string( + "Maven coordinate to add, as group:artifact:version." + ), + "build_file" to ToolSchema.string( + "Project-relative build file to add it to. Defaults to the app module's." + ), + required = listOf("dependency"), + ) override val description = "Add a Maven dependency to the project build file" override val requiresApproval = true diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/CreateFileHandler.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/CreateFileHandler.kt index 34e1327c..7f8ac3a1 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/CreateFileHandler.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/CreateFileHandler.kt @@ -5,6 +5,7 @@ import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aicore.models.ToolResult import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema private const val TAG = "$LOG_PREFIX.CreateFileHandler" @@ -15,6 +16,11 @@ class CreateFileHandler( private val pluginContext: PluginContext ) : ToolHandler { override val toolName = "create_file" + override val parametersSchema = ToolSchema.objectOf( + "file_path" to ToolSchema.string("Project-relative path of the file to create."), + "content" to ToolSchema.string("The file's full contents."), + required = listOf("file_path", "content"), + ) override val description = "Create a new file with given content" override val requiresApproval = true // Requires approval for file creation override val pathArgs = listOf("file_path") diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/EditFileHandler.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/EditFileHandler.kt index d7c7b8d6..bd11f8d2 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/EditFileHandler.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/EditFileHandler.kt @@ -3,6 +3,7 @@ package com.itsaky.androidide.plugins.aicore.tool.handlers import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.models.ToolResult import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema import com.itsaky.androidide.plugins.aicore.tool.Validation import com.itsaky.androidide.plugins.aicore.tool.handlers.edit.AtomicFileWriter import com.itsaky.androidide.plugins.aicore.tool.handlers.edit.EditTargetResolver @@ -36,6 +37,17 @@ class EditFileHandler( ) : ToolHandler { override val toolName = TOOL_NAME + override val parametersSchema = ToolSchema.objectOf( + ARG_PATH to ToolSchema.string("Project-relative path of the file to edit."), + ARG_OLD to ToolSchema.string( + "The exact text to find, copied byte-for-byte from the file including indentation." + ), + ARG_NEW to ToolSchema.string("What to put in its place; empty deletes it."), + ARG_REPLACE_ALL to ToolSchema.boolean( + "Replace every occurrence. When false the text must match exactly once." + ), + required = listOf(ARG_PATH, ARG_OLD, ARG_NEW), + ) override val description = "Edit an existing file by replacing an exact snippet: give file_path, old_string " + "(text to find, copied exactly including indentation) and new_string (its " + diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/GenerateFromTemplateHandler.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/GenerateFromTemplateHandler.kt index 135cc1b2..842f427e 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/GenerateFromTemplateHandler.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/GenerateFromTemplateHandler.kt @@ -5,6 +5,7 @@ import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aicore.models.ToolResult import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema import com.itsaky.androidide.plugins.services.IdeTemplateService import org.json.JSONObject @@ -17,9 +18,37 @@ class GenerateFromTemplateHandler( private val pluginContext: PluginContext ) : ToolHandler { override val toolName = "generate_from_template" + override val parametersSchema = ToolSchema.objectOf( + "template_name" to ToolSchema.string("Name of the registered template to generate from."), + "variables" to ToolSchema.freeform( + "Template variables, as a flat object of name to value." + ), + required = listOf("template_name"), + ) override val description = "Generate files from Pebble templates with variable substitution" override val requiresApproval = false + /** + * Reads the `variables` argument, whatever shape the backend delivered it in. + * + * A nested object reaches a handler as [JSONObject], never as a [Map] — both the envelope + * parser and a native call hand org.json's own types over — and a backend that cannot declare + * a free-form object (Gemini) sends its JSON as text instead. + * + * @param value the raw argument. + * @return the variables, empty when there are none or the text will not parse. + */ + internal fun variablesOf(value: Any?): Map = when { + value == null -> emptyMap() + value is JSONObject -> value.keys().asSequence().associateWith { value.get(it) } + value is Map<*, *> -> value.entries.associate { (key, entry) -> key.toString() to entry } + else -> runCatching { JSONObject(value.toString()) } + .onFailure { Log.w(TAG, "variables is not a JSON object: $value") } + .getOrNull() + ?.let { json -> json.keys().asSequence().associateWith { json.get(it) } } + ?: emptyMap() + } + override suspend fun execute(args: Map): ToolResult { val templateName = args["template_name"]?.toString()?.trim() if (templateName.isNullOrBlank()) { @@ -29,9 +58,7 @@ class GenerateFromTemplateHandler( ) } - // Variables is a map for substitution - @Suppress("UNCHECKED_CAST") - val variables = (args["variables"] as? Map) ?: emptyMap() + val variables = variablesOf(args["variables"]) Log.d(TAG, "Generating from template: $templateName with ${variables.size} variables") diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/ListFilesHandler.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/ListFilesHandler.kt index e9d1fbd7..17d3933d 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/ListFilesHandler.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/ListFilesHandler.kt @@ -5,6 +5,7 @@ import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aicore.models.ToolResult import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema import java.io.File private const val TAG = "$LOG_PREFIX.ListFilesHandler" @@ -16,6 +17,11 @@ class ListFilesHandler( private val pluginContext: PluginContext ) : ToolHandler { override val toolName = "list_files" + override val parametersSchema = ToolSchema.objectOf( + "directory" to ToolSchema.string( + "Project-relative directory to list. Empty or omitted lists the project root." + ), + ) override val description = "List files and directories in a given path" override val requiresApproval = false override val pathArgs = listOf("directory") diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/OpenFileHandler.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/OpenFileHandler.kt index 93a2ab43..efa5276d 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/OpenFileHandler.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/OpenFileHandler.kt @@ -5,6 +5,7 @@ import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aicore.models.ToolResult import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema import com.itsaky.androidide.plugins.services.IdeEditorService import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers @@ -22,6 +23,10 @@ class OpenFileHandler( private val mainDispatcher: CoroutineDispatcher = Dispatchers.Main ) : ToolHandler { override val toolName = "open_file" + override val parametersSchema = ToolSchema.objectOf( + "file_path" to ToolSchema.string("Project-relative path of the file to open."), + required = listOf("file_path"), + ) override val description = "Open a file in the IDE editor" override val requiresApproval = false override val pathArgs = listOf("file_path") diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/ReadFileHandler.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/ReadFileHandler.kt index 3413170e..fbc235df 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/ReadFileHandler.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/ReadFileHandler.kt @@ -5,6 +5,7 @@ import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aicore.models.ToolResult import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema private const val TAG = "$LOG_PREFIX.ReadFileHandler" @@ -15,6 +16,10 @@ class ReadFileHandler( private val pluginContext: PluginContext ) : ToolHandler { override val toolName = "read_file" + override val parametersSchema = ToolSchema.objectOf( + "file_path" to ToolSchema.string("Project-relative path of the file to read."), + required = listOf("file_path"), + ) override val description = "Read the contents of a file" override val requiresApproval = false override val pathArgs = listOf("file_path", "path") diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/SearchProjectHandler.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/SearchProjectHandler.kt index 6019655e..aa138f38 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/SearchProjectHandler.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/SearchProjectHandler.kt @@ -5,6 +5,7 @@ import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aicore.models.ToolResult import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema import java.io.File private const val TAG = "$LOG_PREFIX.SearchProjectHandler" @@ -16,6 +17,16 @@ class SearchProjectHandler( private val pluginContext: PluginContext ) : ToolHandler { override val toolName = "search_project" + override val parametersSchema = ToolSchema.objectOf( + "query" to ToolSchema.string("Text to search for; a file name unless searching contents."), + "project_dir" to ToolSchema.string( + "Project-relative directory to search under. Defaults to the whole project." + ), + "search_in_contents" to ToolSchema.boolean( + "Search inside files instead of matching their names." + ), + required = listOf("query"), + ) override val description = "Search for files by name or content in the project" override val requiresApproval = false override val pathArgs = listOf("project_dir") diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/UpdateFileHandler.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/UpdateFileHandler.kt index ffeebb72..a17472c2 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/UpdateFileHandler.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/UpdateFileHandler.kt @@ -5,6 +5,7 @@ import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aicore.logging.LOG_PREFIX import com.itsaky.androidide.plugins.aicore.models.ToolResult import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema private const val TAG = "$LOG_PREFIX.UpdateFileHandler" @@ -15,6 +16,11 @@ class UpdateFileHandler( private val pluginContext: PluginContext ) : ToolHandler { override val toolName = "update_file" + override val parametersSchema = ToolSchema.objectOf( + "file_path" to ToolSchema.string("Project-relative path of the file to overwrite."), + "content" to ToolSchema.string("The file's new full contents."), + required = listOf("file_path", "content"), + ) override val description = "Update an existing file with new content" override val requiresApproval = true // Requires approval for file modification override val pathArgs = listOf("file_path") diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentReplyRenderer.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentReplyRenderer.kt index f98fe320..7f019ad9 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentReplyRenderer.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentReplyRenderer.kt @@ -43,6 +43,7 @@ object AgentReplyRenderer { * @param lastToolFailed whether this run's most recent tool call failed. * @param actionFailedText what to show when the model claims success after a failed tool. * @param noResponseText last-resort text when the turn carries nothing to show. + * @param unparsedReplyText what to show for a reply that meant to call a tool and failed to. * @param renderToolCall renders one tool call as a badge line. * @return the text to display for this turn. */ @@ -53,6 +54,7 @@ object AgentReplyRenderer { lastToolFailed: Boolean, actionFailedText: String, noResponseText: String, + unparsedReplyText: (ToolCallExtractor.UnparsedReply) -> String, renderToolCall: (ToolCall) -> String, ): String { val respondCall = toolCalls.firstOrNull { isTerminalToolName(it.name, terminalTool) } @@ -64,7 +66,9 @@ object AgentReplyRenderer { ?: ToolCallExtractor.proseOutsideToolCalls(rawText) ?: noResponseText toolCalls.isNotEmpty() -> toolCalls.joinToString("\n", transform = renderToolCall) - else -> rawText.ifBlank { noResponseText } + // A call that failed to parse: say so, rather than pasting the raw envelope on screen. + else -> ToolCallExtractor.diagnoseUnparsedReply(rawText)?.let(unparsedReplyText) + ?: rawText.ifBlank { noResponseText } } } } diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ChatViewModel.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ChatViewModel.kt index 84c741a1..c2f69321 100644 --- a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ChatViewModel.kt +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ChatViewModel.kt @@ -27,6 +27,7 @@ import com.itsaky.androidide.plugins.aicore.tool.ToolCall import com.itsaky.androidide.plugins.aicore.tool.ToolCallExtractor import com.itsaky.androidide.plugins.aicore.tool.ToolExecutionTracker import com.itsaky.androidide.plugins.aicore.tool.ToolHandler +import com.itsaky.androidide.plugins.aicore.tool.ToolSchema import com.itsaky.androidide.plugins.aicore.tool.sources.ToolSourceStore import com.itsaky.androidide.plugins.aicore.tool.handlers.AddDependencyHandler import com.itsaky.androidide.plugins.aicore.tool.handlers.CreateFileHandler @@ -482,10 +483,21 @@ class ChatViewModel( private suspend fun buildSystemPrompt(tools: AgentTools): String { // One editor read serves both the IDE CONTEXT block and the paths in the examples. val ide = readIdeSnapshot() + val modules = withContext(Dispatchers.IO) { + ProjectLayout.describe(File(PathGuard.projectRoot())) + } + // Paths, not a count: an empty or wrong one here is what sends the agent walking the tree, + // and these are project-relative directory names rather than the user's content. + AgentTrace.stage( + "LAYOUT", + "modules=${modules.size}" + modules.joinToString("") { + " ${it.name}[src=${it.sourceDir} layout=${it.layoutDir} manifest=${it.manifest}]" + }, + ) val examplePath = ide.exampleFilePath() val base = backendSystemPrompt(tools, examplePath) ?: buildDefaultSystemPrompt(tools, examplePath) - return base + ide.contextBlock() + return base + ide.contextBlock(modules) } /** @@ -508,7 +520,8 @@ class ChatViewModel( backend.getSystemPrompt( LlmInferenceService.SystemPromptRequest( promptToolDefinitions(tools), - TOOL_CALL_SYNTAX, + // Null tells the backend this side parses no envelope; see SystemPromptRequest. + TOOL_CALL_SYNTAX.takeUnless { callsToolsNatively() }, examplePath, ) )?.takeIf { it.isNotBlank() } @@ -521,6 +534,22 @@ class ChatViewModel( } } + /** + * Whether the active backend carries tool calls in its provider's own function-calling API + * rather than in the reply text. + * + * Decides both halves of the protocol at once — the schemas sent with the request and the + * envelope the prompt teaches — so the two can never disagree about which one is live. + * + * @return true when the backend declares [LlmInferenceService.ToolCallingBackend] + */ + private fun callsToolsNatively(): Boolean = try { + getLlmService()?.getBackend(currentBackendId) is LlmInferenceService.ToolCallingBackend + } catch (e: Throwable) { + logWarn("could not resolve backend '$currentBackendId'", e) + false + } + /** * The sampling temperature the active backend asks for. * @@ -549,7 +578,12 @@ class ChatViewModel( RESPOND_TOOL, "Send the user your reply or final answer. It MUST carry a \"message\" holding the " + "text itself — a respond call with no \"message\" shows the user nothing.", - emptyMap(), + // Schema, not emptyMap(): under native calling a parameterless declaration is one the + // model cannot put the answer in, which is the empty respond the description warns of. + ToolSchema.objectOf( + "message" to ToolSchema.string("The reply to show the user."), + required = listOf("message"), + ), ) } @@ -564,7 +598,7 @@ class ChatViewModel( val toolDescriptions = promptToolDefinitions(tools) .joinToString("\n") { "- ${it.name}: ${it.description}" } - return """ + val head = """ You are a coding assistant inside CodeOnTheGo. Reply with exactly ONE tool call and nothing else. After a tool call, stop and wait — the @@ -574,7 +608,13 @@ class ChatViewModel( Tools: $toolDescriptions + """.trimIndent() + + // Under native calling the provider carries the call; teaching an envelope as well invites + // the model to emit both, and the text one would then run the tool a second time. + if (callsToolsNatively()) return head + return head + "\n\n" + """ TOOL CALL FORMAT — emit a single line in EXACTLY this format and nothing after it: $TOOL_CALL_SYNTAX @@ -583,6 +623,17 @@ class ChatViewModel( """.trimIndent() } + /** + * The advice for a reply that meant to call a tool and produced nothing runnable. + * + * @param reason how the call failed to parse. + * @return the string resource to show the user. + */ + private fun unparsedReplyMessage(reason: ToolCallExtractor.UnparsedReply): Int = when (reason) { + ToolCallExtractor.UnparsedReply.TRUNCATED -> R.string.agent_reply_truncated + ToolCallExtractor.UnparsedReply.MALFORMED -> R.string.agent_reply_malformed + } + /** * What the IDE has open, project-relative, read once per prompt. * @property currentFile the focused file, or null when nothing is open. @@ -627,13 +678,16 @@ class ChatViewModel( currentFile ?: otherFiles.firstOrNull() ?: FALLBACK_EXAMPLE_PATH /** - * Describes what the user is looking at: the focused file and other open tabs, project-relative. - * Most requests are about the file on screen and the IDE knows that path exactly; without it the - * model reconstructs one, which is where invented `.java` paths for Kotlin files came from. - * @return a prompt block, or empty when nothing is open. + * Describes what the user is looking at: the focused file and other open tabs, project-relative, + * plus where [modules] keep their code. Most requests are about the file on screen and the IDE + * knows that path exactly; without it the model reconstructs one, which is where invented + * `.java` paths for Kotlin files came from. + * + * @param modules the project's modules, so the agent spends no turns rediscovering them. + * @return a prompt block, or empty when there is nothing to say. */ - private fun IdeSnapshot.contextBlock(): String { - if (currentFile == null && otherFiles.isEmpty()) return "" + private fun IdeSnapshot.contextBlock(modules: List): String { + if (currentFile == null && otherFiles.isEmpty() && modules.isEmpty()) return "" return buildString { append("\n\nIDE CONTEXT (real paths — use these verbatim, do not rewrite them):\n") @@ -641,6 +695,25 @@ class ChatViewModel( if (otherFiles.isNotEmpty()) { append("- Other open files: ").append(otherFiles.joinToString(", ")).append("\n") } + for (module in modules) { + module.sourceDir?.let { + append("- New classes for module '").append(module.name).append("' go in: ") + .append(it).append("\n") + } + module.layoutDir?.let { + append("- Layouts for module '").append(module.name).append("': ") + .append(it).append("\n") + } + module.manifest?.let { + append("- Manifest for module '").append(module.name).append("': ") + .append(it).append("\n") + } + } + if (modules.isNotEmpty()) { + append( + "These directories already exist — do not call list_files to rediscover them.\n" + ) + } append( "If the user names a file that appears above, use that exact path and do not " + "guess a different folder or extension." @@ -865,16 +938,33 @@ class ChatViewModel( ) try { + // The same list the system prompt describes, so a native declaration and the + // prose the model reads can never name different tools. + val toolDefinitions = promptToolDefinitions(tools) + // Which protocol is live for this run. `native=false` against a backend that + // should call natively is the first thing to check when a call reaches the chat + // as text instead of running. + AgentTrace.stage( + "PROTOCOL", + "native=${callsToolsNatively()} tools=${toolDefinitions.size} " + + toolDefinitions.joinToString(",") { it.name }, + ) val loopResult = agentLoop.run( history = history, generate = { turns -> withContext(Dispatchers.Main) { setState(AgentState.Processing(str(R.string.msg_generating))) } - runModelTurn(llmService, turns, config, epoch) + runModelTurn(llmService, turns, config, toolDefinitions, epoch) }, executeTools = { calls -> executeToolCalls(tools, calls) }, events = object : AgentLoop.Events { + // Numbers the turn between its reply and the tools it runs, so the step + // budget a run spent on one tool is counted off the trace, not guessed. + override suspend fun onModelTurn(turn: Int, text: String) { + AgentTrace.stage("TURN", "turn=$turn chars=${text.length}") + } + override suspend fun onToolResults( turn: Int, calls: List, @@ -906,6 +996,38 @@ class ChatViewModel( ) } + override suspend fun onUnparsedReply( + turn: Int, + reason: ToolCallExtractor.UnparsedReply, + ) { + AgentTrace.refusal( + "PARSE", + "turn=$turn reason=$reason", + "reply carried no readable tool call", + ) + addSystemMessage( + str(unparsedReplyMessage(reason)), + MessageStatus.ERROR + ) + } + + override suspend fun onAbandonedAfterFailure(turn: Int) { + // No system message: the reply itself already carries the model's + // account of the failure, rendered as such by AgentReplyRenderer. + AgentTrace.refusal( + "LOOP", + "turn=$turn", + "stopped with a failed tool unaddressed", + ) + } + + override suspend fun onRepeatAfterSuccess(turn: Int) { + AgentTrace.stage( + "LOOP", + "turn=$turn assumed-complete=repeat-after-success", + ) + } + override suspend fun onRepeatedToolCalls(turns: Int) { AgentTrace.refusal("LOOP", "turns=$turns", "identical tool calls repeated") addSystemMessage( @@ -972,6 +1094,7 @@ class ChatViewModel( * @param llmService the inference service. * @param turns the conversation so far; the last entry is the current user turn. * @param config the generation config. + * @param toolDefinitions the tools to offer a natively-calling backend; see [callsToolsNatively]. * @param epoch this run's epoch, for staleness checks against Stop/newer sends. * @return the final response text (raw, for tool-call extraction). */ @@ -979,6 +1102,7 @@ class ChatViewModel( llmService: LlmInferenceService, turns: List, config: LlmInferenceService.LlmConfig, + toolDefinitions: List, epoch: Int ): String { val deferred = CompletableDeferred() @@ -1063,6 +1187,7 @@ class ChatViewModel( lastToolFailed = lastToolFailed, actionFailedText = str(R.string.agent_action_failed), noResponseText = str(R.string.agent_no_response), + unparsedReplyText = { str(unparsedReplyMessage(it)) }, ) { c -> // Capped: edit_file snippets would turn the badge into a wall of source. "🔧 ${c.name}(${c.args.entries.joinToString(", ") { "${it.key}=${abbreviate(it.value)}" }})" @@ -1096,20 +1221,41 @@ class ChatViewModel( } } + // Appended to the reply on completion, so a natively-called tool reaches extraction, + // the transcript badge and the loop's repeat guard by the one path text calls use. + val nativeCalls = mutableListOf() + try { // Every backend takes the structured form: the last turn as the prompt, the rest as - // history. Tools are empty because tool calls travel in the reply text (TOOL_CALL_SYNTAX), - // not through native function calling. + // history. A backend that reports no native calls simply never calls onToolCall, and + // its calls arrive in the reply text as TOOL_CALL_SYNTAX instead. llmService.generateStreamingWithTools( turns.lastOrNull()?.content.orEmpty(), turns.dropLast(1), config, - emptyList(), + toolDefinitions, object : LlmInferenceService.ToolStreamCallback { override fun onToken(token: String) = streamCallback.onToken(token) - override fun onToolCall(request: LlmInferenceService.ToolCallRequest) = Unit - override fun onComplete(response: LlmInferenceService.LlmResponse) = - streamCallback.onComplete(response) + + override fun onToolCall(request: LlmInferenceService.ToolCallRequest) { + if (isStale()) return + // The proof a call came through the provider's API rather than the reply + // text: the PARSE line that follows reports the envelope this one becomes. + AgentTrace.stage( + "NATIVE", + "tool=${request.name} args=${request.args.orEmpty().keys.joinToString(",")}", + AgentTrace.previewArgs(request.args.orEmpty()), + ) + synchronized(nativeCalls) { nativeCalls.add(request) } + } + + override fun onComplete(response: LlmInferenceService.LlmResponse) { + val calls = synchronized(nativeCalls) { nativeCalls.toList() } + streamCallback.onComplete( + if (calls.isEmpty()) response else response.withNativeCalls(calls) + ) + } + override fun onError(error: String) = streamCallback.onError(error) } ) @@ -1125,6 +1271,26 @@ class ChatViewModel( return deferred.await() } + /** + * This response with [calls] appended as canonical `` envelopes. + * + * The model never writes these: [ToolCallExtractor.renderEnvelope] encodes them from arguments + * the provider already parsed, so the mis-escaping that loses a text-mode call cannot lose one. + * + * @param calls the native calls reported during this turn. + * @return a response whose text carries the calls in the form extraction reads back. + */ + private fun LlmInferenceService.LlmResponse.withNativeCalls( + calls: List, + ): LlmInferenceService.LlmResponse { + val envelopes = calls.joinToString("\n") { + ToolCallExtractor.renderEnvelope(it.name, it.args.orEmpty()) + } + val prose = text?.trim().orEmpty() + val merged = if (prose.isEmpty()) envelopes else prose + "\n" + envelopes + return LlmInferenceService.LlmResponse(success, merged, error, tokensGenerated, timeMs) + } + /** * Appends an AGENT message to the chat (terminal state, no streaming dots). * @param text the message text. diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ProjectLayout.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ProjectLayout.kt new file mode 100644 index 00000000..0904b9be --- /dev/null +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ProjectLayout.kt @@ -0,0 +1,107 @@ +package com.itsaky.androidide.plugins.aicore.viewmodel + +import java.io.File + +/** + * Where a project keeps its sources, layouts and manifest, for the prompt's IDE-context block. + * + * The agent otherwise finds this out one `list_files` at a time: a reported run spent seven of its + * sixteen turns walking `app` → `src` → `main` → `java` → `com` → `example`, four of those steps + * returning a single entry, and hit the step limit with the manifest edit still pending. Stating + * the paths costs no turns at all. + * + * Pure and free of Android types, so it unit-tests against a temp directory. + */ +object ProjectLayout { + + /** Modules described, so a many-module project cannot crowd out the rest of the prompt. */ + private const val MAX_MODULES = 4 + + /** Package directories are a few levels deep; the cap only stops a pathological tree. */ + private const val MAX_DEPTH = 12 + + /** Source roots in preference order; the first that holds anything wins. */ + private val SOURCE_ROOTS = listOf("src/main/java", "src/main/kotlin") + + /** + * One module's interesting directories, project-relative with `/` separators. + * + * @property name the module's directory name. + * @property sourceDir the deepest package directory under its source root, or null when absent. + * @property layoutDir its `res/layout`, or null when absent. + * @property manifest its `AndroidManifest.xml`, or null when absent. + */ + data class Module( + val name: String, + val sourceDir: String?, + val layoutDir: String?, + val manifest: String?, + ) + + /** + * Describes the modules under [root]. + * + * @param root the project root. + * @return one entry per module that has a `src/main`, capped at [MAX_MODULES]; empty when the + * root is unreadable or holds no module, which is the "say nothing" case for the prompt. + */ + fun describe(root: File): List = runCatching { + root.listFiles() + .orEmpty() + .filter { it.isDirectory && File(it, "src/main").isDirectory } + // `app` first: it is the module a request is about unless the user says otherwise. + .sortedBy { if (it.name == "app") "" else it.name } + .take(MAX_MODULES) + .map { module -> + Module( + name = module.name, + sourceDir = packageDirOf(module)?.let { relativeTo(root, it) }, + layoutDir = File(module, "src/main/res/layout") + .takeIf { it.isDirectory }?.let { relativeTo(root, it) }, + manifest = File(module, "src/main/AndroidManifest.xml") + .takeIf { it.isFile }?.let { relativeTo(root, it) }, + ) + } + .filterNot { it.sourceDir == null && it.layoutDir == null && it.manifest == null } + }.getOrDefault(emptyList()) + + /** + * The directory a new class in [module] belongs in. + * + * Descends while a directory holds exactly one subdirectory and no files, which is the + * `com/example/myapplication` chain the agent was walking a turn at a time. + * + * @return the deepest such directory, or null when the module has no source root. + */ + private fun packageDirOf(module: File): File? { + val roots = SOURCE_ROOTS.map { File(module, it) }.filter { it.isDirectory } + // The populated root, not merely the first: an AGP project keeps an empty `java` beside + // the `kotlin` tree its code is in, and naming the empty one sends new classes there. + var dir = roots.firstOrNull { holdsAnyFile(it) } ?: roots.firstOrNull() ?: return null + var depth = 0 + while (depth++ < MAX_DEPTH) { + val children = dir.listFiles().orEmpty() + val onlyChild = children.singleOrNull()?.takeIf { it.isDirectory } ?: return dir + dir = onlyChild + } + return dir + } + + /** + * Whether [dir] holds a file anywhere beneath it, i.e. is a source root in use. + * + * @param dir the candidate source root. + * @return true when it contains at least one file within [MAX_DEPTH]. + */ + private fun holdsAnyFile(dir: File): Boolean = + dir.walkTopDown().maxDepth(MAX_DEPTH).any { it.isFile } + + /** + * [file] as a path relative to [root], with `/` separators whatever the platform uses. + * + * @return the relative path, or the file's name when it lies outside [root]. + */ + private fun relativeTo(root: File, file: File): String = + runCatching { file.relativeToOrSelf(root).path.replace(File.separatorChar, '/') } + .getOrDefault(file.name) +} diff --git a/ai-core/src/main/res/values/strings.xml b/ai-core/src/main/res/values/strings.xml index 23308cdb..25df3ae3 100644 --- a/ai-core/src/main/res/values/strings.xml +++ b/ai-core/src/main/res/values/strings.xml @@ -142,6 +142,8 @@ Reached the %1$d-step limit. Ask me to continue if the task isn\'t finished. Stopped: the model kept requesting the same action. Try rephrasing your request. I couldn\'t complete that, the last action failed. See the details above. + The model ran out of room mid-action, so nothing was changed. Ask for a smaller step \u2014 one file, or one method at a time. + The model\'s reply wasn\'t a readable action, so nothing was changed. Try rephrasing, or pick a larger model in AI Settings. (No response. Try rephrasing, or pick a larger model in AI Settings — very small models struggle with tool use.) Generating response… System Log diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/AgentLoopTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/AgentLoopTest.kt index 59532c68..d28910d0 100644 --- a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/AgentLoopTest.kt +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/AgentLoopTest.kt @@ -414,4 +414,93 @@ class AgentLoopTest { // Must not append a trailing "Assistant:" cue (the backend adds its own). assertFalse("must not append a trailing Assistant cue", transcript.trimEnd().endsWith("Assistant:")) } + + @Test + fun givenAReplyWhoseToolCallCouldNotBeParsed_whenTheLoopRuns_thenItStopsUnparsableRatherThanCompleted() = runTest { + // ADFA-5410: this returned COMPLETED, so a run that did nothing looked like a finished one. + val model = ScriptedModel( + listOf("""{"tool":"edit_file","args":{"new_string":""}}""") + ) + val history = mutableListOf(ChatMessage(Role.USER, "add a view")) + var reported: ToolCallExtractor.UnparsedReply? = null + + val result = AgentLoop().run( + history = history, + generate = model::generate, + executeTools = { emptyList() }, + events = object : AgentLoop.Events { + override suspend fun onUnparsedReply(turn: Int, reason: ToolCallExtractor.UnparsedReply) { + reported = reason + } + } + ) + + assertFalse(result.completed) + assertEquals(AgentLoop.StopReason.UNPARSABLE, result.reason) + assertEquals(ToolCallExtractor.UnparsedReply.MALFORMED, reported) + } + + @Test + fun givenAnOrdinaryProseReply_whenTheLoopRuns_thenItStillCompletesWithoutReportingAFailure() = runTest { + val model = ScriptedModel(listOf("Hi! What shall we build?")) + var reported = false + + val result = AgentLoop().run( + history = mutableListOf(ChatMessage(Role.USER, "hi")), + generate = model::generate, + executeTools = { emptyList() }, + events = object : AgentLoop.Events { + override suspend fun onUnparsedReply(turn: Int, reason: ToolCallExtractor.UnparsedReply) { + reported = true + } + } + ) + + assertEquals(AgentLoop.StopReason.COMPLETED, result.reason) + assertFalse(reported) + } + + @Test + fun givenTheModelGivesUpAfterAFailedTool_whenTheLoopRuns_thenTheRunIsNotReportedComplete() = + runTest { + val model = ScriptedModel(listOf(toolCall("open_file"), "I could not open that file.")) + var abandonedTurn = 0 + + val result = AgentLoop().run( + history = mutableListOf(ChatMessage(Role.USER, "open nope")), + generate = model::generate, + executeTools = { listOf(ToolResult.failure("File not found", "does not exist")) }, + events = object : AgentLoop.Events { + override suspend fun onAbandonedAfterFailure(turn: Int) { + abandonedTurn = turn + } + } + ) + + assertFalse("the request was never met", result.completed) + assertEquals(AgentLoop.StopReason.ABANDONED, result.reason) + assertEquals(2, abandonedTurn) + } + + @Test + fun givenTheModelStopsAfterASucceedingTool_whenTheLoopRuns_thenTheRunIsComplete() = runTest { + // The other side of the rule above: nothing is outstanding, so this really is done. + val model = ScriptedModel(listOf(toolCall("open_file"), "Opened it.")) + var abandoned = false + + val result = AgentLoop().run( + history = mutableListOf(ChatMessage(Role.USER, "open A.kt")), + generate = model::generate, + executeTools = { listOf(ToolResult.success("Opened", "A.kt")) }, + events = object : AgentLoop.Events { + override suspend fun onAbandonedAfterFailure(turn: Int) { + abandoned = true + } + } + ) + + assertEquals(AgentLoop.StopReason.COMPLETED, result.reason) + assertTrue(result.completed) + assertFalse(abandoned) + } } diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCallExtractorTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCallExtractorTest.kt index 7ca12790..a3350185 100644 --- a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCallExtractorTest.kt +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCallExtractorTest.kt @@ -162,4 +162,90 @@ class ToolCallExtractorTest { assertEquals(2, calls.size) } + + /** + * The reply from ADFA-5410: Gemini wrote a whole layout into `new_string` without escaping the + * quotes in it, so the envelope closed but its JSON did not parse and nothing ran. + */ + private val malformedLayoutEdit = + """{"tool":"edit_file","args":{"file_path":"app/src/main/res/layout/fragment_home.xml",""" + + """"new_string":""}}""" + + @Test + fun givenAnEnvelopeWhoseJsonDoesNotParse_whenExtracting_thenNothingIsExtracted() { + assertTrue(ToolCallExtractor.extractToolCalls(malformedLayoutEdit).isEmpty()) + } + + @Test + fun givenAnEnvelopeWhoseJsonDoesNotParse_whenDiagnosed_thenItReadsAsMalformed() { + assertEquals( + ToolCallExtractor.UnparsedReply.MALFORMED, + ToolCallExtractor.diagnoseUnparsedReply(malformedLayoutEdit), + ) + } + + @Test + fun givenAnEnvelopeThatWasNeverClosed_whenDiagnosed_thenItReadsAsTruncated() { + assertEquals( + ToolCallExtractor.UnparsedReply.TRUNCATED, + ToolCallExtractor.diagnoseUnparsedReply( + """{"tool":"edit_file","args":{"file_path":"A.kt","new_string":"class""" + ), + ) + } + + @Test + fun givenABareCallTheJsonStrategyCouldNotRead_whenDiagnosed_thenItReadsAsMalformed() { + assertEquals( + ToolCallExtractor.UnparsedReply.MALFORMED, + ToolCallExtractor.diagnoseUnparsedReply("""{"tool":"open_file","args":{"file_path":"A"}"""), + ) + } + + @Test + fun givenAnOrdinaryProseReply_whenDiagnosed_thenNothingIsReportedAsWrong() { + assertNull(ToolCallExtractor.diagnoseUnparsedReply("Hello! What would you like to build?")) + } + + @Test + fun givenProseThatQuotesTheWordTool_whenDiagnosed_thenNothingIsReportedAsWrong() { + // Without the colon there is no key, so this is an answer and not a broken call. Reading + // it as one replaces the reply with an error message. + val reply = """I used the "tool" you asked about; the {curly} braces are just prose.""" + + assertNull(ToolCallExtractor.diagnoseUnparsedReply(reply)) + } + + @Test + fun givenProseThatQuotesTheWordTool_whenStrippingCalls_thenTheProseSurvives() { + val reply = """The "tool" finished.""" + + assertEquals(reply, ToolCallExtractor.proseOutsideToolCalls(reply)) + } + + @Test + fun givenArgumentsWithQuotesAndNewlines_whenRenderedAsAnEnvelope_thenTheyExtractBackUnchanged() { + // The payload that cannot survive the model writing it by hand; rendering escapes it. + val layout = "\n \n" + + val envelope = ToolCallExtractor.renderEnvelope( + "edit_file", + mapOf("file_path" to "app/src/main/res/layout/fragment_home.xml", "new_string" to layout), + ) + val calls = ToolCallExtractor.extractToolCalls(envelope) + + assertEquals(1, calls.size) + assertEquals("edit_file", calls[0].name) + assertEquals(layout, calls[0].args["new_string"]) + } + + @Test + fun givenARenderedEnvelopeBesideProse_whenExtracting_thenTheCallStillRuns() { + val envelope = ToolCallExtractor.renderEnvelope("list_files", mapOf("directory" to "")) + + val calls = ToolCallExtractor.extractToolCalls("Let me look at the project.\n" + envelope) + + assertEquals(1, calls.size) + assertEquals("list_files", calls[0].name) + } } diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCodeParserTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCodeParserTest.kt new file mode 100644 index 00000000..f7c462e2 --- /dev/null +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCodeParserTest.kt @@ -0,0 +1,146 @@ +package com.itsaky.androidide.plugins.aicore.tool + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Unit tests for [ToolCodeParser]. The reply this exists for ended a 13-turn run one call short of + * finishing: Gemini asked to run the app in its own dialect, nothing read it, and the loop reported + * the run COMPLETED with the markup pasted into the chat (ADFA-5410). + */ +class ToolCodeParserTest { + + /** The exact turn-13 reply from the reported run, closing tag and all missing. */ + private val reportedReply = "\nprint(default_api.run_app())" + + @Test + fun givenTheReplyThatEndedTheRun_whenParsed_thenTheCallIsRecovered() { + val calls = ToolCodeParser.parse(reportedReply) + + assertEquals(1, calls.size) + assertEquals("run_app", calls[0].name) + assertTrue(calls[0].args.isEmpty()) + } + + @Test + fun givenTheReplyThatEndedTheRun_whenExtracting_thenTheAgentRunsItLikeAnyOtherCall() { + val calls = ToolCallExtractor.extractToolCalls(reportedReply) + + assertEquals(1, calls.size) + assertEquals("run_app", calls[0].name) + } + + @Test + fun givenKeywordArguments_whenParsed_thenEachOneIsRead() { + val calls = ToolCodeParser.parse( + """print(default_api.edit_file(file_path="A.kt", old_string="a", new_string="b"))""" + ) + + assertEquals(1, calls.size) + assertEquals(mapOf("file_path" to "A.kt", "old_string" to "a", "new_string" to "b"), calls[0].args) + } + + @Test + fun givenAValueHoldingCommasAndBrackets_whenParsed_thenTheArgumentsAreNotSplitInsideIt() { + val calls = ToolCodeParser.parse( + """default_api.create_file(file_path="A.kt", content="fun f(a, b) { g(1, 2) }")""" + ) + + assertEquals("fun f(a, b) { g(1, 2) }", calls[0].args["content"]) + } + + @Test + fun givenAnEscapedValue_whenParsed_thenTheEscapesAreResolved() { + val calls = ToolCodeParser.parse( + """default_api.create_file(content="line\nwith \"quotes\" and a \\ backslash")""" + ) + + assertEquals("line\nwith \"quotes\" and a \\ backslash", calls[0].args["content"]) + } + + @Test + fun givenATripleQuotedValue_whenParsed_thenTheWholePayloadSurvives() { + // A model writing a file's contents reaches for these; reading '' as empty would truncate it. + val calls = ToolCodeParser.parse( + "default_api.create_file(content='''\nsecond line''')" + ) + + assertEquals("\nsecond line", calls[0].args["content"]) + } + + @Test + fun givenNonStringLiterals_whenParsed_thenTheyKeepTheirTypes() { + val calls = ToolCodeParser.parse( + "default_api.search_project(query=\"x\", search_in_contents=True, limit=10, cursor=None)" + ) + + assertEquals(true, calls[0].args["search_in_contents"]) + assertEquals(10L, calls[0].args["limit"]) + assertEquals(null, calls[0].args["cursor"]) + } + + @Test + fun givenAPositionalArgument_whenParsed_thenTheCallIsRefusedRatherThanGuessed() { + // Without the tool's schema the value cannot be named, and a wrong slot is worse than no run. + assertTrue(ToolCodeParser.parse("""default_api.read_file("A.kt")""").isEmpty()) + } + + @Test + fun givenAPositionalArgument_whenDiagnosed_thenItIsStillReportedRatherThanReadAsProse() { + assertEquals( + ToolCallExtractor.UnparsedReply.MALFORMED, + ToolCallExtractor.diagnoseUnparsedReply("""default_api.read_file("A.kt")"""), + ) + } + + @Test + fun givenAnUnterminatedCall_whenParsed_thenNothingIsRecovered() { + val cutOff = """default_api.create_file(file_path="A.kt", content="cl""" + + assertTrue(ToolCodeParser.parse(cutOff).isEmpty()) + } + + @Test + fun givenTwoCallsInOneBlock_whenParsed_thenBothAreRead() { + val calls = ToolCodeParser.parse( + "\nprint(default_api.gradle_sync())\nprint(default_api.run_app())\n" + ) + + assertEquals(listOf("gradle_sync", "run_app"), calls.map { it.name }) + } + + @Test + fun givenOrdinaryProse_whenAskedIfItIsToolCode_thenItIsNot() { + assertFalse(ToolCodeParser.looksLikeToolCode("I added a binary search class and a screen for it.")) + } + + @Test + fun givenAPythonPrefixedStringLiteral_whenParsed_thenThePrefixIsNotPartOfTheValue() { + // `r"…"` and `f"…"` are Python string literals; keeping the prefix and quotes in a path + // argument fails the file operation that receives it. + val calls = ToolCodeParser.parse( + """default_api.read_file(file_path=r"app/src/main/AndroidManifest.xml")""" + ) + + assertEquals("app/src/main/AndroidManifest.xml", calls[0].args["file_path"]) + } + + @Test + fun givenAnFStringLiteral_whenParsed_thenItIsUnquotedLikeAnyOther() { + val calls = ToolCodeParser.parse("""default_api.open_file(file_path=f"app/Main.kt")""") + + assertEquals("app/Main.kt", calls[0].args["file_path"]) + } + + @Test + fun givenAnEnvelopeReply_whenExtracting_thenTheEnvelopeStillWins() { + // Strategy 3 is a fallback; a well-formed envelope must never be re-read as tool_code. + val calls = ToolCallExtractor.extractToolCalls( + """{"tool":"run_app","args":{}} default_api.gradle_sync()""" + ) + + assertEquals(listOf("run_app"), calls.map { it.name }) + } +} diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/GenerateFromTemplateHandlerTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/GenerateFromTemplateHandlerTest.kt new file mode 100644 index 00000000..9e507a7f --- /dev/null +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/GenerateFromTemplateHandlerTest.kt @@ -0,0 +1,55 @@ +package com.itsaky.androidide.plugins.aicore.tool.handlers + +import com.itsaky.androidide.plugins.PluginContext +import io.mockk.mockk +import org.json.JSONObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The `variables` argument. It reaches a handler in a different shape per backend, and a cast to + * one of them silently dropped every variable the model sent. + */ +class GenerateFromTemplateHandlerTest { + + private val handler = GenerateFromTemplateHandler(mockk(relaxed = true)) + + @Test + fun givenANestedObject_whenReadingVariables_thenTheyAreKept() { + // What both the envelope parser and a native call actually hand over: org.json's own type, + // never a Kotlin Map, which is why `as? Map` was always null. + val variables = handler.variablesOf(JSONObject("""{"className":"Main","package":"a.b"}""")) + + assertEquals("Main", variables["className"]) + assertEquals("a.b", variables["package"]) + } + + @Test + fun givenJsonText_whenReadingVariables_thenItIsParsed() { + // Gemini cannot declare a free-form object, so it sends one as text. + val variables = handler.variablesOf("""{"className":"Main"}""") + + assertEquals("Main", variables["className"]) + } + + @Test + fun givenAMap_whenReadingVariables_thenItIsKept() { + assertEquals(mapOf("a" to 1), handler.variablesOf(mapOf("a" to 1))) + } + + @Test + fun givenNothingOrUnparseableText_whenReadingVariables_thenThereAreNone() { + assertTrue(handler.variablesOf(null).isEmpty()) + assertTrue(handler.variablesOf("not json").isEmpty()) + } + + @Test + fun givenTheSchema_whenDeclared_thenTheTemplateNameIsTheOnlyRequiredArgument() { + @Suppress("UNCHECKED_CAST") + val properties = handler.parametersSchema["properties"] as Map + + assertEquals(setOf("template_name", "variables"), properties.keys) + assertEquals(listOf("template_name"), handler.parametersSchema["required"]) + } +} diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentReplyRendererTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentReplyRendererTest.kt index dcde3399..442acce8 100644 --- a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentReplyRendererTest.kt +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentReplyRendererTest.kt @@ -1,6 +1,7 @@ package com.itsaky.androidide.plugins.aicore.viewmodel import com.itsaky.androidide.plugins.aicore.tool.ToolCall +import com.itsaky.androidide.plugins.aicore.tool.ToolCallExtractor import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue @@ -17,6 +18,8 @@ class AgentReplyRendererTest { const val TERMINAL = "respond" const val FAILED = "(action failed)" const val NO_RESPONSE = "(no response)" + const val TRUNCATED = "(truncated)" + const val MALFORMED = "(malformed)" } private fun render( @@ -30,6 +33,12 @@ class AgentReplyRendererTest { lastToolFailed = lastToolFailed, actionFailedText = FAILED, noResponseText = NO_RESPONSE, + unparsedReplyText = { + when (it) { + ToolCallExtractor.UnparsedReply.TRUNCATED -> TRUNCATED + ToolCallExtractor.UnparsedReply.MALFORMED -> MALFORMED + } + }, ) { call -> "🔧 ${call.name}" } private fun respond(vararg args: Pair) = @@ -171,4 +180,26 @@ class AgentReplyRendererTest { assertFalse(AgentReplyRenderer.isDuplicateTurn(calls, null, TERMINAL)) } + + @Test + fun givenAReplyWhoseEnvelopeDidNotParse_whenRendered_thenTheAdviceReplacesTheRawJson() { + val raw = """{"tool":"edit_file","args":{"new_string":""}}""" + + val text = render(raw) + + // The bug this pins down: the unreadable call was pasted into the transcript verbatim. + assertEquals(MALFORMED, text) + } + + @Test + fun givenAReplyCutOffMidEnvelope_whenRendered_thenTheTruncationAdviceIsShown() { + val text = render("""{"tool":"edit_file","args":{"file_path":"A.kt",""") + + assertEquals(TRUNCATED, text) + } + + @Test + fun givenAnOrdinaryProseReply_whenRendered_thenItIsShownUnchanged() { + assertEquals("Hi! What shall we build?", render("Hi! What shall we build?")) + } } diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ProjectLayoutTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ProjectLayoutTest.kt new file mode 100644 index 00000000..b1a2dcb3 --- /dev/null +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ProjectLayoutTest.kt @@ -0,0 +1,138 @@ +package com.itsaky.androidide.plugins.aicore.viewmodel + +import java.io.File +import java.nio.file.Files +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +/** + * Unit tests for [ProjectLayout]. The run this exists for spent seven of its sixteen turns walking + * `app` → `src` → `main` → `java` → `com` → `example` and hit the step limit with work outstanding; + * every path it was looking for is one this collapses into the prompt. + */ +class ProjectLayoutTest { + + private lateinit var root: File + + @Before + fun setup() { + root = Files.createTempDirectory("project-layout").toFile().canonicalFile + } + + private fun dir(path: String): File = File(root, path).apply { mkdirs() } + + private fun file(path: String): File = File(root, path).apply { + parentFile.mkdirs() + writeText("x") + } + + @Test + fun givenASingleChildPackageChain_whenDescribed_thenItCollapsesToTheDeepestDirectory() { + dir("app/src/main/java/com/example/myapplication27") + + val modules = ProjectLayout.describe(root) + + assertEquals(1, modules.size) + assertEquals("app", modules[0].name) + assertEquals("app/src/main/java/com/example/myapplication27", modules[0].sourceDir) + } + + @Test + fun givenAPackageDirectoryHoldingSources_whenDescribed_thenDescentStopsThere() { + // A directory with files in it is the package, not another level to walk through. + file("app/src/main/java/com/example/app/MainActivity.java") + dir("app/src/main/java/com/example/app/ui") + + assertEquals("app/src/main/java/com/example/app", ProjectLayout.describe(root)[0].sourceDir) + } + + @Test + fun givenABranchingTree_whenDescribed_thenDescentStopsAtTheBranch() { + dir("app/src/main/java/com/example") + dir("app/src/main/java/org/other") + + // Two packages: there is no single directory a new class obviously belongs in. + assertEquals("app/src/main/java", ProjectLayout.describe(root)[0].sourceDir) + } + + @Test + fun givenAKotlinSourceRoot_whenDescribed_thenItIsFoundToo() { + dir("app/src/main/kotlin/com/example/app") + + assertEquals("app/src/main/kotlin/com/example/app", ProjectLayout.describe(root)[0].sourceDir) + } + + @Test + fun givenLayoutsAndAManifest_whenDescribed_thenBothArePointedAt() { + dir("app/src/main/java/com/example/app") + dir("app/src/main/res/layout") + file("app/src/main/AndroidManifest.xml") + + val module = ProjectLayout.describe(root)[0] + + assertEquals("app/src/main/res/layout", module.layoutDir) + assertEquals("app/src/main/AndroidManifest.xml", module.manifest) + } + + @Test + fun givenNoLayoutsOrManifest_whenDescribed_thenNothingIsInvented() { + dir("app/src/main/java/com/example/app") + + val module = ProjectLayout.describe(root)[0] + + assertNull(module.layoutDir) + assertNull(module.manifest) + } + + @Test + fun givenSeveralModules_whenDescribed_thenTheAppModuleComesFirst() { + dir("core/src/main/java/com/example/core") + dir("app/src/main/java/com/example/app") + dir("zebra/src/main/java/com/example/zebra") + + assertEquals(listOf("app", "core", "zebra"), ProjectLayout.describe(root).map { it.name }) + } + + @Test + fun givenADirectoryThatIsNotAModule_whenDescribed_thenItIsLeftOut() { + dir("app/src/main/java/com/example/app") + dir("build/intermediates") + dir("gradle/wrapper") + + assertEquals(listOf("app"), ProjectLayout.describe(root).map { it.name }) + } + + @Test + fun givenAnEmptyJavaRootBesideAPopulatedKotlinOne_whenDescribed_thenKotlinIsNamed() { + // AGP keeps an empty `java` beside the `kotlin` tree, and naming the empty one tells the + // agent to write new Kotlin classes where nothing else lives. + dir("app/src/main/java") + file("app/src/main/kotlin/com/example/app/MainActivity.kt") + + assertEquals( + "app/src/main/kotlin/com/example/app", + ProjectLayout.describe(root)[0].sourceDir, + ) + } + + @Test + fun givenTwoEmptySourceRoots_whenDescribed_thenThePreferredOneIsStillNamed() { + dir("app/src/main/java") + dir("app/src/main/kotlin") + + assertEquals("app/src/main/java", ProjectLayout.describe(root)[0].sourceDir) + } + + @Test + fun givenAnEmptyProject_whenDescribed_thenTheBlockHasNothingToSay() { + assertTrue(ProjectLayout.describe(root).isEmpty()) + } + + @Test + fun givenAMissingRoot_whenDescribed_thenItDegradesToSayingNothing() { + assertTrue(ProjectLayout.describe(File(root, "gone")).isEmpty()) + } +}