Skip to content

fix(server): resolve a namespaced tool call to the offered tool - #1248

Closed
justinchuby wants to merge 1 commit into
fix/honour-declared-samplingfrom
fix/namespaced-tool-names
Closed

justinchuby wants to merge 1 commit into
fix/honour-declared-samplingfrom
fix/namespaced-tool-names

Conversation

@justinchuby

@justinchuby justinchuby commented Aug 18, 2026 •

Copy link
Copy Markdown
Owner

Stacked on #1236.

Symptom

Driving this model from OpenCode, a tool call fails outright:

✗ Invalid Tool — Model tried to call unavailable tool glob.glob. Available tools: bash, edit, glob, grep, read, write.

Cause

Two different authors meet here. The caller supplies tool definitions over the API — OpenCode offers the flat names bash, edit, glob, grep, read, write. The instruction text the model actually reads is rendered by this package's own chat template, which the caller never sees.

That template assumes tool names are namespaced. It takes name.split('.')[0] as the namespace and builds the addressing grammar from it, so a flat offer renders:

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

The model is told in as many words that the valid recipient is glob.*, never glob. Under that grammar the only well-formed address it can write is glob.<something>, and the natural completion is glob.glob. The worked example reinforces it:

(If the tool namespace is not specified, invoke the function directly as
`example_function_name` rather than `example_tool_name.example_function_name`)

to=example_tool_name.example_function_name
<atem:invoke name="example_tool_name.example_function_name">

So the model is following its instructions correctly; the instructions are the ones that are wrong for a flat offer. The client can only dispatch the name it offered, so it rejects the call and the turn is wasted.

The server is the only place that sees both what the caller offered and what the model said, which is where this can be reconciled.

Fix

When a call names a tool that is not on offer but whose final segment is, the call is resolved to the offered tool. Anything else is left exactly as the model spelled it, so a caller keeps reporting the unavailable tool it already knows how to report rather than being handed a target it never offered.

Parsing stays about the wire format; resolution happens where the request — and therefore the offer — is in scope, at the three call sites that already had it.

Verification

  • 213 lib + 31 http + 11 bundle tests pass; clippy --all-targets clean.
  • Three new tests cover the namespaced call, an offered name containing a dot (left alone), and an unknown suffix (left for the caller to reject).
  • End to end from OpenCode: the model now globs, reads the file, and answers — where it previously failed on the first call.

@codecov

codecov Bot commented Aug 18, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.90%. Comparing base (cba7254) to head (d17e10b).

Additional details and impacted files

Impacted file tree graph

@@                      Coverage Diff                       @@
##           fix/honour-declared-sampling    #1248    +/-   ##
==============================================================
  Coverage                         79.89%   79.90%            
==============================================================
  Files                               359      359            
  Lines                            157720   157905   +185     
  Branches                         157720   157905   +185     
==============================================================
+ Hits                             126011   126171   +160     
- Misses                            27100    27121    +21     
- Partials                           4609     4613     +4     
Flag Coverage Δ
mlas 85.36% <ø> (-0.14%) ⬇️
offline 79.79% <ø> (+0.01%) ⬆️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This package's tool instructions carry a namespaced example —
`example_tool_name.example_function_name` — so a model offered a bare
name sometimes answers with a qualified one. OpenCode rejects the call
outright: "Model tried to call unavailable tool 'glob.glob'. Available
tools: bash, edit, glob, grep, read, write."

The namespace is the model's spelling, not the caller's, and a client
can only dispatch a name it offered. When the qualified name is not on
offer but its final segment is, the call names that tool and is now
resolved to it.

Anything else is left exactly as the model spelled it, so a caller keeps
reporting the unavailable tool it already knows how to report rather
than being handed a target it never offered.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e
@justinchuby
justinchuby force-pushed the fix/honour-declared-sampling branch from ea24844 to cba7254 Compare August 18, 2026 14:44
@justinchuby
justinchuby force-pushed the fix/namespaced-tool-names branch from 71964fc to d17e10b Compare August 18, 2026 14:44
justinchuby added a commit that referenced this pull request Aug 18, 2026
…aces #1248) (#1254)

Replacement for #1248, rebased onto current `main` after
#1197/#1224/#1236 landed as squashes (the stacked base diverged). The
original functional commit plus two added tests, re-validated on top of
the merged stack.

## Symptom

A model driven with flat tool names (`bash`, `edit`, `glob`, `grep`,
`read`, `write`) emits a namespaced call such as `glob.glob`, which the
client rejects: it can only dispatch a name it offered.

## Cause (as claimed by the PR, and what I could and could not confirm)

The caller offers flat tool *definitions* over the API. The instruction
text the model reads is rendered by the **package's own chat template**,
which the PR says assumes namespaced names, takes `name.split('.')[0]`,
and renders `# Valid recipients: "self", "glob.*", ..., "user".` — so
the model follows a grammar that is wrong for a flat offer and writes
`glob.glob`.

**I could not inspect that template with my own eyes.** It ships inside
the model package (Muse Glimmer / harmony-style), which is not in this
repository or in my local model set; searching the repo and local
packages for the `split('.')` / "Valid recipients" grammar found
nothing. What I *can* confirm from the server side corroborates the
addressing scheme: the ATEM parser recognises `to=functions.<name>`
channels and `<atem:invoke name="...">` blocks
(`extract_atem_tool_calls`, `ATEM_USER_CHANNEL = "to=user<|message|>"`),
so a namespaced recipient like `functions.glob` producing a call named
`glob.glob` is consistent with the diagnosis.

**Template fix vs dispatch fix — my conclusion:** resolving at dispatch
is the right layer, and this is a considered choice, not merely the
first available. The template belongs to the model author and is
rendered in the jinja sandbox; the server cannot rewrite an arbitrary
third-party package's template at runtime, and the namespaced grammar is
not universally wrong (it is correct for a caller that offers namespaced
tools — only a flat offer breaks it). The server is the **only**
component that sees both the caller's offer and the model's emitted
call, so it is the only place that can reconcile them without editing
model files. A template correction for flat offers would still be
worthwhile in the package, but it cannot be the server's runtime
responsibility.

## Fix

When a parsed call names a tool that is not on offer but whose **final
`.`-segment exactly equals an offered tool's whole name**, the call is
resolved to that offered tool; anything else is left exactly as the
model spelled it. Applied at the three call sites that already have the
request in scope (buffered, streaming, and the logprob path), via
`ParsedAssistantOutput::aligned_to` / `align_tool_calls`.

**Ambiguity is impossible by construction, not merely avoided:** the
resolution target is always the single final segment string, and it
resolves only if that exact string is an offered (unique) name — there
is never a choice between two candidates. When two offered tools merely
share a final segment (`fs.read`, `net.read`) and the model writes a
third namespace (`svc.read`), the bare segment `read` is offered by
neither, so the call is left alone for the caller to reject.

## Verification (mine, on this branch atop the merged stack)

- **216 lib + 31 http + 11 bundle tests pass**; `cargo clippy -p
onnx-genai-server --all-targets -- -D warnings` clean.
- **Falsified the positive claim** (revert → RED → restore → GREEN):
neutering `align_tool_calls` to a no-op made
`a_namespaced_call_resolves_to_the_offered_tool` and the new
full-parse-path test return `glob.glob` instead of `glob`.
- **Confirmed the negative tests have teeth, and characterised their
limits honestly:** a no-op revert leaves the negatives green (they guard
against *over*-resolution, not absence). Dropping the exact-offer guard
makes `an_offered_name_is_left_alone` go RED (`fs.read` → `read`), which
is exactly what that test protects.
- **Added tests:** `a_shared_final_segment_is_not_resolved_arbitrarily`
(the ambiguity case above) and
`a_namespaced_atem_call_dispatches_through_the_full_parse_path`, which
drives a raw `<atem:invoke name="glob.glob">` turn through the real
`parse_assistant_output` → `align_tool_calls` path and asserts the
dispatched name is `glob`.
- **End-to-end note:** I could not reproduce the OpenCode/Muse path — no
local model emits ATEM namespaced calls — so the end-to-end evidence is
the full-parse-path unit test above (the server's actual code path),
clearly labelled as such rather than an HTTP round-trip.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e
@justinchuby

Copy link
Copy Markdown
Owner Author

Landed as #1254 (squash 39771f3), rebased onto current main after #1197/#1224/#1236 merged and the stacked base diverged. The replacement carries the original functional commit plus two added tests (ambiguity guard + full-parse-path e2e-equivalent). Validation summary is in the #1254 body. Closing this in favour of #1254.

@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
🔴 block_quantized_matmul_cached_dense/mxfp4_cached_dense_repeated_call/1x1024x1024 43.64 µs 127.42 µs +192.0%
🔴 block_quantized_matmul_cached_dense/mxfp4_uncached_dequant_each_call/1x1024x1024 502.24 µs 1.18 ms +135.9%
🔴 block_quantized_matmul_cached_dense/mxfp4_preexpanded_dense_oncelock_like_proxy/1x1024x1024 42.29 µs 96.73 µs +128.8%
🔴 matmul/medium_generic_f32_threads=8/32x512x512 930.94 µs 1.77 ms +90.3%
🔴 matmul/medium_generic_bf16_threads=8/32x512x512 368.65 µs 664.69 µs +80.3%
🔴 matmul/large_generic_bf16_threads=8/32x1024x1024 1.28 ms 2.19 ms +72.1%
🔴 matmul/large_generic_f32_threads=8/32x1024x1024 4.54 ms 7.29 ms +60.5%
🔴 block_quantized_moe_cached_dense/mxfp4_uncached_expert_dequant_each_call/rows=1,H=256,I=256,E=4,top_k=1 433.86 µs 650.36 µs +49.9%
🔴 matmul/medium_generic_f16_threads=8/32x512x512 33.34 µs 49.76 µs +49.3%
🔴 matmul/small_generic_f32_threads=8/1x256x256 34.98 µs 49.79 µs +42.3%
🔴 matmul/small_generic_bf16_threads=1/1x256x256 29.35 µs 40.00 µs +36.3%
🔴 matmul/medium_generic_f16_threads=1/32x512x512 28.30 µs 38.19 µs +34.9%
🔴 matmul/large_generic_f16_threads=8/32x1024x1024 79.08 µs 105.64 µs +33.6%
⚠️ matmul/small_generic_bf16_threads=8/1x256x256 30.21 µs 39.25 µs +29.9%
⚠️ gather/large_f32_threads=1-internal/131072 25.14 µs 32.15 µs +27.9%
⚠️ matmul/large_generic_bf16_threads=1/32x1024x1024 1.84 ms 2.24 ms +22.2%
⚠️ matmul/small_generic_f32_threads=1/1x256x256 34.62 µs 42.26 µs +22.1%
⚠️ matmul/small_generic_f16_threads=8/1x256x256 30.78 µs 37.47 µs +21.7%
⚠️ gather/large_f16_threads=1-internal/131072 10.47 µs 12.72 µs +21.6%
⚠️ matmul/small_generic_f16_threads=1/1x256x256 33.83 µs 39.61 µs +17.1%
⚠️ matmul/medium_generic_bf16_threads=1/32x512x512 514.59 µs 600.91 µs +16.8%
⚠️ matmul/large_generic_f32_threads=1/32x1024x1024 8.79 ms 10.15 ms +15.5%
✅ sampling_latency/greedy_per_token 3.29 µs 3.76 µs +14.3%
✅ block_quantized_moe_cached_dense/mxfp4_cached_dense_expert_repeated_call/rows=1,H=256,I=256,E=4,top_k=1 156.47 µs 178.70 µs +14.2%
✅ grammar_masking/llguidance_compute_mask/32 76.40 µs 85.45 µs +11.9%
✅ matmul/medium_generic_f32_threads=1/32x512x512 2.19 ms 2.42 ms +10.4%
✅ logit_processing/seven_processor_chain_per_step 327.38 µs 360.24 µs +10.0%
✅ kv_cache/alloc_dealloc_pages 40.66 µs 44.25 µs +8.8%
✅ gather/medium_bf16_threads=1-internal/32768 2.15 µs 2.32 µs +8.1%
✅ gather/medium_f16_threads=1-internal/32768 2.18 µs 2.32 µs +6.0%
✅ gather/small_bf16_threads=1-internal/4096 453.7 ns 477.0 ns +5.1%
✅ tokenization/encode_tokens_per_second 392.31 µs 409.06 µs +4.3%
✅ matmul/large_generic_f16_threads=1/32x1024x1024 84.90 µs 87.56 µs +3.1%
✅ qwen3_sampling_processors/top_k_top_p_full_sort_baseline 6.45 ms 6.59 ms +2.3%
✅ sampling_latency/top_k_per_token 55.93 µs 56.10 µs +0.3%
✅ reduce_mean/small_f32_threads=1-internal/4096 15.61 µs 15.59 µs -0.1%
✅ gather/large_bf16_threads=1-internal/131072 10.82 µs 10.74 µs -0.8%
✅ qwen3_sampling_processors/top_p_fast_after_top_k 558.81 µs 553.01 µs -1.0%
✅ add/large_bf16_threads=1-internal/4194304 1.67 ms 1.62 ms -2.8%
✅ sampling_latency/min_p_per_token 235.95 µs 227.79 µs -3.5%
✅ sampling_latency/top_p_per_token 422.21 µs 405.92 µs -3.9%
✅ gather/medium_f32_threads=1-internal/32768 3.74 µs 3.59 µs -4.1%
✅ qwen3_sampling_processors/top_k_top_p_fast 730.80 µs 698.43 µs -4.4%
✅ tokenization/decode_tokens_per_second 7.11 ms 6.69 ms -5.9%
✅ add/medium_bf16_threads=1-internal/262144 116.96 µs 108.63 µs -7.1%
✅ reduce_mean/large_f32_threads=1-internal/262144 1.06 ms 980.60 µs -7.2%
✅ gather/small_f16_threads=1-internal/4096 523.9 ns 482.0 ns -8.0%
✅ qwen3_sampling_processors/top_p_full_sort_after_top_k_baseline 3.80 ms 3.42 ms -10.1%
✅ add/large_f16_threads=1-internal/4194304 1.83 ms 1.65 ms -10.1%
✅ add/medium_f16_threads=1-internal/262144 111.42 µs 98.56 µs -11.5%
✅ reduce_mean/medium_f32_threads=1-internal/65536 266.18 µs 233.57 µs -12.3%
🟢 qwen3_sampling_processors/top_k_full_sort_baseline 2.65 ms 2.24 ms -15.5%
🟢 gather/small_f32_threads=1-internal/4096 820.8 ns 671.6 ns -18.2%
🟢 qwen3_sampling_processors/top_k_partial_selection 171.94 µs 134.62 µs -21.7%
🟢 add/small_f32_threads=1-internal/1024 251.3 ns 196.2 ns -21.9%
🟢 add/medium_f32_threads=1-internal/262144 31.68 µs 24.35 µs -23.1%
🟢 add/small_f16_threads=1-internal/1024 699.1 ns 537.1 ns -23.2%
🟢 add/large_f32_threads=1-internal/4194304 788.51 µs 602.04 µs -23.6%
🟢 add/small_bf16_threads=1-internal/1024 756.2 ns 499.1 ns -34.0%

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.94 3.51 6.28 }
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)

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