fix(server): give a reasoning turn room to finish, and keep it private by justinchuby · Pull Request #1224 · justinchuby/onnx-genai

justinchuby · 2026-08-18T06:01:15Z

Stacked on #1197.

A real agent client (OpenCode) driving this server through the OpenAI API exposed two defects that share a cause: the server was answering questions the client never asked.

A reasoning turn was cut off mid-thought

OpenAI accepts only max_completion_tokens for a reasoning model and rejects max_tokens, and we implemented neither — grep max_completion_tokens returned nothing across the repository. Serde fell back to a fixed 256-token default, which a reasoning model spends entirely on its private thinking, so the turn ended before it ever addressed the caller.

Measured against the live 30B model on the same puzzle prompt:

request	completion tokens	finish reason	answer
no budget named	1050	`stop`	1465 characters
`max_completion_tokens: 256`	256	`length`	empty

max_tokens and max_completion_tokens are now both optional, the newer field wins, and a request that names neither falls back to the server cap rather than to 256 — that is what OpenAI means by an unspecified limit.

The cap is a ceiling on generosity, not a reservation the caller made, so an unnamed budget also yields to the prompt instead of being admitted against it. Without that, a short-context model would reject every budget-less request, because cap plus prompt always exceeds its context. A budget the caller did name still reports honestly that it does not fit.

Private reasoning streamed to the client verbatim

The model spells a turn as channels addressed to different recipients — to=self for private thinking, to=user for the answer, to=functions.* for a call. Streaming sent tokens as they arrived, and a streamed token has its special tokens stripped, so the markers that decide visibility were invisible token by token. The buffered path filtered correctly; the two disagreed.

PrivateChannelGate keeps the turn's special-token spelling and asks the existing atem_visible_content what is visible so far, emitting only the growth. Channel semantics stay in one place and streaming agrees with the buffered path by construction.

Two properties matter more than the mechanism:

Armed by declaration, not by guesswork. The gate reads the package's own response_template to learn whether it has a private channel. A model that declares none is never gated and streams exactly what it generated.
Fails closed. A token the gate cannot place in the caller's channel is withheld. A withheld answer costs a retry; a released thought cannot be taken back. This also covers the session path, where the prompt is not wrapped in the chat template and the model spells its own turn header.

Three paths bypassed the gate and now share one answer to "what may the caller see": per-token logprob chunks, constrained-JSON completions, and the drain of a turn that ended on a stop sequence, which silently dropped whatever the gate still held.

Verification

208 lib + 31 http + 11 bundle tests pass; clippy --all-targets is clean.
Live 30B model, native CUDA: streaming answers arrive with zero channel markers; tool calls parse in both streaming and buffered mode; a four-turn agent loop under a 4.8k-token system prompt with tool results completes with finish_reason: stop and a real answer — the exact scenario that previously returned length and nothing.

Running OpenCode against the OpenAI-compatible server surfaced a set of gaps between what a real agent client sends and what the server accepted. - Ignore a client's `X-Session-Id` on a pipeline model rather than rejecting the request. The header is a transport hint; the client still resends the full history, and a pipeline retains its own context. - Decode `function.arguments` from its JSON string before rendering. Chat templates index it as a mapping, and the jinja sandbox cannot parse a string back into one. - Carry a tool result's `name`/`tool_call_id` into the template, so a tool-calling template can name the function a result answers. - Accept OpenAI's `reasoning_effort` and expose it to the template, so a reasoning model can be asked to think less. Without it a template stays on its own default, which is often maximal, and an agent turn can spend its whole token budget thinking. - Honor a pipeline package's declared sampling regime instead of forcing greedy, and stop treating an absent `top_k` as a request to disable the model's declared one. A model shipping `do_sample: true` otherwise decoded greedily and degenerated into repetition. - Parse the ATEM tool-call and channel format, and never present the private reasoning channel as assistant content. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

A reasoning model spends the same output budget on its private thinking as on its answer, so two independent gaps each cost the caller the whole turn. The budget was one. OpenAI deprecated `max_tokens` for chat completions and accepts only `max_completion_tokens` for reasoning models, and the server did not read that field at all: an agent asking for 8192 tokens silently got serde's fallback of 256, exhausted it mid-thought, and returned `finish_reason: length` with empty content. Both fields are now read, the newer one winning as OpenAI specifies, and a request that names neither now gets the server's cap rather than a small fixed default — matching OpenAI, where an omitted budget means "decode until the model stops". Measured on Muse Glimmer 30B, one puzzle prompt, budget absent: | budget | tokens | finish_reason | answer | | --- | --- | --- | --- | | absent (was 256) | 1050 | `stop` | 1465 chars | | `max_completion_tokens: 256` | 256 | `length` | empty | Privacy was the other. An ATEM turn is a sequence of addressed channels and only the one addressed to the user is an answer, which the buffered path already honours. Streaming did not: a streamed token's text has its special tokens stripped, so the channel markers are invisible token by token and the model's private reasoning went out live, as content, to any client that asked for `stream: true`. `AtemChannelGate` closes that by keeping the turn's special-token spelling and asking the existing `atem_visible_content` what is visible so far, emitting only what has grown since the previous token — so streaming and buffered output agree by construction rather than by parallel parsers. A turn opens with its channel address, so the gate withholds output only while that address could still be forming: the first token of an unaddressed turn decides the turn is not ATEM and everything streams unchanged from there, which costs non-ATEM models nothing. A turn cut off mid-address is released rather than swallowed. Nine tests cover both, over the budget precedence, the cap, the field named in a rejection, and the gate's channel, passthrough, cut-off and unspellable-token paths. End to end against Muse Glimmer 30B on native CUDA, a streamed turn now opens on the answer and carries no channel marker. Stacked on #1197, whose `atem_visible_content` this consumes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

A reasoning model spells its turn as channels addressed to different recipients, and only one of them is the answer. Streaming sent the tokens as they arrived, so the private channel reached the client verbatim; the buffered path filtered correctly, and the two disagreed. The gate is now armed from the package's own `response_template` declaration rather than from guessing at the shape of a turn, and an armed gate fails closed: a token it cannot place in the caller's channel is withheld, because a withheld answer costs a retry and a released thought cannot be taken back. A model that declares no private channel is never gated and streams exactly what it generated. Three paths bypassed the gate and now share one answer to "what may the caller see": per-token logprob chunks, constrained-JSON completions, and the drain of a turn that ended on a stop sequence, which dropped whatever the gate still held. The unspecified output budget is also reconciled with the model context rather than reserved against it. Falling back to the server cap made every budget-less request exceed the context of a short-context model; the cap is a ceiling on generosity, so an unnamed budget now shrinks to what the prompt left behind, while a budget the caller named still reports that it does not fit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

codecov · 2026-08-18T11:35:52Z

Codecov Report

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

Additional details and impacted files

@@                       Coverage Diff                        @@
##           feat/openai-agent-client-compat    #1224   +/-   ##
================================================================
  Coverage                            79.90%   79.90%           
================================================================
  Files                                  359      359           
  Lines                               157905   157905           
  Branches                            157905   157905           
================================================================
+ Hits                                126169   126173    +4     
+ Misses                               27123    27120    -3     
+ Partials                              4613     4612    -1

Flag	Coverage Δ
mlas	`85.46% <ø> (+0.13%)`	⬆️
offline	`79.79% <ø> (ø)`

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-08-18T11:48:49Z

⚠️ Benchmark Change 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_preexpanded_dense_oncelock_like_proxy/1x1024x1024`	94.13 µs	118.26 µs	+25.6%
⚠️	`matmul/medium_generic_bf16_threads=8/32x512x512`	591.87 µs	731.87 µs	+23.7%
⚠️	`gather/large_f32_threads=1-internal/131072`	32.09 µs	38.55 µs	+20.1%
✅	`add/medium_f32_threads=1-internal/262144`	30.62 µs	34.51 µs	+12.7%
✅	`add/small_bf16_threads=1-internal/1024`	470.1 ns	526.4 ns	+12.0%
✅	`matmul/medium_generic_bf16_threads=1/32x512x512`	584.99 µs	652.60 µs	+11.6%
✅	`block_quantized_matmul_cached_dense/mxfp4_cached_dense_repeated_call/1x1024x1024`	104.04 µs	114.20 µs	+9.8%
✅	`sampling_latency/top_p_per_token`	391.86 µs	425.81 µs	+8.7%
✅	`sampling_latency/top_k_per_token`	54.23 µs	58.21 µs	+7.3%
✅	`matmul/small_generic_f32_threads=1/1x256x256`	38.11 µs	39.72 µs	+4.2%
✅	`matmul/large_generic_bf16_threads=8/32x1024x1024`	1.96 ms	2.04 ms	+4.2%
✅	`qwen3_sampling_processors/top_k_top_p_fast`	699.48 µs	717.31 µs	+2.5%
✅	`matmul/large_generic_f16_threads=1/32x1024x1024`	85.29 µs	87.30 µs	+2.4%
✅	`tokenization/decode_tokens_per_second`	6.87 ms	7.02 ms	+2.3%
✅	`matmul/large_generic_f32_threads=1/32x1024x1024`	9.61 ms	9.72 ms	+1.2%
✅	`gather/medium_f32_threads=1-internal/32768`	4.42 µs	4.45 µs	+0.7%
✅	`qwen3_sampling_processors/top_p_full_sort_after_top_k_baseline`	3.72 ms	3.75 ms	+0.7%
✅	`matmul/large_generic_bf16_threads=1/32x1024x1024`	2.27 ms	2.28 ms	+0.7%
✅	`block_quantized_moe_cached_dense/mxfp4_cached_dense_expert_repeated_call/rows=1,H=256,I=256,E=4,top_k=1`	176.01 µs	177.00 µs	+0.6%
✅	`grammar_masking/llguidance_compute_mask/32`	78.73 µs	79.09 µs	+0.5%
✅	`block_quantized_matmul_cached_dense/mxfp4_uncached_dequant_each_call/1x1024x1024`	1.15 ms	1.15 ms	-0.2%
✅	`matmul/medium_generic_f16_threads=8/32x512x512`	42.68 µs	42.51 µs	-0.4%
✅	`qwen3_sampling_processors/top_k_partial_selection`	143.25 µs	140.94 µs	-1.6%
✅	`qwen3_sampling_processors/top_p_fast_after_top_k`	538.11 µs	528.92 µs	-1.7%
✅	`sampling_latency/min_p_per_token`	212.79 µs	207.51 µs	-2.5%
✅	`qwen3_sampling_processors/top_k_full_sort_baseline`	2.20 ms	2.13 ms	-2.9%
✅	`sampling_latency/greedy_per_token`	3.50 µs	3.39 µs	-3.0%
✅	`add/medium_bf16_threads=1-internal/262144`	116.18 µs	112.72 µs	-3.0%
✅	`kv_cache/alloc_dealloc_pages`	40.39 µs	39.01 µs	-3.4%
✅	`logit_processing/seven_processor_chain_per_step`	336.25 µs	318.42 µs	-5.3%
✅	`add/medium_f16_threads=1-internal/262144`	117.54 µs	111.08 µs	-5.5%
✅	`tokenization/encode_tokens_per_second`	428.75 µs	404.61 µs	-5.6%
✅	`matmul/small_generic_bf16_threads=1/1x256x256`	37.28 µs	34.59 µs	-7.2%
✅	`block_quantized_moe_cached_dense/mxfp4_uncached_expert_dequant_each_call/rows=1,H=256,I=256,E=4,top_k=1`	567.88 µs	523.12 µs	-7.9%
✅	`qwen3_sampling_processors/top_k_top_p_full_sort_baseline`	6.29 ms	5.78 ms	-8.2%
✅	`matmul/large_generic_f16_threads=8/32x1024x1024`	93.77 µs	86.00 µs	-8.3%
✅	`matmul/medium_generic_f32_threads=1/32x512x512`	2.60 ms	2.36 ms	-9.1%
✅	`gather/small_bf16_threads=1-internal/4096`	532.5 ns	474.6 ns	-10.9%
✅	`gather/small_f32_threads=1-internal/4096`	788.2 ns	697.2 ns	-11.5%
✅	`matmul/small_generic_bf16_threads=8/1x256x256`	38.97 µs	34.21 µs	-12.2%
✅	`matmul/small_generic_f16_threads=8/1x256x256`	38.94 µs	34.04 µs	-12.6%
✅	`add/large_f16_threads=1-internal/4194304`	1.98 ms	1.70 ms	-14.1%
🟢	`add/large_f32_threads=1-internal/4194304`	868.73 µs	718.35 µs	-17.3%
🟢	`gather/medium_bf16_threads=1-internal/32768`	2.98 µs	2.43 µs	-18.5%
🟢	`matmul/small_generic_f16_threads=1/1x256x256`	39.25 µs	31.75 µs	-19.1%
🟢	`gather/large_bf16_threads=1-internal/131072`	15.28 µs	12.32 µs	-19.4%
🟢	`add/small_f32_threads=1-internal/1024`	254.2 ns	204.3 ns	-19.6%
🟢	`gather/small_f16_threads=1-internal/4096`	598.7 ns	479.7 ns	-19.9%
🟢	`gather/medium_f16_threads=1-internal/32768`	3.57 µs	2.84 µs	-20.4%
🟢	`add/small_f16_threads=1-internal/1024`	629.5 ns	488.4 ns	-22.4%
🟢	`matmul/medium_generic_f16_threads=1/32x512x512`	40.80 µs	30.70 µs	-24.8%
🟢	`matmul/small_generic_f32_threads=8/1x256x256`	51.87 µs	38.70 µs	-25.4%
🟢	`reduce_mean/large_f32_threads=1-internal/262144`	1.34 ms	1.00 ms	-25.4%
🟢	`add/large_bf16_threads=1-internal/4194304`	2.48 ms	1.79 ms	-27.8%
🟢	`reduce_mean/small_f32_threads=1-internal/4096`	22.33 µs	16.03 µs	-28.2%
🟢	`gather/large_f16_threads=1-internal/131072`	18.88 µs	13.51 µs	-28.4%
🟢	`matmul/large_generic_f32_threads=8/32x1024x1024`	5.97 ms	4.17 ms	-30.2%
🟢	`reduce_mean/medium_f32_threads=1-internal/65536`	386.21 µs	253.22 µs	-34.4%
🟢	`matmul/medium_generic_f32_threads=8/32x512x512`	2.49 ms	1.04 ms	-58.2%

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.21 3.65 7.08 }

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)

`cargo fmt --check` is a gate in the Fast job, and these two test cases exceeded the line width, which failed formatting and skipped every build, test and clippy step behind it on this branch and the two stacked on it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

…e (replaces #1224) (#1249) Replacement for #1224, rebased onto current `main` after #1197 landed as a squash (the stacked base diverged, so the original went `CONFLICTING`). Same two commits, re-validated on top of the merged #1197. A real agent client driving this server through the OpenAI API exposed two defects that share a cause: the server answered questions the client never asked. ## A reasoning turn was cut off mid-thought OpenAI accepts only `max_completion_tokens` for a reasoning model and rejects `max_tokens`, and the server implemented neither — serde fell back to a fixed 256-token default, which a reasoning model can spend entirely on private thinking, ending the turn before it addresses the caller. `max_tokens` and `max_completion_tokens` are now both optional, the newer field wins, and a request naming neither falls back to the server cap rather than to 256 — what OpenAI means by an unspecified limit. The cap is a ceiling on generosity, not a reservation, so an unnamed budget yields to the prompt (`admit_output_budget`) instead of rejecting every budget-less request on a short-context model; a budget the caller *did* name still reports honestly that it does not fit. ## Private reasoning must not reach the client The model spells a turn as channels addressed to `self` (private), `user` (the answer), or a tool (a call). The buffered path already filtered via `atem_visible_content`; streaming sent tokens as they arrived, and a streamed token has its special tokens stripped, so the markers deciding visibility were invisible token-by-token and private reasoning went out live as content. `PrivateChannelGate` keeps the turn's special-token spelling and asks the same `atem_visible_content` what is visible so far, emitting only the growth — streaming agrees with the buffered path by construction. Two properties: - **Armed by declaration, not guesswork.** The gate is armed from the package's own `response_template` (`declares_private_channels`); a model that declares no private channel is never gated and streams exactly what it generated. - **Fails closed.** A token the gate cannot place is withheld. Also applied to per-token logprob chunks, constrained-JSON completions, and the stop-sequence drain, which previously dropped whatever the gate still held. ## Verification (mine, on this branch atop merged #1197) - **208 lib + 31 http + 11 bundle tests pass**; `cargo clippy -p onnx-genai-server --all-targets -- -D warnings` clean. - **Falsified both claims** (revert hunk → RED → restore → GREEN): - Privacy: forcing the streaming gate to pass tokens through made `reasoning_never_streams` leak `" to=selfsecret plan"` and `only_the_user_channel_streams` leak the `secret` channel; restored, both assert the reasoning is absent. - Budget: reverting the unspecified-budget fallback to 256 made `an_unspecified_budget_is_the_server_cap` return 256 instead of the 4096 cap; restored, GREEN. - **End to end** (`qwen05b-q4`, native): a budget-less request now completes with `finish_reason: stop` (no 256 truncation); streaming for a non-ATEM model streams unchanged (un-gated), confirming no regression for ordinary OpenAI clients; over-cap `max_completion_tokens` is rejected naming the exact field. The ATEM streaming-leak path itself is covered by the unit gate tests above rather than e2e, since that model declares no private channel. 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 · 2026-08-18T14:48:20Z

Superseded by #1249, which is these same two commits rebased onto current main after #1197 landed as a squash (the stacked base diverged, so this PR went CONFLICTING). #1249 was validated (208 lib + 31 http + 11 bundle tests, clippy clean, privacy + budget claims falsified revert->RED->restore->GREEN, and an e2e smoke on qwen05b-q4) and merged as dcf9c0f. Closing in favour of it.

…is silent (replaces #1236) (#1250) Replacement for #1236, rebased onto current `main` after #1197/#1224 landed as squashes (the stacked base diverged). Same fix, plus one added guard test, re-validated on top of the merged stack. ## The defect `temperature` and `top_p` carried the OpenAI schema's documented `1.0` defaults via `#[serde(default = ...)]`, so the server could not tell a caller who *chose* 1.0 from a caller who said nothing. Both were then handed to `resolve_sampling_defaults` as explicit overrides, so a model that declares its own sampling regime lost it on every request. An absent field is "no opinion", not the documented default. ## The fix Both fields become `Option<f32>`; an absent one is passed as `None`, so the package's declared value wins — the same treatment `top_k` already had. An explicit value (including `temperature: 0`, greedy) still wins. Validation (`validate_sampling_range`) now judges only what the caller actually sent. The same fix applies to legacy `/v1/completions`. ## Verification (mine, on this branch atop merged #1197 + #1224) - **211 lib + 31 http + 11 bundle tests pass**; `cargo clippy -p onnx-genai-server --all-targets -- -D warnings` clean. - **Falsified the core claim** (revert hunk → RED → restore → GREEN): reverting `chat_sampling_overrides` to forward `Some(1.0)` for an absent field made `a_silent_request_keeps_the_declared_temperature_and_top_p` see `Some(1.0)` instead of `None`, and `silent_chat_request_honors_model_do_sample` override the declared `0.6` with `1.0`; restored, the declared value survives. - **Absent-vs-default trap, added test:** `an_explicit_default_valued_temperature_still_overrides_the_package` asserts a caller that explicitly sends `temperature: 1.0` (equal to the old schema default) still overrides a package declaring `0.6`. This documents that absence (`None` → package wins) and an explicitly-sent default (`Some(1.0)` → caller wins) are now distinct — the exact distinction this PR exists to make. - **End to end** (`qwen05b-q4`, native): a silent request answers normally; legacy `/v1/completions` at `temperature: 0` is bit-identical across two runs, so the greedy path is untouched; an explicit `temperature: 1.0` is accepted and a negative one rejected. 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

…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 · 2026-08-18T15:13:54Z

Owner has withdrawn the private-reasoning gate added here (reasoning content should now be sent to the client). Important: removing PrivateChannelGate alone only exposes reasoning on the streamed path — the non-streamed/buffered path hides it independently via atem_visible_content from #1197 (ccc24ba9). Full separability map (budget = keep, privacy = reverse), the tests to invert, and the required acceptance tests are in #1256. Read that before starting the reversal.

justinchuby force-pushed the feat/openai-agent-client-compat branch from 846628d to e5507b6 Compare August 18, 2026 06:07

justinchuby and others added 2 commits August 18, 2026 06:10

justinchuby force-pushed the fix/reasoning-turn-budget-and-privacy branch from c4d6cd5 to 6c3ad6f Compare August 18, 2026 06:23

justinchuby mentioned this pull request Aug 18, 2026

fix(server): let a package's declared sampling stand when the caller is silent #1236

Closed

Base automatically changed from feat/openai-agent-client-compat to main August 18, 2026 14:42

justinchuby mentioned this pull request Aug 18, 2026

fix(server): give a reasoning turn room to finish, and keep it private (replaces #1224) #1249

Merged

justinchuby closed this Aug 18, 2026

justinchuby mentioned this pull request Aug 18, 2026

fix(server): let a package's declared sampling stand when the caller is silent (replaces #1236) #1250

Merged

justinchuby mentioned this pull request Aug 18, 2026

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

Merged

This was referenced Aug 18, 2026

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

Closed

Making reasoning visible spans #1197, not just #1224 — the buffered path filters independently #1256

Closed

justinchuby mentioned this pull request Aug 18, 2026

feat(server): report a turn's private thinking on its own channel #1257

Merged

Navigation Menu

Repository navigation

fix(server): give a reasoning turn room to finish, and keep it private - #1224

fix(server): give a reasoning turn room to finish, and keep it private#1224
justinchuby wants to merge 4 commits into
mainfrom
fix/reasoning-turn-budget-and-privacy

justinchuby commented Aug 18, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

justinchuby commented Aug 18, 2026

Uh oh!

justinchuby commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

justinchuby commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A reasoning turn was cut off mid-thought

Private reasoning streamed to the client verbatim

Verification

Uh oh!

codecov Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Aug 18, 2026

⚠️ Benchmark Change Detected

Uh oh!

justinchuby commented Aug 18, 2026

Uh oh!

justinchuby commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

justinchuby commented Aug 18, 2026 •

edited

Loading

codecov Bot commented Aug 18, 2026 •

edited

Loading