fix(dspy): avoid KeyError in truncation warning for reasoning models - #88
Open
detail-app[bot] wants to merge 1 commit into
Open
Conversation
Greptile SummaryThis PR prevents truncated responses from reasoning models from raising
Confidence Score: 5/5The PR appears safe to merge and fixes the reported reasoning-model truncation crash without changing provider-request behavior. Reasoning-model construction stores the limit as Important Files Changed
Reviews (1): Last reviewed commit: "fix(dspy): avoid KeyError in truncation ..." | 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:
LM._check_truncation(dspy/clients/lm.py) interpolates the configured token limit into its truncation warning viaself.kwargs['max_tokens']. OpenAI reasoning models (openai/o3,o1,gpt-5, etc.) store that limit undermax_completion_tokensinstead — an asserted invariant ("max_tokens" not in lm.kwargs). So a truncated chat completion (finish_reason == "length") raisedKeyError: 'max_tokens'out of bothLM.forwardandLM.aforward, turning a successful provider response into an unhandled crash. This was missed when_check_truncationwas added without reconciling it against the reasoning-model kwargs shape.Fix: Read the token limit defensively from whichever key is present, then interpolate the local variable:
This is the minimal idiomatic fix — it preserves the reasoning-model kwargs invariant (no change to
self.kwargs) and covers both reasoning (max_completion_tokens) and non-reasoning (max_tokens) chat paths.forwardandaforwardshare the method, so both are fixed.Testing:
tests/clients/test_lm.pycovering the previously-untested_check_truncation: a reasoning-model truncated response warns and returns the partial result (the exact bug path), a non-reasoning truncated response still warns (the other branch of the defensive read), and a non-truncated response emits no warning (guards the precondition).KeyError: 'max_tokens'atdspy/clients/lm.py:440when the fix is reverted, and passes with the fix in place.pytest tests/clients/test_lm.py(90 passed), the fulltests/clients/run (157 passed, 22 live-only skipped),ruff checkclean, andpre-commit run --filespassed.dspy.Predictwith a mocked reasoning-model truncated response — the adapter returns partial output and logs the warning instead of raising.OPENAI_API_KEYis not set in this environment, so the provider call fails authentication before_check_truncationis reached. The mocked tests cover the same code path up to the provider boundary.✅ Contributor Checklist
ruff format --checkreports drift in a few pre-existing, untouched lines ofdspy/clients/lm.pyandtests/clients/test_lm.py(identical at HEAD); the changed lines are format-compliant, so this PR does not introduce new format drift.AI disclosure: this fix was produced with assistance from Detail (an AI coding assistant).
Automatic Fixes PRs can be configured here.