test(reborn): add semantic judge fallback to live QA - #5531
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an optional semantic judge to live QA reply validation, with workflow defaults, runner wiring, and tests for the judge path and helper behavior. ChangesSemantic Judge Integration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces an LLM-based semantic judge to evaluate assistant replies in the live QA script when exact literal text matches fail, adding helper functions and corresponding unit tests. The review feedback suggests improving error handling: first, by moving the LLM response extraction inside the try-except block to prevent crashes on unexpected response structures, and second, by adding type checks and catching general exceptions in _parse_json_object to handle non-string inputs robustly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| try: | ||
| import httpx | ||
|
|
||
| async with httpx.AsyncClient(timeout=30.0) as client: | ||
| response = await client.post( | ||
| f"{base_url}/chat/completions", | ||
| headers={ | ||
| "Authorization": f"Bearer {api_key}", | ||
| "Content-Type": "application/json", | ||
| }, | ||
| json=payload, | ||
| ) | ||
| response.raise_for_status() | ||
| body = response.json() | ||
| except Exception as exc: | ||
| return {"enabled": True, "error": str(exc)} | ||
| content = ( | ||
| (((body.get("choices") or [{}])[0].get("message") or {}).get("content") or "") | ||
| if isinstance(body, dict) | ||
| else "" | ||
| ) |
There was a problem hiding this comment.
The extraction of content from the response body is performed outside of the try-except block. If the response structure from the LLM provider is unexpected (e.g., if choices is not a list or contains non-dictionary elements), nested dictionary lookups like body.get("choices")[0].get("message") can raise AttributeError or TypeError. Moving this extraction inside the try block ensures that any unexpected response structure is caught gracefully and returned as an error instead of crashing the live QA run.
try:
import httpx
async with httpx.AsyncClient(timeout=30.0) as client:
response = await client.post(
f"{base_url}/chat/completions",
headers={
"Authorization": f"Bearer {api_key}",
"Content-Type": "application/json",
},
json=payload,
)
response.raise_for_status()
body = response.json()
content = (
(((body.get("choices") or [{}])[0].get("message") or {}).get("content") or "")
if isinstance(body, dict)
else ""
)
except Exception as exc:
return {"enabled": True, "error": str(exc)}| def _parse_json_object(text: str) -> object: | ||
| try: | ||
| return json.loads(text) | ||
| except json.JSONDecodeError: | ||
| start = text.find("{") | ||
| end = text.rfind("}") | ||
| if start == -1 or end <= start: | ||
| return None | ||
| try: | ||
| return json.loads(text[start : end + 1]) | ||
| except json.JSONDecodeError: | ||
| return None |
There was a problem hiding this comment.
The _parse_json_object function only catches json.JSONDecodeError. If text is not a string (e.g., if the LLM response content is parsed as a non-string type like an integer or boolean), json.loads will raise a TypeError, and calling .find() or .rfind() on it will raise an AttributeError. Adding a type check and catching general exceptions ensures the function is robust against unexpected input types.
| def _parse_json_object(text: str) -> object: | |
| try: | |
| return json.loads(text) | |
| except json.JSONDecodeError: | |
| start = text.find("{") | |
| end = text.rfind("}") | |
| if start == -1 or end <= start: | |
| return None | |
| try: | |
| return json.loads(text[start : end + 1]) | |
| except json.JSONDecodeError: | |
| return None | |
| def _parse_json_object(text: str) -> object: | |
| if not isinstance(text, str): | |
| return None | |
| try: | |
| return json.loads(text) | |
| except Exception: | |
| start = text.find("{") | |
| end = text.rfind("}") | |
| if start == -1 or end <= start: | |
| return None | |
| try: | |
| return json.loads(text[start : end + 1]) | |
| except Exception: | |
| return None |
Reborn integration-tier coverageLine coverage (Reborn crates): 15.11% — 9538 / 63132 lines Per-crate breakdown (12 crates, lowest-covered first)
This signal is informational: coverage never gates the PR — not the percentage, not the per-crate holes, not the 0-coverage callout. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/reborn_webui_v2_live_qa/test_run_live_qa.py (1)
660-727: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo integration test covers the judge-rejects path.
Both new tests cover the judge-approves path and the marker-short-circuit path, but nothing drives
_wait_for_assistant_replyend-to-end through a judge rejection (completed=False, orcompleted=Truewith confidence below threshold) to confirm the resultingAssertionErroractually surfacessemantic_judge=...in its message._semantic_judge_passeditself is unit-tested (Line 729-748), but the integration wiring at theAssertionErrorcall site isn't.🤖 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 `@scripts/reborn_webui_v2_live_qa/test_run_live_qa.py` around lines 660 - 727, Add an integration-style test for the judge-rejects path in _wait_for_assistant_reply: stub _judge_assistant_reply_completion to return a rejection (either completed=False or confidence below the pass threshold) and verify the resulting AssertionError from _wait_for_assistant_reply includes semantic_judge=... in its message. Reuse the existing test helpers like _fake_assistant_reply_page and patch asyncio.sleep, and place the new coverage alongside the current _wait_for_assistant_reply tests so the rejection behavior is exercised end-to-end.
🤖 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 `@scripts/reborn_webui_v2_live_qa/semantic_judge.py`:
- Around line 93-122: The three helpers `_judge_api_key_env`, `_judge_base_url`,
and `_judge_model` all repeat the same nested environment fallback pattern, so
factor that logic into a shared helper such as `_env_str_with_fallbacks(...)`
and have each function call it with its specific env names and default. Update
the three functions to use the shared helper so the fallback behavior stays
consistent and won’t drift across future edits.
In `@scripts/reborn_webui_v2_live_qa/test_run_live_qa.py`:
- Around line 42-81: The new _fake_assistant_reply_page helper duplicates the
existing FakePage/FakeAssistantBlocks/FakeApprove test fixture logic, so
consolidate them into one reusable fake-page builder. Update
_fake_assistant_reply_page to accept a list of assistant block texts and derive
count/inner_text/all_inner_texts from that list, then refactor
test_wait_for_assistant_reply_matches_combined_assistant_blocks to use the same
helper instead of keeping a separate inline fake page implementation.
---
Outside diff comments:
In `@scripts/reborn_webui_v2_live_qa/test_run_live_qa.py`:
- Around line 660-727: Add an integration-style test for the judge-rejects path
in _wait_for_assistant_reply: stub _judge_assistant_reply_completion to return a
rejection (either completed=False or confidence below the pass threshold) and
verify the resulting AssertionError from _wait_for_assistant_reply includes
semantic_judge=... in its message. Reuse the existing test helpers like
_fake_assistant_reply_page and patch asyncio.sleep, and place the new coverage
alongside the current _wait_for_assistant_reply tests so the rejection behavior
is exercised end-to-end.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2517821d-5c61-4bb4-b18b-45041de55cf6
📒 Files selected for processing (3)
scripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/semantic_judge.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
| def _judge_api_key_env() -> str: | ||
| return os.environ.get( | ||
| "REBORN_WEBUI_V2_LIVE_QA_LLM_JUDGE_API_KEY_ENV", | ||
| os.environ.get( | ||
| "REBORN_WEBUI_V2_LIVE_QA_LLM_API_KEY_ENV", | ||
| "NEARAI_API_KEY" | ||
| if os.environ.get("NEARAI_API_KEY") or os.environ.get("NEARAI_API_KEY_PATH") | ||
| else "LIVE_OPENAI_COMPATIBLE_API_KEY", | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
| def _judge_base_url() -> str: | ||
| return os.environ.get( | ||
| "REBORN_WEBUI_V2_LIVE_QA_LLM_JUDGE_BASE_URL", | ||
| os.environ.get( | ||
| "REBORN_WEBUI_V2_LIVE_QA_LLM_BASE_URL", | ||
| os.environ.get("LIVE_OPENAI_COMPATIBLE_BASE_URL", "https://cloud-api.near.ai/v1"), | ||
| ), | ||
| ).rstrip("/") | ||
|
|
||
|
|
||
| def _judge_model() -> str: | ||
| return os.environ.get( | ||
| "REBORN_WEBUI_V2_LIVE_QA_LLM_JUDGE_MODEL", | ||
| os.environ.get( | ||
| "REBORN_WEBUI_V2_LIVE_QA_LLM_MODEL", | ||
| os.environ.get("LIVE_OPENAI_COMPATIBLE_MODEL", "deepseek-ai/DeepSeek-V4-Flash"), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Dedupe the three-level env-fallback lookup pattern.
_judge_api_key_env, _judge_base_url, and _judge_model each hand-roll the same os.environ.get(new, os.environ.get(old, os.environ.get(fallback, default))) chain. A single helper (e.g. _env_str_with_fallbacks(*names, default)) would remove the triplication and prevent the three lookup chains from silently drifting when one is edited but not the others.
♻️ Proposed helper
+def _env_str_with_fallbacks(*names: str, default: str) -> str:
+ for name in names:
+ value = os.environ.get(name)
+ if value:
+ return value
+ return default
+
+
def _judge_api_key_env() -> str:
- return os.environ.get(
- "REBORN_WEBUI_V2_LIVE_QA_LLM_JUDGE_API_KEY_ENV",
- os.environ.get(
- "REBORN_WEBUI_V2_LIVE_QA_LLM_API_KEY_ENV",
- "NEARAI_API_KEY"
- if os.environ.get("NEARAI_API_KEY") or os.environ.get("NEARAI_API_KEY_PATH")
- else "LIVE_OPENAI_COMPATIBLE_API_KEY",
- ),
- )
+ nearai_default = (
+ "NEARAI_API_KEY"
+ if os.environ.get("NEARAI_API_KEY") or os.environ.get("NEARAI_API_KEY_PATH")
+ else "LIVE_OPENAI_COMPATIBLE_API_KEY"
+ )
+ return _env_str_with_fallbacks(
+ "REBORN_WEBUI_V2_LIVE_QA_LLM_JUDGE_API_KEY_ENV",
+ "REBORN_WEBUI_V2_LIVE_QA_LLM_API_KEY_ENV",
+ default=nearai_default,
+ )🤖 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 `@scripts/reborn_webui_v2_live_qa/semantic_judge.py` around lines 93 - 122, The
three helpers `_judge_api_key_env`, `_judge_base_url`, and `_judge_model` all
repeat the same nested environment fallback pattern, so factor that logic into a
shared helper such as `_env_str_with_fallbacks(...)` and have each function call
it with its specific env names and default. Update the three functions to use
the shared helper so the fallback behavior stays consistent and won’t drift
across future edits.
| def _fake_assistant_reply_page(self, response_text: str): | ||
| class FakeApprove: | ||
| @property | ||
| def last(self): | ||
| return self | ||
|
|
||
| async def is_visible(self, **_kwargs): | ||
| return False | ||
|
|
||
| class FakeAssistantBlocks: | ||
| @property | ||
| def last(self): | ||
| return self | ||
|
|
||
| async def count(self): | ||
| return 1 | ||
|
|
||
| async def inner_text(self, **_kwargs): | ||
| return response_text | ||
|
|
||
| async def all_inner_texts(self): | ||
| return [response_text] | ||
|
|
||
| class FakeMain: | ||
| async def inner_text(self, **_kwargs): | ||
| return response_text | ||
|
|
||
| class FakePage: | ||
| def locator(self, selector): | ||
| if selector == "[data-testid='msg-assistant']": | ||
| return FakeAssistantBlocks() | ||
| if selector == "main": | ||
| return FakeMain() | ||
| raise AssertionError(f"unexpected selector: {selector}") | ||
|
|
||
| def get_by_role(self, _role, **_kwargs): | ||
| return FakeApprove() | ||
|
|
||
| return FakePage() | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
New fake-page helper duplicates the existing inline fixture at Line 613-647.
_fake_assistant_reply_page is structurally the same as the FakePage/FakeAssistantBlocks/FakeApprove trio already defined inline in test_wait_for_assistant_reply_matches_combined_assistant_blocks (differs only in block count: 1 vs 2). Consider generalizing the new helper to accept a list of block texts and refactor the older test to use it, instead of maintaining two parallel fake-page implementations.
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 42-42: Missing return type annotation for private function _fake_assistant_reply_page
(ANN202)
[warning] 45-45: Missing return type annotation for private function last
(ANN202)
[warning] 48-48: Missing return type annotation for private function is_visible
Add return type annotation: bool
(ANN202)
[warning] 48-48: Missing type annotation for **_kwargs
(ANN003)
[warning] 53-53: Missing return type annotation for private function last
(ANN202)
[warning] 56-56: Missing return type annotation for private function count
Add return type annotation: int
(ANN202)
[warning] 59-59: Missing return type annotation for private function inner_text
(ANN202)
[warning] 59-59: Missing type annotation for **_kwargs
(ANN003)
[warning] 62-62: Missing return type annotation for private function all_inner_texts
(ANN202)
[warning] 66-66: Missing return type annotation for private function inner_text
(ANN202)
[warning] 66-66: Missing type annotation for **_kwargs
(ANN003)
[warning] 70-70: Missing return type annotation for private function locator
(ANN202)
[warning] 75-75: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 77-77: Missing return type annotation for private function get_by_role
(ANN202)
[warning] 77-77: Missing type annotation for **_kwargs
(ANN003)
🤖 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 `@scripts/reborn_webui_v2_live_qa/test_run_live_qa.py` around lines 42 - 81,
The new _fake_assistant_reply_page helper duplicates the existing
FakePage/FakeAssistantBlocks/FakeApprove test fixture logic, so consolidate them
into one reusable fake-page builder. Update _fake_assistant_reply_page to accept
a list of assistant block texts and derive count/inner_text/all_inner_texts from
that list, then refactor
test_wait_for_assistant_reply_matches_combined_assistant_blocks to use the same
helper instead of keeping a separate inline fake page implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/reborn_webui_v2_live_qa/semantic_judge.py`:
- Around line 189-196: In semantic_judge.py, narrow the broad exception handling
in the JSON recovery path inside the parsing logic so it only catches malformed
JSON decode failures. Update the try/except blocks around the text slicing and
json.loads call to handle json.JSONDecodeError for invalid judge output, and let
other unexpected runtime bugs surface instead of returning None. Keep the
behavior in the parsing helper that extracts the JSON object from text, but
avoid swallowing unrelated exceptions.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f08916a3-923d-4cfc-a88c-5de1b33cf0b8
📒 Files selected for processing (2)
scripts/reborn_webui_v2_live_qa/semantic_judge.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
| except Exception: | ||
| start = text.find("{") | ||
| end = text.rfind("}") | ||
| if start == -1 or end <= start: | ||
| return None | ||
| try: | ||
| return json.loads(text[start : end + 1]) | ||
| except Exception: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Narrow JSON parsing catches to decode failures.
Line 189 and Line 196 catch all Exception, but this path only expects malformed judge JSON. Catch json.JSONDecodeError so unexpected parser/runtime bugs are not silently converted to None.
Proposed fix
- except Exception:
+ except json.JSONDecodeError:
start = text.find("{")
end = text.rfind("}")
if start == -1 or end <= start:
return None
try:
return json.loads(text[start : end + 1])
- except Exception:
+ except json.JSONDecodeError:
return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except Exception: | |
| start = text.find("{") | |
| end = text.rfind("}") | |
| if start == -1 or end <= start: | |
| return None | |
| try: | |
| return json.loads(text[start : end + 1]) | |
| except Exception: | |
| except json.JSONDecodeError: | |
| start = text.find("{") | |
| end = text.rfind("}") | |
| if start == -1 or end <= start: | |
| return None | |
| try: | |
| return json.loads(text[start : end + 1]) | |
| except json.JSONDecodeError: |
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 189-189: Do not catch blind exception: Exception
(BLE001)
[warning] 196-196: Do not catch blind exception: Exception
(BLE001)
🤖 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 `@scripts/reborn_webui_v2_live_qa/semantic_judge.py` around lines 189 - 196, In
semantic_judge.py, narrow the broad exception handling in the JSON recovery path
inside the parsing logic so it only catches malformed JSON decode failures.
Update the try/except blocks around the text slicing and json.loads call to
handle json.JSONDecodeError for invalid judge output, and let other unexpected
runtime bugs surface instead of returning None. Keep the behavior in the parsing
helper that extracts the JSON object from text, but avoid swallowing unrelated
exceptions.
Source: Linters/SAST tools
Summary
scripts/reborn_webui_v2_live_qa/semantic_judge.pyChange Type
Linked Issue
Security Impact
REBORN_WEBUI_V2_LIVE_QA_LLM_API_KEY_ENV, defaulting toNEARAI_API_KEY) viaenv_secret./chat/completionsrequest only after a visible assistant response fails literal text matching.Reborn Trust-Boundary Checklist
Validation
python3 -m py_compile scripts/reborn_webui_v2_live_qa/run_live_qa.py scripts/reborn_webui_v2_live_qa/semantic_judge.py scripts/reborn_webui_v2_live_qa/test_run_live_qa.pypython3 scripts/reborn_webui_v2_live_qa/test_run_live_qa.py→ 59 tests, 4 skippedgit diff --checkactionlint .github/workflows/live-canary.ymlnot run locally becauseactionlintis not installed