fix(qa): prevent grader truncation - #631
Conversation
The QA grader inherits the default 4096 output-token ceiling, which on Gemini is shared with dynamic thinking, so long grading JSON is cut off mid-object, fails to parse, and score/summary come back empty. Thread an optional max_tokens through the LLM factory and have the QA path request a generous ceiling (16384) with a bounded thinking budget on the Google branch, so a full grade completes. The param is optional; non-QA callers are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="api/services/workflow/qa/llm_config.py">
<violation number="1" location="api/services/workflow/qa/llm_config.py:54">
P2: max_tokens is threaded unconditionally from the QA path for every provider, but create_llm_service_from_provider only honors it in the GOOGLE branch (service_factory.py ~1005). The GOOGLE_VERTEX and Anthropic branches accept the new parameter and silently ignore it. The comment added next to QA_MAX_OUTPUT_TOKENS explicitly notes that Anthropic also defaults to 4096 output tokens, so an Anthropic (or Vertex) QA grader will still hit the same mid-object truncation that this PR is fixing, while the caller believes a generous ceiling was requested. Consider applying the output ceiling on the Anthropic branch (and Vertex if it shares the same cap) or scoping the QA flag to the provider actually covered, so the fix is not silently ineffective for other providers.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| api_key, | ||
| correlation_id=correlation_id, | ||
| usage_context=QA_USAGE_CONTEXT, | ||
| max_tokens=QA_MAX_OUTPUT_TOKENS, |
There was a problem hiding this comment.
P2: max_tokens is threaded unconditionally from the QA path for every provider, but create_llm_service_from_provider only honors it in the GOOGLE branch (service_factory.py ~1005). The GOOGLE_VERTEX and Anthropic branches accept the new parameter and silently ignore it. The comment added next to QA_MAX_OUTPUT_TOKENS explicitly notes that Anthropic also defaults to 4096 output tokens, so an Anthropic (or Vertex) QA grader will still hit the same mid-object truncation that this PR is fixing, while the caller believes a generous ceiling was requested. Consider applying the output ceiling on the Anthropic branch (and Vertex if it shares the same cap) or scoping the QA flag to the provider actually covered, so the fix is not silently ineffective for other providers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/services/workflow/qa/llm_config.py, line 54:
<comment>max_tokens is threaded unconditionally from the QA path for every provider, but create_llm_service_from_provider only honors it in the GOOGLE branch (service_factory.py ~1005). The GOOGLE_VERTEX and Anthropic branches accept the new parameter and silently ignore it. The comment added next to QA_MAX_OUTPUT_TOKENS explicitly notes that Anthropic also defaults to 4096 output tokens, so an Anthropic (or Vertex) QA grader will still hit the same mid-object truncation that this PR is fixing, while the caller believes a generous ceiling was requested. Consider applying the output ceiling on the Anthropic branch (and Vertex if it shares the same cap) or scoping the QA flag to the provider actually covered, so the fix is not silently ineffective for other providers.</comment>
<file context>
@@ -47,6 +51,7 @@ async def create_qa_llm_service(
api_key,
correlation_id=correlation_id,
usage_context=QA_USAGE_CONTEXT,
+ max_tokens=QA_MAX_OUTPUT_TOKENS,
**kwargs,
)
</file context>
thinking_budget is only valid for Gemini 2.5 — Gemini 3 uses thinking_level and legacy/custom models may reject a thinking config, which could make the QA grader fail instead of returning a score. Select the compatible knob per model family (2.5 -> budget, 3 -> level, else omit) via _google_thinking_for_model, and propagate max_tokens + the guarded thinking into the Google Vertex branch, which previously kept the default 4096 output cap and could still truncate. Addresses the greptile/cubic review on b69119b. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="api/services/pipecat/service_factory.py">
<violation number="1" location="api/services/pipecat/service_factory.py:945">
P1: QA grading still sends `thinking_level`/`thinking_budget` to custom or future model IDs containing these family substrings, so incompatible Google models can reject the request. Use an explicit capability allowlist (including supported revisions) and return `None` for every unrecognized ID.</violation>
</file>
<file name="api/tests/test_google_thinking_config.py">
<violation number="1" location="api/tests/test_google_thinking_config.py:15">
P2: These tests only call the isolated helper `_google_thinking_for_model`, so they never validate the actual QA wiring the PR is supposed to guard: that `max_tokens=16384` flows through `create_llm_service_from_provider` and attaches the thinking config to the `GoogleLLMSettings`/`GoogleVertexLLMSettings`, or that those settings serialize without the grader rejecting the request (the stated P1). The PR review-focus explicitly asked for a legacy/custom model at QA max_tokens=16384 and a Gemini 2.5 serialization test; consider adding one that constructs the settings with `max_tokens` + thinking and serializes them.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ``max_tokens`` ceiling alone protects the grader output from truncation. | ||
| """ | ||
| ml = (model or "").lower() | ||
| if "gemini-2.5" in ml or "gemini-2-5" in ml: |
There was a problem hiding this comment.
P1: QA grading still sends thinking_level/thinking_budget to custom or future model IDs containing these family substrings, so incompatible Google models can reject the request. Use an explicit capability allowlist (including supported revisions) and return None for every unrecognized ID.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/services/pipecat/service_factory.py, line 945:
<comment>QA grading still sends `thinking_level`/`thinking_budget` to custom or future model IDs containing these family substrings, so incompatible Google models can reject the request. Use an explicit capability allowlist (including supported revisions) and return `None` for every unrecognized ID.</comment>
<file context>
@@ -933,6 +933,22 @@ def _migrate_deprecated_google_model(model: str) -> str:
+ ``max_tokens`` ceiling alone protects the grader output from truncation.
+ """
+ ml = (model or "").lower()
+ if "gemini-2.5" in ml or "gemini-2-5" in ml:
+ return GoogleLLMService.ThinkingConfig(thinking_budget=4096)
+ if "gemini-3" in ml:
</file context>
| def test_gemini_25_uses_thinking_budget(): | ||
| tc = _google_thinking_for_model("gemini-2.5-flash") | ||
| assert tc is not None | ||
| assert tc.thinking_budget == 4096 |
There was a problem hiding this comment.
P2: These tests only call the isolated helper _google_thinking_for_model, so they never validate the actual QA wiring the PR is supposed to guard: that max_tokens=16384 flows through create_llm_service_from_provider and attaches the thinking config to the GoogleLLMSettings/GoogleVertexLLMSettings, or that those settings serialize without the grader rejecting the request (the stated P1). The PR review-focus explicitly asked for a legacy/custom model at QA max_tokens=16384 and a Gemini 2.5 serialization test; consider adding one that constructs the settings with max_tokens + thinking and serializes them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/tests/test_google_thinking_config.py, line 15:
<comment>These tests only call the isolated helper `_google_thinking_for_model`, so they never validate the actual QA wiring the PR is supposed to guard: that `max_tokens=16384` flows through `create_llm_service_from_provider` and attaches the thinking config to the `GoogleLLMSettings`/`GoogleVertexLLMSettings`, or that those settings serialize without the grader rejecting the request (the stated P1). The PR review-focus explicitly asked for a legacy/custom model at QA max_tokens=16384 and a Gemini 2.5 serialization test; consider adding one that constructs the settings with `max_tokens` + thinking and serializes them.</comment>
<file context>
@@ -0,0 +1,29 @@
+def test_gemini_25_uses_thinking_budget():
+ tc = _google_thinking_for_model("gemini-2.5-flash")
+ assert tc is not None
+ assert tc.thinking_budget == 4096
+ assert getattr(tc, "thinking_level", None) is None
+
</file context>
…iring Match the thinking-config model family by prefix instead of substring so ids that merely embed 'gemini-2.5'/'gemini-3' (custom/legacy/future) fall through to None and never receive an unsupported thinking config. Add a test that builds the real GoogleLLMSettings and GoogleVertexLLMSettings with QA max_tokens=16384 plus the guarded thinking (2.5 attaches, legacy omits). Addresses the follow-up greptile/cubic review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The QA grader inherits the default 4096 output-token ceiling, which on Gemini is shared with dynamic thinking, so long grading JSON is cut off mid-object, fails to parse, and score/summary come back empty.
Thread an optional max_tokens through the LLM factory and have the QA path request a generous ceiling (16384) with a bounded thinking budget on the Google branch, so a full grade completes. The param is optional; non-QA callers are unaffected.
Fixes #636
Summary by cubic
Fixes QA grader truncation on Gemini by raising the output token limit and using model-compatible thinking settings, now applied to both Google and Vertex paths. Adds stricter prefix-based model matching to avoid unsupported configs; only the QA path opts in.
max_tokensthrough LLM factory methods.max_tokensand choose thinking by model-family prefix (2.5 ->thinking_budget=4096, 3 ->thinking_level="low", others omit); covered by tests, including settings wiring forGoogleLLMSettingsandGoogleVertexLLMSettings.QA_MAX_OUTPUT_TOKENS=16384and passes it withusage_context="qa_analysis".Written for commit 2464dd2. Summary will update on new commits.
Greptile Summary
QA grading now carries an explicit output-token limit through the LLM factory and uses thinking configuration only for supported Gemini model families. The previously reported custom-model failure was disproved: Google and Vertex requests for legacy or custom model IDs retain the output limit while omitting unsupported thinking settings.
Confidence Score: 5/5
No blocking failure remains.
Runtime checks exercised Google and Vertex factory behavior for supported, legacy, and custom model identifiers, confirming that incompatible thinking configuration is not attached.
What T-Rex did
Reviews (3): Last reviewed commit: "fix(qa): tighten Google model-family mat..." | Re-trigger Greptile