fix(saving): tolerate unknown dependency keys during load - #91
Open
detail-app[bot] wants to merge 1 commit into
Open
fix(saving): tolerate unknown dependency keys during load#91detail-app[bot] wants to merge 1 commit into
detail-app[bot] wants to merge 1 commit into
Conversation
Greptile SummaryThis PR makes both DSPy persistence loaders tolerate dependency keys that are absent from the current version registry.
Confidence Score: 5/5The PR appears safe to merge; both loader paths handle unknown dependency keys without weakening pickle trust gates or known-version mismatch checks. No actionable failures remain: the missing-key handling matches the dependency-version provider’s string-valued contract, and the tests exercise the real metadata persistence and loading paths. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Read saved dependency metadata] --> B{Key tracked currently?}
B -->|No| C[Log untracked dependency warning]
C --> D[Continue checking dependencies]
B -->|Yes| E{Versions match?}
E -->|No| F[Log version mismatch warning]
E -->|Yes| D
F --> D
D --> G[Load program or module state]
Reviews (1): Last reviewed commit: "fix(saving): tolerate unknown dependency..." | 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
Summary
Closes Unknown issue
The version-check loop in both loaders (
dspy.utils.saving.loadandBaseModule.load) iterated the saved dependency-version keys and indexed the current environment's dict with an unguardeddependency_versions[key]. A saved metadata entry the loading environment doesn't track — e.g. a future DSPy that adds a key toget_dependency_versions()— raised an unhandledKeyError, aborting the entire load before any state was read. This contradicts the documented "version mismatches warn, don't block" / "old saves load against newer DSPy versions" contract, and bit hardest on the safe, pickle-free JSON state path (module.load("x.json"), noallow_pickleneeded).Fix
Replaced the unguarded
dependency_versions[key]lookup withdependency_versions.get(key)in both loaders. When a saved key is absent from the current environment, the loader now logs a warning naming the dependency andcontinues; the load proceeds. Known-key mismatch handling and theallow_picklegates are unchanged. The same one-line-pattern fix applies to both the full-program path (dspy/utils/saving.py) and the state-only.json/.pklpath (dspy/primitives/base_module.py).Testing
tests/utils/test_saving.py::test_load_with_unknown_dependency_key(full-program path)tests/primitives/test_base_module.py::test_json_load_with_unknown_dependency_key(JSON state path, noallow_pickle)test_load_with_version_mismatch(both files),test_json_file_loading_works_without_permission, the pickle-permission tests, and the same-version round-trips (test_save_and_load_with_json,test_save_and_load_with_pkl). Full sweep oftests/utils/test_saving.pyandtests/primitives/test_base_module.pywith--extra: 24 passed, 3 skipped (the skipped tests need a live LM).continue-vs-breakinteraction — but kept the committed test set minimal for this simple bug.dspy.Predictto.json, injecting an extranumpykey into the embedded metadata, and loading withoutallow_picklepreviously crashed withKeyError -> 'numpy'; it now logs the untracked-key warning and loads with matching state. The identical scenario on the full-program path (dspy.load(path, allow_pickle=True)with an extra key inmetadata.json) also now succeeds.@pytest.mark.llm_callusage-tracker tests were run against a local Ollama server (ollama/llama3.2:3b) and pass; they exercise usage tracking, not this fix.test_usage_tracker_in_parallelhardcodesopenai/gpt-4o-mini/gpt-3.5-turboand is gated onOPENAI_API_KEY; a dummy key was attempted and the test ran but failed with an OpenAI authentication error. No valid OpenAI key was available. This test is unrelated to saving/loading.ruff checkandruff format --checkshow no new violations on the changed lines; the three remaining findings are pre-existing in unrelated tests and present in the upstream parent commit.AI-Generated Contributions
Authored by Detail: Automatic Fixes. The fix and tests were generated from the bug report and verified against the included reproduction; no separate prompts were used beyond the report itself.
Automatic Fixes PRs can be configured here.