fix(dspy): Guard use_tip on tip value in GroundedProposer to avoid rendering None in prompt - #83
Open
detail-app[bot] wants to merge 1 commit into
Conversation
…ndering None in prompt
Greptile SummaryThis PR prevents
Confidence Score: 5/5The PR appears safe to merge; the focused guard fixes the unwanted prompt field without changing valid tip behavior. No actionable failures remain: all reachable falsey tip values represent the intended absence of a tip, non-empty tips remain rendered, and the regression tests exercise the affected direct and end-to-end paths. Important Files Changed
Reviews (1): Last reviewed commit: "fix(dspy): Guard use_tip on tip value in..." | 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
GroundedProposer(MIPROv2's instruction proposer) acceptsuse_tipandset_tip_randomlyflags. Whenset_tip_randomly=False— a configuration explicitly supported by PR stanfordnlp#3919 —selected_tipstaysNone, but the constructor defaultuse_tip=Truewas left unchanged. The generation signature still declared thetipinput field andtip=Nonewas forwarded into the prompt, where DSPy's adapter rendered it as the literal string"None", producing a meaningless[[ ## tip ## ]]\nNoneblock the LM is told to follow. The analogoususe_instruct_historyflag was already guarded withand instruction_history;use_tiplacked the equivalent guard.Fix
Guard
use_tipon whether a real tip value was supplied, mirroring the existinguse_instruct_historyguard indspy/propose/grounded_proposer.py:This omits the
tipfield from the signature when no real tip is present, so the adapter no longer rendersNone. Only the buggy configuration (use_tip=Truewithtip=None) changes; all other paths (real tip supplied,use_tip=False,set_tip_randomly=Truedrawing "none" or a real tip) behave identically.Testing
tests/propose/test_grounded_proposer.pyusing aRecordingDummyLMthat captures the exact prompt sent to the rollout LM copy (shared by reference throughBaseLM's shallowcopy). They assert thetipblock is omitted whentip=None— both via the directpropose_instruction_for_predictorand the end-to-endpropose_instructions_for_programwithset_tip_randomly=False— and that a real tip is still rendered. These tests fail without the one-line fix and pass with it.tests/propose/), the broader optimizer suite (tests/teleprompt/),ruff check, andruff format --checkon the test file all pass. The project configures no typechecker; an ad-hocmypyrun shows no new type errors versus the original file (all pre-existing).LMServerError: Missing credentialsandLMTransportError: Connection erroron attempted setup). The prompt-content invariant the smoke would verify is instead asserted deterministically by theRecordingDummyLMregression tests.✅ Contributor Checklist
ruff check); remote CI pending runfix(dspy): ...)fix(dspy): Guard use_tip on tip value in GroundedProposer to avoid rendering None in promptgrounded_proposer.pyretains some pre-existingruff formatstyle nits unrelated to this change; left untouched to keep the diff minimal (CI enforcesruff check, notruff format).Automatic Fixes PRs can be configured here.