fix(session): count every real output block as a served turn

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.
This commit is contained in:
enieuwy
2026-08-08 13:14:35 +08:00
parent 837b1ac0e2
commit ae5965241e
2 changed files with 41 additions and 9 deletions
+22 -8
View File
@@ -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
@@ -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: