Skip to content

[fix] handle OpenAI thinking parameter compatibility #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ice789ice wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[fix] handle OpenAI thinking parameter compatibility #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions backend/app/services/llm/resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from app.models.llm import Model, ModelCategoryKey, ModelSettings, Provider
from app.services.common import entity_not_found
from app.services.llm.provider_resolver import resolve_effective_base_url
from app.services.llm.provider_registry import resolve_provider_key_from_name


def _settings_model_id(settings_row: ModelSettings | None, category: ModelCategoryKey) -> str | None:
Expand Down Expand Up @@ -178,8 +179,13 @@ def _build_chat_openai_model(
kwargs.setdefault("base_url", base_url)

if not thinking:
extra_body = dict(kwargs.get("extra_body") or {})
extra_body["enable_thinking"] = False
kwargs["extra_body"] = extra_body
provider_key = resolve_provider_key_from_name(provider.name)
Comment on lines 181 to +182

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 13, 2026

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 Document the changed LLM compatibility logic

This adds non-obvious, provider-specific parameter behavior to _build_chat_openai_model, but the function still has no documentation explaining what it constructs or why this compatibility branch exists; the changed sync constructor is likewise undocumented. Add synchronized function documentation for both builders so the required provider behavior and its rationale remain maintainable.

AGENTS.md reference: AGENTS.md:L11-L14

Useful? React with 👍 / 👎.


if provider_key == "openai":
kwargs.setdefault("reasoning_effort", "none")

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 13, 2026

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 Override configured reasoning effort when disabling thinking

When an OpenAI model's configurable model.params already contains reasoning_effort such as "high", setdefault preserves that value, so all thinking=False callers—including script division, extraction, and frame-prompt generation—continue to use reasoning rather than disabling it. This branch should force "none", as the non-OpenAI branch already forces its toggle, and the mirrored sync implementation should be changed consistently.

Useful? React with 👍 / 👎.

else:
extra_body = dict(kwargs.get("extra_body") or {})
extra_body["enable_thinking"] = False
kwargs["extra_body"] = extra_body

return ChatOpenAI(**kwargs)
12 changes: 9 additions & 3 deletions backend/app/services/llm/runtime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

from app.models.llm import Model, ModelCategoryKey, ModelSettings, Provider
from app.services.llm.provider_resolver import resolve_effective_base_url
from app.services.llm.provider_registry import resolve_provider_key_from_name


def _default_model_id(settings_row: ModelSettings | None, category: ModelCategoryKey) -> str | None:
Expand Down Expand Up @@ -69,8 +70,13 @@ def build_default_text_llm_sync(
kwargs.setdefault("base_url", base_url)

if not thinking:
extra_body = dict(kwargs.get("extra_body") or {})
extra_body["enable_thinking"] = False
kwargs["extra_body"] = extra_body
provider_key = resolve_provider_key_from_name(provider.name)

if provider_key == "openai":
kwargs.setdefault("reasoning_effort", "none")
else:
extra_body = dict(kwargs.get("extra_body") or {})
extra_body["enable_thinking"] = False
kwargs["extra_body"] = extra_body

return ChatOpenAI(**kwargs)
5 changes: 3 additions & 2 deletions backend/tests/test_llm_resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,10 @@ def __init__(self, **kwargs): # noqa: ANN003, ANN204

assert isinstance(thinking_llm, FakeChatOpenAI)
assert "extra_body" not in thinking_llm.kwargs

assert isinstance(nothinking_llm, FakeChatOpenAI)
assert nothinking_llm.kwargs["extra_body"]["enable_thinking"] is False
assert nothinking_llm.kwargs["reasoning_effort"] == "none"
assert "extra_body" not in nothinking_llm.kwargs

await engine.dispose()

Expand All @@ -229,4 +231,3 @@ def test_resolve_effective_base_url_prefers_category_specific_url() -> None:
resolve_effective_base_url(provider=provider, category=ModelCategoryKey.video)
== "https://video-gateway.example/v1"
)