bugfix: remove identity-keyed multimodal gateway adapter cache - #3887
Merged
Conversation
(tenant_id, modality, slot, model_name, factory), omitting base_url, api_key, ssl_verify, timeout_seconds and every sampling parameter. That exposed the LLM, long-context LLM and VLM paths to three failures: - Editing a model in the management UI had no effect until the process restarted. The staleness was doubled: adapters also memoize their wrapped model via `if self._model is None`, so the config was baked in twice. - Two model records sharing a repo/name but pointing at different endpoints resolved to the same instance, so the second used the first's base_url and api_key. - Construction-time parameters froze across unrelated call sites. Title generation asks for temperature=0.7 and stream=False, but when system prompt generation had already built an adapter for the same model it inherited temperature=0.3, because LLMAdapter.__call__ forwards no per-call sampling arguments. VLM and long-context adapters bake in temperature, top_p, max_tokens, frequency_penalty and truncation_strategy the same way. invalidate() is removed with the cache; it had no production callers, which is precisely why config edits were never picked up. ModelContext.cache_key() is removed as dead code once the cache is gone. get_adapter() now builds a fresh adapter per call, making it equivalent to build_adapter_fresh(). Both names are kept so the ten existing per-call construction sites (health checks, knowledge-base resolvers, voice sessions) stay untouched; merging them is a separate cleanup. Unaffected before and after: knowledge-base and memory embedding/rerank plus all health checks already used build_adapter_fresh(), and STT/TTS never routed through the gateway. test: replace the four cache-pinning gateway tests with factory-semantics tests, including a guard that a rotated endpoint and api_key yield an independent adapter, and a guard that the gateway exposes no cache. Replace the two cache_key context tests with one asserting the connection fields the old key used to drop are carried per instance.
Follow-up to 0bd0249. Removing the caches also rewrote several docstrings that never mentioned caching, and added long narratives explaining why the caches were unsafe. That rationale belongs in the commit message, not in the code, so only the cache references are touched now: - MultimodalGateway's class docstring is back to a single line, minus the "and cache" it used to carry. - get_adapter_from_config and build_adapter_fresh keep their original wording with the "cached by the gateway" and "shared cache key" phrases dropped. - resolver.py no longer calls out "(no gateway cache)" for a cache that no longer exists. - get_embedding_client's module and function docstrings describe what the factory does instead of arguing against the removed cache. - The new regression tests lose their narrative docstrings and match the one-line style of the surrounding tests. No logic changes; the 193 tests covering these modules still pass.
jeffwu-1999
changed the title
jeffwu-1999
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Dallas98
approved these changes
Sep 8, 2026
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
3 participants
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.
(tenant_id, modality, slot, model_name, factory), omitting base_url, api_key, ssl_verify, timeout_seconds and every sampling parameter. That exposed the LLM, long-context LLM and VLM paths to three failures:
if self._model is None, so the config was baked in twice.invalidate() is removed with the cache; it had no production callers, which is precisely why config edits were never picked up. ModelContext.cache_key() is removed as dead code once the cache is gone.
get_adapter() now builds a fresh adapter per call, making it equivalent to build_adapter_fresh(). Both names are kept so the ten existing per-call construction sites (health checks, knowledge-base resolvers, voice sessions) stay untouched; merging them is a separate cleanup.
Unaffected before and after: knowledge-base and memory embedding/rerank plus all health checks already used build_adapter_fresh(), and STT/TTS never routed through the gateway.
test: replace the four cache-pinning gateway tests with factory-semantics tests, including a guard that a rotated endpoint and api_key yield an independent adapter, and a guard that the gateway exposes no cache. Replace the two cache_key context tests with one asserting the connection fields the old key used to drop are carried per instance.
case：用一个能跑的 LLM 发条消息生成标题（成功），去模型管理把 api_key 改成无效值 → 点连通性测试（失败）→ 再发条消息生成标题。修复后应该同样失败报 401；
