fix(judge): reject ambiguous JSON responses - #772
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe LLM judge now validates JSON responses against an exact schema. It rejects duplicate, missing, and unexpected fields. It also requires ChangesJudge response validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Validation update for exact head 7ccef2c:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
python/fast_mlsirm/llm_judge.py (1)
425-434: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for non-boolean
accepted.The new validation rejects values such as
0,"true", andnull. The changed tests cover duplicate and unexpected fields, but not this type contract. Add parameterized cases for non-boolean JSON values.🤖 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/fast_mlsirm/llm_judge.py` around lines 425 - 434, Add a regression test for the validation in the response-judging path around _response_object and advisory_accepted, parameterizing non-boolean JSON values such as 0, "true", and null. Assert each case raises JudgeFormatError with the expected boolean-validation message, while preserving existing valid-boolean coverage.
🤖 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 `@tests/test_llm_judge.py`:
- Around line 111-126: Update the pytest.raises call in
test_judge_rejects_duplicate_and_unknown_top_level_fields to use a raw regex
string for its match argument, changing the existing "exactly|duplicate" pattern
to the raw-string form while preserving the test behavior.
---
Nitpick comments:
In `@python/fast_mlsirm/llm_judge.py`:
- Around line 425-434: Add a regression test for the validation in the
response-judging path around _response_object and advisory_accepted,
parameterizing non-boolean JSON values such as 0, "true", and null. Assert each
case raises JudgeFormatError with the expected boolean-validation message, while
preserving existing valid-boolean coverage.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f1ea7d49-235e-4e1f-94ea-b5e3c32b7f48
📒 Files selected for processing (2)
python/fast_mlsirm/llm_judge.pytests/test_llm_judge.py
| def test_judge_rejects_duplicate_and_unknown_top_level_fields() -> None: | ||
| duplicate = ( | ||
| '{"score":0.8,"accepted":true,"rationale":"supported",' | ||
| '"criterion_scores":{"task_alignment":0.8,"factual_support":0.8},' | ||
| '"score":0.2}' | ||
| ) | ||
| unknown = json.loads(_payload()) | ||
| unknown["unexpected"] = "ignored fields are unsafe" | ||
| for answer in (duplicate, json.dumps(unknown)): | ||
| with pytest.raises(JudgeFormatError, match="exactly|duplicate"): | ||
| ContextualOrchestratorJudge(_FakeOrchestrator(answer)).judge( | ||
| task="task", | ||
| answer="answer", | ||
| criteria=CRITERIA, | ||
| ) | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target lines ---'
cat -n tests/test_llm_judge.py | sed -n '100,135p'
printf '%s\n' '--- Ruff configuration and references ---'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'RUF043|ruff|pytest-raises|exactly\|duplicate' \
pyproject.toml setup.cfg tox.ini .ruff.toml ruff.toml tests 2>/dev/null || true
printf '%s\n' '--- available Ruff executable ---'
if command -v ruff >/dev/null 2>&1; then
ruff --version
ruff check tests/test_llm_judge.py --select RUF043
else
echo 'ruff is not installed'
fi
printf '%s\n' '--- relevant dependency declarations ---'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'ruff|pytest' . 2>/dev/null | head -80Repository: ContextualWisdomLab/fast-mlsirm
Length of output: 8441
Use a raw regex literal for match.
Ruff RUF043 reports match="exactly|duplicate". Change it to match=r"exactly|duplicate".
🧰 Tools
🪛 ast-grep (0.45.1)
[info] 118-118: use jsonify instead of json.dumps for JSON output
Context: json.dumps(unknown)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
[warning] 120-120: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
🤖 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 `@tests/test_llm_judge.py` around lines 111 - 126, Update the pytest.raises
call in test_judge_rejects_duplicate_and_unknown_top_level_fields to use a raw
regex string for its match argument, changing the existing "exactly|duplicate"
pattern to the raw-string form while preserving the test behavior.
Source: Linters/SAST tools
|
@opencode-agent address Exact-current bounded reconciliation/repair handoff for PR #772. Immediately before any write, refetch source head RCA: the PR is currently non-mergeable because its lineage diverged from current Then fix the two valid current CodeRabbit findings test-first: (1) use a raw regex literal for the existing Run focused LLM-judge/IRT tests, Ruff for changed tests/source, then the complete applicable Python suite and repository-required Rust/PyO3/package/security checks on one unchanged reconciled head. Update the PR body with exact head/base/run evidence; keep the PR Ready only if source is stable. Do not change dependencies, provider credentials, numerical psychometrics, version/release state, or unrelated docs. Stop writing this branch after the coherent repair so exact-head review/merge can proceed. |
Combine nesting-depth bounds with duplicate-key rejection and exact mode-specific top-level schema requirements so untrusted judge payloads cannot smuggle alternate scores or unknown fields.
7ccef2c to
2ceeaea
Compare
1 participant
Summary
Validation
This change is intentionally fail-closed for LLM-as-a-Judge. Please review the exact-head checks and merge only through the repository's normal review and protection gates.
Summary by CodeRabbit