fix(dspy): reject duplicate tool names in FlexContext.context_names - #96
Open
detail-app[bot] wants to merge 1 commit into
Open
Conversation
Greptile SummaryAdds fail-fast validation for duplicate Flex tool names, aligning Flex with RLM behavior.
Confidence Score: 5/5The PR appears safe to merge; the validation is applied at the shared tool-context chokepoint and is covered by focused regression tests. No actionable correctness, security, or repository-rule issues remain; duplicate names now fail before overwrite while unique names continue through baseline generation. Important Files Changed
Reviews (1): Last reviewed commit: "fix(dspy): reject duplicate tool names i..." | 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
This MR/PR contains the following changes:
Bug:
dspy.Flex(..., tools=[...])silently overwrote duplicate tool names (last-wins) instead of rejecting them.FlexContext.context_names(dspy/predict/flex/ctx.py) built the sandbox globals dict with a plainout[name] = toolassignment and no uniqueness check, so twodspy.Toolinstances sharing a name (e.g. twoTool(f, name="lookup"), or two functions sharing__name__) caused the earlier tool to be silently dropped — both from the sandbox namespace and from thedspy.RLM(...)baseline emitted intomodule_src— with no error. The same input is already rejected bydspy.RLM._normalize_toolswithValueError("Duplicate tool name '...'"), soFlex's baseline contract andRLM's diverged silently.Fix: Added a duplicate-name check in
FlexContext.context_names, the single chokepoint used both during baseline-source emission (Flex._baseline_src) and runtime tool registration (BridgeRuntime._tool_callables/_resolve_tool). Since_baseline_srccallscontext_names()duringFlex.__init__, theValueErrornow fires at construction time — before any interpreter is created orforwardruns. The message matchesRLM's exactly:Duplicate tool name '<name>'. This mirrors the existing guard indspy/predict/rlm.py:214-216.Tests: Added two regression tests in
tests/flex/test_tools.py:test_duplicate_tool_names_rejected(mirrorstest_rlm.py::test_duplicate_tool_names_rejectedat theFlexconstructor) andtest_unique_tool_names_are_accepted(guards against over-rejection of valid multi-tool input).Closes Unknown issue
✅ Contributor Checklist
dspy.Flexis an@experimentalmodule; this hardens its constructor validation to matchdspy.RLM's existing contract. The change is additive (a new raise) and affects only the duplicate-name path; existing single-tool and multi-unique-tool usage is unchanged.Testing summary
uv run ruff check --fixanduv run ruff formatleave both files unchanged;uv buildsucceeds andimport dspyworks.tests/flex/test_tools.pypasses (9 passed, 8@deno_requiredskipped without Deno; 17 passed with Deno installed viauv sync --extra deno).tests/flex/suite passes (75 passed / 39 skipped without Deno; 114 passed with Deno).tests/predict/test_rlm.py -k "tool or duplicate or reserved") passes (21 without--deno; 28 with--deno), confirming no cross-module regression.BridgeRuntime.forwardpath: the duplicate input raises at construction (before any forward), and the positive single-tool path dispatches correctly ("abc"→"ABC").Automatic Fixes PRs can be configured here.