Greptile Summary

This PR rewrites extract_judge_json_from_response to handle real-world LLM output variability: it now iterates all markdown fences in reverse (returning the last valid dict), falls through to a direct json.loads, and finally uses a bracket-containment heuristic to reject prose-wrapped JSON arrays without blocking valid dicts surrounded by unrelated bracket tokens. All three P1 issues raised in previous review threads — fenced-array bypass, premature raise on saw_array_fence, and false-positive bracket rejection — are resolved in this revision, and 13 test cases validate the new paths.

Confidence Score: 4/5

Safe to merge; all previously-flagged P1 issues are resolved and only a P2 style finding remains.

No P0 or P1 issues found. The one P2 (bare json.loads that can leak JSONDecodeError) is minor and pre-existing. Logic was thoroughly traced through all 13 test cases and edge cases — no correctness defects remain.

No files require special attention, though the bracket-containment logic in llm_eval_judge.py lines 87–109 is dense and would benefit from inline comments if maintained further.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Input: text.strip] --> B{Markdown fences found?}
    B -- Yes --> C[Iterate fences in reverse]
    C --> D{Parse fence as JSON}
    D -- dict --> E[Return dict]
    D -- list --> F[continue to next fence]
    D -- error --> F
    F --> G{More fences?}
    G -- Yes --> C
    G -- No --> H
    B -- No --> H[json.loads full text]
    H -- dict --> E
    H -- list --> I[Raise: must be object]
    H -- error --> J[Scan brace and bracket positions]
    J --> K{arr span contains obj span?}
    K -- Yes --> L[json.loads arr candidate]
    L -- list --> I
    L -- error --> M[Inner loop: each bracket before obj]
    M -- valid array found --> I
    M -- none found --> N{has_obj?}
    K -- No --> N
    N -- No --> O[Raise: no JSON object]
    N -- Yes --> P[json.loads obj span]
    P --> E

_{Reviews (4): Last reviewed commit: "eval: fix judge JSON parsing (array fenc..." | Re-trigger Greptile}

Hey @cerencamkiran, thanks for tightening up the judge JSON parsing and adding targeted tests 👍

I see one behavior gap to fix before we merge:

• Right now the startswith("[") guard only catches arrays when the response literally begins with [. If there is any leading whitespace before the array, we can still end up slicing out a {...} from inside an array and treating it as valid. To fully meet the “reject JSON arrays” goal, can you change that to use text.lstrip().startswith("[") (and then parse from the stripped text)?

Also, a small test alignment:

• In test_extract_judge_json_raises_for_json_array you can simplify the match to just "JSON must be an object" once the guard is in place, since that will be the consistent error.

Optionally, adding a test for " \n [{\"overall_pass\": true}]" would lock in the whitespace-tolerant behavior.

Once those tweaks are in and tests are green, this should be good to go. The fenced array test you added already matches what Greptile flagged, which is great.

Great catch @rrajan94.
I'll update the guard to be whitespace-tolerant and add the extra test case. Thanks!

Hey @rrajan94! I updated the guard to handle leading whitespace and added the test. All passing 👍

nice work @cerencamkiran. fenced-array test is the kind of thing that would silently corrupt benchmark numbers if missed.

one thing not blocking. fence regex went from ({.}) to ([\s\S]?) which is broader than just a fix. old required {...} inside fence. new captures any content. so a prose-only fence with a JSON object outside used to parse fine, now fence eats the prose and the function raises. visible failure beats silent mis-parse so probably right, just flagging.

lgtm

Thanks @yashksaini-coder.

The multiple-fence case is now handled by scanning all fenced blocks and selecting the last valid JSON object, with a regression test added.

For the [INFO] ... case, I now parse first and only reject if the result is actually a list, so the salvage path is preserved.

All tests and checks are passing.

A prose-wrapped JSON array still slips through because the brace-substring fallback (text.find("{") / text.rfind("}")) slices out the first object inside the array.

repro:

extract_judge_json_from_response('Here is the result: [{"overall_pass": true}] Thanks')
# returns {'overall_pass': True} instead of raising

flow at HEAD: no fences → json.loads of the whole text fails → isinstance(raw, list) guard is skipped → find("{")/rfind("}") salvage path returns the inner object.

can you add a regression test for this and tighten the fallback so arrays are rejected even when there's prose around them? matching the existing "JSON must be an object" error keeps it consistent with the other array tests.

Thanks @rrajan94 added a regression test for the prose-wrapped array case and tightened the fallback so arrays are rejected before the {...} salvage path when the array appears before any JSON object.

Validation:
pytest tests/integrations/opensre/test_llm_eval_judge.py -v
10 passed

Thanks for fixing the prose-wrapped array case @cerencamkiran
I think the new bracket-based fix still has one related edge case.

Example: [INFO] result: [{"overall_pass": true}]

Because the check parses from the first [ to the last ], the substring starts with [INFO], fails JSON parsing, and then falls through to the {...} fallback. That still returns {"overall_pass": true} instead of rejecting the array.

Can you add this as a regression test too? It may be worth checking the fallback against a few bracket/prose variants so we don’t keep finding the same partial-parse issue one shape at a time.

@rrajan94 added a regression test for the [INFO] result: [...] case as well and updated the array detection to try parsing from each [ candidate before the {...} fallback.

Validation:
pytest tests/integrations/opensre/test_llm_eval_judge.py -v
11 passed

Thanks @cerencamkiran, the [INFO] result: [...] array case is covered now.

I found one false positive from the new scan though.

This valid object now raises:

[INFO] result: {"overall_pass": true, "score_0_100": 90, "rubric_items": [], "summary": "ok"}

The loop tries each [ before the {...} fallback, so it eventually parses the nested rubric_items: [] array and rejects the whole response before the full object can be salvaged.

Can you add this as a regression test too? The array rejection should catch top-level/salvaged arrays, but not arrays nested inside the valid judge object.

Thanks @rrajan94.

I’ve updated the logic to distinguish top-level arrays from nested ones and added the regression test.

Validation:
pytest tests/integrations/opensre/test_llm_eval_judge.py -v
12 passed

@greptileai review

@greptileai review

@greptileai review

@rrajan94 thanks for the review.

All previously flagged P1 issues are now addressed. The bracket handling has been updated to only reject when the span actually parses as a JSON array, so cases like [INFO] ... {dict} ... [end] no longer fail.

Added regression tests for these scenarios as well.

All tests are passing (15/15), and all checks are green.

I’ll leave the remaining P2 style note as non-blocking for now.

💼 Interviewer: describe a time you shipped something impactful.

@cerencamkiran: points at this PR

Interviewer: you're hired. 🤝

👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome.