Skip to content

Resolve a qualified tool call from any of its segments - #1478

Merged
justinchuby merged 1 commit into
mainfrom
fix/tool-name-alignment-any-segment
Aug 19, 2026
Merged

justinchuby merged 1 commit into
mainfrom
fix/tool-name-alignment-any-segment

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

The symptom

Every OpenCode session opened with a failed tool call:

✗ Invalid Tool
Model tried to call unavailable tool 'read.filePath'

The agent recovered on the retry, so the session worked, but every task paid for a wasted round trip.

The root cause is in the chat template

chat_template.jinja infers a namespace with fn.name.split('.')[0] and renders it unconditionally as a pattern. OpenCode offers bare tool names, so the system prompt became:

# Valid recipients: "self", "read.*", "bash.*", "glob.*", "user".

The model was told it must address read.*. It supplied a second segment by borrowing something at hand -- typically a parameter name. read.filePath is the model doing exactly what the prompt asked.

That template lives in the model package, and it has been fixed there. This PR is the server-side half.

The change

align_tool_calls already rewrote qualified names, but only tried the last segment. That handles functions.read; it cannot handle read.filePath, where the tool is the first segment.

Now every segment is tried, and the rewrite happens only when the offered tools agree on exactly one target. The candidate set is deduplicated so that:

  • glob.glob resolves to glob (both segments name the same tool),
  • read.write is left alone (two different offered tools -- no way to tell which was meant).

Verification

  • cargo test -p onnx-genai-server --lib -- 239 passed, 0 failed. tool_name_alignment_tests is 7/7 with two new cases covering the shapes above.
  • End to end against muse-glimmer-30b-int4 with OpenCode: a read task and a multi-tool debug task (read, edit, bash, todowrite) both completed with no invalid tool errors.

CI runners are down; validated locally.

A chat template that renders bare tool names as namespace patterns tells
the model to address `read.*`, so the model invents a second segment --
usually by borrowing a parameter name, giving `read.filePath`. Tool name
alignment only tried the last segment, which handles `functions.read` but
not this shape, so the call was rejected as an unavailable tool.

Try every segment and rewrite only when the offered tools agree on exactly
one target, deduplicated so `glob.glob` resolves while `read.write` stays
untouched.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e
@justinchuby
justinchuby merged commit 8c750eb into main Aug 19, 2026
4 checks passed
@justinchuby
justinchuby deleted the fix/tool-name-alignment-any-segment branch August 19, 2026 15:50
@github-actions

Copy link
Copy Markdown

🔴 Benchmark Regression Detected

Comparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).

ℹ️ Absolute times are informational only — they vary with runner load. The % change column is the reliable signal because both sides ran under identical conditions.

Status Scenario Base PR Change
🔴 sampling_latency/top_k_per_token 48.28 µs 72.72 µs +50.6%
🔴 sampling_latency/greedy_per_token 2.98 µs 4.20 µs +40.9%
🔴 tokenization/decode_tokens_per_second 6.23 ms 8.44 ms +35.6%
⚠️ sampling_latency/top_p_per_token 350.80 µs 425.53 µs +21.3%
⚠️ qwen3_sampling_processors/top_k_partial_selection 129.56 µs 150.26 µs +16.0%
✅ logit_processing/seven_processor_chain_per_step 298.95 µs 341.14 µs +14.1%
✅ tokenization/encode_tokens_per_second 357.98 µs 405.55 µs +13.3%
✅ qwen3_sampling_processors/top_k_top_p_full_sort_baseline 5.36 ms 6.07 ms +13.2%
✅ qwen3_sampling_processors/top_k_top_p_fast 620.89 µs 686.37 µs +10.5%
✅ qwen3_sampling_processors/top_p_full_sort_after_top_k_baseline 3.32 ms 3.57 ms +7.8%
✅ sampling_latency/min_p_per_token 220.54 µs 228.56 µs +3.6%
✅ kv_cache/alloc_dealloc_pages 37.13 µs 38.31 µs +3.2%
✅ qwen3_sampling_processors/top_p_fast_after_top_k 484.16 µs 492.13 µs +1.6%
✅ grammar_masking/llguidance_compute_mask/32 70.24 µs 70.55 µs +0.4%
✅ qwen3_sampling_processors/top_k_full_sort_baseline 2.27 ms 2.16 ms -5.2%
✅ gather/small_f16_threads=1-internal/4096 547.4 ns 509.2 ns -7.0%
✅ matmul/large_generic_f16_threads=8/32x1024x1024 85.06 µs 78.55 µs -7.7%
✅ block_quantized_moe_cached_dense/mxfp4_uncached_expert_dequant_each_call/rows=1,H=256,I=256,E=4,top_k=1 459.33 µs 419.23 µs -8.7%
🟢 matmul/large_generic_f16_threads=1/32x1024x1024 94.16 µs 79.31 µs -15.8%
🟢 gather/small_bf16_threads=1-internal/4096 556.0 ns 462.3 ns -16.8%
🟢 gather/medium_bf16_threads=1-internal/32768 2.73 µs 2.23 µs -18.2%
🟢 reduce_mean/large_f32_threads=1-internal/262144 1.15 ms 922.79 µs -19.7%
🟢 matmul/medium_generic_f32_threads=1/32x512x512 2.69 ms 2.16 ms -19.9%
🟢 matmul/small_generic_bf16_threads=1/1x256x256 35.83 µs 28.47 µs -20.6%
🟢 gather/medium_f32_threads=1-internal/32768 4.51 µs 3.55 µs -21.1%
🟢 matmul/small_generic_f16_threads=1/1x256x256 36.26 µs 28.51 µs -21.4%
🟢 reduce_mean/medium_f32_threads=1-internal/65536 289.76 µs 226.19 µs -21.9%
🟢 matmul/large_generic_f32_threads=1/32x1024x1024 11.62 ms 8.80 ms -24.2%
🟢 matmul/medium_generic_f16_threads=1/32x512x512 36.81 µs 27.80 µs -24.5%
🟢 matmul/large_generic_bf16_threads=1/32x1024x1024 2.47 ms 1.86 ms -24.8%
🟢 reduce_mean/small_f32_threads=1-internal/4096 19.02 µs 14.24 µs -25.1%
🟢 matmul/medium_generic_bf16_threads=8/32x512x512 480.41 µs 358.76 µs -25.3%
🟢 matmul/small_generic_f32_threads=1/1x256x256 47.18 µs 34.00 µs -27.9%
🟢 gather/medium_f16_threads=1-internal/32768 3.14 µs 2.25 µs -28.4%
🟢 block_quantized_moe_cached_dense/mxfp4_cached_dense_expert_repeated_call/rows=1,H=256,I=256,E=4,top_k=1 99.24 µs 69.83 µs -29.6%
🟢 gather/small_f32_threads=1-internal/4096 919.4 ns 628.2 ns -31.7%
🟢 matmul/small_generic_bf16_threads=8/1x256x256 43.68 µs 29.51 µs -32.4%
🟢 block_quantized_matmul_cached_dense/mxfp4_cached_dense_repeated_call/1x1024x1024 62.09 µs 41.82 µs -32.6%
🟢 matmul/medium_generic_bf16_threads=1/32x512x512 734.41 µs 490.95 µs -33.2%
🟢 matmul/small_generic_f32_threads=8/1x256x256 48.27 µs 31.80 µs -34.1%
🟢 add/medium_f32_threads=1-internal/262144 34.41 µs 22.47 µs -34.7%
🟢 matmul/small_generic_f16_threads=8/1x256x256 44.30 µs 28.08 µs -36.6%
🟢 gather/large_bf16_threads=1-internal/131072 18.59 µs 11.40 µs -38.7%
🟢 matmul/large_generic_f32_threads=8/32x1024x1024 6.20 ms 3.69 ms -40.4%
🟢 matmul/medium_generic_f32_threads=8/32x512x512 1.55 ms 889.05 µs -42.7%
🟢 gather/large_f16_threads=1-internal/131072 17.54 µs 9.76 µs -44.4%
🟢 add/small_bf16_threads=1-internal/1024 757.3 ns 418.8 ns -44.7%
🟢 add/medium_bf16_threads=1-internal/262144 173.10 µs 94.86 µs -45.2%
🟢 matmul/medium_generic_f16_threads=8/32x512x512 50.17 µs 27.34 µs -45.5%
🟢 matmul/large_generic_bf16_threads=8/32x1024x1024 2.31 ms 1.24 ms -46.2%
🟢 add/medium_f16_threads=1-internal/262144 186.50 µs 98.63 µs -47.1%
🟢 add/large_f16_threads=1-internal/4194304 2.88 ms 1.52 ms -47.2%
🟢 add/small_f32_threads=1-internal/1024 345.4 ns 180.4 ns -47.8%
🟢 add/small_f16_threads=1-internal/1024 818.0 ns 414.6 ns -49.3%
🟢 gather/large_f32_threads=1-internal/131072 44.59 µs 22.49 µs -49.5%
🟢 add/large_f32_threads=1-internal/4194304 1.19 ms 566.70 µs -52.2%
🟢 add/large_bf16_threads=1-internal/4194304 3.33 ms 1.57 ms -52.8%
🟢 block_quantized_matmul_cached_dense/mxfp4_preexpanded_dense_oncelock_like_proxy/1x1024x1024 103.28 µs 42.96 µs -58.4%
🟢 block_quantized_matmul_cached_dense/mxfp4_uncached_dequant_each_call/1x1024x1024 1.50 ms 499.48 µs -66.8%

Visual flags: ⚠️ ≥ 15% slower, 🔴 ≥ 30% slower — calibrated against measured runner noise (~27% worst-case on multi-threaded matmul)

Host info
CPU: Apple M1 (Virtual)
Cores: 3
OS: Darwin 25.5.0 arm64
Rust: rustc 1.97.1 (8bab26f4f 2026-07-14)
Load avg: { 3.00 3.31 6.71 }
What this cannot catch
  • Regressions in code paths not covered by these benchmarks (e.g., end-to-end decode with a real model)
  • Sub-threshold regressions that compound over multiple PRs
  • Performance changes that only manifest under GPU execution
  • Latency changes in the ORT integration path (these benchmarks exercise the native Rust kernels)

@codecov

codecov Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.64%. Comparing base (a91557c) to head (c42ccbf).
⚠️ Report is 67 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1478      +/-   ##
==========================================
+ Coverage   82.10%   82.64%   +0.54%     
==========================================
  Files          12       12              
  Lines        5471     5475       +4     
  Branches     5471     5475       +4     
==========================================
+ Hits         4492     4525      +33     
+ Misses        780      757      -23     
+ Partials      199      193       -6     
Flag Coverage Δ
cli-ort-linux 82.60% <ø> (?)
cli-ort-windows 82.19% <ø> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant