fix(litellm): injection_mode, context manager restore, validation, error consistency#1711
Draft
DK09876 wants to merge 4 commits into
Draft
fix(litellm): injection_mode, context manager restore, validation, error consistency#1711DK09876 wants to merge 4 commits into
DK09876 wants to merge 4 commits into
Conversation
…fault URL - recall(): add include_entities, trace, recall_tags, recall_tags_match params (previously only supported via the callback/enable() path, not the manual API) - reflect(): add recall_tags, recall_tags_match params (same gap) - hindsight_memory(): default URL now matches configure()/wrap_openai()/wrap_anthropic() instead of hardcoding localhost; add session_id, use_reflect, reflect_context, tags, recall_tags, recall_tags_match params - Document that enable() and HindsightCallback are mutually exclusive injection paths to prevent accidental double injection - Add 17 tests covering all new behaviour Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… and add sync param to aretain - hindsight_bank_id kwarg was leaking into LiteLLM as extra_body, causing OpenAI 400 errors; now popped in completion(), _wrapped_completion(), _wrapped_acompletion() and propagated as bank_id_override throughout injection and storage paths - _inject_memories() accepts bank_id_override to honour per-call bank without mutating globals - _store_conversation() and _store_conversation_from_text() accept bank_id_override for consistent per-call storage routing - _LiteLLMStreamWrapper and _LiteLLMAsyncStreamWrapper carry bank_id_override so streamed responses store to the right bank - aretain() now accepts sync=True, forwarding it to retain() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…estore, validation, error consistency - config.py: remove DEFAULT_BANK_ID footgun (configure() without bank_id now leaves bank_id=None; is_configured() and enable() correctly require explicit bank_id). Add _restore_config() for atomic state restoration. Add budget/recall_tags_match validation in configure() and set_defaults(). Emit DeprecationWarning for document_id usage. - __init__.py: _inject_memories() now respects injection_mode (PREPEND_USER prepends to last user message; SYSTEM_MESSAGE keeps existing behaviour). Wire up defaults.query as fallback recall query. Fix ValueError → HindsightError for missing bank_id. hindsight_memory() finally block now calls _restore_config() to atomically restore all settings (previously lost: sync_storage, tags, recall_tags, recall_tags_match, reflect_context, reflect_response_schema). Add _enabled_lock and _debug_lock for thread safety on shared mutable state. - callbacks.py: ValueError → HindsightError in log_pre_api_call and async_log_pre_api_call for missing bank_id, consistent with __init__.py. - tests: update tests that relied on DEFAULT_BANK_ID behaviour; add TestValidation, TestInjectionMode, TestQueryField, TestHindsightErrorConsistency, TestContextManagerFullRestore (83 tests, all passing). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t-bank-id behaviour - Run ruff format on __init__.py and wrappers.py to match CI lint expectations - test_config.py: update test_configure_with_no_arguments to assert bank_id is None (not DEFAULT_BANK_ID) and rename test_is_configured_true_with_defaults to test_is_configured_false_without_explicit_bank_id with corrected assertion, matching the removed DEFAULT_BANK_ID footgun Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
configure()footgun removed —configure()withoutbank_idno longer silently uses"default".is_configured()andenable()now both require an explicitbank_id.injection_modeactually honoured —PREPEND_USERnow prepends memories to the last user message; previously all monkeypatch paths ignored the setting and always injected into a system message.hindsight_memory()full restore — context managerfinallyblock now atomically restores all settings via_restore_config(). Previously 6 fields were silently lost on exit:sync_storage,tags,recall_tags,recall_tags_match,reflect_context,reflect_response_schema.defaults.querywired up — was dead code;_inject_memories()now uses it as a fallback query when nohindsight_querykwarg is passed.HindsightErrorconsistency —_inject_memories()and bothlog_pre_api_callmethods incallbacks.pynow raiseHindsightErrorinstead ofValueErrorfor missingbank_id.configure()andset_defaults()validatebudgetandrecall_tags_match, raisingValueErrorimmediately on bad values.document_idemitsDeprecationWarning._enabled_lockand_debug_lockprotect shared mutable state inenable()/disable()and_last_injection_debug.Test plan
uv run pytest tests/test_integration.py -v— 83 unit tests, all pass (addedTestValidation,TestInjectionMode,TestQueryField,TestHindsightErrorConsistency,TestContextManagerFullRestore)TestDesignFixesclass that verifies each fix end-to-end🤖 Generated with Claude Code