parse LLM replies whose strings contain code fences - #22
Open
teddytennant wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_parse_llm_responsestrips the fence unconditionally whenever```jsonappears anywhere in the text, and takes the first```after the opening marker as the close. So a reply that is already valid JSON gets mangled when a justification quotes a```jsonblock, and a quoted```pythonblock truncates the slice.It fails quietly, which is the part that matters: an unparseable reply isn't an error, it's scored
IGNOREDat evaluate_rubrics.py:138-140 and drops out ofmust_have_passandagg_score.The
findtorfindchange in the issue fixes the reported payload but not the other two shapes:Rather than pick a different single delimiter rule, this collects candidate slices and lets
json.loadsdecide: the untouched text first, so anything already valid survives whatever its strings contain, then closing-fence candidates outermost-first, then brace extraction. It returns the first candidate that is a dict containing"ratings"and falls back to the first merely-parseable one, which preserves the old behaviour on a top-level array.Sweep over 16 payload shapes, scored on the caller's actual gate at line 139: 1 improvement, 0 regressions.
On scope: this is the 70 identical copies of
evaluate_rubrics.py, which is what the issue names. All 70 pre-images share one md5 and all 70 post-images share one md5, so the fan-out is mechanical.evaluate_answer.py(124 copies) andevaluate_tests.py(90 copies) have the same bug, and I checked they still fail on the issue's payload after this change. I can extend it to those in one pass if you'd rather;evaluate_answer.pyneeds a slightly different shape since it normalizes the rating inline.Closes #15.
Greptile Summary
The PR updates the RF rubric-response parser across 70 self-contained task bundles so valid JSON survives embedded code fences and fenced responses are evaluated through multiple candidate slices.
ratingswhile retaining prior fallback behavior for other JSON values.Confidence Score: 5/5
The PR appears safe to merge because no concrete changed-code failure remains.
The parser now preserves already-valid responses and tries progressively extracted candidates, while the identified edge-case limitations either predate the change or lack an established realistic trigger under the current response contract.
Important Files Changed
Reviews (1): Last reviewed commit: "parse LLM replies whose strings contain ..." | Re-trigger Greptile
Context used: