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

[lidge-jun]

Summary

Keep an unresolved native Ollama tool-call batch open when assistant text or thinking appears before its results. Emit genuine results in call order, then deferred conversation in arrival order. New tool-call batches and EOF retain explicit unknown-result settlement.

Carries all four commits from #6509 at de5bb1f215c613670e03585e918a344c510ba97a with original authors and cherry-pick provenance. Adds ten adversarial tests for orphan/duplicate/name/namespace validation, reused/empty call IDs, cross-batch results, EOF, ordering and deep-frozen input. The source PR remains open. Also adopts the coordinator's JSON-equivalent shared registry normalization 29a9e91f400d24ac746a18dfe3af357f5da6dec3, without its lane's auth runtime.

Co-authored-by: potota90 85318310+adtumk@users.noreply.github.com

Verification

• Current local head: 917fa4122977b7d890ba1115724e2b5a9a7940cd.
• Regression proof: additional cases on the original adapter yielded 31 pass / 5 fail; after carry, all 42 native tests pass.
• Focused related checks: 130 pass / 0 fail across 10 native/parser/reasoning/structured-output/conformance/layout/file-size files.
• bun scripts/test.ts --changed=dev tests/providers/ollama: 112 pass / 0 fail across 6 import-connected files.
• bun node_modules/typescript/bin/tsc --noEmit, bun scripts/structure-ssot.ts, bun scripts/privacy-scan.ts and git diff --check passed. Direct checker entrypoints avoid the uninitialized local npm Bun shim after scripts-disabled frozen installation.
• cd docs-site && bun run build: passed, 561 pages / 77,932 internal links.
• Independent implementation/security review PASS: 42 native tests and 2,316 independent in-memory replay permutations passed, including missing/partial results, deferred ordering, EOF/new-batch settlement and frozen input. No material findings. Applicable current-head hosted CI passed (receipt below).

Full local suite exception: concurrent stabilization worktrees share the workstation. The earlier unrestricted import-graph run expanded to 1,490 files and had unrelated, baseline-identical fixture/home-isolation failures; it is not claimed passing. This slice uses the focused and bounded import-graph commands above, with required hosted CI still mandatory. No live Ollama request, interactive Windows/packaged-desktop execution, or physical external-tool completion is claimed. Hosted Windows keyring/npm-global checks are separate CI evidence.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Current-head CI receipt: Cross-platform CI 37131234397, attempt 1, event pull_request, completed success for head 917fa4122977b7d890ba1115724e2b5a9a7940cd. Checkout log confirms merge SHA 374570e2b0775b6a1bdd25675fc67adfc1e402f1 into dev e77bfb4901d405724edc4295caf5d9f0675a672e (including Lane A). All 17 applicable workflow jobs succeeded, including ci, all four shards, gates, storage/API, Docker, keyring and npm-global Ubuntu/Windows, docs and structure. Eight scope-gated producers were skipped; full native platform/desktop coverage is not claimed. Other policy checks succeeded. No unresolved review threads or maintainer change requests were observed at readiness.

Summary by CodeRabbit

• Bug Fixes
  • Ollama conversations now preserve genuine tool results when assistant commentary or user/developer messages arrive before all results are received. Deferred messages appear after the results, in their original order.
  • A new tool-call batch or the end of the conversation settles any outstanding results; missing results retain an unknown-status marker. Orphaned, duplicate, or mismatched results remain invalid.
• Documentation
  • Updated the adapter reference to describe how commentary and pending tool results are handled.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 1fc062ae-411e-42bc-b709-1a2b7c48cce7

📥 Commits

Reviewing files that changed from the base of the PR and between e77bfb4 and 917fa41.

📒 Files selected for processing (8)

• devlog/_plan/261003_ollama_commentary_replay/000_plan.md
• devlog/_plan/261003_ollama_commentary_replay/010_replay.md
• docs-site/src/content/docs/reference/adapters.md
• scripts/test-layout/layout.json
• src/adapters/ollama-native.ts
• structure/providers/chat-compat.md
• tests/fixtures/test-layout-expected.json
• tests/providers/ollama/ollama-native.test.ts

💤 Files with no reviewable changes (2)

• scripts/test-layout/layout.json
• tests/fixtures/test-layout-expected.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The Ollama adapter now defers assistant commentary while tool results remain outstanding. It emits accepted results in call order before deferred conversation, while retaining existing validation and missing-result handling.

