Skip to content

fix(ollama-native): key response tool calls by native id before index - #6858

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
MarcTCruz:fix/ollama-native-tool-call-identity
Oct 10, 2026
Merged

lidge-jun merged 1 commit into
lidge-jun:devfrom
MarcTCruz:fix/ollama-native-tool-call-identity

Conversation

@MarcTCruz

@MarcTCruz MarcTCruz commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #6857.

Ollama Cloud's mistral-large-4 streams several distinct tool calls in one native frame, each with its own id, but all with function.index: 0:

{"message":{"role":"assistant","content":"","tool_calls":[
  {"id":"PpZ8JqRmm","function":{"index":0,"name":"read_file","arguments":{"path":"/etc/hostname"}}},
  {"id":"DDe4wZqkx","function":{"index":0,"name":"read_file","arguments":{"path":"/etc/os-release"}}}]},"done":false}

nativeMessageEvents in src/adapters/ollama-native.ts used index:${index} as the call's identity, so the second call collided with the first. The turn then failed with changed a tool-call id for an existing index, or with reused a tool-call index for another function when the names differed.

  • A valid native id is now the call's identity. An entry without an id falls back to its index, then to its position.
  • A later id adopts the call that was first seen without one at the same index, as before.
  • Two entries in the same frame are one call only when they share an id.
  • Budget keys follow first-seen order instead of the index, so distinct calls at one index keep separate argument and metadata accounting. The 128-call cap still counts them.
  • The parallelToolCalls:false guard and its feat(providers): native Ollama /api/chat transport with /api/show metadata for ollama-cloud #2863 test are unchanged. The captured shape now reaches that guard and fails closed with the parallel-call error, instead of the misleading id error.
  • structure/providers/chat-compat.md and docs-site/.../reference/adapters.md describe the identity rule.

For the maintainer: the generated catalog advertises supports_parallel_tool_calls: false for Ollama models, because native adapters advertise it only on explicit opt-in (src/codex/catalog/model-hints.ts:349). Ollama's /api/chat has no field to forward that preference, and this model emits parallel calls regardless. So with default settings these turns still fail, now with the correct error. Should ollama-native advertise parallel calls by default, like openai-chat? I did not change that here.

src/adapters/openai-chat.ts:508 resolves tool calls index-first in the same way. I have no provider capture showing it collide, so it is out of scope here.

Verification

Test runner: Bun 1.4.0 (OCX_TEST_RUNNER_BUN).

  • bun run test -- tests/providers/ollama/ --parallel=1: 155 pass, 0 fail.
  • 17 new cases in tests/providers/ollama/ollama-native-parser.test.ts. With the source change reverted, 10 of them fail: the captured frame (streamed and buffered), the parallel-guard error text, and the id-first identity rules. The other 7, including a later id adopting a call first seen without one, pin behavior that was already correct and must survive the change.
  • bun run typecheck, bun run structure:check, bun run privacy:scan: pass.
  • bun run test -- tests/providers/ tests/adapters/: 9418 pass, 13 fail. Rerun serially, 3 still fail: codebuddy-mcp-server, codebuddy-adapter (SIGTERM abort), and openai-chat-image-normalization. All 3 fail the same way on unmodified dev (93ed1a40b) on this host, and none touches ollama-native.
  • I did not run the full bun run test: the host's load average was ~33 on 4 cores, and suite-wide 5 s timeouts made the results meaningless. Remaining coverage is left to CI.
  • docs-site build not run locally.

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.

🤖 Generated with Claude Code

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Ollama streaming tool-call matching so valid call IDs take priority, repeated IDs update existing calls, and calls without IDs fall back to index or position.
    • Preserved first-seen call order and rejected distinct parallel calls when parallel tool calls are disabled.
    • Kept function-name consistency and pending-call limits enforced.
  • Documentation
    • Clarified tool-call identity, ordering, and parallel-call behavior in the adapter reference and compatibility guide.

Ollama Cloud's mistral-large-4 streams several distinct tool calls in one
frame, each with its own id but all with function.index 0. The parser keyed
calls by index, so the second call collided with the first and the stream
failed with "changed a tool-call id for an existing index" (or "reused a
tool-call index for another function" when the names differ).

A valid native id is now the call's identity. Index, then position, is the
fallback only for entries without an id, and a later id adopts the call that
was first seen without one at the same index. Two entries in one frame share
a call only when they share an id. Budget keys follow first-seen order
instead of the index, so distinct calls at one index keep separate argument
and metadata accounting. The parallelToolCalls:false guard is unchanged: the
captured shape now reaches it and fails closed with the parallel-call error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@github-actions
github-actions Bot marked this pull request as draft October 9, 2026 22:34
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: d0d992f0-6084-4102-88c3-2bbf2e3cfaa7

📥 Commits

Reviewing files that changed from the base of the PR and between 006cec9 and 7b23c35.


📒 Files selected for processing (4)
  • docs-site/src/content/docs/reference/adapters.md
  • src/adapters/ollama-native.ts
  • structure/providers/chat-compat.md
  • tests/providers/ollama/ollama-native-parser.test.ts

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



📝 Walkthrough

Walkthrough

The Ollama native adapter now matches streamed tool calls by native ID before using index or position fallbacks. Tests and documentation cover repeated IDs, call ordering, call limits, and rejection when parallel calls are disabled.

Changes

Ollama Native Tool-Call Identity

Layer / File(s) Summary
Identity matching and call limits
src/adapters/ollama-native.ts, tests/providers/ollama/ollama-native-parser.test.ts, docs-site/src/content/docs/reference/adapters.md, structure/providers/chat-compat.md
At src/adapters/ollama-native.ts:84,642-681,701-702, the adapter tracks calls by budget key, prefers native IDs, and uses index or position for fallback matching. The tests at tests/providers/ollama/ollama-native-parser.test.ts:315-525 cover identity updates, ordering, function-name consistency, parallel-call rejection, and call limits. Both documentation files describe the matching and ordering rules.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium


Merge Risk: ⚪ Minimal · up to 7b23c

This change lets Ollama native streams with distinct tool-call IDs sharing the same index be handled as separate calls. The parallel-call rejection when disabled remains by design. No merge-blocking risk is identified.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Issue #6857 requires ollama-native to keep distinct native tool calls distinct when they share function.index. The reviewed implementation in src/adapters/ollama-native.ts defines native-ID, ind…
Out of Scope Changes check Passed The changed paths are limited to the ollama-native identity implementation, its focused parser tests, and documentation of that behavior. The tests validate the required regression and related safet…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the main change: Ollama native response tool calls now use native IDs before function indexes for identity.

Full details: Docstring Coverage

Explanation

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


  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · 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

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

@lidge-jun
lidge-jun marked this pull request as ready for review October 10, 2026 10:31
@lidge-jun
lidge-jun merged commit aa95a48 into lidge-jun:dev Oct 10, 2026
35 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

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-10T10:36:21.419453Z 7b23c35 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants