Skip to content

[staging CI] unslothai/unsloth#8665 - #299

Open
danielhanchen wants to merge 7 commits into
mainfrom
pr-8665-xplat-ci
Open

[staging CI] unslothai/unsloth#8665#299
danielhanchen wants to merge 7 commits into
mainfrom
pr-8665-xplat-ci

Conversation

@danielhanchen

Copy link
Copy Markdown
Collaborator

Disposable CI run for unslothai/unsloth#8665. Do not merge; closed after CI.

shimmyshimmer and others added 7 commits August 13, 2026 10:21
Studio's tools execute on the Studio host, not on the model, but only one
external provider could use them: the gate was a hardcoded
provider_type == "openai_codex" in routes/inference.py and again in
research_runs.py. Point Studio at your own llama.cpp, vLLM, Ollama or
OpenRouter and Search, Code, MCP, Docs and Deep Research all greyed out.

Generalise the capability instead of adding a second implementation:

- providers.py declares studio_tools per provider type and
  provider_runs_local_tools() replaces the hardcoded comparison. Hidden
  entries now reach /registry flagged rather than filtered out, which is
  why the UI could never learn the self-hosted presets are capable.
- The loop moves to core/inference/studio_tool_loop.py behind a Transport
  protocol. CodexTransport keeps the app-server call; OAICompatTransport
  wraps ExternalProviderClient.stream_chat_completion, the one function
  every provider re-yields through.
- Self-hosted models often write a call as text instead of emitting
  structured tool_calls, so the loop feeds content through
  StreamToolCallHealer, the same bounded buffer the client-tool
  passthrough uses. Only a partial-signal window is ever withheld and an
  unparseable block flushes verbatim, so a turn that never terminates
  renders its text instead of nothing.
- Deep Research accepts any capable saved connection. Research hops keep
  tool_choice "none" and enabled_tools [], so a prompt-injected page in
  the evidence set still cannot reach python.
- response_format now reaches OpenAI-compatible providers at all; JSON
  mode was silently dropped for every one of them.

Anthropic is deliberately excluded: _stream_anthropic never forwards a
caller's function-tool schemas, so the loop would advertise a catalog the
model never sees. It needs schema plus tool_use/tool_result translation.

The local, GGUF, safetensors and MLX paths are untouched -- every hunk in
routes/inference.py is inside _proxy_to_external_provider. Codex behaviour
is unchanged, and the healer is only constructed when a tool catalog is
actually selected.

Fixes #7282. Fixes #7761.

Also fixes a live bug: chat-runtime-store disarmed Deep Research for any
external model id including openai_codex, which the backend and the menu
both permit, so Codex users had to re-enable it after every model switch.
The first cut generalised the Codex loop, which never had to deal with a
model that writes a call as text, repeats one, or stalls. Everything the
llama.cpp and safetensors loops learned about that is now shared here,
taken from the work in #8630:

- ToolLoopController owns dedup, one-shot withdrawal and the
  force-final-answer transition, so a repeated call cannot spend the
  budget twice.
- stream_tool_execution streams live stdout with heartbeats, so a long
  python or terminal call cannot idle the stream out.
- The approval card is flushed on its own write before the stream blocks,
  so Allow and Deny paint immediately in the desktop app.
- A stalled model that says what it will do gets one nudge to do it.
- Usage is summed and reported once instead of a burst of partial counts.
- append_assistant_turn merges a continued partial, and replayed content
  is stripped of markup since the call replays structurally.

Fixes found by testing the above:

- An intermediate turn's [DONE] was relayed, so a spec-compliant client
  stopped before the tool cards and the final answer.
- A structured call was dropped unless finish_reason was exactly
  tool_calls. Ollama and several proxies close with stop, or omit it.
  A truncated turn (finish_reason length) still never executes.
- Argument fragments without an index started a phantom call and ran the
  real one with empty arguments.
- Two distinct calls at one index merged into unparseable JSON.
- max_tool_calls_per_message capped what was advertised, not what ran: a
  provider that called anyway was executed. It is a safety limit, so it
  now gates execution, per call rather than per turn.
- Provider turns are bounded, not just executions. A model asking for
  tools that can never run cannot trade turns forever.
- Call ids are unique for the whole run, so a provider that restarts its
  numbering cannot put two results under one id.
- An empty tool_calls list no longer counts as grammar mode working and
  disables healing for the rest of the turn.
- A call with no id is given one instead of being silently dropped.
- The provider stream is closed from this frame rather than left to the
  async-generator finalisation hook.
- Content parts reach the replayed assistant message, not just the client.

Compatibility: /api/providers/registry hides the backend-only entries
again by default and takes include_hidden=true, so a browser holding a
pre-capability bundle sees exactly the list it saw before instead of
rendering the self-hosted presets as duplicate options.

Also fixes the frontend build: thread.tsx used
providerModelSupportsStudioTools without importing it.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97c03aa612

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

tool_choice: Optional[Any] = None,
fast_mode: Optional[bool] = None,
continue_final_message: Optional[bool] = None,
response_format: Optional[dict[str, Any]] = None,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Forward response_format through provider-specific transports

When response_format is supplied for OpenAI cloud or native Gemini, this parameter is accepted here but silently discarded because both providers return through _stream_openai_responses or _stream_gemini before the generic body adds it. This breaks structured-output requests and the newly enabled Deep Research flow for these providers, whose planning and decision phases depend on JSON mode; translate and forward the format into each provider-specific request.

Useful? React with 👍 / 👎.

Comment on lines +656 to +660
tool_messages.append(
{
"role": "tool",
"tool_call_id": call["id"],
"name": call["function"]["name"],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Replay budget-rejected calls in the assistant message

When one provider turn emits more parallel calls than the remaining budget, earlier calls consume the budget and this branch appends a role="tool" result for each rejected call, but those calls are never added to assistant_tool_calls. The next provider request therefore contains an orphan tool result with no matching assistant call, which strict Chat Completions, OpenAI Responses, and Gemini transports reject instead of producing the final answer.

Useful? React with 👍 / 👎.

Comment on lines +622 to +627
if (
tools_available
and not controller.force_final_answer
and reprompts < max_reprompts
and is_short_intent_without_action(visible_answer)
and not is_reprompt_repeat(visible_answer, last_reprompt_text)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor the per-request tool-call nudge setting

Whenever an external or refactored Codex turn returns a short intent without a call, this condition nudges unconditionally even when the request explicitly sends nudge_tool_calls: false. The frontend now forwards that user setting, but it is absent from ToolLoopPolicy, so disabling nudges still causes an extra billed provider turn and may prompt an action the user chose not to encourage; gate this branch with the same nudge_enabled semantics used by the other loops.

Useful? React with 👍 / 👎.

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.

2 participants