Changes

Ollama tool-result replay

Layer / File(s)	Summary
Track and settle pending tool batches src/adapters/ollama-native.ts, structure/providers/chat-compat.md, docs-site/src/content/docs/reference/adapters.md, devlog/_plan/261003_ollama_commentary_replay/*	PendingToolBatch tracks unresolved calls. Assistant text and thinking are deferred while results are pending. The adapter emits results in call order before deferred messages, and a new tool-call batch settles its predecessor. Documentation describes the behavior and retained validation rules.
Verify replay ordering and validation tests/providers/ollama/ollama-native.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, devlog/_plan/261003_ollama_commentary_replay/*	Tests cover result ordering, partial batches, settlement, compaction, validation failures, EOF handling, and frozen-history immutability. Test-layout mappings and expected fixtures are also updated.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant History as Parsed history
  participant Replay as buildNativeMessages
  participant Batch as PendingToolBatch
  participant Output as Native messages
  History->>Replay: Provide assistant calls and conversation
  Replay->>Batch: Track unresolved call count
  History->>Replay: Provide tool results and commentary
  Replay->>Batch: Decrement unresolved count for each accepted result
  Batch->>Output: Emit tool results in call order
  Batch->>Output: Emit deferred conversation in arrival order

Loading

Merge Risk: ⚪ Minimal · up to 917fa

The layout mappings remain intact, and no merge-blocking issue is established. The PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 917fa

No confirmed security regression was found in the reviewed replay behavior. Tool-result validation remains enforced, and deferred commentary retains its original role. Remaining uncertainty concerns concurrent request isolation and changes outside that behavior.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is changed history ordering for requests using the native Ollama path. The reviewed diff does not change endpoint selection, credentials or tool-execution authority; it changes which validated history messages become adjacent in the provider request.

Trust Boundaries and Controls

• observed — A tool result must match an open batch's call ID, tool name and namespace and must not duplicate an accepted result. Missing or reused call IDs are rejected. Deferred assistant text remains assistant-role content and cannot satisfy a tool result. These attribution controls remain enforced across the newly accepted commentary ordering.

Resilience and Maintainability Implications

• observed — Interrupted or partial histories settle missing results with an explicit execution-status-unknown marker rather than inventing success or failure. New batches and EOF retain this recovery behavior before deferred conversation is released.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (4 skipped: 4…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: keeping Ollama tool batches open across assistant commentary.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (4 skipped: 4 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-03T15:08:18.876664Z	917fa41	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[lidge-jun]

Owner-authorized progressive maintainer integration into dev for the release stabilization train; an integration decision, not a self-approval.

I reviewed the native Ollama replay change and its validation path. The unresolved counter decrements only after a result passes the existing ID/name/duplicate checks; assistant commentary is deferred only while results remain outstanding. New batches and EOF still settle missing results explicitly, and the original history is not mutated. Orphan, duplicate, reused-ID and namespace/name checks remain in place. Explicit scoped security review: no blocker; this neither executes tools nor introduces a credential destination. The separate independent review and 2,316 synthetic replay permutations are recorded in the lane evidence.

Source #6509's author and all four carry commits are retained, with added adversarial regressions. Focused local tests, typecheck, structure/privacy, docs and bounded import-connected checks passed. The unrelated broad local run is not represented as passing and no live Ollama/physical tool execution claim is made.

I also checked dev's #6513 exact-canonical-ChatGPT large HTTP upload conversion against this slice. Ollama destinations do not match it and the replay runtime paths do not overlap. I also verified the candidate's merged test-registry mappings against current dev: they are identical, with only redundant identical registrations removed. No test or cap was removed/raised. The conflict-free union is additional evidence, not a replacement claim for exact-current-head CI. Full integrated regression and cross-platform CI still precede publication.

Hosted receipt: https://github.com/lidge-jun/opencodex/actions/runs/37131234397, attempt 1, pull_request, tested head 917fa4122977b7d890ba1115724e2b5a9a7940cd / base e77bfb4901d405724edc4295caf5d9f0675a672e. Current reviewed dev base e77bfb4901d405724edc4295caf5d9f0675a672e; conflict-free union tree f33ff37ec2bb26801d4eebc2109293ee5c1d9dd9. All four Linux shards and selected gates/storage/API/docs/structure/Docker/keyring/npm-global jobs succeeded. Skipped full-platform suites are not claimed passing; final integrated lane=all remains required.