fix(dspy): compute partial parse reward from field count ratio - #95
Open
detail-app[bot] wants to merge 1 commit into
Open
Conversation
Greptile SummaryThis PR fixes partial-parse reward calculation by dividing the number of parsed fields by the number of expected fields rather than attempting to divide two lists.
Confidence Score: 5/5The PR appears safe to merge; the corrected calculation matches the adapter’s partial-result representation and is covered by focused tests. No actionable failures remain: parsed fields are constrained to expected outputs, the denominator cannot be empty in the partial-result branch, and test-global configuration is restored automatically. Important Files Changed
Reviews (1): Last reviewed commit: "fix(dspy): compute partial parse reward ..." | Re-trigger Greptile |
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.
Warning
GitHub issue creation failed
Detail attempted to publish this bug to GitHub, but the issue could not be created. This fix PR was created without that issue, and missing tracker references are shown as
Unknown issue.You can review and merge this PR normally. Please review your tracker integration settings before the next publish run.
Detail bug report: View on Detail
📝 Changes Description
Closes Unknown issue
Bug
bootstrap_trace_data's partial-parse handler divided twolist[str]values (present / expected) instead of their lengths, raisingTypeError: unsupported operand type(s) for /: 'list' and 'list'whenever an LM returned a parseable, non-empty dict that was missing one or more required (non-default, non-nullable) output fields.raise_on_error=False, theTypeErrorwas swallowed by the evaluator's error budget and the affected example was silently dropped from the returneddata(via theValueError→continuepath), producing no partial-creditFailedPrediction. GRPO/GEPA thus lost training signal on exactly the malformed-but-parseable responses the partial-credit path is meant to train on. The in-tree TODO atbootstrap_trace.py:136-137documents this failure mode occurring in practice (Qwen/Qwen2.5-Coder-0.5B-Instruct on MATH).Fix
dspy/teleprompt/bootstrap_trace.py, replacepresent / expectedwithlen(present) / len(expected). The partial-parse reward now interpolates correctly:format_failure_score + (failure_score - format_failure_score) * (fraction of expected output fields present)— a numeric value betweenformat_failure_scoreandfailure_score. The example is retained as aFailedPrediction(with its completion text and trace) instead of being silently dropped.tests/teleprompt/test_bootstrap_trace.pyusing a two-required-output-field signature (answer,reasoning) and a parseable response missing a required field ({"answer": "4"}).Testing
TypeErrorand silent drop) and pass with it. The fulltests/teleprompt/suite (90 tests, including the GRPO/GEPA callers ofbootstrap_trace_data) and the CI-equivalent non-extra suite (pytest -m 'not extra and not deno' tests/, 1253 tests) pass with no regressions.bootstrap_trace_dataagainstQwen/Qwen2.5-Coder-0.5B-Instruct(via Ollama) on the MATH algebra split with a two-required-output-field signature. With truncated JSON (max_tokens=12) the model emits a partial dict missingreasoning; with the fix all 6 examples are retained asFailedPredictionwithformat_reward=-0.5and zerolist/listerror logs, whereas without the fix all 6 are silently dropped with 6×ERROR ... 'list' and 'list'log lines. The happy-path control (max_tokens=400) retains all 6 as normalPredictions both before and after the fix.✅ Contributor Checklist
fix(dspy): ...Authored by Detail (automatic fix). The change is a one-line numeric correction plus two focused regression tests; the happy-path and the fully-unparseable (
present is None) branches are unchanged.Automatic Fixes PRs can be configured here.