From ae5965241eec67188a2bba8cc12539709e796ef0 Mon Sep 17 00:00:00 2001 From: enieuwy <121954036+enieuwy@users.noreply.github.com> Date: Sat, 8 Aug 2026 13:14:35 +0800 Subject: [PATCH] fix(session): count every real output block as a served turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex found the shared attribution predicate recognising only tool calls, text and signed thinking. A native image response often arrives with no text and no tool call at all, so an image-only turn was read as producing nothing: attribution stayed on whichever model spoke before it, and the empty-stop rule could classify a successful generation as empty. Everything the assistant can emit now counts except two: unsigned thinking, which is not provider-authenticated and was already excluded, and Anthropic's `fallback` marker, which records that a request was routed elsewhere rather than carrying output. Redacted thinking and server-tool blocks are real work by the same argument as the image. The `toolUse` arm keeps its stricter rule — an orphaned toolUse stop needs a tool_use block to anchor a later tool_result, and an image cannot. Also switches the new test to the namespace import AGENTS.md requires for node builtins. --- packages/coding-agent/src/session/messages.ts | 30 ++++++++++++++----- .../persisted-agent-attribution.test.ts | 20 ++++++++++++- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/packages/coding-agent/src/session/messages.ts b/packages/coding-agent/src/session/messages.ts index ce1297b26..ae5eba84b 100644 --- a/packages/coding-agent/src/session/messages.ts +++ b/packages/coding-agent/src/session/messages.ts @@ -504,15 +504,29 @@ function hasText(content: { text?: unknown }): boolean { return typeof content.text === "string" && content.text.trim().length > 0; } -/** A block a consumer can act on: a call, real text, or provider-signed thinking. */ +/** + * A block that is real output from the model. + * + * Everything the assistant can emit counts except two: unsigned thinking, which + * is not provider-authenticated and not actionable, and Anthropic's `fallback` + * marker, which records that the request was routed elsewhere rather than + * carrying any output. A native image response often arrives with no text and + * no tool call at all, so recognising only those would call it nothing. + */ function isActionableContent(content: AssistantMessage["content"][number] | undefined): boolean { - if (!content) return false; - if (content.type === "toolCall") return true; - if (content.type === "text") return hasText(content); - // Unsigned thinking is not actionable; a signature is provider-authenticated. - return content.type === "thinking" && typeof content.thinkingSignature === "string" - ? content.thinkingSignature.trim().length > 0 - : false; + switch (content?.type) { + case "toolCall": + case "image": + case "redactedThinking": + case "anthropicServerTool": + return true; + case "text": + return hasText(content); + case "thinking": + return typeof content.thinkingSignature === "string" && content.thinkingSignature.trim().length > 0; + default: + return false; + } } /** A `stop`/`toolUse` turn that produced nothing actionable. Any other stop diff --git a/packages/coding-agent/test/registry/persisted-agent-attribution.test.ts b/packages/coding-agent/test/registry/persisted-agent-attribution.test.ts index da839c104..5a41ae004 100644 --- a/packages/coding-agent/test/registry/persisted-agent-attribution.test.ts +++ b/packages/coding-agent/test/registry/persisted-agent-attribution.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import path from "node:path"; +import * as path from "node:path"; import { AgentRegistry } from "@oh-my-pi/pi-coding-agent/registry/agent-registry"; import { registerPersistedSubagents } from "@oh-my-pi/pi-coding-agent/registry/persisted-agents"; import { TempDir } from "@oh-my-pi/pi-utils"; @@ -126,6 +126,24 @@ describe("persisted agent model attribution", () => { expect(history?.resolvedModelIsFallback).toBe(false); }); + it("credits a turn whose only output is an image", async () => { + using tempDir = TempDir.createSync("@omp-attribution-image-"); + // A native image response can arrive with no text and no tool call at all. + // Recognising only those would call it nothing and leave the run credited + // to whichever model spoke before it. + const registry = await historyFor(tempDir.path(), "Painter", [ + ...transcriptHead(), + assistant("a1", "si", SONNET, "stop", [{ type: "text", text: "sonnet did the work" }]), + assistant("e1", "a1", SONNET, "error", []), + modelChange("m2", "e1", "openai-codex/gpt-5.6-sol", "fallback", true), + assistant("a2", "m2", SOL, "stop", [{ type: "image", data: "aGk=", mimeType: "image/png" }]), + ]); + + const history = registry.get("Painter")?.history; + expect(history?.resolvedModel).toBe("openai-codex/gpt-5.6-sol"); + expect(history?.resolvedModelIsFallback).toBe(true); + }); + it("treats a budget-exhausted length stop with nothing usable as unserved", async () => { using tempDir = TempDir.createSync("@omp-attribution-length-"); // `length` is not an "empty stop", so the empty-stop rule never inspects it: