mirror of
https://github.com/usestrix/strix.git
synced 2026-08-17 09:29:49 +02:00
review: merge cache extra_args explicitly + note graceful degradation
Address Greptile feedback on #772: - Build extra_args as {**existing, **cache} at the call site rather than leaning on ModelSettings.resolve()'s dict-merge — makes preservation of unrelated LiteLLM options obvious to a reader (resolve() does merge, but it's non-obvious). No behaviour change: make_model_settings builds from scratch so the base extra_args is None today. - Document that an unrecognised injection-point location degrades gracefully (not injected, no error) on older LiteLLM pins; tool_config is honoured by the Bedrock Converse transform on versions that support it (litellm 1.90.1 verified).
This commit is contained in:
+16
-1
@@ -142,8 +142,17 @@ def make_model_settings(
|
||||
if force_required_tool_choice and _accepts_required_tool_choice(model_name):
|
||||
model_settings = model_settings.resolve(ModelSettings(tool_choice="required"))
|
||||
if _is_claude_model(model_name):
|
||||
# Merge into any existing extra_args rather than relying on resolve()'s
|
||||
# dict-merge semantics — makes it obvious at the call site that unrelated
|
||||
# LiteLLM options are preserved (make_model_settings currently builds
|
||||
# from scratch, so extra_args is None here today, but this keeps the
|
||||
# invariant local if that changes).
|
||||
merged_extra_args = {
|
||||
**(model_settings.extra_args or {}),
|
||||
**_claude_prompt_cache_extra_args(),
|
||||
}
|
||||
model_settings = model_settings.resolve(
|
||||
ModelSettings(extra_args=_claude_prompt_cache_extra_args()),
|
||||
ModelSettings(extra_args=merged_extra_args),
|
||||
)
|
||||
return model_settings
|
||||
|
||||
@@ -183,6 +192,12 @@ def _claude_prompt_cache_extra_args() -> dict[str, Any]:
|
||||
Two breakpoints on the stable prefix (2 of the 4 allowed), leaving headroom:
|
||||
- the system prompt (``role: system``) — the largest repeated span
|
||||
- the tool schemas (``tool_config``) — sizeable and identical every turn
|
||||
|
||||
Both points degrade gracefully on older LiteLLM: an unrecognised location is
|
||||
simply not injected (no error), so a stale pin still gets whatever caching it
|
||||
supports — the system-prompt point (the dominant win) has the widest support,
|
||||
and the tool_config point is applied by LiteLLM's Bedrock Converse transform
|
||||
on versions that recognise it (verified on litellm 1.90.1).
|
||||
"""
|
||||
return {
|
||||
"cache_control_injection_points": [
|
||||
|
||||
Reference in New Issue
Block a user