fix(openai): make store opt-in so it stops leaking to non-OpenAI backends (#4757)
This commit is contained in:
@@ -29,7 +29,7 @@ class OpenAIConfig(BaseLlmConfig):
|
||||
openrouter_base_url: Optional[str] = None,
|
||||
site_url: Optional[str] = None,
|
||||
app_name: Optional[str] = None,
|
||||
store: bool = False,
|
||||
store: Optional[bool] = None,
|
||||
# Response monitoring callback
|
||||
response_callback: Optional[Callable[[Any, dict, dict], None]] = None,
|
||||
):
|
||||
@@ -53,6 +53,11 @@ class OpenAIConfig(BaseLlmConfig):
|
||||
openrouter_base_url: OpenRouter base URL, defaults to None
|
||||
site_url: Site URL for OpenRouter, defaults to None
|
||||
app_name: Application name for OpenRouter, defaults to None
|
||||
store: Whether to store the conversation on OpenAI's server. Opt-in;
|
||||
defaults to None (not sent). Set to True or False only if you
|
||||
want the value forwarded to the OpenAI API. Leaving it None
|
||||
avoids leaking the field into OpenAI-compatible backends that
|
||||
reject unknown fields (Gemini, Groq, vLLM, etc.).
|
||||
response_callback: Optional callback for monitoring LLM responses.
|
||||
"""
|
||||
# Initialize base parameters
|
||||
|
||||
+6
-5
@@ -126,11 +126,12 @@ class OpenAILLM(LLMBase):
|
||||
params.update(**openrouter_params)
|
||||
|
||||
else:
|
||||
openai_specific_generation_params = ["store"]
|
||||
for param in openai_specific_generation_params:
|
||||
if hasattr(self.config, param):
|
||||
params[param] = getattr(self.config, param)
|
||||
|
||||
# Only send OpenAI-specific parameters when the user has explicitly
|
||||
# configured them. OpenAI-compatible backends (Gemini, Groq, vLLM, etc.)
|
||||
# reject unknown fields, so `store` must be opt-in, not opt-out.
|
||||
if self.config.store is not None:
|
||||
params["store"] = self.config.store
|
||||
|
||||
if response_format:
|
||||
params["response_format"] = response_format
|
||||
if tools: # TODO: Remove tools if no issues found with new memory addition logic
|
||||
|
||||
@@ -55,7 +55,7 @@ def test_generate_response_without_tools(mock_openai_client):
|
||||
response = llm.generate_response(messages)
|
||||
|
||||
mock_openai_client.chat.completions.create.assert_called_once_with(
|
||||
model="gpt-4.1-nano-2025-04-14", messages=messages, temperature=0.7, max_tokens=100, top_p=1.0, store=False
|
||||
model="gpt-4.1-nano-2025-04-14", messages=messages, temperature=0.7, max_tokens=100, top_p=1.0
|
||||
)
|
||||
assert response == "I'm doing well, thank you for asking!"
|
||||
|
||||
@@ -97,7 +97,7 @@ def test_generate_response_with_tools(mock_openai_client):
|
||||
response = llm.generate_response(messages, tools=tools)
|
||||
|
||||
mock_openai_client.chat.completions.create.assert_called_once_with(
|
||||
model="gpt-4.1-nano-2025-04-14", messages=messages, temperature=0.7, max_tokens=100, top_p=1.0, tools=tools, tool_choice="auto", store=False
|
||||
model="gpt-4.1-nano-2025-04-14", messages=messages, temperature=0.7, max_tokens=100, top_p=1.0, tools=tools, tool_choice="auto"
|
||||
)
|
||||
|
||||
assert response["content"] == "I've added the memory for you."
|
||||
@@ -231,6 +231,60 @@ def test_reasoning_effort_config_values():
|
||||
assert config.reasoning_effort is None
|
||||
|
||||
|
||||
def test_store_not_sent_by_default(mock_openai_client):
|
||||
"""`store` must NOT be injected into requests when the user has not
|
||||
explicitly configured it. Regression test for issue #4709, where
|
||||
`store=False` was unconditionally sent and rejected by OpenAI-compatible
|
||||
backends such as Google Gemini."""
|
||||
config = OpenAIConfig(model="gpt-4.1-nano-2025-04-14", temperature=0.1)
|
||||
assert config.store is None # new opt-in default
|
||||
llm = OpenAILLM(config)
|
||||
messages = [{"role": "user", "content": "Hello"}]
|
||||
|
||||
mock_response = Mock()
|
||||
mock_response.choices = [Mock(message=Mock(content="Response"))]
|
||||
mock_openai_client.chat.completions.create.return_value = mock_response
|
||||
|
||||
llm.generate_response(messages)
|
||||
|
||||
call_kwargs = mock_openai_client.chat.completions.create.call_args.kwargs
|
||||
assert "store" not in call_kwargs
|
||||
|
||||
|
||||
def test_store_sent_when_explicitly_true(mock_openai_client):
|
||||
"""When the user explicitly sets `store=True`, the field must be forwarded."""
|
||||
config = OpenAIConfig(model="gpt-4.1-nano-2025-04-14", store=True)
|
||||
llm = OpenAILLM(config)
|
||||
messages = [{"role": "user", "content": "Hello"}]
|
||||
|
||||
mock_response = Mock()
|
||||
mock_response.choices = [Mock(message=Mock(content="Response"))]
|
||||
mock_openai_client.chat.completions.create.return_value = mock_response
|
||||
|
||||
llm.generate_response(messages)
|
||||
|
||||
call_kwargs = mock_openai_client.chat.completions.create.call_args.kwargs
|
||||
assert call_kwargs["store"] is True
|
||||
|
||||
|
||||
def test_store_sent_when_explicitly_false(mock_openai_client):
|
||||
"""When the user explicitly sets `store=False`, the field must still be
|
||||
forwarded — explicit opt-out is a valid configuration for users who rely on
|
||||
it for OpenAI's zero-data-retention behavior."""
|
||||
config = OpenAIConfig(model="gpt-4.1-nano-2025-04-14", store=False)
|
||||
llm = OpenAILLM(config)
|
||||
messages = [{"role": "user", "content": "Hello"}]
|
||||
|
||||
mock_response = Mock()
|
||||
mock_response.choices = [Mock(message=Mock(content="Response"))]
|
||||
mock_openai_client.chat.completions.create.return_value = mock_response
|
||||
|
||||
llm.generate_response(messages)
|
||||
|
||||
call_kwargs = mock_openai_client.chat.completions.create.call_args.kwargs
|
||||
assert call_kwargs["store"] is False
|
||||
|
||||
|
||||
def test_callback_with_tools(mock_openai_client):
|
||||
mock_callback = Mock()
|
||||
config = OpenAIConfig(model="gpt-4.1-nano-2025-04-14", response_callback=mock_callback)
|
||||
|
||||
Reference in New Issue
Block a user