fix(ollama-native): keep tool batches open across assistant commentary

docs-site/src/content/docs/reference/adapters.md

@@ -118,6 +118,10 @@ be configured on a separately named custom or self-hosted Ollama provider with
   is refused rather than mis-sent, and remote image URLs are not fetched.
 - **Tools:** declared in Ollama's native shape, streamed tool calls are whole-call records with
   object-valued `arguments`, and tool-result replay is paired strictly by call id and tool name.
+  Codex may record assistant commentary before a pending call's results. Text/thinking with no
+  new tool calls is deferred until the batch is settled, so genuine results remain beside their
+  originating calls. A new tool-call batch still settles the preceding one; missing results retain
+  an explicit unknown-status marker, and orphan or duplicate results remain invalid.
   `tool_choice: "none"` and `auto` behave normally; **`required` or an exact named choice fails
   closed**, because Ollama's `/api/chat` has no `tool_choice` field to enforce it with.
 - **Structured output is refused on canonical Ollama Cloud.** Ollama currently documents structured

src/adapters/ollama-native.ts

@@ -77,6 +77,7 @@ interface PendingToolCall {
 interface PendingToolBatch {
   calls: PendingToolCall[];
   byId: Map<string, PendingToolCall>;
+  unresolvedCount: number;
 }
 
 interface NativeStreamToolCall {
@@ -302,6 +303,12 @@ function assistantTextThinkingAndCalls(message: OcxAssistantMessage): {
   };
 }
 
+/**
+ * Build native replay messages without mutating the parsed history. Keep tool results
+ * beside their originating batch, deferring intervening conversation until settlement.
+ * Reserve replayed call IDs for response translation and mark missing results explicitly.
+ * @throws When call IDs are invalid or results are orphaned, duplicated, or mismatched.
+ */
 function buildNativeMessages(
   parsed: OcxParsedRequest,
   reservedToolCallIds: Set<string>,
@@ -323,12 +330,14 @@ function buildNativeMessages(
   // early. The openai-chat adapter defers them the same way; refusing the replay killed the turn.
   let deferred: OllamaNativeMessage[] = [];
 
+  /** Emit held conversation messages in their recorded arrival order after settlement. */
   const releaseDeferred = (): void => {
     if (deferred.length === 0) return;
     messages.push(...deferred);
     deferred = [];
   };
 
+  /** Settle the open batch with genuine results or unknown markers, then release deferred messages. */
   const flushPending = (): void => {
     if (!pending) return;
     for (const call of pending.calls) {
@@ -376,13 +385,14 @@ function buildNativeMessages(
         throw new Error(`ollama-native tool result ${message.toolCallId} names the wrong originating tool`);
       }
       call.result = message;
+      pending.unresolvedCount--;
       continue;
     }
 
     // Native Ollama requires the whole assistant tool-call turn followed by its tool results. A
     // conversational message that arrives while the batch is still open is held aside instead of
     // closing it, so the call keeps its results adjacent; it is released right after the batch
-    // flushes. Anything else (a new assistant turn) settles the batch first.
+    // flushes. A new assistant tool-call batch settles the preceding batch first.
     if (pending) {
       if (message.role === "user" || message.role === "developer") {
         const translated = message.role === "user"
@@ -393,6 +403,19 @@ function buildNativeMessages(
           : { role: "system", content: translated.content });
         continue;
       }
+      if (message.role === "assistant" && pending.unresolvedCount > 0) {
+        const extracted = assistantTextThinkingAndCalls(message);
+        if (extracted.calls.length === 0) {
+          // Commentary can follow the call items but precede their recorded results.
+          // Keep the batch open and release its text/thinking after the actual results.
+          deferred.push({
+            role: "assistant",
+            content: extracted.content,
+            ...(extracted.thinking ? { thinking: extracted.thinking } : {}),
+          });
+          continue;
+        }
+      }
       flushPending();
     }
 
@@ -443,7 +466,11 @@ function buildNativeMessages(
         };
         messages.push(native);
         if (wireCalls.length > 0) {
-          pending = { calls: wireCalls, byId: new Map(wireCalls.map(call => [call.id, call])) };
+          pending = {
+            calls: wireCalls,
+            byId: new Map(wireCalls.map(call => [call.id, call])),
+            unresolvedCount: wireCalls.length,
+          };
         }
         break;
       }

structure/providers/chat-compat.md

@@ -122,13 +122,13 @@ and synthesizing explicit "no tool result was recorded" answers only when no rea
 (Kimi/Moonshot 400 `ocx-mrqaiw05-269`; unit `devlog/_fin/260718_dangling_toolcall_hardening`).
 
 The native Ollama wire carries the same contract. `src/adapters/ollama-native.ts`
-`buildNativeMessages` defers `user`/`developer` messages that arrive while a batch is open and
-releases them after the tool messages, and answers a call with no result anywhere in the replayed
-history with the same `[ocx] no tool result was recorded for "<name>"` marker. The shape it
-absorbs is ordinary Codex history, not a malformed one: Codex records mid-turn items (a
-`PostToolUse` hook verdict, a context notice) between an assistant `tool_calls` message and that
-call's own result. The strict pair checks (orphan result, duplicate result, result naming another
-tool) still throw on both wires (#4842).
+`buildNativeMessages` defers `user`/`developer` messages while a batch is open; assistant text/thinking
+with no new tool calls is also deferred while results remain outstanding. Deferred messages retain
+arrival order after recorded results. An unresolved counter avoids rescanning calls per message.
+A new tool-call batch settles its predecessor; completed batches keep subsequent messages in place.
+Codex can record hook notices or commentary between calls and results. Missing results keep the
+`[ocx] no tool result was recorded for "<name>"` marker; orphan IDs, duplicates, and mismatched names
+still throw (#4842). `tests/providers/ollama/ollama-native.test.ts` covers replay and compaction.
 
 Forward-mode OpenAI passthrough also repairs replayed `call_id` values longer than the Responses
 API's 64-character limit. Sidechat/fork replay can namespace routed-provider ids beyond that limit,

tests/providers/ollama/ollama-native.test.ts

@@ -9,12 +9,14 @@ import { getProviderRegistryEntry } from "../../../src/providers/registry";
 import { withStubbedProviderFetch } from "../../helpers/catalog-provider-fetch";
 import type { OcxParsedRequest, OcxProviderConfig } from "../../../src/types";
 
+/** Discover routed test models with provider fetches stubbed to avoid live requests. */
 const gatherRoutedModels: typeof gatherRoutedModelsDirect = (config, options) =>
   gatherRoutedModelsDirect(withStubbedProviderFetch(config), options);
 
 /** The four ids this transport is maintained against. */
 const TARGETS = ["glm-5.3-flash", "deepseek-v4-flash:0731", "glm-5.2", "kimi-k3"] as const;
 
+/** Configure a native Ollama test provider with inert credentials and caller overrides. */
 function ollamaProvider(overrides: Partial<OcxProviderConfig> = {}): OcxProviderConfig {
   return {
     adapter: "ollama-native",
@@ -28,6 +30,7 @@ function ollamaProvider(overrides: Partial<OcxProviderConfig> = {}): OcxProvider
   } as OcxProviderConfig;
 }
 
+/** Build a minimal streamed replay request from synthetic messages and caller options. */
 function parsedWith(
   messages: unknown[],
   options: Record<string, unknown> = {},
@@ -285,6 +288,120 @@ describe("ollama-native — request shape", () => {
     expect(messages[3].content).toBe("[hook] design findings requiring review");
   });
 
+  test("assistant commentary before parallel results keeps the original batch open", async () => {
+    const adapter = createOllamaNativeAdapter(ollamaProvider());
+    const { body } = await adapter.buildRequest(parsedWith([
+      {
+        role: "assistant",
+        content: [
+          { type: "toolCall", id: "call_commentary_first", name: "exec", arguments: { input: "text('first')" } },
+          { type: "toolCall", id: "call_commentary_second", name: "exec", arguments: { input: "text('second')" } },
+        ],
+        timestamp: 1,
+      },
+      { role: "assistant", content: [{ type: "text", text: "checking both results" }], timestamp: 2 },
+      { role: "toolResult", toolCallId: "call_commentary_second", toolName: "exec", content: "second result", isError: false, timestamp: 3 },
+      { role: "toolResult", toolCallId: "call_commentary_first", toolName: "exec", content: "first result", isError: false, timestamp: 4 },
+    ]));
+    const messages = JSON.parse(String(body)).messages;
+    expect(messages.map((message: { role: string }) => message.role))
+      .toEqual(["assistant", "tool", "tool", "assistant"]);
+    expect(messages[1]).toMatchObject({ tool_call_id: "call_commentary_first", content: "first result" });
+    expect(messages[2]).toMatchObject({ tool_call_id: "call_commentary_second", content: "second result" });
+    expect(messages[3].content).toBe("checking both results");
+  });
+
+  test("assistant commentary after one parallel result preserves the remaining genuine result", async () => {
+    const adapter = createOllamaNativeAdapter(ollamaProvider());
+    const { body } = await adapter.buildRequest(parsedWith([
+      {
+        role: "assistant",
+        content: [
+          { type: "toolCall", id: "call_partial_first", name: "exec", arguments: {} },
+          { type: "toolCall", id: "call_partial_second", name: "exec", arguments: {} },
+        ],
+        timestamp: 1,
+      },
+      { role: "toolResult", toolCallId: "call_partial_first", toolName: "exec", content: "first done", isError: false, timestamp: 2 },
+      { role: "assistant", content: [{ type: "text", text: "waiting for the second result" }], timestamp: 3 },
+      { role: "toolResult", toolCallId: "call_partial_second", toolName: "exec", content: "second done", isError: false, timestamp: 4 },
+    ]));
+    const messages = JSON.parse(String(body)).messages;
+    expect(messages.map((message: { role: string }) => message.role))
+      .toEqual(["assistant", "tool", "tool", "assistant"]);
+    expect(messages[1].content).toBe("first done");
+    expect(messages[2].content).toBe("second done");
+    expect(messages[3].content).toBe("waiting for the second result");
+  });
+
+  test("deferred assistant text and thinking retain their order without mutating parsed history", async () => {
+    const parsed = parsedWith([
+      { role: "assistant", content: [{ type: "toolCall", id: "call_thinking", name: "exec", arguments: {} }], timestamp: 1 },
+      { role: "developer", content: "context notice", timestamp: 2 },
+      { role: "assistant", content: [{ type: "text", text: "first comment" }, { type: "thinking", thinking: "synthetic reasoning" }], timestamp: 3 },
+      { role: "assistant", content: [{ type: "text", text: "second comment" }], timestamp: 4 },
+      { role: "toolResult", toolCallId: "call_thinking", toolName: "exec", content: "recorded result", isError: false, timestamp: 5 },
+    ]);
+    const original = JSON.stringify(parsed);
+    const { body } = await createOllamaNativeAdapter(ollamaProvider()).buildRequest(parsed);
+    const messages = JSON.parse(String(body)).messages;
+    expect(messages.map((message: { role: string }) => message.role))
+      .toEqual(["assistant", "tool", "system", "assistant", "assistant"]);
+    expect(messages[1].content).toBe("recorded result");
+    expect(messages[2].content).toBe("context notice");
+    expect(messages[3]).toMatchObject({ content: "first comment", thinking: "synthetic reasoning" });
+    expect(messages[4].content).toBe("second comment");
+    expect(JSON.stringify(parsed)).toBe(original);
+  });
+
+  test("assistant commentary after a complete batch keeps its existing wire position", async () => {
+    const { body } = await createOllamaNativeAdapter(ollamaProvider()).buildRequest(parsedWith([
+      { role: "assistant", content: [{ type: "toolCall", id: "call_complete", name: "exec", arguments: {} }], timestamp: 1 },
+      { role: "toolResult", toolCallId: "call_complete", toolName: "exec", content: "done", isError: false, timestamp: 2 },
+      { role: "assistant", content: [{ type: "text", text: "completed comment" }], timestamp: 3 },
+      { role: "user", content: "next turn", timestamp: 4 },
+    ]));
+    const messages = JSON.parse(String(body)).messages;
+    expect(messages.map((message: { role: string }) => message.role))
+      .toEqual(["assistant", "tool", "assistant", "user"]);
+    expect(messages[1].content).toBe("done");
+    expect(messages[2].content).toBe("completed comment");
+    expect(messages[3].content).toBe("next turn");
+  });
+
+  test("a new tool-call batch still settles an unresolved batch after assistant commentary", async () => {
+    const { body } = await createOllamaNativeAdapter(ollamaProvider()).buildRequest(parsedWith([
+      { role: "assistant", content: [{ type: "toolCall", id: "call_unfinished", name: "exec", arguments: {} }], timestamp: 1 },
+      { role: "assistant", content: [{ type: "text", text: "before the next batch" }], timestamp: 2 },
+      { role: "assistant", content: [{ type: "toolCall", id: "call_next", name: "exec", arguments: {} }], timestamp: 3 },
+      { role: "toolResult", toolCallId: "call_next", toolName: "exec", content: "next result", isError: false, timestamp: 4 },
+    ]));
+    const messages = JSON.parse(String(body)).messages;
+    expect(messages.map((message: { role: string }) => message.role))
+      .toEqual(["assistant", "tool", "assistant", "assistant", "tool"]);
+    expect(messages[1].tool_call_id).toBe("call_unfinished");
+    expect(messages[1].content).toContain("execution status unknown");
+    expect(messages[2].content).toBe("before the next batch");
+    expect(messages[4]).toMatchObject({ tool_call_id: "call_next", content: "next result" });
+  });
+
+  test("routed compaction preserves a result recorded after assistant commentary", async () => {
+    const parsed = parsedWith([
+      { role: "assistant", content: [{ type: "toolCall", id: "call_compaction", name: "exec", arguments: {} }], timestamp: 1 },
+      { role: "assistant", content: [{ type: "text", text: "commentary before compaction" }], timestamp: 2 },
+      { role: "toolResult", toolCallId: "call_compaction", toolName: "exec", content: "genuine result", isError: false, timestamp: 3 },
+      { role: "user", content: "summarize this history", timestamp: 4 },
+    ], { toolChoice: "none" });
+    parsed._compactionRequest = true;
+    const { body } = await createOllamaNativeAdapter(ollamaProvider()).buildRequest(parsed);
+    const request = JSON.parse(String(body));
+    expect(request.messages.map((message: { role: string }) => message.role))
+      .toEqual(["assistant", "tool", "assistant", "user"]);
+    expect(request.messages[1]).toMatchObject({ tool_call_id: "call_compaction", content: "genuine result" });
+    expect(request.messages[2].content).toBe("commentary before compaction");
+    expect(request.tools).toBeUndefined();
+  });
+
   test("a deferred user message keeps its text and images after the tool result", async () => {
     const adapter = createOllamaNativeAdapter(ollamaProvider());
     const png = "data:image/png;base64,iVBORw0KGgo=";