From 6fa029a95ca9a05ecc5d2ffb9ad7de2b1cdae632 Mon Sep 17 00:00:00 2001 From: John Trujillo Date: Thu, 3 Sep 2026 12:29:20 -0500 Subject: [PATCH 1/4] fix(ai): ADFA-5410 make Gemini and OpenAI agent tool calls actually run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declare tools natively on both backends so calls arrive as structured data instead of unparseable prose, and read the dialects models still emit — Gemini's print(default_api.x()) and OpenAI's tool_calls deltas that carried no content — with a fallback to undeclared requests where a server refuses them. Report unreadable calls and abandoned runs honestly rather than COMPLETED, and name the source, layout and manifest paths up front. --- .../aiagentgemini/backend/GeminiBackend.kt | 169 ++++++++++-- .../backend/GeminiToolProtocol.kt | 164 +++++++++++ .../errors/GeminiErrorFormatter.kt | 6 + .../prompt/GeminiSystemPrompt.kt | 14 +- .../src/main/res/values/strings.xml | 1 + .../backend/GeminiBackendTest.kt | 7 +- .../backend/GeminiToolProtocolTest.kt | 203 ++++++++++++++ .../prompt/GeminiSystemPromptTest.kt | 61 +++++ .../aiagentopenai/backend/OpenAiBackend.kt | 185 +++++++++++-- .../backend/OpenAiRequestBuilder.kt | 12 +- .../backend/OpenAiToolProtocol.kt | 212 ++++++++++++++ .../aiagentopenai/backend/RequestTuning.kt | 50 +++- .../plugins/aiagentopenai/backend/SseChunk.kt | 16 ++ .../prompt/OpenAiSystemPrompt.kt | 20 +- .../backend/OpenAiBackendTest.kt | 35 +++ .../backend/OpenAiRequestBuilderTest.kt | 37 ++- .../backend/OpenAiToolProtocolTest.kt | 201 ++++++++++++++ .../backend/RequestTuningTest.kt | 23 ++ .../aiagentopenai/backend/SseChunkTest.kt | 39 +++ .../prompt/OpenAiSystemPromptTest.kt | 30 +- .../plugins/aicore/logging/AgentTrace.kt | 18 +- .../plugins/aicore/tool/AgentLoop.kt | 69 ++++- .../plugins/aicore/tool/Executor.kt | 22 +- .../plugins/aicore/tool/ToolCallExtractor.kt | 107 +++++++- .../plugins/aicore/tool/ToolCodeParser.kt | 258 ++++++++++++++++++ .../plugins/aicore/tool/ToolSchema.kt | 57 ++++ .../tool/handlers/AddDependencyHandler.kt | 10 + .../aicore/tool/handlers/CreateFileHandler.kt | 6 + .../aicore/tool/handlers/EditFileHandler.kt | 12 + .../handlers/GenerateFromTemplateHandler.kt | 33 ++- .../aicore/tool/handlers/ListFilesHandler.kt | 6 + .../aicore/tool/handlers/OpenFileHandler.kt | 5 + .../aicore/tool/handlers/ReadFileHandler.kt | 5 + .../tool/handlers/SearchProjectHandler.kt | 11 + .../aicore/tool/handlers/UpdateFileHandler.kt | 6 + .../aicore/viewmodel/AgentReplyRenderer.kt | 6 +- .../plugins/aicore/viewmodel/ChatViewModel.kt | 200 ++++++++++++-- .../plugins/aicore/viewmodel/ProjectLayout.kt | 107 ++++++++ ai-core/src/main/res/values/strings.xml | 2 + .../plugins/aicore/tool/AgentLoopTest.kt | 89 ++++++ .../aicore/tool/ToolCallExtractorTest.kt | 86 ++++++ .../plugins/aicore/tool/ToolCodeParserTest.kt | 146 ++++++++++ .../GenerateFromTemplateHandlerTest.kt | 55 ++++ .../viewmodel/AgentReplyRendererTest.kt | 31 +++ .../aicore/viewmodel/ProjectLayoutTest.kt | 138 ++++++++++ 45 files changed, 2867 insertions(+), 103 deletions(-) create mode 100644 ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiToolProtocol.kt create mode 100644 ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiToolProtocolTest.kt create mode 100644 ai-agent-gemini/src/test/kotlin/com/itsaky/androidide/plugins/aiagentgemini/prompt/GeminiSystemPromptTest.kt create mode 100644 ai-agent-openai/src/main/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiToolProtocol.kt create mode 100644 ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiBackendTest.kt create mode 100644 ai-agent-openai/src/test/kotlin/com/itsaky/androidide/plugins/aiagentopenai/backend/OpenAiToolProtocolTest.kt create mode 100644 ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCodeParser.kt create mode 100644 ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolSchema.kt create mode 100644 ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ProjectLayout.kt create mode 100644 ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/ToolCodeParserTest.kt create mode 100644 ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/GenerateFromTemplateHandlerTest.kt create mode 100644 ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/ProjectLayoutTest.kt 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()) + } +} From 5a10192cb73daf09318dfb832a049028f6a3c084 Mon Sep 17 00:00:00 2001 From: John Trujillo Date: Mon, 7 Sep 2026 15:28:29 -0500 Subject: [PATCH 2/4] fix(ai): ADFA-5410 keep truncated replies and native calls out of history Gemini only errors on MAX_TOKENS when the reply is also blank, and AgentLoop takes a ModelReply so envelopes reach extraction but never the transcript. Also: declarable() on items, a toast when a server refuses tools, unparsed advice left to the bubble. --- .../aiagentgemini/backend/GeminiBackend.kt | 7 +- .../backend/GeminiToolProtocol.kt | 4 +- .../backend/GeminiToolProtocolTest.kt | 29 +++++++++ .../aiagentopenai/backend/OpenAiBackend.kt | 21 ++++++ .../src/main/res/values/strings.xml | 1 + .../plugins/aicore/tool/AgentLoop.kt | 30 +++++++-- .../plugins/aicore/viewmodel/ChatViewModel.kt | 65 ++++++++++--------- .../plugins/aicore/tool/AgentLoopTest.kt | 58 ++++++++++++++++- 8 files changed, 176 insertions(+), 39 deletions(-) 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 586f847c..3a52ba8a 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 @@ -375,8 +375,11 @@ class GeminiBackend( ) 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 -> + // reads as an empty reply unless the reason is reported. Only when the cap left + // nothing behind: onError deletes the bubble, so reporting it for a long answer + // that ran out of room would throw away the prose it did produce. + toolCallCount == 0 && finalText.isBlank() && + finishReason == FINISH_REASON_MAX_TOKENS -> callback.onError(userMessage(GeminiFailure.ReplyTruncated)) toolCallCount == 0 && finalText.isBlank() -> 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 index 0b932fb3..5de42548 100644 --- 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 @@ -146,7 +146,9 @@ internal object GeminiToolProtocol { (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)) } + // Through declarable() like a property: an array of free-form objects is as propertyless + // as a free-form property, and Gemini answers the same 400. + (schema["items"] as? Map<*, *>)?.let { json.put("items", declarable(schemaJson(it))) } (schema["properties"] as? Map<*, *>)?.let { properties -> val rendered = JSONObject() for ((name, value) in properties) { 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 index 90d667a0..ca4b0bec 100644 --- 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 @@ -74,6 +74,35 @@ class GeminiToolProtocolTest { assertTrue(variables.getString("description").contains("JSON object")) } + @Test + fun givenAnArrayOfFreeFormObjects_whenDeclared_thenItsItemsAreDeclaredAsJsonText() { + // The same propertyless OBJECT, one level down: an items schema left as-is 400s the + // request just as a property would, and the streaming path does not catch it. + val declarations = GeminiToolProtocol.functionDeclarations( + listOf( + ToolDefinition( + "add_dependency", + "Add dependencies", + schema( + "entries" to mapOf( + "type" to "array", + "items" to mapOf("type" to "object", "description" to "One entry."), + ), + required = listOf("entries"), + ), + ) + ) + ) + + val items = declarations.getJSONObject(0) + .getJSONObject("parameters") + .getJSONObject("properties") + .getJSONObject("entries") + .getJSONObject("items") + assertEquals("STRING", items.getString("type")) + assertTrue(items.getString("description").contains("JSON object")) + } + @Test fun givenASchemaThatNamesNoProperties_whenDeclared_thenNoParametersAreSent() { // Same rejection at the top level, where there is nothing to degrade it to. 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 5b86828a..ec5d64b4 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,7 +1,10 @@ package com.itsaky.androidide.plugins.aiagentopenai.backend import android.content.SharedPreferences +import android.os.Handler +import android.os.Looper import android.util.Log +import android.widget.Toast import com.itsaky.androidide.plugins.PluginContext import com.itsaky.androidide.plugins.aiagentopenai.R import com.itsaky.androidide.plugins.aiagentopenai.errors.OpenAiErrorFormatter @@ -555,10 +558,28 @@ class OpenAiBackend( "OpenAiBackend: $baseUrl does not accept tool declarations; " + "the agent cannot call tools on this server" ) + // Said out loud, not only logged: from here the agent answers but never touches the + // project, which reads as the tools being broken. Once per server, since the flag + // above short-circuits every later turn. + notifyToolsUnsupported(baseUrl) attempt(emptyList()) } } + /** + * Tells the user this server cannot call tools, as a Toast: the run continues, so there is no + * error message to carry it, and the chat's own turn is an ordinary prose answer. + * + * @param baseUrl the server that refused, named in the message. + */ + private fun notifyToolsUnsupported(baseUrl: String) { + val appContext = context.androidContext.applicationContext + val message = appContext.getString(R.string.openai_error_tools_unsupported, baseUrl) + Handler(Looper.getMainLooper()).post { + Toast.makeText(appContext, message, Toast.LENGTH_LONG).show() + } + } + /** * Run [attempt] and, if the server rejected one optional parameter, run it once more without it. * diff --git a/ai-agent-openai/src/main/res/values/strings.xml b/ai-agent-openai/src/main/res/values/strings.xml index daab0802..3e025241 100644 --- a/ai-agent-openai/src/main/res/values/strings.xml +++ b/ai-agent-openai/src/main/res/values/strings.xml @@ -29,6 +29,7 @@ The server answered but sent no reply text. Check the model is fully loaded in your server, then try again — see the IDE log for what the server sent. The model spent its whole reply on internal reasoning and never answered. Raise the response length limit in your server, or choose a model without a thinking mode. The reply was cut off before any text arrived — the response length limit is too low for this model. Raise it in your server settings. + %1$s does not support tool calling, so the AI can answer but cannot change your project. Point the server setting at one that does. The request to the AI server failed. The request to the AI server failed. %1$s 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 31624aab..bea2e867 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 @@ -38,6 +38,21 @@ class AgentLoop( const val DEFAULT_MAX_CONSECUTIVE_REPEATS = 2 } + /** + * One model turn, as the loop reads it and as the transcript keeps it. + * + * The two differ for a natively-calling backend: [text] carries the provider's calls rendered + * as `` envelopes so extraction reads them by the one path a text-mode call takes, + * while [historyText] is what the model actually wrote. Storing the envelopes would show the + * model its own calls in a format the system prompt says is not read, which invites it to write + * the next one as text. + * + * @property text the reply tool extraction reads. + * @property historyText the ASSISTANT turn to store; the same text unless the caller says + * otherwise. + */ + data class ModelReply(val text: String, val historyText: String = text) + /** Callbacks so the caller can drive UI/state; all no-ops by default. */ interface Events { /** @@ -136,14 +151,15 @@ class AgentLoop( * @param history transcript, mutated in place; seed it with the user message. * @param generate renders one model turn from the transcript so far. Receives the turns * structurally rather than pre-flattened, so a backend that speaks a real chat format can - * emit one turn per message; flattening callers can use [renderTranscript]. + * emit one turn per message; flattening callers can use [renderTranscript]. Returns a + * [ModelReply], whose two texts a natively-calling caller sets apart. * @param executeTools runs a batch of tool calls. * @param events UI/state callbacks. * @return the run [Result]. */ suspend fun run( history: MutableList, - generate: suspend (turns: List) -> String, + generate: suspend (turns: List) -> ModelReply, executeTools: suspend (List) -> List, events: Events = object : Events {}, ): Result { @@ -155,8 +171,14 @@ class AgentLoop( while (turn < maxIterations) { turn++ - val text = generate(history.toList()) - history.add(ChatMessage(Role.ASSISTANT, text)) + val reply = generate(history.toList()) + val text = reply.text + // A native call with no prose beside it leaves nothing to record, and an empty turn is + // not harmless: Gemini rejects a content part whose text is empty. The tool results + // that follow name the call anyway. + if (reply.historyText.isNotBlank()) { + history.add(ChatMessage(Role.ASSISTANT, reply.historyText)) + } events.onModelTurn(turn, text) val calls = extractToolCalls(text) 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 c2f69321..8d5e317f 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 @@ -1000,15 +1000,13 @@ class ChatViewModel( turn: Int, reason: ToolCallExtractor.UnparsedReply, ) { + // No system message: AgentReplyRenderer already puts this same + // advice in the turn's own bubble, in place of the raw envelope. 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) { @@ -1096,7 +1094,8 @@ class ChatViewModel( * @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). + * @return the turn: the reply with any native calls rendered into it for extraction, beside the + * text the model itself wrote, which is what the transcript keeps. */ private suspend fun runModelTurn( llmService: LlmInferenceService, @@ -1104,8 +1103,8 @@ class ChatViewModel( config: LlmInferenceService.LlmConfig, toolDefinitions: List, epoch: Int - ): String { - val deferred = CompletableDeferred() + ): AgentLoop.ModelReply { + val deferred = CompletableDeferred() val agentMessageId = UUID.randomUUID().toString() val startTime = System.currentTimeMillis() val responseBuilder = StringBuilder() @@ -1124,6 +1123,10 @@ class ChatViewModel( syncMessageToSession(agentMessage) } + // Rendered into 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() + val streamCallback = object : LlmInferenceService.StreamCallback { override fun onToken(token: String) { if (isStale()) return // Stop pressed — ignore late tokens. @@ -1145,10 +1148,14 @@ class ChatViewModel( override fun onComplete(response: LlmInferenceService.LlmResponse) { // Null text means a failed response, which arrives through onError instead. - val text = response.text.orEmpty() + val written = response.text.orEmpty() + val calls = synchronized(nativeCalls) { nativeCalls.toList() } + val text = withNativeCalls(written, calls) + // The transcript keeps what the model wrote, never the envelopes below. + val reply = AgentLoop.ModelReply(text = text, historyText = written) if (isStale()) { // Already cancelled; the awaiting loop was unblocked by job cancel. - deferred.complete(text) + deferred.complete(reply) return } val durationMs = System.currentTimeMillis() - startTime @@ -1176,7 +1183,7 @@ class ChatViewModel( viewModelScope.launch(Dispatchers.Main) { _messages.value = _messages.value.filter { it.id != agentMessageId } } - deferred.complete(text) + deferred.complete(reply) return } @@ -1204,8 +1211,9 @@ class ChatViewModel( _messages.value = _messages.value.map { if (it.id == agentMessageId) finalMsg else it } syncMessageToSession(finalMsg) } - // Return the RAW text to the loop so extraction/stop logic is unaffected. - deferred.complete(text) + // Hand the loop the reply with the envelopes, so extraction/stop logic sees + // a native call, and the model's own text for the transcript. + deferred.complete(reply) } override fun onError(error: String) { @@ -1221,10 +1229,6 @@ 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. A backend that reports no native calls simply never calls onToolCall, and @@ -1249,12 +1253,10 @@ class ChatViewModel( 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) - ) - } + // The reported calls are read back in streamCallback.onComplete, which + // renders them into the reply it hands the loop. + override fun onComplete(response: LlmInferenceService.LlmResponse) = + streamCallback.onComplete(response) override fun onError(error: String) = streamCallback.onError(error) } @@ -1272,23 +1274,26 @@ class ChatViewModel( } /** - * This response with [calls] appended as canonical `` envelopes. + * [written] 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. + * For this turn only — see [AgentLoop.ModelReply] for why the transcript keeps [written]. * - * @param calls the native calls reported during this turn. - * @return a response whose text carries the calls in the form extraction reads back. + * @param written the reply text as the model produced it. + * @param calls the native calls reported during this turn; none leaves [written] unchanged. + * @return the text carrying the calls in the form extraction reads back. */ - private fun LlmInferenceService.LlmResponse.withNativeCalls( + private fun withNativeCalls( + written: String, calls: List, - ): LlmInferenceService.LlmResponse { + ): String { + if (calls.isEmpty()) return written 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) + val prose = written.trim() + return if (prose.isEmpty()) envelopes else prose + "\n" + envelopes } /** 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 d28910d0..89d06eab 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 @@ -19,11 +19,11 @@ class AgentLoopTest { private class ScriptedModel(private val responses: List) { val turns = mutableListOf>() var calls = 0 - suspend fun generate(history: List): String { + suspend fun generate(history: List): AgentLoop.ModelReply { turns += history val r = responses.getOrElse(calls) { responses.last() } calls++ - return r + return AgentLoop.ModelReply(r) } } @@ -129,6 +129,60 @@ class AgentLoopTest { assertEquals("All set!", finalMessage) } + @Test + fun givenAReplyWhoseHistoryTextDiffers_whenTheLoopRuns_thenTheTranscriptKeepsTheHistoryText() = runTest { + // A natively-calling backend hands over the envelopes for extraction only: shown its own + // calls as text, the model starts writing the next one in a format that does not run. + val call = toolCall("read_file") + val replies = listOf( + AgentLoop.ModelReply(text = "Reading it. $call", historyText = "Reading it."), + AgentLoop.ModelReply(text = "All done."), + ) + var turn = 0 + val history = mutableListOf(ChatMessage(Role.USER, "read MainActivity.kt")) + val executed = mutableListOf>() + + val result = AgentLoop().run( + history = history, + generate = { replies[turn++] }, + executeTools = { calls -> + executed += calls + listOf(ToolResult.success("contents", "MainActivity.kt")) + } + ) + + assertTrue(result.completed) + // The call still ran, so extraction read the envelope... + assertEquals(1, executed.size) + assertEquals("read_file", executed[0][0].name) + // ...but the stored turn carries neither it nor its envelope. + assertEquals("Reading it.", history[1].content) + assertFalse(history[1].content.contains("")) + } + + @Test + fun givenACallWithNoProseBesideIt_whenTheLoopRuns_thenNoEmptyTurnIsStored() = runTest { + // Gemini rejects a content part with empty text, so the turn is left out rather than + // stored blank; the tool results that follow name the call. + val replies = listOf( + AgentLoop.ModelReply(text = toolCall("read_file"), historyText = ""), + AgentLoop.ModelReply(text = "All done."), + ) + var turn = 0 + val history = mutableListOf(ChatMessage(Role.USER, "read MainActivity.kt")) + + AgentLoop().run( + history = history, + generate = { replies[turn++] }, + executeTools = { listOf(ToolResult.success("contents", "MainActivity.kt")) } + ) + + // user, user(tool results), assistant("All done.") — no blank turn between the first two. + assertEquals(3, history.size) + assertEquals(Role.USER, history[1].role) + assertTrue(history[1].content.startsWith("")) + } + @Test fun givenAModelThatCallsAToolThenAnswers_whenTheLoopRuns_thenItChainsTheToolAndFinishes() = runTest { // Turn 1: model calls a tool. Turn 2: sees results, gives final answer. From 3fbfec9b3edc76b3538086e073f1ec69472adfd0 Mon Sep 17 00:00:00 2001 From: John Trujillo Date: Tue, 8 Sep 2026 09:23:45 -0500 Subject: [PATCH 3/4] fix(gemini): merge consecutive same-role turns in buildContents A native call with no prose leaves no ASSISTANT turn, so the user message and its tool results were sent as two adjacent "user" contents. --- .../aiagentgemini/backend/GeminiBackend.kt | 30 ++++++++++--- .../backend/GeminiBackendTest.kt | 42 +++++++++++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) 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 3a52ba8a..c922c9ef 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 @@ -258,19 +258,32 @@ class GeminiBackend( * acknowledges — the same shape [generateWithHistory] uses, kept in one place so the two * transports cannot drift apart. * + * Consecutive same-role turns are merged into one content, because the transcript no longer + * always alternates: the agent loop drops an ASSISTANT turn that carried only a native call + * and no prose, leaving the user message and the tool results it produced adjacent. + * * @param history the conversation so far, oldest first * @param prompt the current user turn, appended last * @param config supplies the optional system prompt */ - private fun buildContents( + internal fun buildContents( history: List, prompt: String, config: LlmConfig ): JSONArray { - val contents = JSONArray() + val turns = mutableListOf>() + // Folds a turn into the previous one when the role repeats, so the roles alternate. + fun add(role: String, text: String) { + val last = turns.lastOrNull() + if (last != null && last.first == role) { + turns[turns.lastIndex] = role to (last.second + "\n\n" + text) + } else { + turns.add(role to text) + } + } config.systemPrompt?.let { systemPrompt -> - contents.put(contentJson("user", systemPrompt)) - contents.put(contentJson("model", "Understood.")) + add("user", systemPrompt) + add("model", "Understood.") } for (msg in history) { val role = when (msg.role) { @@ -282,9 +295,14 @@ class GeminiBackend( // 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)) + add(role, msg.content) + } + add("user", prompt) + + val contents = JSONArray() + for ((role, text) in turns) { + contents.put(contentJson(role, text)) } - contents.put(contentJson("user", prompt)) return contents } 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 469007ad..e71b6310 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 @@ -30,4 +30,46 @@ class GeminiBackendTest { assertTrue(declared is HistoryCapableBackend) assertTrue(declared is ToolCallingBackend) } + + @Test + fun givenAdjacentUserTurns_whenBuildingContents_thenTheyAreMergedIntoOne() { + // The agent loop stores no ASSISTANT turn for a native call with no prose beside it, so + // the user message and the tool results it produced arrive adjacent. Sent as two user + // contents they break Gemini's alternation; merged, the request stays well-formed. + val contents = backend.buildContents( + history = listOf( + ChatMessage(ChatMessage.Role.USER, "add a dependency"), + ChatMessage(ChatMessage.Role.USER, "Tool add_dependency: ok"), + ), + prompt = "Tool sync_project: ok", + config = LlmConfig("gemini"), + ) + + assertEquals(1, contents.length()) + val turn = contents.getJSONObject(0) + assertEquals("user", turn.getString("role")) + assertEquals( + "add a dependency\n\nTool add_dependency: ok\n\nTool sync_project: ok", + turn.getJSONArray("parts").getJSONObject(0).getString("text"), + ) + } + + @Test + fun givenAlternatingTurns_whenBuildingContents_thenEachStaysItsOwnContent() { + val contents = backend.buildContents( + history = listOf( + ChatMessage(ChatMessage.Role.USER, "hello"), + ChatMessage(ChatMessage.Role.ASSISTANT, "hi"), + ), + prompt = "how are you?", + config = LlmConfig("gemini").apply { systemPrompt = "be brief" }, + ) + + val roles = (0 until contents.length()).map { contents.getJSONObject(it).getString("role") } + assertEquals(listOf("user", "model", "user", "model", "user"), roles) + assertEquals( + "be brief", + contents.getJSONObject(0).getJSONArray("parts").getJSONObject(0).getString("text"), + ) + } } From 25d86553936ce81f6e454872457d71e80e54433e Mon Sep 17 00:00:00 2001 From: John Trujillo Date: Tue, 8 Sep 2026 16:57:02 -0500 Subject: [PATCH 4/4] fix(ai): ADFA-5410 collapse agent tool chatter to one activity line A run now keeps one transcript row, rewritten in place as each tool starts and closed as a one-line summary; failures keep their own message and Retry. Removals also reach the session's message list, or syncMessageToSession republishes a dropped bubble and it animates its dots for the rest of the chat. --- .../plugins/aicore/tool/Executor.kt | 13 +- .../plugins/aicore/viewmodel/AgentActivity.kt | 60 ++++++ .../aicore/viewmodel/AgentReplyRenderer.kt | 32 ++-- .../plugins/aicore/viewmodel/ChatViewModel.kt | 180 +++++++++++++----- ai-core/src/main/res/values/strings.xml | 9 + .../aicore/viewmodel/AgentActivityTest.kt | 86 +++++++++ .../viewmodel/AgentReplyRendererTest.kt | 64 +++---- 7 files changed, 338 insertions(+), 106 deletions(-) create mode 100644 ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentActivity.kt create mode 100644 ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentActivityTest.kt 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 7b6a0991..c98e1d0b 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 @@ -77,9 +77,14 @@ class Executor( * read-only handler, and a batch of four reads must not go sequential over spelling. * * @param toolCalls the calls to run, in the order the model emitted them. + * @param onCallStarted invoked as each call begins, for the UI's activity line; a parallel run + * invokes it once per call in that run, in no guaranteed order. * @return one result per call, positionally aligned with [toolCalls]. */ - suspend fun execute(toolCalls: List): List = coroutineScope { + suspend fun execute( + toolCalls: List, + onCallStarted: suspend (ToolCall) -> Unit = {}, + ): List = coroutineScope { Log.i(TAG, "Executing ${toolCalls.size} tool call(s)...") val results = arrayOfNulls(toolCalls.size) @@ -91,6 +96,7 @@ class Executor( var index = 0 while (index < toolCalls.size) { if (!parallelSafe[index]) { + onCallStarted(toolCalls[index]) results[index] = executeCall(toolCalls[index], handlers[index], "Sequential") index++ continue @@ -99,7 +105,10 @@ class Executor( var end = index while (end < toolCalls.size && parallelSafe[end]) end++ (index until end).map { i -> - async { results[i] = executeCall(toolCalls[i], handlers[i], "Parallel") } + async { + onCallStarted(toolCalls[i]) + results[i] = executeCall(toolCalls[i], handlers[i], "Parallel") + } }.awaitAll() index = end } diff --git a/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentActivity.kt b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentActivity.kt new file mode 100644 index 00000000..c52127e2 --- /dev/null +++ b/ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentActivity.kt @@ -0,0 +1,60 @@ +package com.itsaky.androidide.plugins.aicore.viewmodel + +import com.itsaky.androidide.plugins.aicore.tool.ToolCall + +/** + * What the one activity line says while a run works through its tool calls. + * + * A run appends a single bubble and rewrites it as each call starts, so a ten-call run reads as one + * line rather than twenty messages. Pure and string-injected: the wording is a resource, this is + * only the choice of what to name. + */ +object AgentActivity { + + /** Longest subject shown; past this the line wraps and stops being a line. */ + const val SUBJECT_LIMIT = 40 + + /** + * Argument names that say what a call acts on, most specific first. A tool whose arguments + * are all content (`gradle_sync`, `run_app`) matches none, and shows its name alone. + */ + private val SUBJECT_ARGS = listOf( + "file_path", + "path", + "build_file", + "directory", + "query", + "template_name", + "dependency", + ) + + /** + * The part of [call] worth showing beside the tool name. + * + * A path is reduced to its last segment: the full one is what turned the old badge into three + * wrapped lines, and the file name is what a user recognises. + * + * @param call the call about to run. + * @return the subject, or null when the call has nothing recognisable to name. + */ + fun subjectOf(call: ToolCall): String? { + val raw = SUBJECT_ARGS.firstNotNullOfOrNull { key -> + call.args[key]?.toString()?.trim()?.takeIf { it.isNotEmpty() } + } ?: return null + val flat = raw.replace("\n", " ").trim().trimEnd('/') + val leaf = flat.substringAfterLast('/').ifEmpty { flat } + // "." and ".." are how a model spells the project root: a name, but not one worth showing. + if (leaf.isEmpty() || leaf == "." || leaf == "..") return null + return if (leaf.length <= SUBJECT_LIMIT) leaf else leaf.take(SUBJECT_LIMIT) + "…" + } + + /** + * The tool names a run's summary lists: each one once, in the order the run first used it, so + * a loop that read six files still reads as `read_file`. + * + * @param names every executed tool name, in execution order. + * @return the distinct names, blanks dropped. + */ + fun distinctNames(names: List): List = + names.map { it.trim() }.filter { it.isNotEmpty() }.distinct() +} 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 7f019ad9..24748ec8 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 @@ -13,30 +13,27 @@ import com.itsaky.androidide.plugins.aicore.tool.respondMessageOf object AgentReplyRenderer { /** - * Whether this turn only repeats the calls that already succeeded, so its bubble is dropped. + * Whether this turn leaves no bubble behind. * - * A turn carrying the terminal call is never a duplicate, whatever else it repeats: that bubble - * is the only place the answer is rendered. `AgentLoop` runs the real calls first and so never - * reaches `onFinalAnswer`, and dropping the turn here would end the run "completed" with - * nothing on screen. The match is the loose one a handler is routed by, so a backend answering - * `Respond` is recognised as terminal too. + * A turn that only asks for tools is work in progress, not something to read: the run reports + * it on the single activity line ([AgentActivity]), which is replaced as each call starts, and + * a failure still gets its own message. Before that line existed every such turn appended a + * badge bubble and every result another, which is what buried the answer. + * + * A turn carrying the terminal call always keeps its bubble: that is the only place the answer + * is rendered. * * @param toolCalls the calls parsed out of this turn. - * @param lastSucceededCalls the calls this run last executed successfully, null when none did. * @param terminalTool the name of the answer-carrying pseudo-tool (`respond`). - * @return true when the turn adds nothing and should not reach the transcript. + * @return true when the turn should not reach the transcript. */ - fun isDuplicateTurn( - toolCalls: List, - lastSucceededCalls: List?, - terminalTool: String, - ): Boolean { - if (toolCalls.any { isTerminalToolName(it.name, terminalTool) }) return false - return toolCalls.isNotEmpty() && toolCalls == lastSucceededCalls + fun isSilentTurn(toolCalls: List, terminalTool: String): Boolean { + if (toolCalls.isEmpty()) return false + return toolCalls.none { isTerminalToolName(it.name, terminalTool) } } /** - * Renders one model turn. + * Renders one model turn. Only reached for a turn [isSilentTurn] kept. * @param rawText the model's raw reply. * @param toolCalls the calls parsed out of it. * @param terminalTool the name of the answer-carrying pseudo-tool (`respond`). @@ -44,7 +41,6 @@ object AgentReplyRenderer { * @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. */ fun render( @@ -55,7 +51,6 @@ object AgentReplyRenderer { actionFailedText: String, noResponseText: String, unparsedReplyText: (ToolCallExtractor.UnparsedReply) -> String, - renderToolCall: (ToolCall) -> String, ): String { val respondCall = toolCalls.firstOrNull { isTerminalToolName(it.name, terminalTool) } return when { @@ -65,7 +60,6 @@ object AgentReplyRenderer { respondMessageOf(respondCall.args) ?: ToolCallExtractor.proseOutsideToolCalls(rawText) ?: noResponseText - toolCalls.isNotEmpty() -> toolCalls.joinToString("\n", transform = renderToolCall) // 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 8d5e317f..12d6f677 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 @@ -83,9 +83,6 @@ class ChatViewModel( */ private const val EXTRA_PARAM_GRAMMAR = "grammar" - /** Per-argument cap in the tool badge shown in the transcript. */ - private const val TOOL_BADGE_ARG_LIMIT = 80 - /** * The call envelope this side parses back (see [ToolCallExtractor]) and constrains local * sampling to (see [com.itsaky.androidide.plugins.aicore.tool.ToolCallGrammar]). Handed to @@ -242,9 +239,14 @@ class ChatViewModel( @Volatile private var lastToolFailedThisRun = false - /** The current run's most recent fully-successful tool batch, or null; reset per run. */ - @Volatile - private var lastSucceededCalls: List? = null + /** + * The transcript row this run rewrites in place as each tool starts, closed as a one-line + * summary when the run ends; null before the run's first tool. Main thread only. + */ + private var activityMessageId: String? = null + + /** Every tool name this run executed, in order, for the activity line's closing summary. */ + private val runToolNames = mutableListOf() /** The prompt the last run was started with, for [retryLastRun]; null before the first send. */ @Volatile @@ -408,6 +410,23 @@ class ChatViewModel( _contextFiles = files.toList() } + /** + * Drops a message from the transcript and from the session list behind it. + * + * Both, always: [syncMessageToSession] republishes the transcript from the session's own list, + * so a message removed from one and not the other comes back on the next sync. That is what + * left a silenced turn's empty agent bubble on screen, animating its dots for the rest of the + * conversation. + * + * @param messageId the message to remove; an unknown id is a no-op. + */ + private fun removeMessageFromSession(messageId: String) { + _messages.value = _messages.value.filter { it.id != messageId } + _currentSessionId.value + ?.let { id -> _sessions.value.firstOrNull { it.id == id } } + ?.messages?.removeAll { it.id == messageId } + } + /** * Helper method to synchronize a message to the current session. * Updates or adds the message to the session's message list. @@ -722,9 +741,13 @@ class ChatViewModel( } /** - * Executes a batch of tool calls, renders each result as a TOOL message, and - * returns the results for the [agentLoop] to feed back; leaves [AgentState.Idle] - * to the loop. + * Executes a batch of tool calls and returns the results for the [agentLoop] to feed back; + * leaves [AgentState.Idle] to the loop. + * + * A successful call is reported only on the run's single activity line, which this rewrites as + * each call starts. A failure keeps its own message: it is the one thing here a user has to act + * on, and it carries the Retry button. + * * @param tools the snapshot this run started with; a source registered mid-run does not join it. * @param toolCalls the calls to execute. * @return the results, positionally aligned with [toolCalls]. @@ -743,27 +766,22 @@ class ChatViewModel( withContext(Dispatchers.Main) { setState(executingState) } startStateTimer(executingState) - val results = tools.executor.execute(toolCalls) + val results = tools.executor.execute(toolCalls) { call -> + withContext(Dispatchers.Main) { showActivity(call) } + } // Record whether this batch's last tool failed (read by runModelTurn). lastToolFailedThisRun = results.lastOrNull()?.success == false - lastSucceededCalls = toolCalls.takeIf { results.isNotEmpty() && results.all { r -> r.success } } - - // Add tool results as messages withContext(Dispatchers.Main) { results.forEachIndexed { index, result -> + if (result.success) return@forEachIndexed val toolCall = toolCalls[index] - val resultText = if (result.success) { - "${toolCall.name}: ${result.message}\n${result.data ?: ""}" - } else { - "${toolCall.name} failed: ${result.message}\n${result.error_details ?: ""}" - } val resultMessage = ChatMessage( id = UUID.randomUUID().toString(), - text = resultText, + text = "${toolCall.name} failed: ${result.message}\n${result.error_details ?: ""}", sender = Sender.TOOL, - status = if (result.success) MessageStatus.SENT else MessageStatus.ERROR + status = MessageStatus.ERROR ) _messages.value = _messages.value + resultMessage syncMessageToSession(resultMessage) @@ -894,7 +912,8 @@ class ChatViewModel( AgentTrace.beginRun(currentBackendId, userMessage, contextFiles.size) // Reset per-run tool tracking. lastToolFailedThisRun = false - lastSucceededCalls = null + activityMessageId = null + runToolNames.clear() // Where a Retry has to rewind to; read here, on Main, while no run can be appending. lastRunPrompt = userMessage historySizeBeforeLastRun = _history.value.size @@ -1059,8 +1078,12 @@ class ChatViewModel( // Allow re-entry once the coroutine unwinds. isGenerating.set(false) // The run can finish with the chat off screen, where nothing else writes. - // NonCancellable so a Stop still saves what the run produced before it. - withContext(NonCancellable + Dispatchers.Main) { persistState() } + // NonCancellable so a Stop still saves what the run produced before it, and so the + // activity line is closed rather than left reading as a tool still running. + withContext(NonCancellable + Dispatchers.Main) { + finishActivity() + persistState() + } } } } @@ -1179,9 +1202,9 @@ class ChatViewModel( // Per-run flag (set by executeToolCalls), not a session-wide scan. val lastToolFailed = lastToolFailedThisRun - if (AgentReplyRenderer.isDuplicateTurn(toolCalls, lastSucceededCalls, RESPOND_TOOL)) { + if (AgentReplyRenderer.isSilentTurn(toolCalls, RESPOND_TOOL)) { viewModelScope.launch(Dispatchers.Main) { - _messages.value = _messages.value.filter { it.id != agentMessageId } + removeMessageFromSession(agentMessageId) } deferred.complete(reply) return @@ -1195,10 +1218,7 @@ class ChatViewModel( 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)}" }})" - } + ) viewModelScope.launch(Dispatchers.Main) { if (isStale()) return@launch val finalMsg = ChatMessage( @@ -1223,7 +1243,7 @@ class ChatViewModel( } viewModelScope.launch(Dispatchers.Main) { // Drop the empty/partial bubble; the error surfaces as a SYSTEM message. - _messages.value = _messages.value.filter { it.id != agentMessageId } + removeMessageFromSession(agentMessageId) } deferred.completeExceptionally(RuntimeException(error)) } @@ -1265,7 +1285,7 @@ class ChatViewModel( // A synchronous throw fires no callback; complete deferred so await() doesn't hang. logError("generateStreaming threw synchronously", e) viewModelScope.launch(Dispatchers.Main) { - _messages.value = _messages.value.filter { it.id != agentMessageId } + removeMessageFromSession(agentMessageId) } if (!deferred.isCompleted) deferred.completeExceptionally(e) } @@ -1296,6 +1316,92 @@ class ChatViewModel( return if (prose.isEmpty()) envelopes else prose + "\n" + envelopes } + /** + * Puts [call] on the run's activity line, creating that row on the run's first tool and + * rewriting it in place from then on. One row per run is the whole point: the badge-per-call + * and result-per-call rows it replaces buried the answer under twenty messages. + * + * @param call the call that is starting. + */ + private fun showActivity(call: ToolCall) { + runToolNames += call.name + val subject = AgentActivity.subjectOf(call) + putActivity( + text = if (subject == null) { + str(R.string.agent_activity_running_plain, call.name) + } else { + str(R.string.agent_activity_running, call.name, subject) + }, + status = MessageStatus.SENT, + ) + } + + /** + * Closes the run's activity line: a run that used tools leaves a one-line summary of which + * ones, a run that used none leaves nothing at all. Idempotent, since every exit from a run — + * completion, Stop, error — passes through here. + */ + private fun finishActivity() { + val id = activityMessageId ?: return + val names = AgentActivity.distinctNames(runToolNames) + // Gone already means the chat was cleared or the session switched under the run. + val stillShown = _messages.value.any { it.id == id } + if (stillShown && names.isNotEmpty()) { + putActivity( + text = plural( + R.plurals.agent_activity_done, + runToolNames.size, + runToolNames.size, + names.joinToString(str(R.string.agent_activity_separator)), + ), + status = MessageStatus.COMPLETED, + ) + } else { + removeMessageFromSession(id) + } + activityMessageId = null + runToolNames.clear() + } + + /** + * Writes [text] to the activity row, appending it if this run has not shown one yet. Also + * appends when the row it held has gone — clearing the chat mid-run drops it, and a silently + * discarded update would leave the rest of the run with no progress at all. + * + * @param text the line to show. + * @param status the row's status; [MessageStatus.COMPLETED] closes it. + */ + private fun putActivity(text: String, status: MessageStatus) { + val id = activityMessageId + val existing = id != null && _messages.value.any { it.id == id } + val message = ChatMessage( + id = if (existing) id!! else UUID.randomUUID().toString(), + text = text, + sender = Sender.TOOL, + status = status, + // Any non-null value: a null one is what the adapter animates generating-dots on. + durationMs = 0L, + ) + activityMessageId = message.id + _messages.value = if (existing) { + _messages.value.map { if (it.id == message.id) message else it } + } else { + _messages.value + message + } + // Rewrites the row in place by id, as it does for every other message. + syncMessageToSession(message) + } + + /** + * Resolves a quantity string, empty when the plugin context has gone; see [str]. + * @param resId the plurals resource. + * @param quantity the count the wording is chosen by. + * @param args the format arguments. + * @return the formatted line. + */ + private fun plural(resId: Int, quantity: Int, vararg args: Any?): String = + getContext()?.androidContext?.resources?.getQuantityString(resId, quantity, *args).orEmpty() + /** * Appends an AGENT message to the chat (terminal state, no streaming dots). * @param text the message text. @@ -1350,18 +1456,6 @@ class ChatViewModel( private fun str(resId: Int, vararg args: Any?): String = getContext()?.androidContext?.getString(resId, *args).orEmpty() - /** - * Renders a tool argument for the one-line tool badge: line breaks flattened and the - * value capped, so a code-carrying argument stays a badge instead of a source dump. - * @param value the raw argument value. - * @return a single-line, length-capped rendering. - */ - private fun abbreviate(value: Any?): String { - val text = value?.toString().orEmpty().replace("\n", "⏎") - return if (text.length <= TOOL_BADGE_ARG_LIMIT) text - else text.take(TOOL_BADGE_ARG_LIMIT) + "…" - } - /** * Appends a SYSTEM message to the chat (on the main thread). * @param text the message text. diff --git a/ai-core/src/main/res/values/strings.xml b/ai-core/src/main/res/values/strings.xml index 25df3ae3..5a860103 100644 --- a/ai-core/src/main/res/values/strings.xml +++ b/ai-core/src/main/res/values/strings.xml @@ -146,6 +146,15 @@ 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… + + + \u2699 %1$s \u00b7 %2$s\u2026 + \u2699 %1$s\u2026 + + \u2713 %1$d action \u00b7 %2$s + \u2713 %1$d actions \u00b7 %2$s + + , System Log diff --git a/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentActivityTest.kt b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentActivityTest.kt new file mode 100644 index 00000000..78c06a37 --- /dev/null +++ b/ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/viewmodel/AgentActivityTest.kt @@ -0,0 +1,86 @@ +package com.itsaky.androidide.plugins.aicore.viewmodel + +import com.itsaky.androidide.plugins.aicore.tool.ToolCall +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * Unit tests for [AgentActivity]. QA's complaint these answer: a run's tool calls arrived as a + * wall of wrapped badges carrying whole file paths and whole `edit_file` snippets. Every line the + * activity row shows has to fit on one line of a phone. + */ +class AgentActivityTest { + + @Test + fun givenAFilePath_whenNamed_thenOnlyTheFileNameIsShown() { + val call = ToolCall( + "edit_file", + mapOf("file_path" to "app/src/main/java/com/example/myapplication25/MainActivity.java"), + ) + + assertEquals("MainActivity.java", AgentActivity.subjectOf(call)) + } + + @Test + fun givenAPathArgumentUnderTheAlias_whenNamed_thenItIsStillFound() { + assertEquals("A.kt", AgentActivity.subjectOf(ToolCall("read_file", mapOf("path" to "src/A.kt")))) + } + + @Test + fun givenADirectoryWithATrailingSlash_whenNamed_thenTheLastSegmentIsShown() { + val call = ToolCall("list_files", mapOf("directory" to "app/src/main/java/")) + + assertEquals("java", AgentActivity.subjectOf(call)) + } + + @Test + fun givenBothAPathAndContent_whenNamed_thenThePathWinsOverTheSnippet() { + // The bug in the badge this replaces: `new_string` put a whole class on the line. + val call = ToolCall( + "edit_file", + mapOf("file_path" to "A.kt", "old_string" to "class A", "new_string" to "class A {\n}"), + ) + + assertEquals("A.kt", AgentActivity.subjectOf(call)) + } + + @Test + fun givenAMultiLineQuery_whenNamed_thenItIsFlattenedAndCapped() { + val call = ToolCall("search_project", mapOf("query" to "a".repeat(200) + "\nb")) + + val subject = AgentActivity.subjectOf(call)!! + + assertEquals(AgentActivity.SUBJECT_LIMIT + 1, subject.length) + assertEquals("a".repeat(AgentActivity.SUBJECT_LIMIT) + "…", subject) + } + + @Test + fun givenACallWithNoRecognisableArgument_whenNamed_thenNothingIsShown() { + assertNull(AgentActivity.subjectOf(ToolCall("gradle_sync", emptyMap()))) + } + + @Test + fun givenTheProjectRootAsTheDirectory_whenNamed_thenNothingIsShown() { + // `list_files(directory=".")` is the commonest call of the run; "· ." is not information. + assertNull(AgentActivity.subjectOf(ToolCall("list_files", mapOf("directory" to ".")))) + assertNull(AgentActivity.subjectOf(ToolCall("list_files", mapOf("directory" to "./")))) + } + + @Test + fun givenABlankArgument_whenNamed_thenNothingIsShown() { + assertNull(AgentActivity.subjectOf(ToolCall("list_files", mapOf("directory" to " ")))) + } + + @Test + fun givenARunThatReadSixFiles_whenSummarised_thenTheToolIsNamedOnce() { + val names = listOf("read_file", "read_file", "edit_file", "read_file", "") + + assertEquals(listOf("read_file", "edit_file"), AgentActivity.distinctNames(names)) + } + + @Test + fun givenNoToolsAtAll_whenSummarised_thenThereIsNothingToName() { + assertEquals(emptyList(), AgentActivity.distinctNames(listOf("", " "))) + } +} 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 442acce8..85fdb15e 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 @@ -39,7 +39,7 @@ class AgentReplyRendererTest { ToolCallExtractor.UnparsedReply.MALFORMED -> MALFORMED } }, - ) { call -> "🔧 ${call.name}" } + ) private fun respond(vararg args: Pair) = listOf(ToolCall(TERMINAL, mapOf(*args))) @@ -116,16 +116,6 @@ class AgentReplyRendererTest { assertEquals(FAILED, text) } - @Test - fun givenRealToolCalls_whenRendered_thenBadgesAreShown() { - val text = render( - "x", - listOf(ToolCall("read_file", mapOf("file_path" to "A.kt")), ToolCall("edit_file", emptyMap())), - ) - - assertEquals("🔧 read_file\n🔧 edit_file", text) - } - @Test fun givenPlainProseAndNoToolCalls_whenRendered_thenTheProseIsShown() { assertEquals("Hi! What would you like to build?", render("Hi! What would you like to build?")) @@ -137,48 +127,38 @@ class AgentReplyRendererTest { } @Test - fun givenARepeatedCallBesideACapitalisedRespond_whenChecked_thenTheAnswerIsKept() { - // The turn repeats a call that already succeeded, but it also carries the answer, and this - // bubble is the only place that text is ever rendered: dropping it ends the run - // "completed" with nothing on screen. `Respond` counts as terminal by the loose match. - val succeeded = listOf(ToolCall("read_file", mapOf("file_path" to "A.kt"))) - val repeated = succeeded + ToolCall("Respond", mapOf("message" to "Here is the answer")) - - assertFalse(AgentReplyRenderer.isDuplicateTurn(repeated, succeeded, TERMINAL)) - } - - @Test - fun givenOnlyRepeatedRealCalls_whenChecked_thenTheTurnIsADuplicate() { - // Nothing terminal, nothing new: the bubble would say what the last one already did. - val succeeded = listOf(ToolCall("read_file", mapOf("file_path" to "A.kt"))) + fun givenACallBesideACapitalisedRespond_whenChecked_thenTheTurnIsNotSilent() { + // The turn carries the answer, and this bubble is the only place that text is ever + // rendered: dropping it ends the run "completed" with nothing on screen. `Respond` counts + // as terminal by the loose match. + val calls = listOf( + ToolCall("read_file", mapOf("file_path" to "A.kt")), + ToolCall("Respond", mapOf("message" to "Here is the answer")), + ) - assertTrue(AgentReplyRenderer.isDuplicateTurn(succeeded.toList(), succeeded, TERMINAL)) + assertFalse(AgentReplyRenderer.isSilentTurn(calls, TERMINAL)) } @Test - fun givenADifferentCall_whenChecked_thenTheTurnIsNotADuplicate() { - val succeeded = listOf(ToolCall("read_file", mapOf("file_path" to "A.kt"))) - val next = listOf(ToolCall("read_file", mapOf("file_path" to "B.kt"))) + fun givenOnlyRealCalls_whenChecked_thenTheTurnIsSilent() { + // Work in progress: the activity line reports it, the transcript does not. + val calls = listOf( + ToolCall("read_file", mapOf("file_path" to "A.kt")), + ToolCall("edit_file", mapOf("file_path" to "A.kt")), + ) - assertFalse(AgentReplyRenderer.isDuplicateTurn(next, succeeded, TERMINAL)) + assertTrue(AgentReplyRenderer.isSilentTurn(calls, TERMINAL)) } @Test - fun givenOnlyATerminalCall_whenChecked_thenTheAnswerIsNeverDroppedAsADuplicate() { - // The answer-carrying turn has no real calls left after the filter, so it must fall through - // even when the run's last real call is what it is reporting on. - val succeeded = listOf(ToolCall("read_file", mapOf("file_path" to "A.kt"))) - - assertFalse( - AgentReplyRenderer.isDuplicateTurn(respond("message" to "Done."), succeeded, TERMINAL) - ) + fun givenOnlyATerminalCall_whenChecked_thenTheAnswerIsNeverSilenced() { + assertFalse(AgentReplyRenderer.isSilentTurn(respond("message" to "Done."), TERMINAL)) } @Test - fun givenNothingSucceededYet_whenChecked_thenTheTurnIsNotADuplicate() { - val calls = listOf(ToolCall("read_file", mapOf("file_path" to "A.kt"))) - - assertFalse(AgentReplyRenderer.isDuplicateTurn(calls, null, TERMINAL)) + fun givenNoToolCallsAtAll_whenChecked_thenTheProseTurnIsNotSilent() { + // A plain answer parses to no calls; silencing it would drop the reply entirely. + assertFalse(AgentReplyRenderer.isSilentTurn(emptyList(), TERMINAL)) } @Test