Merge PR #8512: fix(agent): name billed output tokens on capped empty stops (@roboomp)

2026-08-16 02:43:34 +02:00
parent e224493fe2 b6aa92b143
commit 14813330f5
3 changed files with 82 additions and 4 deletions
@@ -100,6 +100,9 @@
 - Fixed Streamable HTTP MCP sessions being invalidated by opening the optional GET SSE stream before sending `notifications/initialized`, which prevented Figma Dev Mode MCP from connecting ([#8514](https://github.com/can1357/oh-my-pi/issues/8514)).
 - Fixed the `/hotkeys` table describing Ctrl+D (`app.exit`) as "Exit (when editor is empty)" when it actually exits unconditionally and saves the current prompt as a resumable draft ([#8530](https://github.com/can1357/oh-my-pi/issues/8530)).
 - Fixed Ctrl+G external editors failing to launch on Windows because Bun re-quoted the embedded `cmd.exe /c` command line ([#8544](https://github.com/can1357/oh-my-pi/issues/8544)).
+### Fixed
+
+- Fixed the capped empty-stop failure always naming the context/`/shake images` hint even when the provider billed output tokens. A zero-block `stop` (no content blocks) with `usage.output > 0` means content was generated and dropped downstream (a filter/refusal flattened to `finish_reason: "stop"` by a proxy, or a lossy API translation), so the message now reports the billed output-token count and points at a provider-side filter/translation instead of a context problem, and logs `outputTokens` alongside the existing warning fields. Thinking-only stops keep a thinking block (and bill output for it), so they retain the context hint ([#8511](https://github.com/can1357/oh-my-pi/issues/8511)).

 ## [17.3.3] - 2026-08-14

@@ -677,15 +677,29 @@ export class TurnRecovery {
 		this.#emptyStopRetryCount++;
 		if (this.#emptyStopRetryCount > EMPTY_STOP_MAX_RETRIES) {
 			const attempts = this.#emptyStopRetryCount - 1;
-			const finalError = providerEmptyOutput
-				? "Assistant returned no final output after retry cap; try switching models"
-				: "Assistant returned empty stop after retry cap; try switching models or `/shake images` to remove archived frames";
+			const outputTokens = assistantMessage.usage.output;
+			let finalError: string;
+			if (providerEmptyOutput) {
+				finalError = "Assistant returned no final output after retry cap; try switching models";
+			} else if (outputTokens > 0 && assistantMessage.content.length === 0) {
+				// Billed output on a truly zero-block stop means content was generated and
+				// then dropped downstream (a filter/refusal flattened to
+				// `finish_reason: "stop"` by a proxy, or a lossy API translation) — the
+				// context/`/shake images` hint is wrong here, so name the billed output
+				// instead. Thinking-only stops keep a thinking block (and bill output for
+				// it), so they fall through to the context hint rather than this path.
+				finalError = `Assistant returned an empty stop after retry cap, but the provider billed ${outputTokens} output token${outputTokens === 1 ? "" : "s"} for it; content was generated and then dropped before delivery, which usually points to a provider-side content filter or a lossy API translation rather than a context problem`;
+			} else {
+				finalError =
+					"Assistant returned empty stop after retry cap; try switching models or `/shake images` to remove archived frames";
+			}
 			assistantMessage.errorMessage = finalError;
 			if (providerEmptyOutput) assistantMessage.errorId = AIError.create();
 			logger.warn(finalError, {
 				attempts,
 				model: assistantMessage.model,
 				provider: assistantMessage.provider,
+				outputTokens,
 			});
 			await this.#host.emitSessionEvent({
 				type: "auto_retry_end",
@@ -57,7 +57,18 @@ function emptyStop(): MockResponse {
 	return {
 		content: [],
 		stopReason: "stop",
-		usage: { output: 1, cacheRead: 100 },
+		usage: { output: 0, cacheRead: 100 },
+	};
+}
+
+// A zero-block `stop` for which the provider still billed output tokens: content
+// was generated and dropped downstream (e.g. a filter/refusal flattened to
+// `finish_reason: "stop"` by a proxy), so the context/`/shake images` hint is wrong.
+function filteredEmptyStop(): MockResponse {
+	return {
+		content: [],
+		stopReason: "stop",
+		usage: { output: 126, cacheRead: 100 },
 	};
 }

@@ -463,6 +474,56 @@ describe("AgentSession empty stop guard", () => {
 		expect(retryEndEvents[0]?.finalError).toContain("/shake images");
 	});

+	it("names billed output tokens instead of the context hint when a capped empty stop billed output", async () => {
+		const { session, mock } = await createHarness([
+			filteredEmptyStop(),
+			filteredEmptyStop(),
+			filteredEmptyStop(),
+			filteredEmptyStop(),
+		]);
+		const retryEndEvents: Array<Extract<AgentSessionEvent, { type: "auto_retry_end" }>> = [];
+		session.subscribe(event => {
+			if (event.type === "auto_retry_end") {
+				retryEndEvents.push(event);
+			}
+		});
+
+		await expectPromptCompletes(session.prompt("answer that gets filtered"));
+		await session.waitForIdle();
+
+		expect(mock.calls).toHaveLength(4);
+		expect(retryEndEvents).toHaveLength(1);
+		expect(retryEndEvents[0]?.success).toBe(false);
+		const finalError = retryEndEvents[0]?.finalError ?? "";
+		expect(finalError).toContain("billed 126 output tokens");
+		expect(finalError).not.toContain("/shake images");
+	});
+
+	it("keeps the context hint for a capped thinking-only stop even though it billed output", async () => {
+		const { session, mock } = await createHarness([
+			thinkingOnlyStop(),
+			thinkingOnlyStop(),
+			thinkingOnlyStop(),
+			thinkingOnlyStop(),
+		]);
+		const retryEndEvents: Array<Extract<AgentSessionEvent, { type: "auto_retry_end" }>> = [];
+		session.subscribe(event => {
+			if (event.type === "auto_retry_end") {
+				retryEndEvents.push(event);
+			}
+		});
+
+		await expectPromptCompletes(session.prompt("think without answering"));
+		await session.waitForIdle();
+
+		expect(mock.calls).toHaveLength(4);
+		expect(retryEndEvents).toHaveLength(1);
+		expect(retryEndEvents[0]?.success).toBe(false);
+		const finalError = retryEndEvents[0]?.finalError ?? "";
+		expect(finalError).toContain("/shake images");
+		expect(finalError).not.toContain("billed");
+	});
+
 	it("ends auto-retry state when empty stop retries hit the cap", async () => {
 		vi.spyOn(scheduler, "wait").mockResolvedValue(undefined);
 		const { session, mock } = await createHarness(