Conversation
|
| Filename | Overview |
|---|---|
| api/streaming.py | Adds _route_rejects_reasoning_extra() helper and gates the reasoning extra_body assignment; the logic is sound for the covered provider strings and base URLs, though CHANGELOG.md is missing and provider normalization ordering is a minor untested edge case. |
| tests/test_issue4161_reasoning_extra_provider_gate.py | Comprehensive regression suite covering True/False paths of the new helper and the full generate_title_raw_via_aux call path for OpenAI, Azure, local, OpenRouter, and MiniMax routes; patching strategy correctly targets agent.auxiliary_client._get_auxiliary_task_config via _get_aux_title_config. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[generate_title_raw_via_aux] --> B[Resolve provider / model / base_url]
B --> C{_route_rejects_reasoning_extra}
C -->|True - OpenAI / Azure| D["reasoning_extra = {}"]
C -->|False - local / openrouter / other| E["reasoning_extra = {reasoning: {enabled: false}}"]
D --> F{_is_minimax_route?}
E --> F
F -->|Yes| G["reasoning_extra += {reasoning_split: true}"]
F -->|No| H["extra_body = reasoning_extra or None"]
G --> H
H --> I[call_llm with extra_body]
I --> J{Response OK?}
J -->|Yes| K["Return title, llm_aux"]
J -->|No| L[Retry / fallback]
Reviews (1): Last reviewed commit: "fix(streaming): provider-gate title-gen ..." | Re-trigger Greptile
Review — the gate is correct and the tests pin the right behaviorPulled the branch into a read-only worktree and read the full Code referenceThe gate replaces the unconditional disable with a route check, and crucially flips reasoning_extra = {}
if not _route_rejects_reasoning_extra(provider, model, base_url):
reasoning_extra["reasoning"] = {"enabled": False}
if _is_minimax_route(provider, model, base_url):
reasoning_extra["reasoning_split"] = True
...
extra_body=reasoning_extra or None,The The test matrix is thorough: the allow/deny split covers One inaccuracy in the PR's own risk noteThe "Risks / Follow-ups" section says the agent's fixed_temperature = _fixed_temperature_for_model(model, base_url)
if fixed_temperature is OMIT_TEMPERATURE:
temperature = None # strip — let server choose
...
if temperature is not None:
from agent.anthropic_adapter import _forbids_sampling_params
if _forbids_sampling_params(model):
temperature = NoneThere is no Minor: openai-codex gating is safe but redundant
VerificationThe unit + integration split looks complete for the helper and the patched call path. No execution from my side (review is read-only) — the green suite is on your CI. LGTM once the risk-note wording is corrected. |
…a#4161, nesquena#2083) Consolidates nesquena#4162 + nesquena#3944. Aux path: skip the reasoning-disable inject for routes that 400 on it via _route_rejects_reasoning_extra — hostname-matched OpenAI / Azure OpenAI / Azure AI Foundry (+ provider aliases openai*/azure*/ azure-foundry/azure-ai*) and OpenRouter Anthropic mandatory-reasoning; keep it for local/OpenRouter-non-anthropic/other reasoning routes (nesquena#4161). Agent path: rely on the existing agent.reasoning_config={enabled:False} + _build_api_kwargs() (route-correct per provider profile) rather than re-injecting a generic disable on top of it; only MiniMax reasoning_split is added (unchanged from master). Co-authored-by: Rod Boev <rod.boev@gmail.com>
…a#4161, nesquena#2083) Consolidates nesquena#4162 + nesquena#3944. Aux path: skip the reasoning-disable inject for routes that 400 on it via _route_rejects_reasoning_extra — hostname-matched OpenAI / Azure OpenAI / Azure AI Foundry (+ provider aliases openai*/azure*/ azure-foundry/azure-ai*) and OpenRouter Anthropic mandatory-reasoning; keep it for local/OpenRouter-non-anthropic/other reasoning routes (nesquena#4161). Agent path: rely on the existing agent.reasoning_config={enabled:False} + _build_api_kwargs() (route-correct per provider profile) rather than re-injecting a generic disable on top of it; only MiniMax reasoning_split is added (unchanged from master). Co-authored-by: Rod Boev <rod.boev@gmail.com>
Release NV (v0.51.409): provider-gate title-gen reasoning extra_body (nesquena#4161/nesquena#2083, consolidates nesquena#4162+nesquena#3944)
Fixes #4161
Thinking Path
extra_body={"reasoning": {"enabled": False}}to suppress thinking on reasoning models, but OpenAI Chat Completions rejects thereasoningparameter as unknown, causing a 400 and fallback to low-quality local titles.reasoningextra_body for routes known to reject it (OpenAI, Azure), while preserving it for local endpoints, OpenRouter proxies, and other reasoning-aware providers.What Changed
api/streaming.py: added_route_rejects_reasoning_extra()helper near_is_minimax_route(), and gated thereasoning_extraassignment ingenerate_title_raw_via_aux()behind ittests/test_issue4161_reasoning_extra_provider_gate.py: focused regression tests for the helper and the patched call pathWhy It Matters
Users with OpenAI as their main or auxiliary title-generation provider get silent 400 errors on every new session, falling back to heuristic titles like partial word bags. This fix restores LLM-quality titles for OpenAI routes while preserving the reasoning-disable behavior for local models.
Verification
Full-suite CI context, not a required local check unless requested:
pytest tests/ -v --timeout=60.Risks / Follow-ups
reasoningbut doesn't match the detection patterns would still get the parameter; the agent'sauxiliary_client.pyhandlestemperatureproactively via model-pattern gating (_fixed_temperature_for_modeland_forbids_sampling_params), not a reactive retry-on-400 loop. A defense-in-depth follow-up could add a similar proactive gate forreasoningthere, or introduce a new reactiveunknown_parameterretry mechanism — neither exists today.generate_title_raw_via_agentuses a different mechanism (agent.reasoning_config) that is provider-aware by design, so it is not affected.