fix(judge): declare reasoning off on tool-carrying calls to reasoning models

[drewdrewthis]

Why

Closes #864. Platform-side epic: langwatch/langwatch#6594 (sibling PR langwatch/langwatch#6607 applies the same design to the platform's judge model client).

Some reasoning models reject requests that carry function tools unless reasoning_effort is "none":

litellm.BadRequestError: OpenAIException - Function tools with
reasoning_effort are not supported for <model> in /v1/chat/completions.
To use function tools, use /v1/responses or set reasoning_effort to 'none'.

The judge always sends tools, so a criteria-graded run against such a model failed with a provider 400 instead of returning a verdict. Verified live: the same request answers 400 without reasoning_effort and 200 + a finish_test tool call with reasoning_effort="none".

What changed

Reasoning is disabled by retry, never preemptively. The first design on this branch gated a preemptive reasoning_effort="none" on litellm.supports_reasoning, and this repo's own CI falsified it: the Gemini judge example failed with "Budget 0 is invalid. This model only works in thinking mode." — Gemini 2.5 Pro accepts tool-carrying requests but refuses to disable reasoning. "Supports reasoning_effort" and "accepts reasoning off" are different questions, and only the provider answers the second one.

• judge_agent.py: _completion_with_reasoning_off_retry sends every litellm.completion call untouched; when a tool-carrying call with no caller-set reasoning_effort raises a rejection carrying the remediation directive set reasoning_effort to 'none', it retries once with reasoning off. Wired at all three call sites (standard call, discovery loop, forced verdict). Unrelated rejections surface unchanged; a caller-set effort is never rewritten; models that work today are never sent anything new.
• examples/test_audio_to_text.py: removed the CI skip. This example exercises exactly this path and would have caught the regression; skipped, the class recurs silently (per the epic's sequencing note).

Test plan

• tests/test_judge_transport_reasoning.py — 7 offline deterministic tests: rejected call retried once with reasoning off and reaches a verdict; a caller-set effort surfaces the provider's own error; an accepting (Gemini-shaped) judge gets exactly one untouched request; an unrelated 400 is not retried; a rejection that merely mentions reasoning_effort and 'none' without the directive (e.g. "reasoning_effort 'none' is invalid for this model") surfaces instead of retrying; the discovery-loop and forced-verdict call sites each retry. Falsifiability: with judge_agent.py reverted to main, 3 of them fail; the directive-vs-mention test was proven red against the earlier two-token matcher.
• The Gemini judge example (examples/test_weather_agent__gemini.py) is the live regression gate for the retry design itself — it is what failed against the first design.

Human verification

Nothing UI to look at — this is a Python SDK transport change. If you want to see it live: run examples/test_weather_agent__gemini.py (thinking-only model, must never be asked to disable reasoning) and examples/test_audio_to_text.py (the un-skipped example that exercises the rejection-retry path); both run in this repo's CI against real providers and are green at HEAD.

How I can prove I was successful

• CI at HEAD (5b7c262) is fully green, 14/14 checks, including the real-LLM examples job — the same job that falsified the first (preemptive) design same-day.
• The offline suite fails 3/7 with the fix reverted to main, and the directive-vs-mention regression test fails against the previous looser matcher — both reverts were actually run, not reasoned about.
• No capability oracle remains: grep -r supports_reasoning python/scenario/ is empty.

Anything surprising?

The rejection is matched on the remediation directive substring set reasoning_effort to 'none' rather than a structured field: litellm re-wraps provider errors and the OpenAI param field does not survive uniformly. Matching the directive (not just the two tokens) keeps errors like "reasoning_effort 'none' is invalid for this model" — which mention both tokens but are not asking us to turn reasoning off — surfacing instead of looping. The platform sibling keys on error.param first, because it reads the raw wire body.

[coderabbitai]

Walkthrough

The judge retries matching tool-call provider rejections with reasoning_effort="none". Tests cover all judge call paths and error cases. The live audio-to-text end-to-end test now runs in CI.

Changes

Judge reasoning retry

Layer / File(s)	Summary
Targeted completion retry python/scenario/judge_agent.py	The judge detects the specific provider rejection and retries standard, discovery, and forced-verdict completions with reasoning disabled.
Retry behavior regression coverage python/tests/test_judge_transport_reasoning.py	Offline tests cover retry, explicit reasoning settings, accepted requests, unrelated errors, discovery, and forced-verdict paths.

Live audio CI coverage

Layer / File(s)	Summary
Live audio-to-text CI execution python/examples/test_audio_to_text.py	The live end-to-end test no longer skips in CI. Comments identify the offline transport test as the deterministic test.

Sequence Diagram(s)

sequenceDiagram
  participant JudgeAgent
  participant LiteLLM
  participant Provider
  JudgeAgent->>LiteLLM: Send tool-call completion
  LiteLLM->>Provider: Forward request
  Provider-->>LiteLLM: Reject missing reasoning_effort none
  LiteLLM-->>JudgeAgent: Return provider rejection
  JudgeAgent->>LiteLLM: Retry with reasoning_effort none
  LiteLLM-->>JudgeAgent: Return completion

Loading

Suggested labels: review: targeted

Poem

A rabbit checks the retry call,
Tools hop past the blocking wall.
“None,” says the model, then replies,
While audio tests wake CI skies.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The description clearly explains the reasoning retry behavior, affected call sites, regression tests, and CI example change.
Title check	✅ Passed	The title clearly identifies the main change: disabling reasoning on tool-carrying judge calls when providers require it.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue864/judge-reasoning-transport

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

python/tests/test_judge_transport_reasoning.py (1)

182-187: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Declare _TOOLS as typed class state.

Ruff reports RUF012 because _TOOLS is a mutable class attribute. Add a ClassVar annotation, or create a fresh tool list per test. This also satisfies the required class-attribute type annotation.

Proposed fix

-from typing import Any, Optional
+from typing import Any, ClassVar, Optional
@@
-    _TOOLS = [
+    _TOOLS: ClassVar[list[dict[str, object]]] = [

As per coding guidelines, “Always use explicit type annotations for function parameters, return types, and class attributes in Python.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/tests/test_judge_transport_reasoning.py` around lines 182 - 187,
Annotate the mutable class attribute `_TOOLS` with `ClassVar` and its
appropriate list type, adding the required typing import if needed. Keep the
existing tool definition and class-level reuse unchanged.

Sources: Coding guidelines, Linters/SAST tools

python/scenario/judge_agent.py (1)

59-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Declare module constants as Final.

• python/scenario/judge_agent.py#L59-L59: declare _REASONING_OFF as Final[str].
• python/tests/test_judge_transport_reasoning.py#L41-L45: declare _REJECTION_MESSAGE as Final[str].

As per coding guidelines, “Use Final for constants that should not be reassigned in Python.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/scenario/judge_agent.py` at line 59, Declare the module constant
_REASONING_OFF in python/scenario/judge_agent.py (lines 59-59) as Final[str],
and declare _REJECTION_MESSAGE in python/tests/test_judge_transport_reasoning.py
(lines 41-45) as Final[str], adding or reusing the appropriate Final import in
each file.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@python/scenario/judge_agent.py`:
- Around line 67-68: Update the rejection-detection helper around the visible
message check to require both the function-tool rejection text and the directive
to set reasoning_effort to 'none', rather than matching only the field and
value. Preserve unrelated reasoning_effort validation errors so they are not
retried, and add a regression case covering both tokens without the
function-tool rejection.

In `@python/tests/test_judge_transport_reasoning.py`:
- Around line 97-112: Update the test helper containing the _call_judge flow to
capture the existing ScenarioConfig.default_config before execution, then
restore that saved value in the finally block instead of always assigning None.
Preserve the current context_scenario reset and mock behavior.

---

Nitpick comments:
In `@python/scenario/judge_agent.py`:
- Line 59: Declare the module constant _REASONING_OFF in
python/scenario/judge_agent.py (lines 59-59) as Final[str], and declare
_REJECTION_MESSAGE in python/tests/test_judge_transport_reasoning.py (lines
41-45) as Final[str], adding or reusing the appropriate Final import in each
file.

In `@python/tests/test_judge_transport_reasoning.py`:
- Around line 182-187: Annotate the mutable class attribute `_TOOLS` with
`ClassVar` and its appropriate list type, adding the required typing import if
needed. Keep the existing tool definition and class-level reuse unchanged.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 33645a9b-d549-492b-88bb-7f81e8b438f5

📥 Commits

Reviewing files that changed from the base of the PR and between 32abdb1 and 14de69d.

📒 Files selected for processing (3)

• python/examples/test_audio_to_text.py
• python/scenario/judge_agent.py
• python/tests/test_judge_transport_reasoning.py

[drewdrewthis]

Review verdict: READY

Reviewed at: 5b7c262 · Run: pr-review (reviewer: @you)

No blocking concerns. All review threads resolved at this SHA.

Verification performed this run

• CI enumerated check-by-check (gh pr checks 878): 17 checks, all pass or legitimately skipping (conditional jobs), zero failures — including the real-LLM test (3.12) job's Test (Examples) step. Confirmed from the raw run log that examples/test_audio_to_text.py::test_audio_to_text actually executed (no longer skipped) and PASSED against real credentials, and that examples/test_weather_agent__gemini.py (the live regression gate the PR body cites) ran and PASSED. All 7 new tests in tests/test_judge_transport_reasoning.py ran and PASSED.
• CodeRabbit thread resolution claim verified independently via the GraphQL reviewThreads API: 3 threads (2 CodeRabbit functional-correctness + 1 code-quality unused-import), all isResolved: true, each with an explicit @drewdrewthis, confirmed reply from the bot.
• Own two-pass review (line-by-line + structural) of judge_agent.py, test_audio_to_text.py, and the new test file: the retry gate (not kwargs.get("tools") or "reasoning_effort" in kwargs) correctly no-ops for non-tool calls and never overwrites a caller-set effort; the rejection matcher is keyed on the remediation directive (not just field-name substring), matching the already-resolved CodeRabbit finding; all 3 litellm.completion call sites (standard, discovery loop, forced verdict) are wired through the same retry helper and each has a dedicated pinning test (TestEveryCallSiteThatSendsTools); no unused imports, no multitenancy/PII/secret concerns (this is a Python SDK transport fix, no tenant-scoped data involved); no reinvention of an existing litellm/provider retry primitive.

No new acceptance criteria or blocking findings from this pass.

---

Verdict is prose, not a GitHub approval. The pr-review procedure never flips approve state.

[drewdrewthis]

Full review complete — verdict: READY. No blocking findings, no inline threads needed.

Verified independently (not taken on the PR description's word):

• CI: gh pr checks 878 enumerated all 17 checks row-by-row — pass/skipping, zero failures. Pulled the raw test (3.12) job log directly and confirmed examples/test_audio_to_text.py::test_audio_to_text actually executed (un-skipped) and PASSED against real OpenAI credentials, examples/test_weather_agent__gemini.py (the PR's cited live regression gate) ran and PASSED, and all 7 new tests in tests/test_judge_transport_reasoning.py PASSED.
• CodeRabbit: queried reviewThreads via GraphQL directly rather than trusting the resolved-status claim — 3 threads (2 functional-correctness + 1 unused-import), all isResolved: true with explicit bot confirmation replies.

Own two-pass review of judge_agent.py, the un-skip in test_audio_to_text.py, and the new test file:

• Retry gate if not kwargs.get("tools") or "reasoning_effort" in kwargs: raise correctly no-ops for tool-less calls and never overwrites a caller-set reasoning_effort — confirmed by test_preserves_an_explicitly_requested_effort.
• _rejection_asks_for_reasoning_off matches on the full remediation directive ("set reasoning_effort to 'none'"), not just field-name substring — this is the fix for the already-resolved CodeRabbit finding on the same line, confirmed still present at HEAD.
• All 3 litellm.completion call sites (call, _run_discovery_loop, _force_verdict) are wired through _completion_with_reasoning_off_retry, each independently pinned by TestEveryCallSiteThatSendsTools so an unwired site can't pass by way of the others.
• No unused imports left behind by the skip removal (os and pytest both still used elsewhere in the file). No multitenancy/PII/secret concerns — pure SDK transport logic, no tenant-scoped data. No reinvention of an existing litellm/provider retry primitive (Pass 3 check).

See the upserted verdict comment for the full writeup: #878 (comment)

[github-actions]

Automated low-risk assessment

This PR was evaluated against the repository's Low-Risk Pull Requests procedure and does not qualify as low risk.

The PR changes runtime behavior of the judge by retrying litellm.completion calls with reasoning_effort="none" based on provider error text, which alters integration behavior with external LLM providers and the judge's verdict logic. Because this is a change to third‑party API interaction and business‑critical agent behavior (not just docs/tests/UI), it does not meet the low‑risk criteria.

This PR requires a manual review before merging.