From 70ad7eccc88f3b95ad9c0719c695e7f3e1aa013b Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Tue, 20 Oct 2026 15:13:11 -0700 Subject: [PATCH] fix(coach): retry a refused Gemini grounding turn without web search (#123) Free-tier Gemini keys can be refused google_search grounding with HTTP 429 RESOURCE_EXHAUSTED while plain, function and schema calls on the same key succeed, so every coach question failed with Web Search on. When the grounding turn gets a 429, GeminiClient re-sends it once without google_search, as the schema turn it would otherwise have been, with a note asking the model to say in data_quality_note that web search was skipped. A 429 on any other turn, or on the retry itself, still surfaces as before. Once the ungrounded retry succeeds, the key is remembered for the process so later questions skip grounding without the extra round trip. Settings shows a hint under the Web Search toggle for the Gemini provider. --- .../pulseloop/coach/gemini/GeminiClient.kt | 89 ++++++++++-- .../ui/screens/SettingsSubScreens.kt | 5 + .../com/pulseloop/coach/GeminiClientTest.kt | 130 ++++++++++++++++++ 3 files changed, 216 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt b/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt index 255ad767..8d9328ce 100644 --- a/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt +++ b/app/src/main/java/com/pulseloop/coach/gemini/GeminiClient.kt @@ -31,6 +31,8 @@ class GeminiClient( private val apiKey: String, model: String = GeminiModel.DEFAULT.slug, private val baseURL: String = "https://generativelanguage.googleapis.com/v1beta/models", + // Network seam: a test can answer or fail requests without an HTTP stack. + private val transport: suspend (String, ByteArray) -> String = { u, b -> ResponsesHttp.post(u, b) }, ) : ResponsesClient { // The model the orchestrator selected, taken from each request body's `model` // field (see `send`); the constructor value is only a fallback if that's absent. @@ -54,6 +56,10 @@ class GeminiClient( // Sources cited by a google_search turn, queued to prepend to the next user // message so the schema turn can fill the response `sources` array. private var pendingSourcesNote: String? = null + // Inputs of the last built body, so a refused grounding turn can be rebuilt + // as the plain schema turn it would otherwise have been (issue #123). + private var lastTextFormat: JsonObject? = null + private var lastThinkingLevel: String? = null override suspend fun send(requestBody: ByteArray): OpenAIResponse { if (apiKey.isBlank()) throw ResponsesError.MissingAPIKey @@ -65,9 +71,23 @@ class GeminiClient( } val geminiBody = buildRequestBody(req) - val bodyBytes = json.encodeToString(JsonObject.serializer(), geminiBody).toByteArray() - - val body = postWithOverloadRetry("$baseURL/$model:generateContent?key=$apiKey", bodyBytes) + val body = try { + post(geminiBody) + } catch (e: ResponsesError.Http) { + if (e.status != 429 || !isGroundingBody(geminiBody)) throw e + // A free-tier key can be refused `google_search` grounding outright (429 + // RESOURCE_EXHAUSTED) while every other call on the same key succeeds (issue + // #123). Re-send this turn once without it — as the schema turn it would + // otherwise have been — so the answer still arrives. The rebuilt body carries + // no google_search, so this cannot loop; a 429 on it is a real quota refusal + // and surfaces as before. + val ungrounded = post(ungroundedRetryBody()) + // Only now is it proven that grounding, not the key, was refused: stop asking + // for it on this key for the rest of the process so later questions don't pay + // the extra round trip. An app restart (e.g. after upgrading the plan) re-tries. + groundingRefusedKeys.add(apiKey) + ungrounded + } val root = try { json.parseToJsonElement(body).jsonObject @@ -95,7 +115,7 @@ class GeminiClient( url: String, bodyBytes: ByteArray, backoffMs: Long = OVERLOAD_BACKOFF_MS, - post: suspend (String, ByteArray) -> String = { u, b -> ResponsesHttp.post(u, b) }, + post: suspend (String, ByteArray) -> String = transport, ): String { for (attempt in 0..MAX_OVERLOAD_RETRIES) { try { @@ -108,9 +128,51 @@ class GeminiClient( throw IllegalStateException("overload retry loop never returned") } + private suspend fun post(geminiBody: JsonObject): String { + val bodyBytes = json.encodeToString(JsonObject.serializer(), geminiBody).toByteArray() + return postWithOverloadRetry("$baseURL/$model:generateContent?key=$apiKey", bodyBytes) + } + + private fun isGroundingBody(body: JsonObject): Boolean = + (body["tools"] as? JsonArray)?.any { (it as? JsonObject)?.containsKey("google_search") == true } == true + + /** The refused grounding turn rebuilt without `google_search` (issue #123): same + * conversation, the response schema applied, and a note so the answer says web + * search was skipped. `webSearchGrounded` is already set, so it never re-grounds. */ + private fun ungroundedRetryBody(): JsonObject { + noteWebSearchSkipped() + return buildGeminiBody(emptyList(), lastTextFormat, lastThinkingLevel) + } + + /** Tells the model, on the current user turn, that web search didn't run — so it + * leaves `sources` empty and says so in `data_quality_note`, which the chat shows + * under the answer. Prompt-level on purpose: no UI plumbing (issue #123). */ + private fun noteWebSearchSkipped() { + val note = JsonObject(mapOf("text" to JsonPrimitive(WEB_SEARCH_SKIPPED_NOTE))) + val last = contents.lastOrNull() + if (last != null && (last["role"] as? JsonPrimitive)?.contentOrNull == "user") { + val parts = (last["parts"] as? JsonArray).orEmpty() + note + contents[contents.lastIndex] = JsonObject(last + ("parts" to JsonArray(parts))) + } else { + contents.add(JsonObject(mapOf( + "role" to JsonPrimitive("user"), + "parts" to JsonArray(listOf(note)), + ))) + } + } + private companion object { const val MAX_OVERLOAD_RETRIES = 2 const val OVERLOAD_BACKOFF_MS = 2_000L + + const val WEB_SEARCH_SKIPPED_NOTE = "Web search was not available for this answer " + + "(Google refused Search grounding for this API key; free-tier keys often can't use it). " + + "Answer from the health data alone, leave `sources` empty, and say briefly in " + + "`data_quality_note` that web search was skipped." + + // Keys whose grounding turn was refused with a 429 while the same turn succeeded + // without it (issue #123). Process-wide because each agent turn gets a fresh client. + val groundingRefusedKeys: MutableSet = java.util.concurrent.ConcurrentHashMap.newKeySet() } // ── Request assembly (internal for unit tests) ─────────────────────── @@ -137,6 +199,8 @@ class GeminiClient( appendContinuation(previousResponseId, input) } + lastTextFormat = textFormat + lastThinkingLevel = thinkingLevel return buildGeminiBody(convertTools(tools), textFormat, thinkingLevel) } @@ -362,12 +426,21 @@ class GeminiClient( // and skip the schema. The grounded prose + cited sources land in history; // the orchestrator's subsequent tool-less repair turn then enforces the // coach_response schema (and can fill the `sources` array from them). - val groundingTurn = tools.isEmpty() && webSearchRequested && !webSearchGrounded + // + // A key already known to be refused grounding (issue #123) skips it here and gets + // the plain schema turn with a "web search skipped" note instead. + var groundingTurn = tools.isEmpty() && webSearchRequested && !webSearchGrounded if (groundingTurn) { - body["tools"] = JsonArray(listOf(JsonObject(mapOf( - "google_search" to JsonObject(emptyMap()), - )))) webSearchGrounded = true + if (apiKey in groundingRefusedKeys) { + groundingTurn = false + noteWebSearchSkipped() + body["contents"] = JsonArray(contents) + } else { + body["tools"] = JsonArray(listOf(JsonObject(mapOf( + "google_search" to JsonObject(emptyMap()), + )))) + } } // Gemini rejects function declarations combined with a JSON response diff --git a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt index a710a4c0..cfc1f03b 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt @@ -763,6 +763,11 @@ fun CoachSettingsScreen(onBack: () -> Unit) { Column(Modifier.weight(1f)) { Text("Web Search") Text("Uses additional tokens", style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant) + // Free-tier Gemini keys can be refused Google Search grounding (HTTP 429) + // while every other call works; the coach then answers without it (issue #123). + if (providerMode == CoachProviderMode.USER_GEMINI_KEY) { + Text("Google Search grounding may need a paid Gemini plan — free-tier keys can be refused, and the coach then answers without web results.", style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant) + } } Switch(checked = webSearch, onCheckedChange = { webSearch = it; keyStore.webSearchEnabled = it diff --git a/app/src/test/java/com/pulseloop/coach/GeminiClientTest.kt b/app/src/test/java/com/pulseloop/coach/GeminiClientTest.kt index 388a2e89..446e53e5 100644 --- a/app/src/test/java/com/pulseloop/coach/GeminiClientTest.kt +++ b/app/src/test/java/com/pulseloop/coach/GeminiClientTest.kt @@ -408,4 +408,134 @@ class GeminiClientTest { } assertEquals(1, attempts) } + + // ── Refused grounding turn (issue #123) ──────────────────────────── + + private val schemaFormat = JsonObject(mapOf( + "type" to JsonPrimitive("json_schema"), + "schema" to JsonObject(mapOf("type" to JsonPrimitive("object"))), + )) + + private fun bytes(req: JsonObject) = req.toString().toByteArray() + + private fun isGrounding(body: JsonObject) = + body["tools"]?.jsonArray?.any { it.jsonObject.containsKey("google_search") } == true + + /** Tool turn (web search requested) answered with prose; returns its response id. */ + private suspend fun toolTurn(client: GeminiClient): String = client.send(bytes(request( + listOf(msg("user", "hi")), + tools = listOf(webSearchTool, functionTool("t", JsonObject(mapOf("type" to JsonPrimitive("object"))))), + textFormat = schemaFormat, + ))).id + + private suspend fun repairTurn(client: GeminiClient, previousId: String) = client.send(bytes(request( + listOf(msg("user", "format it")), textFormat = schemaFormat, previousResponseId = previousId, + ))) + + private fun lastUserText(body: JsonObject) = body["contents"]!!.jsonArray.last().jsonObject["parts"]!! + .jsonArray.joinToString(" ") { it.jsonObject["text"]?.jsonPrimitive?.content ?: "" } + + @Test + fun `a 429 on the grounding turn is retried once without google_search`() = kotlinx.coroutines.runBlocking { + // Free-tier keys get 429 RESOURCE_EXHAUSTED for google_search alone while plain, function + // and schema calls succeed — every coach question failed with Web Search on. + val sent = mutableListOf() + val client = GeminiClient("key-123-retry", transport = { _, b -> + val body = Json.parseToJsonElement(String(b)).jsonObject + sent.add(body) + if (isGrounding(body)) throw ResponsesError.Http(429, "You exceeded your current quota") + textResponse(if (sent.size == 1) "prose" else """{"title":"ok"}""").toString() + }) + + val answer = repairTurn(client, toolTurn(client)) + + assertEquals("""{"title":"ok"}""", answer.outputText) + assertEquals(3, sent.size) + assertTrue(isGrounding(sent[1])) + val retry = sent[2] + assertFalse(isGrounding(retry)) + assertNull(retry["tools"]) + // The retry is the schema turn it would otherwise have been, and says search was skipped. + assertNotNull(retry["generationConfig"]!!.jsonObject["responseSchema"]) + assertTrue(lastUserText(retry).contains("Web search was not available")) + } + + @Test + fun `a refused key skips grounding on later turns without the extra round trip`() = kotlinx.coroutines.runBlocking { + val key = "key-123-remembered" + var refusals = 0 + val transport: suspend (String, ByteArray) -> String = { _, b -> + val body = Json.parseToJsonElement(String(b)).jsonObject + if (isGrounding(body)) { refusals++; throw ResponsesError.Http(429, "quota") } + textResponse("prose").toString() + } + val first = GeminiClient(key, transport = transport) + repairTurn(first, toolTurn(first)) + assertEquals(1, refusals) + + // Next agent turn: a fresh client (as the orchestrator builds one per turn). + val sent = mutableListOf() + val second = GeminiClient(key, transport = { u, b -> + sent.add(Json.parseToJsonElement(String(b)).jsonObject) + transport(u, b) + }) + repairTurn(second, toolTurn(second)) + assertEquals(1, refusals) + assertEquals(2, sent.size) + assertFalse(isGrounding(sent[1])) + assertNotNull(sent[1]["generationConfig"]!!.jsonObject["responseSchema"]) + assertTrue(lastUserText(sent[1]).contains("Web search was not available")) + + // A different key is unaffected. + val other = GeminiClient("key-123-other", transport = { _, b -> + sent.add(Json.parseToJsonElement(String(b)).jsonObject) + textResponse("prose").toString() + }) + repairTurn(other, toolTurn(other)) + assertTrue(isGrounding(sent.last())) + } + + @Test + fun `a 429 on a non-grounding turn still surfaces`() = kotlinx.coroutines.runBlocking { + var attempts = 0 + val client = GeminiClient("key-123-plain", transport = { _, _ -> + attempts++ + throw ResponsesError.Http(429, "You exceeded your current quota") + }) + try { + toolTurn(client) + fail("a quota refusal on a normal turn must surface") + } catch (e: ResponsesError.Http) { + assertEquals(429, e.status) + } + assertEquals(1, attempts) + } + + @Test + fun `a 429 on the ungrounded retry surfaces and is not remembered`() = kotlinx.coroutines.runBlocking { + // The key itself is out of quota: the retry fails too, and grounding is not blamed. + val key = "key-123-exhausted" + var calls = 0 + val client = GeminiClient(key, transport = { _, _ -> + if (calls++ == 0) textResponse("prose").toString() + else throw ResponsesError.Http(429, "You exceeded your current quota") + }) + val id = toolTurn(client) + try { + repairTurn(client, id) + fail("a 429 on the retry must surface, not loop") + } catch (e: ResponsesError.Http) { + assertEquals(429, e.status) + } + assertEquals(3, calls) // tool turn, grounding turn, one retry + + // Not marked as grounding-refused: a later turn on the same key still grounds. + val sent = mutableListOf() + val next = GeminiClient(key, transport = { _, b -> + sent.add(Json.parseToJsonElement(String(b)).jsonObject) + textResponse("prose").toString() + }) + repairTurn(next, toolTurn(next)) + assertTrue(isGrounding(sent.last())) + } }